From 9cdfd1d7df81f911e8c4f120f3aaf1b0d981e8a1 Mon Sep 17 00:00:00 2001 From: Chandra Pratap Date: Fri, 12 Jul 2024 11:08:57 +0530 Subject: [PATCH 1/7] t: move reftable/merged_test.c to the unit testing framework reftable/merged_test.c exercises the functions defined in reftable/merged.{c, h}. Migrate reftable/merged_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 according to unit-tests' naming conventions. Also, move strbuf_add_void() and noop_flush() from reftable/test_framework.c to the ported test. This is because both these functions are used in the merged tests and reftable/test_framework.{c, h} is not #included in the ported test. Mentored-by: Patrick Steinhardt Mentored-by: Christian Couder Signed-off-by: Chandra Pratap Signed-off-by: Junio C Hamano --- Makefile | 2 +- reftable/reftable-tests.h | 1 - t/helper/test-reftable.c | 1 - .../unit-tests/t-reftable-merged.c | 111 +++++++++--------- 4 files changed, 59 insertions(+), 56 deletions(-) rename reftable/merged_test.c => t/unit-tests/t-reftable-merged.c (84%) diff --git a/Makefile b/Makefile index 3eab701b10..e5d1b53991 100644 --- a/Makefile +++ b/Makefile @@ -1340,6 +1340,7 @@ UNIT_TEST_PROGRAMS += t-mem-pool UNIT_TEST_PROGRAMS += t-oidtree UNIT_TEST_PROGRAMS += t-prio-queue UNIT_TEST_PROGRAMS += t-reftable-basics +UNIT_TEST_PROGRAMS += t-reftable-merged UNIT_TEST_PROGRAMS += t-strbuf UNIT_TEST_PROGRAMS += t-strcmp-offset UNIT_TEST_PROGRAMS += t-strvec @@ -2679,7 +2680,6 @@ REFTABLE_OBJS += reftable/writer.o 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 diff --git a/reftable/reftable-tests.h b/reftable/reftable-tests.h index 114cc3d053..d5e03dcc1b 100644 --- a/reftable/reftable-tests.h +++ b/reftable/reftable-tests.h @@ -11,7 +11,6 @@ https://developers.google.com/open-source/licenses/bsd int basics_test_main(int argc, const char **argv); int block_test_main(int argc, const char **argv); -int merged_test_main(int argc, const char **argv); int pq_test_main(int argc, const char **argv); int record_test_main(int argc, const char **argv); int readwrite_test_main(int argc, const char **argv); diff --git a/t/helper/test-reftable.c b/t/helper/test-reftable.c index 9160bc5da6..0357718fa8 100644 --- a/t/helper/test-reftable.c +++ b/t/helper/test-reftable.c @@ -10,7 +10,6 @@ int cmd__reftable(int argc, const char **argv) tree_test_main(argc, argv); pq_test_main(argc, argv); readwrite_test_main(argc, argv); - merged_test_main(argc, argv); stack_test_main(argc, argv); return 0; } diff --git a/reftable/merged_test.c b/t/unit-tests/t-reftable-merged.c similarity index 84% rename from reftable/merged_test.c rename to t/unit-tests/t-reftable-merged.c index a9d6661c13..78a864a54f 100644 --- a/reftable/merged_test.c +++ b/t/unit-tests/t-reftable-merged.c @@ -6,20 +6,25 @@ license that can be found in the LICENSE file or at https://developers.google.com/open-source/licenses/bsd */ -#include "merged.h" +#include "test-lib.h" +#include "reftable/blocksource.h" +#include "reftable/constants.h" +#include "reftable/merged.h" +#include "reftable/reader.h" +#include "reftable/reftable-generic.h" +#include "reftable/reftable-merged.h" +#include "reftable/reftable-writer.h" -#include "system.h" +static ssize_t strbuf_add_void(void *b, const void *data, size_t sz) +{ + strbuf_add(b, data, sz); + return sz; +} -#include "basics.h" -#include "blocksource.h" -#include "constants.h" -#include "reader.h" -#include "record.h" -#include "test_framework.h" -#include "reftable-merged.h" -#include "reftable-tests.h" -#include "reftable-generic.h" -#include "reftable-writer.h" +static int noop_flush(void *arg) +{ + return 0; +} static void write_test_table(struct strbuf *buf, struct reftable_ref_record refs[], int n) @@ -49,12 +54,12 @@ static void write_test_table(struct strbuf *buf, for (i = 0; i < n; i++) { uint64_t before = refs[i].update_index; int n = reftable_writer_add_ref(w, &refs[i]); - EXPECT(n == 0); - EXPECT(before == refs[i].update_index); + check_int(n, ==, 0); + check_int(before, ==, refs[i].update_index); } err = reftable_writer_close(w); - EXPECT_ERR(err); + check(!err); reftable_writer_free(w); } @@ -76,11 +81,11 @@ static void write_test_log_table(struct strbuf *buf, for (i = 0; i < n; i++) { int err = reftable_writer_add_log(w, &logs[i]); - EXPECT_ERR(err); + check(!err); } err = reftable_writer_close(w); - EXPECT_ERR(err); + check(!err); reftable_writer_free(w); } @@ -105,12 +110,12 @@ merged_table_from_records(struct reftable_ref_record **refs, err = reftable_new_reader(&(*readers)[i], &(*source)[i], "name"); - EXPECT_ERR(err); + check(!err); reftable_table_from_reader(&tabs[i], (*readers)[i]); } err = reftable_new_merged_table(&mt, tabs, n, GIT_SHA1_FORMAT_ID); - EXPECT_ERR(err); + check(!err); return mt; } @@ -122,7 +127,7 @@ static void readers_destroy(struct reftable_reader **readers, size_t n) reftable_free(readers); } -static void test_merged_between(void) +static void t_merged_single_record(void) { struct reftable_ref_record r1[] = { { .refname = (char *) "b", @@ -150,11 +155,11 @@ static void test_merged_between(void) merged_table_init_iter(mt, &it, BLOCK_TYPE_REF); err = reftable_iterator_seek_ref(&it, "a"); - EXPECT_ERR(err); + check(!err); err = reftable_iterator_next_ref(&it, &ref); - EXPECT_ERR(err); - EXPECT(ref.update_index == 2); + check(!err); + check_int(ref.update_index, ==, 2); reftable_ref_record_release(&ref); reftable_iterator_destroy(&it); readers_destroy(readers, 2); @@ -165,7 +170,7 @@ static void test_merged_between(void) reftable_free(bs); } -static void test_merged(void) +static void t_merged_refs(void) { struct reftable_ref_record r1[] = { { @@ -230,9 +235,9 @@ static void test_merged(void) merged_table_init_iter(mt, &it, BLOCK_TYPE_REF); err = reftable_iterator_seek_ref(&it, "a"); - EXPECT_ERR(err); - EXPECT(reftable_merged_table_hash_id(mt) == GIT_SHA1_FORMAT_ID); - EXPECT(reftable_merged_table_min_update_index(mt) == 1); + check(!err); + check_int(reftable_merged_table_hash_id(mt), ==, GIT_SHA1_FORMAT_ID); + check_int(reftable_merged_table_min_update_index(mt), ==, 1); while (len < 100) { /* cap loops/recursion. */ struct reftable_ref_record ref = { NULL }; @@ -245,9 +250,9 @@ static void test_merged(void) } reftable_iterator_destroy(&it); - EXPECT(ARRAY_SIZE(want) == len); + check_int(ARRAY_SIZE(want), ==, len); for (i = 0; i < len; i++) { - EXPECT(reftable_ref_record_equal(want[i], &out[i], + check(reftable_ref_record_equal(want[i], &out[i], GIT_SHA1_RAWSZ)); } for (i = 0; i < len; i++) { @@ -283,16 +288,16 @@ merged_table_from_log_records(struct reftable_log_record **logs, err = reftable_new_reader(&(*readers)[i], &(*source)[i], "name"); - EXPECT_ERR(err); + check(!err); reftable_table_from_reader(&tabs[i], (*readers)[i]); } err = reftable_new_merged_table(&mt, tabs, n, GIT_SHA1_FORMAT_ID); - EXPECT_ERR(err); + check(!err); return mt; } -static void test_merged_logs(void) +static void t_merged_logs(void) { struct reftable_log_record r1[] = { { @@ -362,9 +367,9 @@ static void test_merged_logs(void) merged_table_init_iter(mt, &it, BLOCK_TYPE_LOG); err = reftable_iterator_seek_log(&it, "a"); - EXPECT_ERR(err); - EXPECT(reftable_merged_table_hash_id(mt) == GIT_SHA1_FORMAT_ID); - EXPECT(reftable_merged_table_min_update_index(mt) == 1); + check(!err); + check_int(reftable_merged_table_hash_id(mt), ==, GIT_SHA1_FORMAT_ID); + check_int(reftable_merged_table_min_update_index(mt), ==, 1); while (len < 100) { /* cap loops/recursion. */ struct reftable_log_record log = { NULL }; @@ -377,19 +382,19 @@ static void test_merged_logs(void) } reftable_iterator_destroy(&it); - EXPECT(ARRAY_SIZE(want) == len); + check_int(ARRAY_SIZE(want), ==, len); for (i = 0; i < len; i++) { - EXPECT(reftable_log_record_equal(want[i], &out[i], + check(reftable_log_record_equal(want[i], &out[i], GIT_SHA1_RAWSZ)); } merged_table_init_iter(mt, &it, BLOCK_TYPE_LOG); err = reftable_iterator_seek_log_at(&it, "a", 2); - EXPECT_ERR(err); + check(!err); reftable_log_record_release(&out[0]); err = reftable_iterator_next_log(&it, &out[0]); - EXPECT_ERR(err); - EXPECT(reftable_log_record_equal(&out[0], &r3[0], GIT_SHA1_RAWSZ)); + check(!err); + check(reftable_log_record_equal(&out[0], &r3[0], GIT_SHA1_RAWSZ)); reftable_iterator_destroy(&it); for (i = 0; i < len; i++) { @@ -405,7 +410,7 @@ static void test_merged_logs(void) reftable_free(bs); } -static void test_default_write_opts(void) +static void t_default_write_opts(void) { struct reftable_write_options opts = { 0 }; struct strbuf buf = STRBUF_INIT; @@ -426,36 +431,36 @@ static void test_default_write_opts(void) reftable_writer_set_limits(w, 1, 1); err = reftable_writer_add_ref(w, &rec); - EXPECT_ERR(err); + check(!err); err = reftable_writer_close(w); - EXPECT_ERR(err); + check(!err); reftable_writer_free(w); block_source_from_strbuf(&source, &buf); err = reftable_new_reader(&rd, &source, "filename"); - EXPECT_ERR(err); + check(!err); hash_id = reftable_reader_hash_id(rd); - EXPECT(hash_id == GIT_SHA1_FORMAT_ID); + check_int(hash_id, ==, GIT_SHA1_FORMAT_ID); reftable_table_from_reader(&tab[0], rd); err = reftable_new_merged_table(&merged, tab, 1, GIT_SHA1_FORMAT_ID); - EXPECT_ERR(err); + check(!err); reftable_reader_free(rd); reftable_merged_table_free(merged); strbuf_release(&buf); } -/* XXX test refs_for(oid) */ -int merged_test_main(int argc, const char *argv[]) +int cmd_main(int argc, const char *argv[]) { - RUN_TEST(test_merged_logs); - RUN_TEST(test_merged_between); - RUN_TEST(test_merged); - RUN_TEST(test_default_write_opts); - return 0; + TEST(t_default_write_opts(), "merged table with default write opts"); + TEST(t_merged_logs(), "merged table with multiple log updates for same ref"); + TEST(t_merged_refs(), "merged table with multiple updates to same ref"); + TEST(t_merged_single_record(), "ref ocurring in only one record can be fetched"); + + return test_done(); } From e8ed7d1974af3d38e7c6f2ad07fd4ea49ae29474 Mon Sep 17 00:00:00 2001 From: Chandra Pratap Date: Fri, 12 Jul 2024 11:08:58 +0530 Subject: [PATCH 2/7] t: harmonize t-reftable-merged.c with coding guidelines Harmonize the newly ported test unit-tests/t-reftable-merged.c with the following guidelines: - Single line control flow statements like 'for' and 'if' must omit curly braces. - Structs must be 0-initialized with '= { 0 }' instead of '= { NULL }'. - Array indices should preferably be of type 'size_t', not 'int'. - It is fine to use C99 initial declaration in 'for' loop. While at it, use 'ARRAY_SIZE(x)' to store the number of elements in an array instead of hardcoding them. Mentored-by: Patrick Steinhardt Mentored-by: Christian Couder Signed-off-by: Chandra Pratap Signed-off-by: Junio C Hamano --- t/unit-tests/t-reftable-merged.c | 68 +++++++++++++------------------- 1 file changed, 28 insertions(+), 40 deletions(-) diff --git a/t/unit-tests/t-reftable-merged.c b/t/unit-tests/t-reftable-merged.c index 78a864a54f..9791f53418 100644 --- a/t/unit-tests/t-reftable-merged.c +++ b/t/unit-tests/t-reftable-merged.c @@ -27,11 +27,11 @@ static int noop_flush(void *arg) } static void write_test_table(struct strbuf *buf, - struct reftable_ref_record refs[], int n) + struct reftable_ref_record refs[], size_t n) { uint64_t min = 0xffffffff; uint64_t max = 0; - int i = 0; + size_t i; int err; struct reftable_write_options opts = { @@ -40,12 +40,10 @@ static void write_test_table(struct strbuf *buf, struct reftable_writer *w = NULL; for (i = 0; i < n; i++) { uint64_t ui = refs[i].update_index; - if (ui > max) { + if (ui > max) max = ui; - } - if (ui < min) { + if (ui < min) min = ui; - } } w = reftable_new_writer(&strbuf_add_void, &noop_flush, buf, &opts); @@ -65,10 +63,9 @@ static void write_test_table(struct strbuf *buf, } static void write_test_log_table(struct strbuf *buf, - struct reftable_log_record logs[], int n, + struct reftable_log_record logs[], size_t n, uint64_t update_index) { - int i = 0; int err; struct reftable_write_options opts = { @@ -79,7 +76,7 @@ static void write_test_log_table(struct strbuf *buf, w = reftable_new_writer(&strbuf_add_void, &noop_flush, buf, &opts); reftable_writer_set_limits(w, update_index, update_index); - for (i = 0; i < n; i++) { + for (size_t i = 0; i < n; i++) { int err = reftable_writer_add_log(w, &logs[i]); check(!err); } @@ -93,7 +90,7 @@ static void write_test_log_table(struct strbuf *buf, static struct reftable_merged_table * merged_table_from_records(struct reftable_ref_record **refs, struct reftable_block_source **source, - struct reftable_reader ***readers, int *sizes, + struct reftable_reader ***readers, size_t *sizes, struct strbuf *buf, size_t n) { struct reftable_merged_table *mt = NULL; @@ -121,8 +118,7 @@ merged_table_from_records(struct reftable_ref_record **refs, static void readers_destroy(struct reftable_reader **readers, size_t n) { - int i = 0; - for (; i < n; i++) + for (size_t i = 0; i < n; i++) reftable_reader_free(readers[i]); reftable_free(readers); } @@ -142,15 +138,14 @@ static void t_merged_single_record(void) } }; struct reftable_ref_record *refs[] = { r1, r2 }; - int sizes[] = { 1, 1 }; + size_t sizes[] = { ARRAY_SIZE(r1), ARRAY_SIZE(r2) }; struct strbuf bufs[2] = { STRBUF_INIT, STRBUF_INIT }; struct reftable_block_source *bs = NULL; struct reftable_reader **readers = NULL; struct reftable_merged_table *mt = merged_table_from_records(refs, &bs, &readers, sizes, bufs, 2); - int i; - struct reftable_ref_record ref = { NULL }; - struct reftable_iterator it = { NULL }; + struct reftable_ref_record ref = { 0 }; + struct reftable_iterator it = { 0 }; int err; merged_table_init_iter(mt, &it, BLOCK_TYPE_REF); @@ -164,9 +159,8 @@ static void t_merged_single_record(void) reftable_iterator_destroy(&it); readers_destroy(readers, 2); reftable_merged_table_free(mt); - for (i = 0; i < ARRAY_SIZE(bufs); i++) { + for (size_t i = 0; i < ARRAY_SIZE(bufs); i++) strbuf_release(&bufs[i]); - } reftable_free(bs); } @@ -220,18 +214,18 @@ static void t_merged_refs(void) }; struct reftable_ref_record *refs[] = { r1, r2, r3 }; - int sizes[3] = { 3, 1, 2 }; + size_t sizes[3] = { ARRAY_SIZE(r1), ARRAY_SIZE(r2), ARRAY_SIZE(r3) }; struct strbuf bufs[3] = { STRBUF_INIT, STRBUF_INIT, STRBUF_INIT }; struct reftable_block_source *bs = NULL; struct reftable_reader **readers = NULL; struct reftable_merged_table *mt = merged_table_from_records(refs, &bs, &readers, sizes, bufs, 3); - struct reftable_iterator it = { NULL }; + struct reftable_iterator it = { 0 }; int err; struct reftable_ref_record *out = NULL; size_t len = 0; size_t cap = 0; - int i = 0; + size_t i; merged_table_init_iter(mt, &it, BLOCK_TYPE_REF); err = reftable_iterator_seek_ref(&it, "a"); @@ -240,7 +234,7 @@ static void t_merged_refs(void) check_int(reftable_merged_table_min_update_index(mt), ==, 1); while (len < 100) { /* cap loops/recursion. */ - struct reftable_ref_record ref = { NULL }; + struct reftable_ref_record ref = { 0 }; int err = reftable_iterator_next_ref(&it, &ref); if (err > 0) break; @@ -251,18 +245,15 @@ static void t_merged_refs(void) reftable_iterator_destroy(&it); check_int(ARRAY_SIZE(want), ==, len); - for (i = 0; i < len; i++) { + for (i = 0; i < len; i++) check(reftable_ref_record_equal(want[i], &out[i], GIT_SHA1_RAWSZ)); - } - for (i = 0; i < len; i++) { + for (i = 0; i < len; i++) reftable_ref_record_release(&out[i]); - } reftable_free(out); - for (i = 0; i < 3; i++) { + for (i = 0; i < 3; i++) strbuf_release(&bufs[i]); - } readers_destroy(readers, 3); reftable_merged_table_free(mt); reftable_free(bs); @@ -271,7 +262,7 @@ static void t_merged_refs(void) static struct reftable_merged_table * merged_table_from_log_records(struct reftable_log_record **logs, struct reftable_block_source **source, - struct reftable_reader ***readers, int *sizes, + struct reftable_reader ***readers, size_t *sizes, struct strbuf *buf, size_t n) { struct reftable_merged_table *mt = NULL; @@ -352,18 +343,18 @@ static void t_merged_logs(void) }; struct reftable_log_record *logs[] = { r1, r2, r3 }; - int sizes[3] = { 2, 1, 1 }; + size_t sizes[3] = { ARRAY_SIZE(r1), ARRAY_SIZE(r2), ARRAY_SIZE(r3) }; struct strbuf bufs[3] = { STRBUF_INIT, STRBUF_INIT, STRBUF_INIT }; struct reftable_block_source *bs = NULL; struct reftable_reader **readers = NULL; struct reftable_merged_table *mt = merged_table_from_log_records( logs, &bs, &readers, sizes, bufs, 3); - struct reftable_iterator it = { NULL }; + struct reftable_iterator it = { 0 }; int err; struct reftable_log_record *out = NULL; size_t len = 0; size_t cap = 0; - int i = 0; + size_t i; merged_table_init_iter(mt, &it, BLOCK_TYPE_LOG); err = reftable_iterator_seek_log(&it, "a"); @@ -372,7 +363,7 @@ static void t_merged_logs(void) check_int(reftable_merged_table_min_update_index(mt), ==, 1); while (len < 100) { /* cap loops/recursion. */ - struct reftable_log_record log = { NULL }; + struct reftable_log_record log = { 0 }; int err = reftable_iterator_next_log(&it, &log); if (err > 0) break; @@ -383,10 +374,9 @@ static void t_merged_logs(void) reftable_iterator_destroy(&it); check_int(ARRAY_SIZE(want), ==, len); - for (i = 0; i < len; i++) { + for (i = 0; i < len; i++) check(reftable_log_record_equal(want[i], &out[i], GIT_SHA1_RAWSZ)); - } merged_table_init_iter(mt, &it, BLOCK_TYPE_LOG); err = reftable_iterator_seek_log_at(&it, "a", 2); @@ -397,14 +387,12 @@ static void t_merged_logs(void) check(reftable_log_record_equal(&out[0], &r3[0], GIT_SHA1_RAWSZ)); reftable_iterator_destroy(&it); - for (i = 0; i < len; i++) { + for (i = 0; i < len; i++) reftable_log_record_release(&out[i]); - } reftable_free(out); - for (i = 0; i < 3; i++) { + for (i = 0; i < 3; i++) strbuf_release(&bufs[i]); - } readers_destroy(readers, 3); reftable_merged_table_free(mt); reftable_free(bs); @@ -422,7 +410,7 @@ static void t_default_write_opts(void) .update_index = 1, }; int err; - struct reftable_block_source source = { NULL }; + struct reftable_block_source source = { 0 }; struct reftable_table *tab = reftable_calloc(1, sizeof(*tab)); uint32_t hash_id; struct reftable_reader *rd = NULL; From c755c2f3519075028047b092dd26706ac75d1aff Mon Sep 17 00:00:00 2001 From: Chandra Pratap Date: Fri, 12 Jul 2024 11:08:59 +0530 Subject: [PATCH 3/7] t-reftable-merged: improve the test t_merged_single_record() In t-reftable-merged.c, the test t_merged_single_record() ensures that a ref ('a') which occurs in only one of the records ('r2') can be retrieved. Improve this test by adding another record 'r3' to ensure that ref 'a' only occurs in 'r2' and that merged tables don't simply read the last record. Mentored-by: Patrick Steinhardt Mentored-by: Christian Couder Signed-off-by: Chandra Pratap Signed-off-by: Junio C Hamano --- t/unit-tests/t-reftable-merged.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/t/unit-tests/t-reftable-merged.c b/t/unit-tests/t-reftable-merged.c index 9791f53418..f4c14c5d47 100644 --- a/t/unit-tests/t-reftable-merged.c +++ b/t/unit-tests/t-reftable-merged.c @@ -136,14 +136,19 @@ static void t_merged_single_record(void) .update_index = 2, .value_type = REFTABLE_REF_DELETION, } }; + struct reftable_ref_record r3[] = { { + .refname = (char *) "c", + .update_index = 3, + .value_type = REFTABLE_REF_DELETION, + } }; - struct reftable_ref_record *refs[] = { r1, r2 }; - size_t sizes[] = { ARRAY_SIZE(r1), ARRAY_SIZE(r2) }; - struct strbuf bufs[2] = { STRBUF_INIT, STRBUF_INIT }; + struct reftable_ref_record *refs[] = { r1, r2, r3 }; + size_t sizes[] = { ARRAY_SIZE(r1), ARRAY_SIZE(r2), ARRAY_SIZE(r3) }; + struct strbuf bufs[3] = { STRBUF_INIT, STRBUF_INIT, STRBUF_INIT }; struct reftable_block_source *bs = NULL; struct reftable_reader **readers = NULL; struct reftable_merged_table *mt = - merged_table_from_records(refs, &bs, &readers, sizes, bufs, 2); + merged_table_from_records(refs, &bs, &readers, sizes, bufs, 3); struct reftable_ref_record ref = { 0 }; struct reftable_iterator it = { 0 }; int err; @@ -157,7 +162,7 @@ static void t_merged_single_record(void) check_int(ref.update_index, ==, 2); reftable_ref_record_release(&ref); reftable_iterator_destroy(&it); - readers_destroy(readers, 2); + readers_destroy(readers, 3); reftable_merged_table_free(mt); for (size_t i = 0; i < ARRAY_SIZE(bufs); i++) strbuf_release(&bufs[i]); From 8d4f8165d88eb43a4fda8af519936fa194acfbb8 Mon Sep 17 00:00:00 2001 From: Chandra Pratap Date: Fri, 12 Jul 2024 11:09:00 +0530 Subject: [PATCH 4/7] t-reftable-merged: improve the const-correctness of helper functions In t-reftable-merged.c, a number of helper functions used by the tests can be re-defined with parameters made 'const' which makes it easier to understand if they're read-only or not. Re-define these functions along these lines. Mentored-by: Patrick Steinhardt Mentored-by: Christian Couder Signed-off-by: Chandra Pratap Signed-off-by: Junio C Hamano --- t/unit-tests/t-reftable-merged.c | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/t/unit-tests/t-reftable-merged.c b/t/unit-tests/t-reftable-merged.c index f4c14c5d47..ff2f448bb6 100644 --- a/t/unit-tests/t-reftable-merged.c +++ b/t/unit-tests/t-reftable-merged.c @@ -15,7 +15,7 @@ https://developers.google.com/open-source/licenses/bsd #include "reftable/reftable-merged.h" #include "reftable/reftable-writer.h" -static ssize_t strbuf_add_void(void *b, const void *data, size_t sz) +static ssize_t strbuf_add_void(void *b, const void *data, const size_t sz) { strbuf_add(b, data, sz); return sz; @@ -27,7 +27,7 @@ static int noop_flush(void *arg) } static void write_test_table(struct strbuf *buf, - struct reftable_ref_record refs[], size_t n) + struct reftable_ref_record refs[], const size_t n) { uint64_t min = 0xffffffff; uint64_t max = 0; @@ -62,9 +62,8 @@ static void write_test_table(struct strbuf *buf, reftable_writer_free(w); } -static void write_test_log_table(struct strbuf *buf, - struct reftable_log_record logs[], size_t n, - uint64_t update_index) +static void write_test_log_table(struct strbuf *buf, struct reftable_log_record logs[], + const size_t n, const uint64_t update_index) { int err; @@ -90,8 +89,8 @@ static void write_test_log_table(struct strbuf *buf, static struct reftable_merged_table * merged_table_from_records(struct reftable_ref_record **refs, struct reftable_block_source **source, - struct reftable_reader ***readers, size_t *sizes, - struct strbuf *buf, size_t n) + struct reftable_reader ***readers, const size_t *sizes, + struct strbuf *buf, const size_t n) { struct reftable_merged_table *mt = NULL; struct reftable_table *tabs; @@ -116,7 +115,7 @@ merged_table_from_records(struct reftable_ref_record **refs, return mt; } -static void readers_destroy(struct reftable_reader **readers, size_t n) +static void readers_destroy(struct reftable_reader **readers, const size_t n) { for (size_t i = 0; i < n; i++) reftable_reader_free(readers[i]); @@ -267,8 +266,8 @@ static void t_merged_refs(void) static struct reftable_merged_table * merged_table_from_log_records(struct reftable_log_record **logs, struct reftable_block_source **source, - struct reftable_reader ***readers, size_t *sizes, - struct strbuf *buf, size_t n) + struct reftable_reader ***readers, const size_t *sizes, + struct strbuf *buf, const size_t n) { struct reftable_merged_table *mt = NULL; struct reftable_table *tabs; From 84958ec754bb1bc311cda92bfd67e86fcff9e996 Mon Sep 17 00:00:00 2001 From: Chandra Pratap Date: Fri, 12 Jul 2024 11:09:01 +0530 Subject: [PATCH 5/7] t-reftable-merged: add tests for reftable_merged_table_max_update_index reftable_merged_table_max_update_index() as defined by reftable/ merged.{c, h} returns the maximum update index in a merged table. Since this function is currently unexercised, add tests for it. Mentored-by: Patrick Steinhardt Mentored-by: Christian Couder Signed-off-by: Chandra Pratap Signed-off-by: Junio C Hamano --- t/unit-tests/t-reftable-merged.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/t/unit-tests/t-reftable-merged.c b/t/unit-tests/t-reftable-merged.c index ff2f448bb6..065b359200 100644 --- a/t/unit-tests/t-reftable-merged.c +++ b/t/unit-tests/t-reftable-merged.c @@ -236,6 +236,7 @@ static void t_merged_refs(void) check(!err); check_int(reftable_merged_table_hash_id(mt), ==, GIT_SHA1_FORMAT_ID); check_int(reftable_merged_table_min_update_index(mt), ==, 1); + check_int(reftable_merged_table_max_update_index(mt), ==, 3); while (len < 100) { /* cap loops/recursion. */ struct reftable_ref_record ref = { 0 }; @@ -365,6 +366,7 @@ static void t_merged_logs(void) check(!err); check_int(reftable_merged_table_hash_id(mt), ==, GIT_SHA1_FORMAT_ID); check_int(reftable_merged_table_min_update_index(mt), ==, 1); + check_int(reftable_merged_table_max_update_index(mt), ==, 3); while (len < 100) { /* cap loops/recursion. */ struct reftable_log_record log = { 0 }; From 40c80eab83f3f7eae4be14cccf756c0a89931b59 Mon Sep 17 00:00:00 2001 From: Chandra Pratap Date: Fri, 12 Jul 2024 11:09:02 +0530 Subject: [PATCH 6/7] t-reftable-merged: use reftable_ref_record_equal to compare ref records In the test t_merged_single_record() defined in t-reftable-merged.c, the 'input' and 'expected' ref records are checked for equality by comparing their update indices. It is very much possible for two different ref records to have the same update indices. Use reftable_ref_record_equal() instead for a stronger check. Mentored-by: Patrick Steinhardt Mentored-by: Christian Couder Signed-off-by: Chandra Pratap Signed-off-by: Junio C Hamano --- t/unit-tests/t-reftable-merged.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/unit-tests/t-reftable-merged.c b/t/unit-tests/t-reftable-merged.c index 065b359200..9f9275f871 100644 --- a/t/unit-tests/t-reftable-merged.c +++ b/t/unit-tests/t-reftable-merged.c @@ -158,7 +158,7 @@ static void t_merged_single_record(void) err = reftable_iterator_next_ref(&it, &ref); check(!err); - check_int(ref.update_index, ==, 2); + check(reftable_ref_record_equal(&r2[0], &ref, GIT_SHA1_RAWSZ)); reftable_ref_record_release(&ref); reftable_iterator_destroy(&it); readers_destroy(readers, 3); From 9a1fb8af9887c99420f41a36667e4a7905d6266b Mon Sep 17 00:00:00 2001 From: Chandra Pratap Date: Fri, 12 Jul 2024 11:09:03 +0530 Subject: [PATCH 7/7] t-reftable-merged: add test for REFTABLE_FORMAT_ERROR When calling reftable_new_merged_table(), if the hash ID of the passed reftable_table parameter doesn't match the passed hash_id parameter, a REFTABLE_FORMAT_ERROR is thrown. This case is currently left unexercised, so add a test for the same. Mentored-by: Patrick Steinhardt Mentored-by: Christian Couder Signed-off-by: Chandra Pratap Signed-off-by: Junio C Hamano --- t/unit-tests/t-reftable-merged.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/t/unit-tests/t-reftable-merged.c b/t/unit-tests/t-reftable-merged.c index 9f9275f871..b6263ee8b5 100644 --- a/t/unit-tests/t-reftable-merged.c +++ b/t/unit-tests/t-reftable-merged.c @@ -11,6 +11,7 @@ https://developers.google.com/open-source/licenses/bsd #include "reftable/constants.h" #include "reftable/merged.h" #include "reftable/reader.h" +#include "reftable/reftable-error.h" #include "reftable/reftable-generic.h" #include "reftable/reftable-merged.h" #include "reftable/reftable-writer.h" @@ -440,6 +441,8 @@ static void t_default_write_opts(void) check_int(hash_id, ==, GIT_SHA1_FORMAT_ID); reftable_table_from_reader(&tab[0], rd); + err = reftable_new_merged_table(&merged, tab, 1, GIT_SHA256_FORMAT_ID); + check_int(err, ==, REFTABLE_FORMAT_ERROR); err = reftable_new_merged_table(&merged, tab, 1, GIT_SHA1_FORMAT_ID); check(!err);