From ba9661b457effa34b6f6188e10546d00e4a49d54 Mon Sep 17 00:00:00 2001 From: Chandra Pratap Date: Tue, 2 Jul 2024 12:52:14 +0530 Subject: [PATCH 01/11] t: move reftable/record_test.c to the unit testing framework reftable/record_test.c exercises the functions defined in reftable/record.{c, h}. Migrate reftable/record_test.c to the unit testing framework. Migration involves refactoring the tests to use the unit testing framework instead of reftable's test framework, and renaming the tests to fit unit-tests' naming scheme. While at it, change the type of index variable 'i' to 'size_t' from 'int'. This is because 'i' is used in comparison against 'ARRAY_SIZE(x)' which is of type 'size_t'. Also, use set_hash() which is defined locally in the test file instead of set_test_hash() which is defined by reftable/test_framework.{c, h}. This is fine to do as both these functions are similarly implemented, and reftable/test_framework.{c, h} is not #included in the ported test. Get rid of reftable_record_print() from the tests as well, because it clutters the test framework's output and we have no way of verifying the output. Mentored-by: Patrick Steinhardt Mentored-by: Christian Couder Signed-off-by: Chandra Pratap Acked-by: Karthik Nayak Signed-off-by: Junio C Hamano --- Makefile | 2 +- t/helper/test-reftable.c | 1 - .../unit-tests/t-reftable-record.c | 131 ++++++++---------- 3 files changed, 61 insertions(+), 73 deletions(-) rename reftable/record_test.c => t/unit-tests/t-reftable-record.c (72%) diff --git a/Makefile b/Makefile index f25b2e80a13014..def3700b4d65ba 100644 --- a/Makefile +++ b/Makefile @@ -1338,6 +1338,7 @@ UNIT_TEST_PROGRAMS += t-hash UNIT_TEST_PROGRAMS += t-mem-pool UNIT_TEST_PROGRAMS += t-prio-queue UNIT_TEST_PROGRAMS += t-reftable-basics +UNIT_TEST_PROGRAMS += t-reftable-record UNIT_TEST_PROGRAMS += t-strbuf UNIT_TEST_PROGRAMS += t-strcmp-offset UNIT_TEST_PROGRAMS += t-strvec @@ -2678,7 +2679,6 @@ REFTABLE_TEST_OBJS += reftable/block_test.o REFTABLE_TEST_OBJS += reftable/dump.o REFTABLE_TEST_OBJS += reftable/merged_test.o REFTABLE_TEST_OBJS += reftable/pq_test.o -REFTABLE_TEST_OBJS += reftable/record_test.o REFTABLE_TEST_OBJS += reftable/readwrite_test.o REFTABLE_TEST_OBJS += reftable/stack_test.o REFTABLE_TEST_OBJS += reftable/test_framework.o diff --git a/t/helper/test-reftable.c b/t/helper/test-reftable.c index 9160bc5da645af..aa6538a8da3264 100644 --- a/t/helper/test-reftable.c +++ b/t/helper/test-reftable.c @@ -5,7 +5,6 @@ int cmd__reftable(int argc, const char **argv) { /* test from simple to complex. */ - record_test_main(argc, argv); block_test_main(argc, argv); tree_test_main(argc, argv); pq_test_main(argc, argv); diff --git a/reftable/record_test.c b/t/unit-tests/t-reftable-record.c similarity index 72% rename from reftable/record_test.c rename to t/unit-tests/t-reftable-record.c index 58290bdba345d0..e799464c365bfe 100644 --- a/reftable/record_test.c +++ b/t/unit-tests/t-reftable-record.c @@ -6,15 +6,11 @@ https://developers.google.com/open-source/licenses/bsd */ -#include "record.h" +#include "test-lib.h" +#include "reftable/constants.h" +#include "reftable/record.h" -#include "system.h" -#include "basics.h" -#include "constants.h" -#include "test_framework.h" -#include "reftable-tests.h" - -static void test_copy(struct reftable_record *rec) +static void t_copy(struct reftable_record *rec) { struct reftable_record copy; uint8_t typ; @@ -24,15 +20,12 @@ static void test_copy(struct reftable_record *rec) reftable_record_copy_from(©, rec, GIT_SHA1_RAWSZ); /* do it twice to catch memory leaks */ reftable_record_copy_from(©, rec, GIT_SHA1_RAWSZ); - EXPECT(reftable_record_equal(rec, ©, GIT_SHA1_RAWSZ)); - - puts("testing print coverage:\n"); - reftable_record_print(©, GIT_SHA1_RAWSZ); + check(reftable_record_equal(rec, ©, GIT_SHA1_RAWSZ)); reftable_record_release(©); } -static void test_varint_roundtrip(void) +static void t_varint_roundtrip(void) { uint64_t inputs[] = { 0, 1, @@ -43,8 +36,8 @@ static void test_varint_roundtrip(void) 4096, ((uint64_t)1 << 63), ((uint64_t)1 << 63) + ((uint64_t)1 << 63) - 1 }; - int i = 0; - for (i = 0; i < ARRAY_SIZE(inputs); i++) { + + for (size_t i = 0; i < ARRAY_SIZE(inputs); i++) { uint8_t dest[10]; struct string_view out = { @@ -55,29 +48,26 @@ static void test_varint_roundtrip(void) int n = put_var_int(&out, in); uint64_t got = 0; - EXPECT(n > 0); + check_int(n, >, 0); out.len = n; n = get_var_int(&got, &out); - EXPECT(n > 0); + check_int(n, >, 0); - EXPECT(got == in); + check_int(got, ==, in); } } static void set_hash(uint8_t *h, int j) { - int i = 0; - for (i = 0; i < hash_size(GIT_SHA1_FORMAT_ID); i++) { + for (int i = 0; i < hash_size(GIT_SHA1_FORMAT_ID); i++) h[i] = (j >> i) & 0xff; - } } -static void test_reftable_ref_record_roundtrip(void) +static void t_reftable_ref_record_roundtrip(void) { struct strbuf scratch = STRBUF_INIT; - int i = 0; - for (i = REFTABLE_REF_DELETION; i < REFTABLE_NR_REF_VALUETYPES; i++) { + for (int i = REFTABLE_REF_DELETION; i < REFTABLE_NR_REF_VALUETYPES; i++) { struct reftable_record in = { .type = BLOCK_TYPE_REF, }; @@ -107,19 +97,19 @@ static void test_reftable_ref_record_roundtrip(void) } in.u.ref.refname = xstrdup("refs/heads/master"); - test_copy(&in); + t_copy(&in); - EXPECT(reftable_record_val_type(&in) == i); + check_int(reftable_record_val_type(&in), ==, i); reftable_record_key(&in, &key); n = reftable_record_encode(&in, dest, GIT_SHA1_RAWSZ); - EXPECT(n > 0); + check_int(n, >, 0); /* decode into a non-zero reftable_record to test for leaks. */ m = reftable_record_decode(&out, key, i, dest, GIT_SHA1_RAWSZ, &scratch); - EXPECT(n == m); + check_int(n, ==, m); - EXPECT(reftable_ref_record_equal(&in.u.ref, &out.u.ref, + check(reftable_ref_record_equal(&in.u.ref, &out.u.ref, GIT_SHA1_RAWSZ)); reftable_record_release(&in); @@ -130,7 +120,7 @@ static void test_reftable_ref_record_roundtrip(void) strbuf_release(&scratch); } -static void test_reftable_log_record_equal(void) +static void t_reftable_log_record_equal(void) { struct reftable_log_record in[2] = { { @@ -143,16 +133,15 @@ static void test_reftable_log_record_equal(void) } }; - EXPECT(!reftable_log_record_equal(&in[0], &in[1], GIT_SHA1_RAWSZ)); + check(!reftable_log_record_equal(&in[0], &in[1], GIT_SHA1_RAWSZ)); in[1].update_index = in[0].update_index; - EXPECT(reftable_log_record_equal(&in[0], &in[1], GIT_SHA1_RAWSZ)); + check(reftable_log_record_equal(&in[0], &in[1], GIT_SHA1_RAWSZ)); reftable_log_record_release(&in[0]); reftable_log_record_release(&in[1]); } -static void test_reftable_log_record_roundtrip(void) +static void t_reftable_log_record_roundtrip(void) { - int i; struct reftable_log_record in[] = { { .refname = xstrdup("refs/heads/master"), @@ -180,12 +169,12 @@ static void test_reftable_log_record_roundtrip(void) } }; struct strbuf scratch = STRBUF_INIT; + set_hash(in[0].value.update.new_hash, 1); + set_hash(in[0].value.update.old_hash, 2); + set_hash(in[2].value.update.new_hash, 3); + set_hash(in[2].value.update.old_hash, 4); - set_test_hash(in[0].value.update.new_hash, 1); - set_test_hash(in[0].value.update.old_hash, 2); - set_test_hash(in[2].value.update.new_hash, 3); - set_test_hash(in[2].value.update.old_hash, 4); - for (i = 0; i < ARRAY_SIZE(in); i++) { + for (size_t i = 0; i < ARRAY_SIZE(in); i++) { struct reftable_record rec = { .type = BLOCK_TYPE_LOG }; struct strbuf key = STRBUF_INIT; uint8_t buffer[1024] = { 0 }; @@ -212,18 +201,18 @@ static void test_reftable_log_record_roundtrip(void) rec.u.log = in[i]; - test_copy(&rec); + t_copy(&rec); reftable_record_key(&rec, &key); n = reftable_record_encode(&rec, dest, GIT_SHA1_RAWSZ); - EXPECT(n >= 0); + check_int(n, >=, 0); valtype = reftable_record_val_type(&rec); m = reftable_record_decode(&out, key, valtype, dest, GIT_SHA1_RAWSZ, &scratch); - EXPECT(n == m); + check_int(n, ==, m); - EXPECT(reftable_log_record_equal(&in[i], &out.u.log, + check(reftable_log_record_equal(&in[i], &out.u.log, GIT_SHA1_RAWSZ)); reftable_log_record_release(&in[i]); strbuf_release(&key); @@ -233,7 +222,7 @@ static void test_reftable_log_record_roundtrip(void) strbuf_release(&scratch); } -static void test_key_roundtrip(void) +static void t_key_roundtrip(void) { uint8_t buffer[1024] = { 0 }; struct string_view dest = { @@ -252,21 +241,21 @@ static void test_key_roundtrip(void) strbuf_addstr(&key, "refs/tags/bla"); extra = 6; n = reftable_encode_key(&restart, dest, last_key, key, extra); - EXPECT(!restart); - EXPECT(n > 0); + check(!restart); + check_int(n, >, 0); strbuf_addstr(&roundtrip, "refs/heads/master"); m = reftable_decode_key(&roundtrip, &rt_extra, dest); - EXPECT(n == m); - EXPECT(0 == strbuf_cmp(&key, &roundtrip)); - EXPECT(rt_extra == extra); + check_int(n, ==, m); + check(!strbuf_cmp(&key, &roundtrip)); + check_int(rt_extra, ==, extra); strbuf_release(&last_key); strbuf_release(&key); strbuf_release(&roundtrip); } -static void test_reftable_obj_record_roundtrip(void) +static void t_reftable_obj_record_roundtrip(void) { uint8_t testHash1[GIT_SHA1_RAWSZ] = { 1, 2, 3, 4, 0 }; uint64_t till9[] = { 1, 2, 3, 4, 500, 600, 700, 800, 9000 }; @@ -289,9 +278,8 @@ static void test_reftable_obj_record_roundtrip(void) }, }; struct strbuf scratch = STRBUF_INIT; - int i = 0; - for (i = 0; i < ARRAY_SIZE(recs); i++) { + for (size_t i = 0; i < ARRAY_SIZE(recs); i++) { uint8_t buffer[1024] = { 0 }; struct string_view dest = { .buf = buffer, @@ -308,16 +296,16 @@ static void test_reftable_obj_record_roundtrip(void) int n, m; uint8_t extra; - test_copy(&in); + t_copy(&in); reftable_record_key(&in, &key); n = reftable_record_encode(&in, dest, GIT_SHA1_RAWSZ); - EXPECT(n > 0); + check_int(n, >, 0); extra = reftable_record_val_type(&in); m = reftable_record_decode(&out, key, extra, dest, GIT_SHA1_RAWSZ, &scratch); - EXPECT(n == m); + check_int(n, ==, m); - EXPECT(reftable_record_equal(&in, &out, GIT_SHA1_RAWSZ)); + check(reftable_record_equal(&in, &out, GIT_SHA1_RAWSZ)); strbuf_release(&key); reftable_record_release(&out); } @@ -325,7 +313,7 @@ static void test_reftable_obj_record_roundtrip(void) strbuf_release(&scratch); } -static void test_reftable_index_record_roundtrip(void) +static void t_reftable_index_record_roundtrip(void) { struct reftable_record in = { .type = BLOCK_TYPE_INDEX, @@ -350,18 +338,18 @@ static void test_reftable_index_record_roundtrip(void) strbuf_addstr(&in.u.idx.last_key, "refs/heads/master"); reftable_record_key(&in, &key); - test_copy(&in); + t_copy(&in); - EXPECT(0 == strbuf_cmp(&key, &in.u.idx.last_key)); + check(!strbuf_cmp(&key, &in.u.idx.last_key)); n = reftable_record_encode(&in, dest, GIT_SHA1_RAWSZ); - EXPECT(n > 0); + check_int(n, >, 0); extra = reftable_record_val_type(&in); m = reftable_record_decode(&out, key, extra, dest, GIT_SHA1_RAWSZ, &scratch); - EXPECT(m == n); + check_int(m, ==, n); - EXPECT(reftable_record_equal(&in, &out, GIT_SHA1_RAWSZ)); + check(reftable_record_equal(&in, &out, GIT_SHA1_RAWSZ)); reftable_record_release(&out); strbuf_release(&key); @@ -369,14 +357,15 @@ static void test_reftable_index_record_roundtrip(void) strbuf_release(&in.u.idx.last_key); } -int record_test_main(int argc, const char *argv[]) +int cmd_main(int argc, const char *argv[]) { - RUN_TEST(test_reftable_log_record_equal); - RUN_TEST(test_reftable_log_record_roundtrip); - RUN_TEST(test_reftable_ref_record_roundtrip); - RUN_TEST(test_varint_roundtrip); - RUN_TEST(test_key_roundtrip); - RUN_TEST(test_reftable_obj_record_roundtrip); - RUN_TEST(test_reftable_index_record_roundtrip); - return 0; + TEST(t_reftable_log_record_equal(), "reftable_log_record_equal works"); + TEST(t_reftable_log_record_roundtrip(), "record operations work on log record"); + TEST(t_reftable_ref_record_roundtrip(), "record operations work on ref record"); + TEST(t_varint_roundtrip(), "put_var_int and get_var_int work"); + TEST(t_key_roundtrip(), "reftable_encode_key and reftable_decode_key work"); + TEST(t_reftable_obj_record_roundtrip(), "record operations work on obj record"); + TEST(t_reftable_index_record_roundtrip(), "record operations work on index record"); + + return test_done(); } From 9008b8a6e8c0c96bbe9bb2ebb4842c89eca39f6d Mon Sep 17 00:00:00 2001 From: Chandra Pratap Date: Tue, 2 Jul 2024 12:52:15 +0530 Subject: [PATCH 02/11] t-reftable-record: add reftable_record_cmp() tests for log records In the current testing setup for log records, only reftable_log_record_equal() among log record's comparison functions is tested. Modify the existing tests to exercise reftable_log_record_cmp_void() (using the wrapper function reftable_record_cmp()) alongside reftable_log_record_equal(). Note that to achieve this, we'll need to replace instances of reftable_log_record_equal() with the wrapper function reftable_record_equal(). Rename the now modified test to reflect its nature of exercising all comparison operations, not just equality. Mentored-by: Patrick Steinhardt Mentored-by: Christian Couder Signed-off-by: Chandra Pratap Acked-by: Karthik Nayak Signed-off-by: Junio C Hamano --- t/unit-tests/t-reftable-record.c | 38 +++++++++++++++++++++----------- 1 file changed, 25 insertions(+), 13 deletions(-) diff --git a/t/unit-tests/t-reftable-record.c b/t/unit-tests/t-reftable-record.c index e799464c365bfe..dd64e71f3b6d0e 100644 --- a/t/unit-tests/t-reftable-record.c +++ b/t/unit-tests/t-reftable-record.c @@ -120,24 +120,36 @@ static void t_reftable_ref_record_roundtrip(void) strbuf_release(&scratch); } -static void t_reftable_log_record_equal(void) +static void t_reftable_log_record_comparison(void) { - struct reftable_log_record in[2] = { + struct reftable_record in[3] = { { - .refname = xstrdup("refs/heads/master"), - .update_index = 42, + .type = BLOCK_TYPE_LOG, + .u.log.refname = (char *) "refs/heads/master", + .u.log.update_index = 42, }, { - .refname = xstrdup("refs/heads/master"), - .update_index = 22, - } + .type = BLOCK_TYPE_LOG, + .u.log.refname = (char *) "refs/heads/master", + .u.log.update_index = 22, + }, + { + .type = BLOCK_TYPE_LOG, + .u.log.refname = (char *) "refs/heads/main", + .u.log.update_index = 22, + }, }; - check(!reftable_log_record_equal(&in[0], &in[1], GIT_SHA1_RAWSZ)); - in[1].update_index = in[0].update_index; - check(reftable_log_record_equal(&in[0], &in[1], GIT_SHA1_RAWSZ)); - reftable_log_record_release(&in[0]); - reftable_log_record_release(&in[1]); + check(!reftable_record_equal(&in[0], &in[1], GIT_SHA1_RAWSZ)); + check(!reftable_record_equal(&in[1], &in[2], GIT_SHA1_RAWSZ)); + check_int(reftable_record_cmp(&in[1], &in[2]), >, 0); + /* comparison should be reversed for equal keys, because + * comparison is now performed on the basis of update indices */ + check_int(reftable_record_cmp(&in[0], &in[1]), <, 0); + + in[1].u.log.update_index = in[0].u.log.update_index; + check(reftable_record_equal(&in[0], &in[1], GIT_SHA1_RAWSZ)); + check(!reftable_record_cmp(&in[0], &in[1])); } static void t_reftable_log_record_roundtrip(void) @@ -359,7 +371,7 @@ static void t_reftable_index_record_roundtrip(void) int cmd_main(int argc, const char *argv[]) { - TEST(t_reftable_log_record_equal(), "reftable_log_record_equal works"); + TEST(t_reftable_log_record_comparison(), "comparison operations work on log record"); TEST(t_reftable_log_record_roundtrip(), "record operations work on log record"); TEST(t_reftable_ref_record_roundtrip(), "record operations work on ref record"); TEST(t_varint_roundtrip(), "put_var_int and get_var_int work"); From b7bbb58c14ba92dbfaeb4e995a61b633a8754194 Mon Sep 17 00:00:00 2001 From: Chandra Pratap Date: Tue, 2 Jul 2024 12:52:16 +0530 Subject: [PATCH 03/11] t-reftable-record: add comparison tests for ref records In the current testing setup for ref records, the comparison functions for ref records, reftable_ref_record_cmp_void() and reftable_ref_record_equal() are left untested. Add tests for the same by using the wrapper functions reftable_record_cmp() and reftable_record_equal() for reftable_ref_record_cmp_void() and reftable_ref_record_equal() respectively. Mentored-by: Patrick Steinhardt Mentored-by: Christian Couder Signed-off-by: Chandra Pratap Acked-by: Karthik Nayak Signed-off-by: Junio C Hamano --- t/unit-tests/t-reftable-record.c | 33 ++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/t/unit-tests/t-reftable-record.c b/t/unit-tests/t-reftable-record.c index dd64e71f3b6d0e..99534acd176878 100644 --- a/t/unit-tests/t-reftable-record.c +++ b/t/unit-tests/t-reftable-record.c @@ -63,6 +63,38 @@ static void set_hash(uint8_t *h, int j) h[i] = (j >> i) & 0xff; } +static void t_reftable_ref_record_comparison(void) +{ + struct reftable_record in[3] = { + { + .type = BLOCK_TYPE_REF, + .u.ref.refname = (char *) "refs/heads/master", + .u.ref.value_type = REFTABLE_REF_VAL1, + }, + { + .type = BLOCK_TYPE_REF, + .u.ref.refname = (char *) "refs/heads/master", + .u.ref.value_type = REFTABLE_REF_DELETION, + }, + { + .type = BLOCK_TYPE_REF, + .u.ref.refname = (char *) "HEAD", + .u.ref.value_type = REFTABLE_REF_SYMREF, + .u.ref.value.symref = (char *) "refs/heads/master", + }, + }; + + check(!reftable_record_equal(&in[0], &in[1], GIT_SHA1_RAWSZ)); + check(!reftable_record_cmp(&in[0], &in[1])); + + check(!reftable_record_equal(&in[1], &in[2], GIT_SHA1_RAWSZ)); + check_int(reftable_record_cmp(&in[1], &in[2]), >, 0); + + in[1].u.ref.value_type = in[0].u.ref.value_type; + check(reftable_record_equal(&in[0], &in[1], GIT_SHA1_RAWSZ)); + check(!reftable_record_cmp(&in[0], &in[1])); +} + static void t_reftable_ref_record_roundtrip(void) { struct strbuf scratch = STRBUF_INIT; @@ -371,6 +403,7 @@ static void t_reftable_index_record_roundtrip(void) int cmd_main(int argc, const char *argv[]) { + TEST(t_reftable_ref_record_comparison(), "comparison operations work on ref record"); TEST(t_reftable_log_record_comparison(), "comparison operations work on log record"); TEST(t_reftable_log_record_roundtrip(), "record operations work on log record"); TEST(t_reftable_ref_record_roundtrip(), "record operations work on ref record"); From 85ca39e79b3fc3baed8ae218ac52af953121977a Mon Sep 17 00:00:00 2001 From: Chandra Pratap Date: Tue, 2 Jul 2024 12:52:17 +0530 Subject: [PATCH 04/11] t-reftable-record: add comparison tests for index records In the current testing setup for index records, the comparison functions for index records, reftable_index_record_cmp() and reftable_index_record_equal() are left untested. Add tests for the same by using the wrapper functions reftable_record_cmp() and reftable_record_equal() for reftable_index_record_cmp() and reftable_index_record_equal() respectively. Mentored-by: Patrick Steinhardt Mentored-by: Christian Couder Signed-off-by: Chandra Pratap Acked-by: Karthik Nayak Signed-off-by: Junio C Hamano --- t/unit-tests/t-reftable-record.c | 38 ++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/t/unit-tests/t-reftable-record.c b/t/unit-tests/t-reftable-record.c index 99534acd176878..e8db337eb82f2b 100644 --- a/t/unit-tests/t-reftable-record.c +++ b/t/unit-tests/t-reftable-record.c @@ -357,6 +357,43 @@ static void t_reftable_obj_record_roundtrip(void) strbuf_release(&scratch); } +static void t_reftable_index_record_comparison(void) +{ + struct reftable_record in[3] = { + { + .type = BLOCK_TYPE_INDEX, + .u.idx.offset = 22, + .u.idx.last_key = STRBUF_INIT, + }, + { + .type = BLOCK_TYPE_INDEX, + .u.idx.offset = 32, + .u.idx.last_key = STRBUF_INIT, + }, + { + .type = BLOCK_TYPE_INDEX, + .u.idx.offset = 32, + .u.idx.last_key = STRBUF_INIT, + }, + }; + strbuf_addstr(&in[0].u.idx.last_key, "refs/heads/master"); + strbuf_addstr(&in[1].u.idx.last_key, "refs/heads/master"); + strbuf_addstr(&in[2].u.idx.last_key, "refs/heads/branch"); + + check(!reftable_record_equal(&in[0], &in[1], GIT_SHA1_RAWSZ)); + check(!reftable_record_cmp(&in[0], &in[1])); + + check(!reftable_record_equal(&in[1], &in[2], GIT_SHA1_RAWSZ)); + check_int(reftable_record_cmp(&in[1], &in[2]), >, 0); + + in[1].u.idx.offset = in[0].u.idx.offset; + check(reftable_record_equal(&in[0], &in[1], GIT_SHA1_RAWSZ)); + check(!reftable_record_cmp(&in[0], &in[1])); + + for (size_t i = 0; i < ARRAY_SIZE(in); i++) + reftable_record_release(&in[i]); +} + static void t_reftable_index_record_roundtrip(void) { struct reftable_record in = { @@ -405,6 +442,7 @@ int cmd_main(int argc, const char *argv[]) { TEST(t_reftable_ref_record_comparison(), "comparison operations work on ref record"); TEST(t_reftable_log_record_comparison(), "comparison operations work on log record"); + TEST(t_reftable_index_record_comparison(), "comparison operations work on index record"); TEST(t_reftable_log_record_roundtrip(), "record operations work on log record"); TEST(t_reftable_ref_record_roundtrip(), "record operations work on ref record"); TEST(t_varint_roundtrip(), "put_var_int and get_var_int work"); From abb1834f2af0c958f418968760abf4e4c62aef34 Mon Sep 17 00:00:00 2001 From: Chandra Pratap Date: Tue, 2 Jul 2024 12:52:18 +0530 Subject: [PATCH 05/11] t-reftable-record: add comparison tests for obj records In the current testing setup for obj records, the comparison functions for obj records, reftable_obj_record_cmp_void() and reftable_obj_record_equal_void() are left untested. Add tests for the same by using the wrapper functions reftable_record_cmp() and reftable_record_equal() for reftable_index_record_cmp_void() and reftable_index_record_equal_void() respectively. Mentored-by: Patrick Steinhardt Mentored-by: Christian Couder Signed-off-by: Chandra Pratap Acked-by: Karthik Nayak Signed-off-by: Junio C Hamano --- t/unit-tests/t-reftable-record.c | 39 ++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/t/unit-tests/t-reftable-record.c b/t/unit-tests/t-reftable-record.c index e8db337eb82f2b..46614de948147e 100644 --- a/t/unit-tests/t-reftable-record.c +++ b/t/unit-tests/t-reftable-record.c @@ -299,6 +299,44 @@ static void t_key_roundtrip(void) strbuf_release(&roundtrip); } +static void t_reftable_obj_record_comparison(void) +{ + + uint8_t id_bytes[] = { 0, 1, 2, 3, 4, 5, 6 }; + uint64_t offsets[] = { 0, 16, 32, 48, 64, 80, 96, 112}; + struct reftable_record in[3] = { + { + .type = BLOCK_TYPE_OBJ, + .u.obj.hash_prefix = id_bytes, + .u.obj.hash_prefix_len = 7, + .u.obj.offsets = offsets, + .u.obj.offset_len = 8, + }, + { + .type = BLOCK_TYPE_OBJ, + .u.obj.hash_prefix = id_bytes, + .u.obj.hash_prefix_len = 7, + .u.obj.offsets = offsets, + .u.obj.offset_len = 5, + }, + { + .type = BLOCK_TYPE_OBJ, + .u.obj.hash_prefix = id_bytes, + .u.obj.hash_prefix_len = 5, + }, + }; + + check(!reftable_record_equal(&in[0], &in[1], GIT_SHA1_RAWSZ)); + check(!reftable_record_cmp(&in[0], &in[1])); + + check(!reftable_record_equal(&in[1], &in[2], GIT_SHA1_RAWSZ)); + check_int(reftable_record_cmp(&in[1], &in[2]), >, 0); + + in[1].u.obj.offset_len = in[0].u.obj.offset_len; + check(reftable_record_equal(&in[0], &in[1], GIT_SHA1_RAWSZ)); + check(!reftable_record_cmp(&in[0], &in[1])); +} + static void t_reftable_obj_record_roundtrip(void) { uint8_t testHash1[GIT_SHA1_RAWSZ] = { 1, 2, 3, 4, 0 }; @@ -443,6 +481,7 @@ int cmd_main(int argc, const char *argv[]) TEST(t_reftable_ref_record_comparison(), "comparison operations work on ref record"); TEST(t_reftable_log_record_comparison(), "comparison operations work on log record"); TEST(t_reftable_index_record_comparison(), "comparison operations work on index record"); + TEST(t_reftable_obj_record_comparison(), "comparison operations work on obj record"); TEST(t_reftable_log_record_roundtrip(), "record operations work on log record"); TEST(t_reftable_ref_record_roundtrip(), "record operations work on ref record"); TEST(t_varint_roundtrip(), "put_var_int and get_var_int work"); From aa3fef4ff3df005c6ad5b524429caef57c7b00d6 Mon Sep 17 00:00:00 2001 From: Chandra Pratap Date: Tue, 2 Jul 2024 12:52:19 +0530 Subject: [PATCH 06/11] t-reftable-record: add ref tests for reftable_record_is_deletion() reftable_record_is_deletion() is a function defined in reftable/record.{c, h} that determines whether a record is of type deletion or not. In the current testing setup, this function is left untested for all the four record types (ref, log, obj, index). Add tests for this function in the case of ref records. Mentored-by: Patrick Steinhardt Mentored-by: Christian Couder Signed-off-by: Chandra Pratap Acked-by: Karthik Nayak Signed-off-by: Junio C Hamano --- t/unit-tests/t-reftable-record.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/t/unit-tests/t-reftable-record.c b/t/unit-tests/t-reftable-record.c index 46614de948147e..170c804825ca6b 100644 --- a/t/unit-tests/t-reftable-record.c +++ b/t/unit-tests/t-reftable-record.c @@ -102,6 +102,7 @@ static void t_reftable_ref_record_roundtrip(void) for (int i = REFTABLE_REF_DELETION; i < REFTABLE_NR_REF_VALUETYPES; i++) { struct reftable_record in = { .type = BLOCK_TYPE_REF, + .u.ref.value_type = i, }; struct reftable_record out = { .type = BLOCK_TYPE_REF }; struct strbuf key = STRBUF_INIT; @@ -132,6 +133,7 @@ static void t_reftable_ref_record_roundtrip(void) t_copy(&in); check_int(reftable_record_val_type(&in), ==, i); + check_int(reftable_record_is_deletion(&in), ==, i == REFTABLE_REF_DELETION); reftable_record_key(&in, &key); n = reftable_record_encode(&in, dest, GIT_SHA1_RAWSZ); From 09ca34799b1fa23132330bff309dfb0d8c019f37 Mon Sep 17 00:00:00 2001 From: Chandra Pratap Date: Tue, 2 Jul 2024 12:52:20 +0530 Subject: [PATCH 07/11] t-reftable-record: add log tests for reftable_record_is_deletion() reftable_record_is_deletion() is a function defined in reftable/record.{c, h} that determines whether a record is of type deletion or not. In the current testing setup, this function is left untested for three of the four record types (log, obj, index). Add tests for this function in the case of log records. Mentored-by: Patrick Steinhardt Mentored-by: Christian Couder Signed-off-by: Chandra Pratap Acked-by: Karthik Nayak Signed-off-by: Junio C Hamano --- t/unit-tests/t-reftable-record.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/t/unit-tests/t-reftable-record.c b/t/unit-tests/t-reftable-record.c index 170c804825ca6b..6ece76e4166741 100644 --- a/t/unit-tests/t-reftable-record.c +++ b/t/unit-tests/t-reftable-record.c @@ -220,6 +220,10 @@ static void t_reftable_log_record_roundtrip(void) set_hash(in[2].value.update.new_hash, 3); set_hash(in[2].value.update.old_hash, 4); + check(!reftable_log_record_is_deletion(&in[0])); + check(reftable_log_record_is_deletion(&in[1])); + check(!reftable_log_record_is_deletion(&in[2])); + for (size_t i = 0; i < ARRAY_SIZE(in); i++) { struct reftable_record rec = { .type = BLOCK_TYPE_LOG }; struct strbuf key = STRBUF_INIT; From 9aa3814b2fd5ef135ebc63f99f1f473722b53e52 Mon Sep 17 00:00:00 2001 From: Chandra Pratap Date: Tue, 2 Jul 2024 12:52:21 +0530 Subject: [PATCH 08/11] t-reftable-record: add obj tests for reftable_record_is_deletion() reftable_record_is_deletion() is a function defined in reftable/record.{c, h} that determines whether a record is of type deletion or not. In the current testing setup, this function is left untested for two of the four record types (obj, index). Add tests for this function in the case of obj records. Note that since obj records cannot be of type deletion, this function must always return '0' when called on an obj record. Mentored-by: Patrick Steinhardt Mentored-by: Christian Couder Signed-off-by: Chandra Pratap Acked-by: Karthik Nayak Signed-off-by: Junio C Hamano --- t/unit-tests/t-reftable-record.c | 1 + 1 file changed, 1 insertion(+) diff --git a/t/unit-tests/t-reftable-record.c b/t/unit-tests/t-reftable-record.c index 6ece76e4166741..1b52085f4680bd 100644 --- a/t/unit-tests/t-reftable-record.c +++ b/t/unit-tests/t-reftable-record.c @@ -384,6 +384,7 @@ static void t_reftable_obj_record_roundtrip(void) int n, m; uint8_t extra; + check(!reftable_record_is_deletion(&in)); t_copy(&in); reftable_record_key(&in, &key); n = reftable_record_encode(&in, dest, GIT_SHA1_RAWSZ); From 8a1f1f88bbcac191d5904fd966e6a767caf97d4b Mon Sep 17 00:00:00 2001 From: Chandra Pratap Date: Tue, 2 Jul 2024 12:52:22 +0530 Subject: [PATCH 09/11] t-reftable-record: add index tests for reftable_record_is_deletion() reftable_record_is_deletion() is a function defined in reftable/record.{c, h} that determines whether a record is of type deletion or not. In the current testing setup, this function is left untested for index records. Add tests for this function in the case of index records. Note that since index records cannot be of type deletion, this function must always return '0' when called on an index record. Mentored-by: Patrick Steinhardt Mentored-by: Christian Couder Signed-off-by: Chandra Pratap Acked-by: Karthik Nayak Signed-off-by: Junio C Hamano --- t/unit-tests/t-reftable-record.c | 1 + 1 file changed, 1 insertion(+) diff --git a/t/unit-tests/t-reftable-record.c b/t/unit-tests/t-reftable-record.c index 1b52085f4680bd..43b5d408998ea0 100644 --- a/t/unit-tests/t-reftable-record.c +++ b/t/unit-tests/t-reftable-record.c @@ -466,6 +466,7 @@ static void t_reftable_index_record_roundtrip(void) reftable_record_key(&in, &key); t_copy(&in); + check(!reftable_record_is_deletion(&in)); check(!strbuf_cmp(&key, &in.u.idx.last_key)); n = reftable_record_encode(&in, dest, GIT_SHA1_RAWSZ); check_int(n, >, 0); From f7ec13b53896966c233514c3906c284a40b00c3c Mon Sep 17 00:00:00 2001 From: Chandra Pratap Date: Tue, 2 Jul 2024 12:52:23 +0530 Subject: [PATCH 10/11] t-reftable-record: add tests for reftable_ref_record_compare_name() reftable_ref_record_compare_name() is a function defined by reftable/record.{c, h} and is used to compare the refname of two ref records when sorting multiple ref records using 'qsort'. In the current testing setup, this function is left unexercised. Add a testing function for the same. Mentored-by: Patrick Steinhardt Mentored-by: Christian Couder Signed-off-by: Chandra Pratap Acked-by: Karthik Nayak Signed-off-by: Junio C Hamano --- t/unit-tests/t-reftable-record.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/t/unit-tests/t-reftable-record.c b/t/unit-tests/t-reftable-record.c index 43b5d408998ea0..c0668cd8b4fc49 100644 --- a/t/unit-tests/t-reftable-record.c +++ b/t/unit-tests/t-reftable-record.c @@ -95,6 +95,25 @@ static void t_reftable_ref_record_comparison(void) check(!reftable_record_cmp(&in[0], &in[1])); } +static void t_reftable_ref_record_compare_name(void) +{ + struct reftable_ref_record recs[3] = { + { + .refname = (char *) "refs/heads/a" + }, + { + .refname = (char *) "refs/heads/b" + }, + { + .refname = (char *) "refs/heads/a" + }, + }; + + check_int(reftable_ref_record_compare_name(&recs[0], &recs[1]), <, 0); + check_int(reftable_ref_record_compare_name(&recs[1], &recs[0]), >, 0); + check_int(reftable_ref_record_compare_name(&recs[0], &recs[2]), ==, 0); +} + static void t_reftable_ref_record_roundtrip(void) { struct strbuf scratch = STRBUF_INIT; @@ -490,6 +509,7 @@ int cmd_main(int argc, const char *argv[]) TEST(t_reftable_log_record_comparison(), "comparison operations work on log record"); TEST(t_reftable_index_record_comparison(), "comparison operations work on index record"); TEST(t_reftable_obj_record_comparison(), "comparison operations work on obj record"); + TEST(t_reftable_ref_record_compare_name(), "reftable_ref_record_compare_name works"); TEST(t_reftable_log_record_roundtrip(), "record operations work on log record"); TEST(t_reftable_ref_record_roundtrip(), "record operations work on ref record"); TEST(t_varint_roundtrip(), "put_var_int and get_var_int work"); From b942fda6702994dc1a7ed91d33faceea68aedbc9 Mon Sep 17 00:00:00 2001 From: Chandra Pratap Date: Tue, 2 Jul 2024 12:52:24 +0530 Subject: [PATCH 11/11] t-reftable-record: add tests for reftable_log_record_compare_key() reftable_log_record_compare_key() is a function defined by reftable/record.{c, h} and is used to compare the keys of two log records when sorting multiple log records using 'qsort'. In the current testing setup, this function is left unexercised. Add a testing function for the same. Mentored-by: Patrick Steinhardt Mentored-by: Christian Couder Signed-off-by: Chandra Pratap Acked-by: Karthik Nayak Signed-off-by: Junio C Hamano --- t/unit-tests/t-reftable-record.c | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/t/unit-tests/t-reftable-record.c b/t/unit-tests/t-reftable-record.c index c0668cd8b4fc49..cb649ee419a015 100644 --- a/t/unit-tests/t-reftable-record.c +++ b/t/unit-tests/t-reftable-record.c @@ -205,6 +205,35 @@ static void t_reftable_log_record_comparison(void) check(!reftable_record_cmp(&in[0], &in[1])); } +static void t_reftable_log_record_compare_key(void) +{ + struct reftable_log_record logs[3] = { + { + .refname = (char *) "refs/heads/a", + .update_index = 1, + }, + { + .refname = (char *) "refs/heads/b", + .update_index = 2, + }, + { + .refname = (char *) "refs/heads/a", + .update_index = 3, + }, + }; + + check_int(reftable_log_record_compare_key(&logs[0], &logs[1]), <, 0); + check_int(reftable_log_record_compare_key(&logs[1], &logs[0]), >, 0); + + logs[1].update_index = logs[0].update_index; + check_int(reftable_log_record_compare_key(&logs[0], &logs[1]), <, 0); + + check_int(reftable_log_record_compare_key(&logs[0], &logs[2]), >, 0); + check_int(reftable_log_record_compare_key(&logs[2], &logs[0]), <, 0); + logs[2].update_index = logs[0].update_index; + check_int(reftable_log_record_compare_key(&logs[0], &logs[2]), ==, 0); +} + static void t_reftable_log_record_roundtrip(void) { struct reftable_log_record in[] = { @@ -510,6 +539,7 @@ int cmd_main(int argc, const char *argv[]) TEST(t_reftable_index_record_comparison(), "comparison operations work on index record"); TEST(t_reftable_obj_record_comparison(), "comparison operations work on obj record"); TEST(t_reftable_ref_record_compare_name(), "reftable_ref_record_compare_name works"); + TEST(t_reftable_log_record_compare_key(), "reftable_log_record_compare_key works"); TEST(t_reftable_log_record_roundtrip(), "record operations work on log record"); TEST(t_reftable_ref_record_roundtrip(), "record operations work on ref record"); TEST(t_varint_roundtrip(), "put_var_int and get_var_int work");