{"thread":{"id":"61730","subject":"[GSoC][PATCH 0/5] t: port reftable/merged_test.c to the unit testing framework","startedAt":"2024-07-03T17:12:13Z","lastAt":"2024-07-24T09:12:10Z","messageCount":41,"participants":["Chandra Pratap","Karthik Nayak","Justin Tobler","Junio C Hamano","Patrick Steinhardt"],"isPatch":true,"patchVersion":1,"patchTotal":5},"messages":[{"id":"498043","messageId":"20240703171131.3929-1-chandrapratap3519@gmail.com","threadId":"61730","inReplyTo":null,"subject":"[GSoC][PATCH 0/5] t: port reftable/merged_test.c to the unit testing framework","fromName":"Chandra Pratap","fromEmail":"chandrapratap3519@gmail.com","sentAt":"2024-07-03T17:01:40Z","receivedAt":"2024-07-03T17:12:13Z","isPatch":true,"sender":{"key":"chandrapratap3519@gmail.com","avatar":null},"body":"In the recent codebase update (commit 8bf6fbd, 2023-12-09), a new unit\ntesting framework written entirely in C was introduced to the Git project\naimed at simplifying testing and reducing test run times.\nCurrently, tests for the reftable refs-backend are performed by a custom\ntesting framework defined by reftable/test_framework.{c, h}. Port\nreftable/merged_test.c to the unit testing framework and improve upon\nthe ported test.\n\nThe first patch in the series moves the test to the unit testing framework,\nand the rest of the patches improve upon the ported test.\n\nMentored-by: Patrick Steinhardt \nMentored-by: Christian Couder \nSigned-off-by: Chandra Pratap \n\n---\nCI/PR: https://github.com/gitgitgadget/git/pull/1755\n\nChandra Pratap (5):\n[PATCH 1/5] t: move reftable/merged_test.c to the unit testing framework\n[PATCH 2/5] t: harmonize t-reftable-merged.c with coding guidelines\n[PATCH 3/5] t-reftable-merged: add test for reftable_merged_table_max_update_index()\n[PATCH 4/5] t-reftable-merged: use reftable_ref_record_equal to compare ref records\n[PATCH 5/5] t-reftable-merged: add test for REFTABLE_FORMAT_ERROR\n\nMakefile | 2 +-\nt/helper/test-reftable.c | 1 -\nreftable/merged_test.c => t/unit-tests/t-reftable-merged.c | 170 +++++++++++++++----------------\n3 files changed, 86 insertions(+), 87 deletions(-)\n"},{"id":"498044","messageId":"20240703171131.3929-2-chandrapratap3519@gmail.com","threadId":"61730","inReplyTo":"20240703171131.3929-1-chandrapratap3519@gmail.com","subject":"[PATCH 1/5] t: move reftable/merged_test.c to the unit testing framework","fromName":"Chandra Pratap","fromEmail":"chandrapratap3519@gmail.com","sentAt":"2024-07-03T17:01:41Z","receivedAt":"2024-07-03T17:12:16Z","isPatch":true,"sender":{"key":"chandrapratap3519@gmail.com","avatar":null},"body":"reftable/merged_test.c exercises the functions defined in\nreftable/merged.{c, h}. Migrate reftable/merged_test.c to the unit\ntesting framework. Migration involves refactoring the tests\nto use the unit testing framework instead of reftable's test\nframework and renaming the tests according to unit-tests' naming\nconventions.\n\nAlso, move strbuf_add_void() and noop_flush() from\nreftable/test_framework.c to the ported test. This is because\nboth these functions are used in the merged tests and\nreftable/test_framework.{c, h} is not #included in the ported test.\n\nMentored-by: Patrick Steinhardt \nMentored-by: Christian Couder \nSigned-off-by: Chandra Pratap \n---\n Makefile | 2 +-\n t/helper/test-reftable.c | 1 -\n .../unit-tests/t-reftable-merged.c | 112 +++++++++---------\n 3 files changed, 60 insertions(+), 55 deletions(-)\n rename reftable/merged_test.c => t/unit-tests/t-reftable-merged.c (84%)\n\ndiff --git a/Makefile b/Makefile\nindex 3eab701b10..e5d1b53991 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1340,6 +1340,7 @@ UNIT_TEST_PROGRAMS += t-mem-pool\n UNIT_TEST_PROGRAMS += t-oidtree\n UNIT_TEST_PROGRAMS += t-prio-queue\n UNIT_TEST_PROGRAMS += t-reftable-basics\n+UNIT_TEST_PROGRAMS += t-reftable-merged\n UNIT_TEST_PROGRAMS += t-strbuf\n UNIT_TEST_PROGRAMS += t-strcmp-offset\n UNIT_TEST_PROGRAMS += t-strvec\n@@ -2679,7 +2680,6 @@ REFTABLE_OBJS += reftable/writer.o\n \n REFTABLE_TEST_OBJS += reftable/block_test.o\n REFTABLE_TEST_OBJS += reftable/dump.o\n-REFTABLE_TEST_OBJS += reftable/merged_test.o\n REFTABLE_TEST_OBJS += reftable/pq_test.o\n REFTABLE_TEST_OBJS += reftable/record_test.o\n REFTABLE_TEST_OBJS += reftable/readwrite_test.o\ndiff --git a/t/helper/test-reftable.c b/t/helper/test-reftable.c\nindex 9160bc5da6..0357718fa8 100644\n--- a/t/helper/test-reftable.c\n+++ b/t/helper/test-reftable.c\n@@ -10,7 +10,6 @@ int cmd__reftable(int argc, const char **argv)\n \ttree_test_main(argc, argv);\n \tpq_test_main(argc, argv);\n \treadwrite_test_main(argc, argv);\n-\tmerged_test_main(argc, argv);\n \tstack_test_main(argc, argv);\n \treturn 0;\n }\ndiff --git a/reftable/merged_test.c b/t/unit-tests/t-reftable-merged.c\nsimilarity index 84%\nrename from reftable/merged_test.c\nrename to t/unit-tests/t-reftable-merged.c\nindex a9d6661c13..1718489f06 100644\n--- a/reftable/merged_test.c\n+++ b/t/unit-tests/t-reftable-merged.c\n@@ -6,20 +6,25 @@ license that can be found in the LICENSE file or at\n https://developers.google.com/open-source/licenses/bsd\n */\n \n-#include \"merged.h\"\n-\n-#include \"system.h\"\n+#include \"test-lib.h\"\n+#include \"reftable/blocksource.h\"\n+#include \"reftable/constants.h\"\n+#include \"reftable/merged.h\"\n+#include \"reftable/reader.h\"\n+#include \"reftable/reftable-generic.h\"\n+#include \"reftable/reftable-merged.h\"\n+#include \"reftable/reftable-writer.h\"\n+\n+static ssize_t strbuf_add_void(void *b, const void *data, size_t sz)\n+{\n+\tstrbuf_add(b, data, sz);\n+\treturn sz;\n+}\n \n-#include \"basics.h\"\n-#include \"blocksource.h\"\n-#include \"constants.h\"\n-#include \"reader.h\"\n-#include \"record.h\"\n-#include \"test_framework.h\"\n-#include \"reftable-merged.h\"\n-#include \"reftable-tests.h\"\n-#include \"reftable-generic.h\"\n-#include \"reftable-writer.h\"\n+static int noop_flush(void *arg)\n+{\n+\treturn 0;\n+}\n \n static void write_test_table(struct strbuf *buf,\n \t\t\t struct reftable_ref_record refs[], int n)\n@@ -49,12 +54,12 @@ static void write_test_table(struct strbuf *buf,\n \tfor (i = 0; i < n; i++) {\n \t\tuint64_t before = refs[i].update_index;\n \t\tint n = reftable_writer_add_ref(w, &refs[i]);\n-\t\tEXPECT(n == 0);\n-\t\tEXPECT(before == refs[i].update_index);\n+\t\tcheck_int(n, ==, 0);\n+\t\tcheck_int(before, ==, refs[i].update_index);\n \t}\n \n \terr = reftable_writer_close(w);\n-\tEXPECT_ERR(err);\n+\tcheck(!err);\n \n \treftable_writer_free(w);\n }\n@@ -76,11 +81,11 @@ static void write_test_log_table(struct strbuf *buf,\n \n \tfor (i = 0; i < n; i++) {\n \t\tint err = reftable_writer_add_log(w, &logs[i]);\n-\t\tEXPECT_ERR(err);\n+\t\tcheck(!err);\n \t}\n \n \terr = reftable_writer_close(w);\n-\tEXPECT_ERR(err);\n+\tcheck(!err);\n \n \treftable_writer_free(w);\n }\n@@ -105,12 +110,12 @@ merged_table_from_records(struct reftable_ref_record **refs,\n \n \t\terr = reftable_new_reader(&(*readers)[i], &(*source)[i],\n \t\t\t\t\t \"name\");\n-\t\tEXPECT_ERR(err);\n+\t\tcheck(!err);\n \t\treftable_table_from_reader(&tabs[i], (*readers)[i]);\n \t}\n \n \terr = reftable_new_merged_table(&mt, tabs, n, GIT_SHA1_FORMAT_ID);\n-\tEXPECT_ERR(err);\n+\tcheck(!err);\n \treturn mt;\n }\n \n@@ -122,7 +127,7 @@ static void readers_destroy(struct reftable_reader **readers, size_t n)\n \treftable_free(readers);\n }\n \n-static void test_merged_between(void)\n+static void t_merged_between(void)\n {\n \tstruct reftable_ref_record r1[] = { {\n \t\t.refname = (char *) \"b\",\n@@ -150,11 +155,11 @@ static void test_merged_between(void)\n \n \tmerged_table_init_iter(mt, &it, BLOCK_TYPE_REF);\n \terr = reftable_iterator_seek_ref(&it, \"a\");\n-\tEXPECT_ERR(err);\n+\tcheck(!err);\n \n \terr = reftable_iterator_next_ref(&it, &ref);\n-\tEXPECT_ERR(err);\n-\tEXPECT(ref.update_index == 2);\n+\tcheck(!err);\n+\tcheck_int(ref.update_index, ==, 2);\n \treftable_ref_record_release(&ref);\n \treftable_iterator_destroy(&it);\n \treaders_destroy(readers, 2);\n@@ -165,7 +170,7 @@ static void test_merged_between(void)\n \treftable_free(bs);\n }\n \n-static void test_merged(void)\n+static void t_merged(void)\n {\n \tstruct reftable_ref_record r1[] = {\n \t\t{\n@@ -230,9 +235,9 @@ static void test_merged(void)\n \n \tmerged_table_init_iter(mt, &it, BLOCK_TYPE_REF);\n \terr = reftable_iterator_seek_ref(&it, \"a\");\n-\tEXPECT_ERR(err);\n-\tEXPECT(reftable_merged_table_hash_id(mt) == GIT_SHA1_FORMAT_ID);\n-\tEXPECT(reftable_merged_table_min_update_index(mt) == 1);\n+\tcheck(!err);\n+\tcheck_int(reftable_merged_table_hash_id(mt), ==, GIT_SHA1_FORMAT_ID);\n+\tcheck_int(reftable_merged_table_min_update_index(mt), ==, 1);\n \n \twhile (len < 100) { /* cap loops/recursion. */\n \t\tstruct reftable_ref_record ref = { NULL };\n@@ -245,9 +250,9 @@ static void test_merged(void)\n \t}\n \treftable_iterator_destroy(&it);\n \n-\tEXPECT(ARRAY_SIZE(want) == len);\n+\tcheck_int(ARRAY_SIZE(want), ==, len);\n \tfor (i = 0; i < len; i++) {\n-\t\tEXPECT(reftable_ref_record_equal(want[i], &out[i],\n+\t\tcheck(reftable_ref_record_equal(want[i], &out[i],\n \t\t\t\t\t\t GIT_SHA1_RAWSZ));\n \t}\n \tfor (i = 0; i < len; i++) {\n@@ -283,16 +288,16 @@ merged_table_from_log_records(struct reftable_log_record **logs,\n \n \t\terr = reftable_new_reader(&(*readers)[i], &(*source)[i],\n \t\t\t\t\t \"name\");\n-\t\tEXPECT_ERR(err);\n+\t\tcheck(!err);\n \t\treftable_table_from_reader(&tabs[i], (*readers)[i]);\n \t}\n \n \terr = reftable_new_merged_table(&mt, tabs, n, GIT_SHA1_FORMAT_ID);\n-\tEXPECT_ERR(err);\n+\tcheck(!err);\n \treturn mt;\n }\n \n-static void test_merged_logs(void)\n+static void t_merged_logs(void)\n {\n \tstruct reftable_log_record r1[] = {\n \t\t{\n@@ -362,9 +367,9 @@ static void test_merged_logs(void)\n \n \tmerged_table_init_iter(mt, &it, BLOCK_TYPE_LOG);\n \terr = reftable_iterator_seek_log(&it, \"a\");\n-\tEXPECT_ERR(err);\n-\tEXPECT(reftable_merged_table_hash_id(mt) == GIT_SHA1_FORMAT_ID);\n-\tEXPECT(reftable_merged_table_min_update_index(mt) == 1);\n+\tcheck(!err);\n+\tcheck_int(reftable_merged_table_hash_id(mt), ==, GIT_SHA1_FORMAT_ID);\n+\tcheck_int(reftable_merged_table_min_update_index(mt), ==, 1);\n \n \twhile (len < 100) { /* cap loops/recursion. */\n \t\tstruct reftable_log_record log = { NULL };\n@@ -377,19 +382,19 @@ static void test_merged_logs(void)\n \t}\n \treftable_iterator_destroy(&it);\n \n-\tEXPECT(ARRAY_SIZE(want) == len);\n+\tcheck_int(ARRAY_SIZE(want), ==, len);\n \tfor (i = 0; i < len; i++) {\n-\t\tEXPECT(reftable_log_record_equal(want[i], &out[i],\n+\t\tcheck(reftable_log_record_equal(want[i], &out[i],\n \t\t\t\t\t\t GIT_SHA1_RAWSZ));\n \t}\n \n \tmerged_table_init_iter(mt, &it, BLOCK_TYPE_LOG);\n \terr = reftable_iterator_seek_log_at(&it, \"a\", 2);\n-\tEXPECT_ERR(err);\n+\tcheck(!err);\n \treftable_log_record_release(&out[0]);\n \terr = reftable_iterator_next_log(&it, &out[0]);\n-\tEXPECT_ERR(err);\n-\tEXPECT(reftable_log_record_equal(&out[0], &r3[0], GIT_SHA1_RAWSZ));\n+\tcheck(!err);\n+\tcheck(reftable_log_record_equal(&out[0], &r3[0], GIT_SHA1_RAWSZ));\n \treftable_iterator_destroy(&it);\n \n \tfor (i = 0; i < len; i++) {\n@@ -405,7 +410,7 @@ static void test_merged_logs(void)\n \treftable_free(bs);\n }\n \n-static void test_default_write_opts(void)\n+static void t_default_write_opts(void)\n {\n \tstruct reftable_write_options opts = { 0 };\n \tstruct strbuf buf = STRBUF_INIT;\n@@ -426,23 +431,23 @@ static void test_default_write_opts(void)\n \treftable_writer_set_limits(w, 1, 1);\n \n \terr = reftable_writer_add_ref(w, &rec);\n-\tEXPECT_ERR(err);\n+\tcheck(!err);\n \n \terr = reftable_writer_close(w);\n-\tEXPECT_ERR(err);\n+\tcheck(!err);\n \treftable_writer_free(w);\n \n \tblock_source_from_strbuf(&source, &buf);\n \n \terr = reftable_new_reader(&rd, &source, \"filename\");\n-\tEXPECT_ERR(err);\n+\tcheck(!err);\n \n \thash_id = reftable_reader_hash_id(rd);\n-\tEXPECT(hash_id == GIT_SHA1_FORMAT_ID);\n+\tcheck_int(hash_id, ==, GIT_SHA1_FORMAT_ID);\n \n \treftable_table_from_reader(&tab[0], rd);\n \terr = reftable_new_merged_table(&merged, tab, 1, GIT_SHA1_FORMAT_ID);\n-\tEXPECT_ERR(err);\n+\tcheck(!err);\n \n \treftable_reader_free(rd);\n \treftable_merged_table_free(merged);\n@@ -451,11 +456,12 @@ static void test_default_write_opts(void)\n \n /* XXX test refs_for(oid) */\n \n-int merged_test_main(int argc, const char *argv[])\n+int cmd_main(int argc, const char *argv[])\n {\n-\tRUN_TEST(test_merged_logs);\n-\tRUN_TEST(test_merged_between);\n-\tRUN_TEST(test_merged);\n-\tRUN_TEST(test_default_write_opts);\n-\treturn 0;\n+\tTEST(t_merged_logs(), \"merged table with log records\");\n+\tTEST(t_merged_between(), \"seek ref in a merged table\");\n+\tTEST(t_merged(), \"merged table with multiple updates to same ref\");\n+\tTEST(t_default_write_opts(), \"merged table with default write opts\");\n+\n+\treturn test_done();\n }\n-- \n2.45.2.404.g9eaef5822c\n\n"},{"id":"498045","messageId":"20240703171131.3929-3-chandrapratap3519@gmail.com","threadId":"61730","inReplyTo":"20240703171131.3929-1-chandrapratap3519@gmail.com","subject":"[PATCH 2/5] t: harmonize t-reftable-merged.c with coding guidelines","fromName":"Chandra Pratap","fromEmail":"chandrapratap3519@gmail.com","sentAt":"2024-07-03T17:01:42Z","receivedAt":"2024-07-03T17:12:18Z","isPatch":true,"sender":{"key":"chandrapratap3519@gmail.com","avatar":null},"body":"Harmonize the newly ported test unit-tests/t-reftable-merged.c\nwith the following guidelines:\n- Single line control flow statements like 'for' and 'if'\n must omit curly braces.\n- Structs must be 0-initialized with '= { 0 }' instead of '= { NULL }'.\n- Array indices must be of type 'size_t', not 'int'.\n- It is fine to use C99 initial declaration in 'for' loop.\n\nMentored-by: Patrick Steinhardt \nMentored-by: Christian Couder \nSigned-off-by: Chandra Pratap \n---\n t/unit-tests/t-reftable-merged.c | 52 ++++++++++++--------------------\n 1 file changed, 20 insertions(+), 32 deletions(-)\n\ndiff --git a/t/unit-tests/t-reftable-merged.c b/t/unit-tests/t-reftable-merged.c\nindex 1718489f06..28bf6f6696 100644\n--- a/t/unit-tests/t-reftable-merged.c\n+++ b/t/unit-tests/t-reftable-merged.c\n@@ -40,12 +40,10 @@ static void write_test_table(struct strbuf *buf,\n \tstruct reftable_writer *w = NULL;\n \tfor (i = 0; i < n; i++) {\n \t\tuint64_t ui = refs[i].update_index;\n-\t\tif (ui > max) {\n+\t\tif (ui > max)\n \t\t\tmax = ui;\n-\t\t}\n-\t\tif (ui < min) {\n+\t\tif (ui < min)\n \t\t\tmin = ui;\n-\t\t}\n \t}\n \n \tw = reftable_new_writer(&strbuf_add_void, &noop_flush, buf, &opts);\n@@ -68,7 +66,6 @@ static void write_test_log_table(struct strbuf *buf,\n \t\t\t\t struct reftable_log_record logs[], int n,\n \t\t\t\t uint64_t update_index)\n {\n-\tint i = 0;\n \tint err;\n \n \tstruct reftable_write_options opts = {\n@@ -79,7 +76,7 @@ static void write_test_log_table(struct strbuf *buf,\n \tw = reftable_new_writer(&strbuf_add_void, &noop_flush, buf, &opts);\n \treftable_writer_set_limits(w, update_index, update_index);\n \n-\tfor (i = 0; i < n; i++) {\n+\tfor (int i = 0; i < n; i++) {\n \t\tint err = reftable_writer_add_log(w, &logs[i]);\n \t\tcheck(!err);\n \t}\n@@ -121,8 +118,7 @@ merged_table_from_records(struct reftable_ref_record **refs,\n \n static void readers_destroy(struct reftable_reader **readers, size_t n)\n {\n-\tint i = 0;\n-\tfor (; i < n; i++)\n+\tfor (size_t i = 0; i < n; i++)\n \t\treftable_reader_free(readers[i]);\n \treftable_free(readers);\n }\n@@ -148,9 +144,8 @@ static void t_merged_between(void)\n \tstruct reftable_reader **readers = NULL;\n \tstruct reftable_merged_table *mt =\n \t\tmerged_table_from_records(refs, &bs, &readers, sizes, bufs, 2);\n-\tint i;\n-\tstruct reftable_ref_record ref = { NULL };\n-\tstruct reftable_iterator it = { NULL };\n+\tstruct reftable_ref_record ref = { 0 };\n+\tstruct reftable_iterator it = { 0 };\n \tint err;\n \n \tmerged_table_init_iter(mt, &it, BLOCK_TYPE_REF);\n@@ -164,9 +159,8 @@ static void t_merged_between(void)\n \treftable_iterator_destroy(&it);\n \treaders_destroy(readers, 2);\n \treftable_merged_table_free(mt);\n-\tfor (i = 0; i < ARRAY_SIZE(bufs); i++) {\n+\tfor (size_t i = 0; i < ARRAY_SIZE(bufs); i++)\n \t\tstrbuf_release(&bufs[i]);\n-\t}\n \treftable_free(bs);\n }\n \n@@ -226,12 +220,12 @@ static void t_merged(void)\n \tstruct reftable_reader **readers = NULL;\n \tstruct reftable_merged_table *mt =\n \t\tmerged_table_from_records(refs, &bs, &readers, sizes, bufs, 3);\n-\tstruct reftable_iterator it = { NULL };\n+\tstruct reftable_iterator it = { 0 };\n \tint err;\n \tstruct reftable_ref_record *out = NULL;\n \tsize_t len = 0;\n \tsize_t cap = 0;\n-\tint i = 0;\n+\tsize_t i;\n \n \tmerged_table_init_iter(mt, &it, BLOCK_TYPE_REF);\n \terr = reftable_iterator_seek_ref(&it, \"a\");\n@@ -240,7 +234,7 @@ static void t_merged(void)\n \tcheck_int(reftable_merged_table_min_update_index(mt), ==, 1);\n \n \twhile (len < 100) { /* cap loops/recursion. */\n-\t\tstruct reftable_ref_record ref = { NULL };\n+\t\tstruct reftable_ref_record ref = { 0 };\n \t\tint err = reftable_iterator_next_ref(&it, &ref);\n \t\tif (err > 0)\n \t\t\tbreak;\n@@ -251,18 +245,15 @@ static void t_merged(void)\n \treftable_iterator_destroy(&it);\n \n \tcheck_int(ARRAY_SIZE(want), ==, len);\n-\tfor (i = 0; i < len; i++) {\n+\tfor (i = 0; i < len; i++)\n \t\tcheck(reftable_ref_record_equal(want[i], &out[i],\n \t\t\t\t\t\t GIT_SHA1_RAWSZ));\n-\t}\n-\tfor (i = 0; i < len; i++) {\n+\tfor (i = 0; i < len; i++)\n \t\treftable_ref_record_release(&out[i]);\n-\t}\n \treftable_free(out);\n \n-\tfor (i = 0; i < 3; i++) {\n+\tfor (i = 0; i < 3; i++)\n \t\tstrbuf_release(&bufs[i]);\n-\t}\n \treaders_destroy(readers, 3);\n \treftable_merged_table_free(mt);\n \treftable_free(bs);\n@@ -358,12 +349,12 @@ static void t_merged_logs(void)\n \tstruct reftable_reader **readers = NULL;\n \tstruct reftable_merged_table *mt = merged_table_from_log_records(\n \t\tlogs, &bs, &readers, sizes, bufs, 3);\n-\tstruct reftable_iterator it = { NULL };\n+\tstruct reftable_iterator it = { 0 };\n \tint err;\n \tstruct reftable_log_record *out = NULL;\n \tsize_t len = 0;\n \tsize_t cap = 0;\n-\tint i = 0;\n+\tsize_t i = 0;\n \n \tmerged_table_init_iter(mt, &it, BLOCK_TYPE_LOG);\n \terr = reftable_iterator_seek_log(&it, \"a\");\n@@ -372,7 +363,7 @@ static void t_merged_logs(void)\n \tcheck_int(reftable_merged_table_min_update_index(mt), ==, 1);\n \n \twhile (len < 100) { /* cap loops/recursion. */\n-\t\tstruct reftable_log_record log = { NULL };\n+\t\tstruct reftable_log_record log = { 0 };\n \t\tint err = reftable_iterator_next_log(&it, &log);\n \t\tif (err > 0)\n \t\t\tbreak;\n@@ -383,10 +374,9 @@ static void t_merged_logs(void)\n \treftable_iterator_destroy(&it);\n \n \tcheck_int(ARRAY_SIZE(want), ==, len);\n-\tfor (i = 0; i < len; i++) {\n+\tfor (i = 0; i < len; i++)\n \t\tcheck(reftable_log_record_equal(want[i], &out[i],\n \t\t\t\t\t\t GIT_SHA1_RAWSZ));\n-\t}\n \n \tmerged_table_init_iter(mt, &it, BLOCK_TYPE_LOG);\n \terr = reftable_iterator_seek_log_at(&it, \"a\", 2);\n@@ -397,14 +387,12 @@ static void t_merged_logs(void)\n \tcheck(reftable_log_record_equal(&out[0], &r3[0], GIT_SHA1_RAWSZ));\n \treftable_iterator_destroy(&it);\n \n-\tfor (i = 0; i < len; i++) {\n+\tfor (i = 0; i < len; i++)\n \t\treftable_log_record_release(&out[i]);\n-\t}\n \treftable_free(out);\n \n-\tfor (i = 0; i < 3; i++) {\n+\tfor (i = 0; i < 3; i++)\n \t\tstrbuf_release(&bufs[i]);\n-\t}\n \treaders_destroy(readers, 3);\n \treftable_merged_table_free(mt);\n \treftable_free(bs);\n@@ -422,7 +410,7 @@ static void t_default_write_opts(void)\n \t\t.update_index = 1,\n \t};\n \tint err;\n-\tstruct reftable_block_source source = { NULL };\n+\tstruct reftable_block_source source = { 0 };\n \tstruct reftable_table *tab = reftable_calloc(1, sizeof(*tab));\n \tuint32_t hash_id;\n \tstruct reftable_reader *rd = NULL;\n-- \n2.45.2.404.g9eaef5822c\n\n"},{"id":"498046","messageId":"20240703171131.3929-4-chandrapratap3519@gmail.com","threadId":"61730","inReplyTo":"20240703171131.3929-1-chandrapratap3519@gmail.com","subject":"[PATCH 3/5] t-reftable-merged: add tests for reftable_merged_table_max_update_index","fromName":"Chandra Pratap","fromEmail":"chandrapratap3519@gmail.com","sentAt":"2024-07-03T17:01:43Z","receivedAt":"2024-07-03T17:12:20Z","isPatch":true,"sender":{"key":"chandrapratap3519@gmail.com","avatar":null},"body":"reftable_merged_table_max_update_index() as defined by reftable/\nmerged.{c, h} returns the maximum update index in a merged table.\nSince this function is currently unexercised, add tests for it.\n\nMentored-by: Patrick Steinhardt \nMentored-by: Christian Couder \nSigned-off-by: Chandra Pratap \n---\n t/unit-tests/t-reftable-merged.c | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/t/unit-tests/t-reftable-merged.c b/t/unit-tests/t-reftable-merged.c\nindex 28bf6f6696..543113f3d4 100644\n--- a/t/unit-tests/t-reftable-merged.c\n+++ b/t/unit-tests/t-reftable-merged.c\n@@ -232,6 +232,7 @@ static void t_merged(void)\n \tcheck(!err);\n \tcheck_int(reftable_merged_table_hash_id(mt), ==, GIT_SHA1_FORMAT_ID);\n \tcheck_int(reftable_merged_table_min_update_index(mt), ==, 1);\n+\tcheck_int(reftable_merged_table_max_update_index(mt), ==, 3);\n \n \twhile (len < 100) { /* cap loops/recursion. */\n \t\tstruct reftable_ref_record ref = { 0 };\n@@ -361,6 +362,7 @@ static void t_merged_logs(void)\n \tcheck(!err);\n \tcheck_int(reftable_merged_table_hash_id(mt), ==, GIT_SHA1_FORMAT_ID);\n \tcheck_int(reftable_merged_table_min_update_index(mt), ==, 1);\n+\tcheck_int(reftable_merged_table_max_update_index(mt), ==, 3);\n \n \twhile (len < 100) { /* cap loops/recursion. */\n \t\tstruct reftable_log_record log = { 0 };\n-- \n2.45.2.404.g9eaef5822c\n\n"},{"id":"498047","messageId":"20240703171131.3929-5-chandrapratap3519@gmail.com","threadId":"61730","inReplyTo":"20240703171131.3929-1-chandrapratap3519@gmail.com","subject":"[PATCH 4/5] t-reftable-merged: use reftable_ref_record_equal to compare ref records","fromName":"Chandra Pratap","fromEmail":"chandrapratap3519@gmail.com","sentAt":"2024-07-03T17:01:44Z","receivedAt":"2024-07-03T17:12:23Z","isPatch":true,"sender":{"key":"chandrapratap3519@gmail.com","avatar":null},"body":"In the test test_merged_between() defined in t-reftable-merged.c,\nthe 'input' and 'expected' ref records are checked for equality\nby comparing their update indices. It is very much possible for\ntwo different ref records to have the same update indices. Use\nreftable_ref_record_equal() as well for a stronger check.\n\nMentored-by: Patrick Steinhardt \nMentored-by: Christian Couder \nSigned-off-by: Chandra Pratap \n---\n t/unit-tests/t-reftable-merged.c | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/t/unit-tests/t-reftable-merged.c b/t/unit-tests/t-reftable-merged.c\nindex 543113f3d4..656193550d 100644\n--- a/t/unit-tests/t-reftable-merged.c\n+++ b/t/unit-tests/t-reftable-merged.c\n@@ -155,6 +155,7 @@ static void t_merged_between(void)\n \terr = reftable_iterator_next_ref(&it, &ref);\n \tcheck(!err);\n \tcheck_int(ref.update_index, ==, 2);\n+\tcheck(reftable_ref_record_equal(&r2[0], &ref, GIT_SHA1_RAWSZ));\n \treftable_ref_record_release(&ref);\n \treftable_iterator_destroy(&it);\n \treaders_destroy(readers, 2);\n-- \n2.45.2.404.g9eaef5822c\n\n"},{"id":"498048","messageId":"20240703171131.3929-6-chandrapratap3519@gmail.com","threadId":"61730","inReplyTo":"20240703171131.3929-1-chandrapratap3519@gmail.com","subject":"[PATCH 5/5] t-reftable-merged: add test for REFTABLE_FORMAT_ERROR","fromName":"Chandra Pratap","fromEmail":"chandrapratap3519@gmail.com","sentAt":"2024-07-03T17:01:45Z","receivedAt":"2024-07-03T17:12:25Z","isPatch":true,"sender":{"key":"chandrapratap3519@gmail.com","avatar":null},"body":"When calling reftable_new_merged_table(), if the hash ID of the\npasssed reftable_table parameter doesn't match the passed hash_id\nparameter, a REFTABLE_FORMAT_ERROR is thrown. This case is\ncurrently left unexercised, so add a test for the same.\n\nMentored-by: Patrick Steinhardt \nMentored-by: Christian Couder \nSigned-off-by: Chandra Pratap \n---\n t/unit-tests/t-reftable-merged.c | 3 +++\n 1 file changed, 3 insertions(+)\n\ndiff --git a/t/unit-tests/t-reftable-merged.c b/t/unit-tests/t-reftable-merged.c\nindex 656193550d..a213867cf5 100644\n--- a/t/unit-tests/t-reftable-merged.c\n+++ b/t/unit-tests/t-reftable-merged.c\n@@ -11,6 +11,7 @@ license that can be found in the LICENSE file or at\n #include \"reftable/constants.h\"\n #include \"reftable/merged.h\"\n #include \"reftable/reader.h\"\n+#include \"reftable/reftable-error.h\"\n #include \"reftable/reftable-generic.h\"\n #include \"reftable/reftable-merged.h\"\n #include \"reftable/reftable-writer.h\"\n@@ -437,6 +438,8 @@ static void t_default_write_opts(void)\n \tcheck_int(hash_id, ==, GIT_SHA1_FORMAT_ID);\n \n \treftable_table_from_reader(&tab[0], rd);\n+\terr = reftable_new_merged_table(&merged, tab, 1, GIT_SHA256_FORMAT_ID);\n+\tcheck_int(err, ==, REFTABLE_FORMAT_ERROR);\n \terr = reftable_new_merged_table(&merged, tab, 1, GIT_SHA1_FORMAT_ID);\n \tcheck(!err);\n \n-- \n2.45.2.404.g9eaef5822c\n\n"},{"id":"498111","messageId":"CAOLa=ZSkfTCVgexzN=UQf-qzkEdfunp6YCmiqcf8gsxCS9aNvw@mail.gmail.com","threadId":"61730","inReplyTo":"20240703171131.3929-2-chandrapratap3519@gmail.com","subject":"Re: [PATCH 1/5] t: move reftable/merged_test.c to the unit testing framework","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2024-07-05T17:40:42Z","receivedAt":"2024-07-05T17:40:44Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Chandra Pratap writes:\n\n> reftable/merged_test.c exercises the functions defined in\n> reftable/merged.{c, h}. Migrate reftable/merged_test.c to the unit\n> testing framework. Migration involves refactoring the tests\n> to use the unit testing framework instead of reftable's test\n> framework and renaming the tests according to unit-tests' naming\n> conventions.\n>\n> Also, move strbuf_add_void() and noop_flush() from\n> reftable/test_framework.c to the ported test. This is because\n> both these functions are used in the merged tests and\n> reftable/test_framework.{c, h} is not #included in the ported test.\n>\n> Mentored-by: Patrick Steinhardt \n> Mentored-by: Christian Couder \n> Signed-off-by: Chandra Pratap \n> ---\n> Makefile | 2 +-\n> t/helper/test-reftable.c | 1 -\n> .../unit-tests/t-reftable-merged.c | 112 +++++++++---------\n> 3 files changed, 60 insertions(+), 55 deletions(-)\n> rename reftable/merged_test.c => t/unit-tests/t-reftable-merged.c (84%)\n>\n> diff --git a/Makefile b/Makefile\n> index 3eab701b10..e5d1b53991 100644\n> --- a/Makefile\n> +++ b/Makefile\n> @@ -1340,6 +1340,7 @@ UNIT_TEST_PROGRAMS += t-mem-pool\n> UNIT_TEST_PROGRAMS += t-oidtree\n> UNIT_TEST_PROGRAMS += t-prio-queue\n> UNIT_TEST_PROGRAMS += t-reftable-basics\n> +UNIT_TEST_PROGRAMS += t-reftable-merged\n> UNIT_TEST_PROGRAMS += t-strbuf\n> UNIT_TEST_PROGRAMS += t-strcmp-offset\n> UNIT_TEST_PROGRAMS += t-strvec\n> @@ -2679,7 +2680,6 @@ REFTABLE_OBJS += reftable/writer.o\n>\n> REFTABLE_TEST_OBJS += reftable/block_test.o\n> REFTABLE_TEST_OBJS += reftable/dump.o\n> -REFTABLE_TEST_OBJS += reftable/merged_test.o\n> REFTABLE_TEST_OBJS += reftable/pq_test.o\n> REFTABLE_TEST_OBJS += reftable/record_test.o\n> REFTABLE_TEST_OBJS += reftable/readwrite_test.o\n> diff --git a/t/helper/test-reftable.c b/t/helper/test-reftable.c\n> index 9160bc5da6..0357718fa8 100644\n> --- a/t/helper/test-reftable.c\n> +++ b/t/helper/test-reftable.c\n> @@ -10,7 +10,6 @@ int cmd__reftable(int argc, const char **argv)\n> \ttree_test_main(argc, argv);\n> \tpq_test_main(argc, argv);\n> \treadwrite_test_main(argc, argv);\n> -\tmerged_test_main(argc, argv);\n> \tstack_test_main(argc, argv);\n> \treturn 0;\n> }\n> diff --git a/reftable/merged_test.c b/t/unit-tests/t-reftable-merged.c\n> similarity index 84%\n> rename from reftable/merged_test.c\n> rename to t/unit-tests/t-reftable-merged.c\n> index a9d6661c13..1718489f06 100644\n> --- a/reftable/merged_test.c\n> +++ b/t/unit-tests/t-reftable-merged.c\n> @@ -6,20 +6,25 @@ license that can be found in the LICENSE file or at\n> https://developers.google.com/open-source/licenses/bsd\n> */\n>\n> -#include \"merged.h\"\n> -\n> -#include \"system.h\"\n> +#include \"test-lib.h\"\n> +#include \"reftable/blocksource.h\"\n> +#include \"reftable/constants.h\"\n> +#include \"reftable/merged.h\"\n> +#include \"reftable/reader.h\"\n> +#include \"reftable/reftable-generic.h\"\n> +#include \"reftable/reftable-merged.h\"\n> +#include \"reftable/reftable-writer.h\"\n> +\n> +static ssize_t strbuf_add_void(void *b, const void *data, size_t sz)\n> +{\n> +\tstrbuf_add(b, data, sz);\n> +\treturn sz;\n> +}\n>\n> -#include \"basics.h\"\n> -#include \"blocksource.h\"\n> -#include \"constants.h\"\n> -#include \"reader.h\"\n> -#include \"record.h\"\n> -#include \"test_framework.h\"\n> -#include \"reftable-merged.h\"\n> -#include \"reftable-tests.h\"\n> -#include \"reftable-generic.h\"\n> -#include \"reftable-writer.h\"\n> +static int noop_flush(void *arg)\n> +{\n> +\treturn 0;\n> +}\n>\n> static void write_test_table(struct strbuf *buf,\n> \t\t\t struct reftable_ref_record refs[], int n)\n> @@ -49,12 +54,12 @@ static void write_test_table(struct strbuf *buf,\n> \tfor (i = 0; i < n; i++) {\n> \t\tuint64_t before = refs[i].update_index;\n> \t\tint n = reftable_writer_add_ref(w, &refs[i]);\n> -\t\tEXPECT(n == 0);\n> -\t\tEXPECT(before == refs[i].update_index);\n> +\t\tcheck_int(n, ==, 0);\n> +\t\tcheck_int(before, ==, refs[i].update_index);\n> \t}\n>\n> \terr = reftable_writer_close(w);\n> -\tEXPECT_ERR(err);\n> +\tcheck(!err);\n>\n\nNit: Couldn't we just do `check(!reftable_writer_close(w))` here and in\na lot of places below?\n\n> \treftable_writer_free(w);\n> }\n> @@ -76,11 +81,11 @@ static void write_test_log_table(struct strbuf *buf,\n>\n> \tfor (i = 0; i < n; i++) {\n> \t\tint err = reftable_writer_add_log(w, &logs[i]);\n> -\t\tEXPECT_ERR(err);\n> +\t\tcheck(!err);\n> \t}\n>\n> \terr = reftable_writer_close(w);\n> -\tEXPECT_ERR(err);\n> +\tcheck(!err);\n>\n> \treftable_writer_free(w);\n> }\n> @@ -105,12 +110,12 @@ merged_table_from_records(struct reftable_ref_record **refs,\n>\n> \t\terr = reftable_new_reader(&(*readers)[i], &(*source)[i],\n> \t\t\t\t\t \"name\");\n> -\t\tEXPECT_ERR(err);\n> +\t\tcheck(!err);\n> \t\treftable_table_from_reader(&tabs[i], (*readers)[i]);\n> \t}\n>\n> \terr = reftable_new_merged_table(&mt, tabs, n, GIT_SHA1_FORMAT_ID);\n> -\tEXPECT_ERR(err);\n> +\tcheck(!err);\n> \treturn mt;\n> }\n>\n> @@ -122,7 +127,7 @@ static void readers_destroy(struct reftable_reader **readers, size_t n)\n> \treftable_free(readers);\n> }\n>\n> -static void test_merged_between(void)\n> +static void t_merged_between(void)\n> {\n> \tstruct reftable_ref_record r1[] = { {\n> \t\t.refname = (char *) \"b\",\n> @@ -150,11 +155,11 @@ static void test_merged_between(void)\n>\n> \tmerged_table_init_iter(mt, &it, BLOCK_TYPE_REF);\n> \terr = reftable_iterator_seek_ref(&it, \"a\");\n> -\tEXPECT_ERR(err);\n> +\tcheck(!err);\n>\n> \terr = reftable_iterator_next_ref(&it, &ref);\n> -\tEXPECT_ERR(err);\n> -\tEXPECT(ref.update_index == 2);\n> +\tcheck(!err);\n> +\tcheck_int(ref.update_index, ==, 2);\n> \treftable_ref_record_release(&ref);\n> \treftable_iterator_destroy(&it);\n> \treaders_destroy(readers, 2);\n> @@ -165,7 +170,7 @@ static void test_merged_between(void)\n> \treftable_free(bs);\n> }\n>\n> -static void test_merged(void)\n> +static void t_merged(void)\n> {\n> \tstruct reftable_ref_record r1[] = {\n> \t\t{\n> @@ -230,9 +235,9 @@ static void test_merged(void)\n>\n> \tmerged_table_init_iter(mt, &it, BLOCK_TYPE_REF);\n> \terr = reftable_iterator_seek_ref(&it, \"a\");\n> -\tEXPECT_ERR(err);\n> -\tEXPECT(reftable_merged_table_hash_id(mt) == GIT_SHA1_FORMAT_ID);\n> -\tEXPECT(reftable_merged_table_min_update_index(mt) == 1);\n> +\tcheck(!err);\n> +\tcheck_int(reftable_merged_table_hash_id(mt), ==, GIT_SHA1_FORMAT_ID);\n> +\tcheck_int(reftable_merged_table_min_update_index(mt), ==, 1);\n>\n> \twhile (len < 100) { /* cap loops/recursion. */\n> \t\tstruct reftable_ref_record ref = { NULL };\n> @@ -245,9 +250,9 @@ static void test_merged(void)\n> \t}\n> \treftable_iterator_destroy(&it);\n>\n> -\tEXPECT(ARRAY_SIZE(want) == len);\n> +\tcheck_int(ARRAY_SIZE(want), ==, len);\n> \tfor (i = 0; i < len; i++) {\n> -\t\tEXPECT(reftable_ref_record_equal(want[i], &out[i],\n> +\t\tcheck(reftable_ref_record_equal(want[i], &out[i],\n> \t\t\t\t\t\t GIT_SHA1_RAWSZ));\n> \t}\n> \tfor (i = 0; i < len; i++) {\n> @@ -283,16 +288,16 @@ merged_table_from_log_records(struct reftable_log_record **logs,\n>\n> \t\terr = reftable_new_reader(&(*readers)[i], &(*source)[i],\n> \t\t\t\t\t \"name\");\n> -\t\tEXPECT_ERR(err);\n> +\t\tcheck(!err);\n> \t\treftable_table_from_reader(&tabs[i], (*readers)[i]);\n> \t}\n>\n> \terr = reftable_new_merged_table(&mt, tabs, n, GIT_SHA1_FORMAT_ID);\n> -\tEXPECT_ERR(err);\n> +\tcheck(!err);\n> \treturn mt;\n> }\n>\n> -static void test_merged_logs(void)\n> +static void t_merged_logs(void)\n> {\n> \tstruct reftable_log_record r1[] = {\n> \t\t{\n> @@ -362,9 +367,9 @@ static void test_merged_logs(void)\n>\n> \tmerged_table_init_iter(mt, &it, BLOCK_TYPE_LOG);\n> \terr = reftable_iterator_seek_log(&it, \"a\");\n> -\tEXPECT_ERR(err);\n> -\tEXPECT(reftable_merged_table_hash_id(mt) == GIT_SHA1_FORMAT_ID);\n> -\tEXPECT(reftable_merged_table_min_update_index(mt) == 1);\n> +\tcheck(!err);\n> +\tcheck_int(reftable_merged_table_hash_id(mt), ==, GIT_SHA1_FORMAT_ID);\n> +\tcheck_int(reftable_merged_table_min_update_index(mt), ==, 1);\n>\n> \twhile (len < 100) { /* cap loops/recursion. */\n> \t\tstruct reftable_log_record log = { NULL };\n> @@ -377,19 +382,19 @@ static void test_merged_logs(void)\n> \t}\n> \treftable_iterator_destroy(&it);\n>\n> -\tEXPECT(ARRAY_SIZE(want) == len);\n> +\tcheck_int(ARRAY_SIZE(want), ==, len);\n> \tfor (i = 0; i < len; i++) {\n> -\t\tEXPECT(reftable_log_record_equal(want[i], &out[i],\n> +\t\tcheck(reftable_log_record_equal(want[i], &out[i],\n> \t\t\t\t\t\t GIT_SHA1_RAWSZ));\n> \t}\n>\n> \tmerged_table_init_iter(mt, &it, BLOCK_TYPE_LOG);\n> \terr = reftable_iterator_seek_log_at(&it, \"a\", 2);\n> -\tEXPECT_ERR(err);\n> +\tcheck(!err);\n> \treftable_log_record_release(&out[0]);\n> \terr = reftable_iterator_next_log(&it, &out[0]);\n> -\tEXPECT_ERR(err);\n> -\tEXPECT(reftable_log_record_equal(&out[0], &r3[0], GIT_SHA1_RAWSZ));\n> +\tcheck(!err);\n> +\tcheck(reftable_log_record_equal(&out[0], &r3[0], GIT_SHA1_RAWSZ));\n> \treftable_iterator_destroy(&it);\n>\n> \tfor (i = 0; i < len; i++) {\n> @@ -405,7 +410,7 @@ static void test_merged_logs(void)\n> \treftable_free(bs);\n> }\n>\n> -static void test_default_write_opts(void)\n> +static void t_default_write_opts(void)\n> {\n> \tstruct reftable_write_options opts = { 0 };\n> \tstruct strbuf buf = STRBUF_INIT;\n> @@ -426,23 +431,23 @@ static void test_default_write_opts(void)\n> \treftable_writer_set_limits(w, 1, 1);\n>\n> \terr = reftable_writer_add_ref(w, &rec);\n> -\tEXPECT_ERR(err);\n> +\tcheck(!err);\n>\n> \terr = reftable_writer_close(w);\n> -\tEXPECT_ERR(err);\n> +\tcheck(!err);\n> \treftable_writer_free(w);\n>\n> \tblock_source_from_strbuf(&source, &buf);\n>\n> \terr = reftable_new_reader(&rd, &source, \"filename\");\n> -\tEXPECT_ERR(err);\n> +\tcheck(!err);\n>\n> \thash_id = reftable_reader_hash_id(rd);\n> -\tEXPECT(hash_id == GIT_SHA1_FORMAT_ID);\n> +\tcheck_int(hash_id, ==, GIT_SHA1_FORMAT_ID);\n>\n> \treftable_table_from_reader(&tab[0], rd);\n> \terr = reftable_new_merged_table(&merged, tab, 1, GIT_SHA1_FORMAT_ID);\n> -\tEXPECT_ERR(err);\n> +\tcheck(!err);\n>\n> \treftable_reader_free(rd);\n> \treftable_merged_table_free(merged);\n> @@ -451,11 +456,12 @@ static void test_default_write_opts(void)\n>\n> /* XXX test refs_for(oid) */\n\nNot sure what this comment is for, let's remove it.\n\n> -int merged_test_main(int argc, const char *argv[])\n> +int cmd_main(int argc, const char *argv[])\n> {\n> -\tRUN_TEST(test_merged_logs);\n> -\tRUN_TEST(test_merged_between);\n> -\tRUN_TEST(test_merged);\n> -\tRUN_TEST(test_default_write_opts);\n> -\treturn 0;\n> +\tTEST(t_merged_logs(), \"merged table with log records\");\n> +\tTEST(t_merged_between(), \"seek ref in a merged table\");\n> +\tTEST(t_merged(), \"merged table with multiple updates to same ref\");\n> +\tTEST(t_default_write_opts(), \"merged table with default write opts\");\n\nNit: Could we order these alphabetically?\n\n> +\treturn test_done();\n> }\n> --\n\nThe diff only shows parts of the file which were renamed, some generic\ncomments though:\n* Some of the parameters passed to functions can be made 'const', this\nmakes it clearer if/not it is read-only.\n* The functions names could definitely be a little more verbose and\nclearer. e.g. `t_merged_between` doesn't even signify what we're doing.\nEven the test descriptions could definitely be improved.\n* Some single lined for loops have '{}' which can be removed.\n\nNow coming to some individual tests:\n- t_merged_between: This test is to check and ensure that a ref which\nonly occurds in one of the records can be retrieved. This is fine, It\nwould be nice if we could however add another record 'r3' and ensure the\nref 'a' only occurs in 'r2', this would also ensure that merged tables\ndon't simply read the last record.\n- t_default_write_opts: What is this test trying to achieve?\n"},{"id":"498113","messageId":"CAOLa=ZTdH5YHxs3wOkO68CCoMQM1WOYFi4LpTuuDj11fCRqK9w@mail.gmail.com","threadId":"61730","inReplyTo":"20240703171131.3929-3-chandrapratap3519@gmail.com","subject":"Re: [PATCH 2/5] t: harmonize t-reftable-merged.c with coding guidelines","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2024-07-05T18:08:26Z","receivedAt":"2024-07-05T18:08:28Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Chandra Pratap writes:\n\n> Harmonize the newly ported test unit-tests/t-reftable-merged.c\n> with the following guidelines:\n> - Single line control flow statements like 'for' and 'if'\n> must omit curly braces.\n> - Structs must be 0-initialized with '= { 0 }' instead of '= { NULL }'.\n> - Array indices must be of type 'size_t', not 'int'.\n> - It is fine to use C99 initial declaration in 'for' loop.\n>\n\nSeems like I spoke too soon, some of my comments from the previous\ncommit have been addressed here. Nice.\n\n[snip]\n"},{"id":"498114","messageId":"CAOLa=ZSTb-kKiGHkGJwSgx3acxcfFe_+KGW=p7O7B7=CgeX7rw@mail.gmail.com","threadId":"61730","inReplyTo":"20240703171131.3929-5-chandrapratap3519@gmail.com","subject":"Re: [PATCH 4/5] t-reftable-merged: use reftable_ref_record_equal to compare ref records","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2024-07-05T18:14:04Z","receivedAt":"2024-07-05T18:14:05Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Chandra Pratap writes:\n\n> In the test test_merged_between() defined in t-reftable-merged.c,\n>\n\ns/test_merged_between/t_merged_between\n\n> the 'input' and 'expected' ref records are checked for equality\n> by comparing their update indices. It is very much possible for\n> two different ref records to have the same update indices. Use\n> reftable_ref_record_equal() as well for a stronger check.\n>\n> Mentored-by: Patrick Steinhardt \n> Mentored-by: Christian Couder \n> Signed-off-by: Chandra Pratap \n> ---\n> t/unit-tests/t-reftable-merged.c | 1 +\n> 1 file changed, 1 insertion(+)\n>\n> diff --git a/t/unit-tests/t-reftable-merged.c b/t/unit-tests/t-reftable-merged.c\n> index 543113f3d4..656193550d 100644\n> --- a/t/unit-tests/t-reftable-merged.c\n> +++ b/t/unit-tests/t-reftable-merged.c\n> @@ -155,6 +155,7 @@ static void t_merged_between(void)\n> \terr = reftable_iterator_next_ref(&it, &ref);\n> \tcheck(!err);\n> \tcheck_int(ref.update_index, ==, 2);\n> +\tcheck(reftable_ref_record_equal(&r2[0], &ref, GIT_SHA1_RAWSZ));\n\nWe can actually remove `check_int(ref.update_index, ==, 2)` since the\nnew check also would compare the update_index.\n\n> \treftable_ref_record_release(&ref);\n> \treftable_iterator_destroy(&it);\n> \treaders_destroy(readers, 2);\n> --\n> 2.45.2.404.g9eaef5822c\n"},{"id":"498115","messageId":"CAOLa=ZT84sVZ67VQyXLeCby+YbF28d_STseME1xsFxzFXLa7_Q@mail.gmail.com","threadId":"61730","inReplyTo":"20240703171131.3929-1-chandrapratap3519@gmail.com","subject":"Re: [GSoC][PATCH 0/5] t: port reftable/merged_test.c to the unit testing framework","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2024-07-05T18:24:18Z","receivedAt":"2024-07-05T18:24:19Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Chandra Pratap writes:\n\n> In the recent codebase update (commit 8bf6fbd, 2023-12-09), a new unit\n> testing framework written entirely in C was introduced to the Git project\n> aimed at simplifying testing and reducing test run times.\n> Currently, tests for the reftable refs-backend are performed by a custom\n> testing framework defined by reftable/test_framework.{c, h}. Port\n> reftable/merged_test.c to the unit testing framework and improve upon\n> the ported test.\n>\n> The first patch in the series moves the test to the unit testing framework,\n> and the rest of the patches improve upon the ported test.\n>\n\nI went through the commits and left some small comments. Looks great\nalready.\n\nThanks\n\n[snip]\n"},{"id":"498160","messageId":"CA+J6zkTvFjfP1pzBCjom8XeJKe+AseYPmWemrJAs+b0NCiEQ7g@mail.gmail.com","threadId":"61730","inReplyTo":"CAOLa=ZSkfTCVgexzN=UQf-qzkEdfunp6YCmiqcf8gsxCS9aNvw@mail.gmail.com","subject":"Re: [PATCH 1/5] t: move reftable/merged_test.c to the unit testing framework","fromName":"Chandra Pratap","fromEmail":"chandrapratap3519@gmail.com","sentAt":"2024-07-06T07:13:31Z","receivedAt":"2024-07-06T07:13:45Z","isPatch":true,"sender":{"key":"chandrapratap3519@gmail.com","avatar":null},"body":"On Fri, 5 Jul 2024 at 23:10, Karthik Nayak wrote:\n>\n> Chandra Pratap writes:\n>\n> > reftable/merged_test.c exercises the functions defined in\n> > reftable/merged.{c, h}. Migrate reftable/merged_test.c to the unit\n> > testing framework. Migration involves refactoring the tests\n> > to use the unit testing framework instead of reftable's test\n> > framework and renaming the tests according to unit-tests' naming\n> > conventions.\n> >\n> > Also, move strbuf_add_void() and noop_flush() from\n> > reftable/test_framework.c to the ported test. This is because\n> > both these functions are used in the merged tests and\n> > reftable/test_framework.{c, h} is not #included in the ported test.\n> >\n> > Mentored-by: Patrick Steinhardt \n> > Mentored-by: Christian Couder \n> > Signed-off-by: Chandra Pratap \n> > ---\n> > Makefile | 2 +-\n> > t/helper/test-reftable.c | 1 -\n> > .../unit-tests/t-reftable-merged.c | 112 +++++++++---------\n> > 3 files changed, 60 insertions(+), 55 deletions(-)\n> > rename reftable/merged_test.c => t/unit-tests/t-reftable-merged.c (84%)\n> >\n> > diff --git a/Makefile b/Makefile\n> > index 3eab701b10..e5d1b53991 100644\n> > --- a/Makefile\n> > +++ b/Makefile\n> > @@ -1340,6 +1340,7 @@ UNIT_TEST_PROGRAMS += t-mem-pool\n> > UNIT_TEST_PROGRAMS += t-oidtree\n> > UNIT_TEST_PROGRAMS += t-prio-queue\n> > UNIT_TEST_PROGRAMS += t-reftable-basics\n> > +UNIT_TEST_PROGRAMS += t-reftable-merged\n> > UNIT_TEST_PROGRAMS += t-strbuf\n> > UNIT_TEST_PROGRAMS += t-strcmp-offset\n> > UNIT_TEST_PROGRAMS += t-strvec\n> > @@ -2679,7 +2680,6 @@ REFTABLE_OBJS += reftable/writer.o\n> >\n> > REFTABLE_TEST_OBJS += reftable/block_test.o\n> > REFTABLE_TEST_OBJS += reftable/dump.o\n> > -REFTABLE_TEST_OBJS += reftable/merged_test.o\n> > REFTABLE_TEST_OBJS += reftable/pq_test.o\n> > REFTABLE_TEST_OBJS += reftable/record_test.o\n> > REFTABLE_TEST_OBJS += reftable/readwrite_test.o\n> > diff --git a/t/helper/test-reftable.c b/t/helper/test-reftable.c\n> > index 9160bc5da6..0357718fa8 100644\n> > --- a/t/helper/test-reftable.c\n> > +++ b/t/helper/test-reftable.c\n> > @@ -10,7 +10,6 @@ int cmd__reftable(int argc, const char **argv)\n> > tree_test_main(argc, argv);\n> > pq_test_main(argc, argv);\n> > readwrite_test_main(argc, argv);\n> > - merged_test_main(argc, argv);\n> > stack_test_main(argc, argv);\n> > return 0;\n> > }\n> > diff --git a/reftable/merged_test.c b/t/unit-tests/t-reftable-merged.c\n> > similarity index 84%\n> > rename from reftable/merged_test.c\n> > rename to t/unit-tests/t-reftable-merged.c\n> > index a9d6661c13..1718489f06 100644\n> > --- a/reftable/merged_test.c\n> > +++ b/t/unit-tests/t-reftable-merged.c\n> > @@ -6,20 +6,25 @@ license that can be found in the LICENSE file or at\n> > https://developers.google.com/open-source/licenses/bsd\n> > */\n> >\n> > -#include \"merged.h\"\n> > -\n> > -#include \"system.h\"\n> > +#include \"test-lib.h\"\n> > +#include \"reftable/blocksource.h\"\n> > +#include \"reftable/constants.h\"\n> > +#include \"reftable/merged.h\"\n> > +#include \"reftable/reader.h\"\n> > +#include \"reftable/reftable-generic.h\"\n> > +#include \"reftable/reftable-merged.h\"\n> > +#include \"reftable/reftable-writer.h\"\n> > +\n> > +static ssize_t strbuf_add_void(void *b, const void *data, size_t sz)\n> > +{\n> > + strbuf_add(b, data, sz);\n> > + return sz;\n> > +}\n> >\n> > -#include \"basics.h\"\n> > -#include \"blocksource.h\"\n> > -#include \"constants.h\"\n> > -#include \"reader.h\"\n> > -#include \"record.h\"\n> > -#include \"test_framework.h\"\n> > -#include \"reftable-merged.h\"\n> > -#include \"reftable-tests.h\"\n> > -#include \"reftable-generic.h\"\n> > -#include \"reftable-writer.h\"\n> > +static int noop_flush(void *arg)\n> > +{\n> > + return 0;\n> > +}\n> >\n> > static void write_test_table(struct strbuf *buf,\n> > struct reftable_ref_record refs[], int n)\n> > @@ -49,12 +54,12 @@ static void write_test_table(struct strbuf *buf,\n> > for (i = 0; i < n; i++) {\n> > uint64_t before = refs[i].update_index;\n> > int n = reftable_writer_add_ref(w, &refs[i]);\n> > - EXPECT(n == 0);\n> > - EXPECT(before == refs[i].update_index);\n> > + check_int(n, ==, 0);\n> > + check_int(before, ==, refs[i].update_index);\n> > }\n> >\n> > err = reftable_writer_close(w);\n> > - EXPECT_ERR(err);\n> > + check(!err);\n> >\n>\n> Nit: Couldn't we just do `check(!reftable_writer_close(w))` here and in\n> a lot of places below?\n\nI guess we could do that here because 'err' is only used once, but in\nmost of the other tests, 'err' is used multiple times and with more\ncomplicated functions, so doing a change like this there would probably\nmake the code less readable.\nIf we aren't doing this change in any of the other tests, I think we're\nbetter off not changing things here as well to preserve uniformity across\ntests.\n\n> > reftable_writer_free(w);\n> > }\n> > @@ -76,11 +81,11 @@ static void write_test_log_table(struct strbuf *buf,\n> >\n> > for (i = 0; i < n; i++) {\n> > int err = reftable_writer_add_log(w, &logs[i]);\n> > - EXPECT_ERR(err);\n> > + check(!err);\n> > }\n> >\n> > err = reftable_writer_close(w);\n> > - EXPECT_ERR(err);\n> > + check(!err);\n> >\n> > reftable_writer_free(w);\n> > }\n> > @@ -105,12 +110,12 @@ merged_table_from_records(struct reftable_ref_record **refs,\n> >\n> > err = reftable_new_reader(&(*readers)[i], &(*source)[i],\n> > \"name\");\n> > - EXPECT_ERR(err);\n> > + check(!err);\n> > reftable_table_from_reader(&tabs[i], (*readers)[i]);\n> > }\n> >\n> > err = reftable_new_merged_table(&mt, tabs, n, GIT_SHA1_FORMAT_ID);\n> > - EXPECT_ERR(err);\n> > + check(!err);\n> > return mt;\n> > }\n> >\n> > @@ -122,7 +127,7 @@ static void readers_destroy(struct reftable_reader **readers, size_t n)\n> > reftable_free(readers);\n> > }\n> >\n> > -static void test_merged_between(void)\n> > +static void t_merged_between(void)\n> > {\n> > struct reftable_ref_record r1[] = { {\n> > .refname = (char *) \"b\",\n> > @@ -150,11 +155,11 @@ static void test_merged_between(void)\n> >\n> > merged_table_init_iter(mt, &it, BLOCK_TYPE_REF);\n> > err = reftable_iterator_seek_ref(&it, \"a\");\n> > - EXPECT_ERR(err);\n> > + check(!err);\n> >\n> > err = reftable_iterator_next_ref(&it, &ref);\n> > - EXPECT_ERR(err);\n> > - EXPECT(ref.update_index == 2);\n> > + check(!err);\n> > + check_int(ref.update_index, ==, 2);\n> > reftable_ref_record_release(&ref);\n> > reftable_iterator_destroy(&it);\n> > readers_destroy(readers, 2);\n> > @@ -165,7 +170,7 @@ static void test_merged_between(void)\n> > reftable_free(bs);\n> > }\n> >\n> > -static void test_merged(void)\n> > +static void t_merged(void)\n> > {\n> > struct reftable_ref_record r1[] = {\n> > {\n> > @@ -230,9 +235,9 @@ static void test_merged(void)\n> >\n> > merged_table_init_iter(mt, &it, BLOCK_TYPE_REF);\n> > err = reftable_iterator_seek_ref(&it, \"a\");\n> > - EXPECT_ERR(err);\n> > - EXPECT(reftable_merged_table_hash_id(mt) == GIT_SHA1_FORMAT_ID);\n> > - EXPECT(reftable_merged_table_min_update_index(mt) == 1);\n> > + check(!err);\n> > + check_int(reftable_merged_table_hash_id(mt), ==, GIT_SHA1_FORMAT_ID);\n> > + check_int(reftable_merged_table_min_update_index(mt), ==, 1);\n> >\n> > while (len < 100) { /* cap loops/recursion. */\n> > struct reftable_ref_record ref = { NULL };\n> > @@ -245,9 +250,9 @@ static void test_merged(void)\n> > }\n> > reftable_iterator_destroy(&it);\n> >\n> > - EXPECT(ARRAY_SIZE(want) == len);\n> > + check_int(ARRAY_SIZE(want), ==, len);\n> > for (i = 0; i < len; i++) {\n> > - EXPECT(reftable_ref_record_equal(want[i], &out[i],\n> > + check(reftable_ref_record_equal(want[i], &out[i],\n> > GIT_SHA1_RAWSZ));\n> > }\n> > for (i = 0; i < len; i++) {\n> > @@ -283,16 +288,16 @@ merged_table_from_log_records(struct reftable_log_record **logs,\n> >\n> > err = reftable_new_reader(&(*readers)[i], &(*source)[i],\n> > \"name\");\n> > - EXPECT_ERR(err);\n> > + check(!err);\n> > reftable_table_from_reader(&tabs[i], (*readers)[i]);\n> > }\n> >\n> > err = reftable_new_merged_table(&mt, tabs, n, GIT_SHA1_FORMAT_ID);\n> > - EXPECT_ERR(err);\n> > + check(!err);\n> > return mt;\n> > }\n> >\n> > -static void test_merged_logs(void)\n> > +static void t_merged_logs(void)\n> > {\n> > struct reftable_log_record r1[] = {\n> > {\n> > @@ -362,9 +367,9 @@ static void test_merged_logs(void)\n> >\n> > merged_table_init_iter(mt, &it, BLOCK_TYPE_LOG);\n> > err = reftable_iterator_seek_log(&it, \"a\");\n> > - EXPECT_ERR(err);\n> > - EXPECT(reftable_merged_table_hash_id(mt) == GIT_SHA1_FORMAT_ID);\n> > - EXPECT(reftable_merged_table_min_update_index(mt) == 1);\n> > + check(!err);\n> > + check_int(reftable_merged_table_hash_id(mt), ==, GIT_SHA1_FORMAT_ID);\n> > + check_int(reftable_merged_table_min_update_index(mt), ==, 1);\n> >\n> > while (len < 100) { /* cap loops/recursion. */\n> > struct reftable_log_record log = { NULL };\n> > @@ -377,19 +382,19 @@ static void test_merged_logs(void)\n> > }\n> > reftable_iterator_destroy(&it);\n> >\n> > - EXPECT(ARRAY_SIZE(want) == len);\n> > + check_int(ARRAY_SIZE(want), ==, len);\n> > for (i = 0; i < len; i++) {\n> > - EXPECT(reftable_log_record_equal(want[i], &out[i],\n> > + check(reftable_log_record_equal(want[i], &out[i],\n> > GIT_SHA1_RAWSZ));\n> > }\n> >\n> > merged_table_init_iter(mt, &it, BLOCK_TYPE_LOG);\n> > err = reftable_iterator_seek_log_at(&it, \"a\", 2);\n> > - EXPECT_ERR(err);\n> > + check(!err);\n> > reftable_log_record_release(&out[0]);\n> > err = reftable_iterator_next_log(&it, &out[0]);\n> > - EXPECT_ERR(err);\n> > - EXPECT(reftable_log_record_equal(&out[0], &r3[0], GIT_SHA1_RAWSZ));\n> > + check(!err);\n> > + check(reftable_log_record_equal(&out[0], &r3[0], GIT_SHA1_RAWSZ));\n> > reftable_iterator_destroy(&it);\n> >\n> > for (i = 0; i < len; i++) {\n> > @@ -405,7 +410,7 @@ static void test_merged_logs(void)\n> > reftable_free(bs);\n> > }\n> >\n> > -static void test_default_write_opts(void)\n> > +static void t_default_write_opts(void)\n> > {\n> > struct reftable_write_options opts = { 0 };\n> > struct strbuf buf = STRBUF_INIT;\n> > @@ -426,23 +431,23 @@ static void test_default_write_opts(void)\n> > reftable_writer_set_limits(w, 1, 1);\n> >\n> > err = reftable_writer_add_ref(w, &rec);\n> > - EXPECT_ERR(err);\n> > + check(!err);\n> >\n> > err = reftable_writer_close(w);\n> > - EXPECT_ERR(err);\n> > + check(!err);\n> > reftable_writer_free(w);\n> >\n> > block_source_from_strbuf(&source, &buf);\n> >\n> > err = reftable_new_reader(&rd, &source, \"filename\");\n> > - EXPECT_ERR(err);\n> > + check(!err);\n> >\n> > hash_id = reftable_reader_hash_id(rd);\n> > - EXPECT(hash_id == GIT_SHA1_FORMAT_ID);\n> > + check_int(hash_id, ==, GIT_SHA1_FORMAT_ID);\n> >\n> > reftable_table_from_reader(&tab[0], rd);\n> > err = reftable_new_merged_table(&merged, tab, 1, GIT_SHA1_FORMAT_ID);\n> > - EXPECT_ERR(err);\n> > + check(!err);\n> >\n> > reftable_reader_free(rd);\n> > reftable_merged_table_free(merged);\n> > @@ -451,11 +456,12 @@ static void test_default_write_opts(void)\n> >\n> > /* XXX test refs_for(oid) */\n>\n> Not sure what this comment is for, let's remove it.\n>\n> > -int merged_test_main(int argc, const char *argv[])\n> > +int cmd_main(int argc, const char *argv[])\n> > {\n> > - RUN_TEST(test_merged_logs);\n> > - RUN_TEST(test_merged_between);\n> > - RUN_TEST(test_merged);\n> > - RUN_TEST(test_default_write_opts);\n> > - return 0;\n> > + TEST(t_merged_logs(), \"merged table with log records\");\n> > + TEST(t_merged_between(), \"seek ref in a merged table\");\n> > + TEST(t_merged(), \"merged table with multiple updates to same ref\");\n> > + TEST(t_default_write_opts(), \"merged table with default write opts\");\n>\n> Nit: Could we order these alphabetically?\n>\n> > + return test_done();\n> > }\n> > --\n>\n> The diff only shows parts of the file which were renamed, some generic\n> comments though:\n> * Some of the parameters passed to functions can be made 'const', this\n> makes it clearer if/not it is read-only.\n> * The functions names could definitely be a little more verbose and\n> clearer. e.g. `t_merged_between` doesn't even signify what we're doing.\n> Even the test descriptions could definitely be improved.\n> * Some single lined for loops have '{}' which can be removed.\n>\n> Now coming to some individual tests:\n> - t_merged_between: This test is to check and ensure that a ref which\n> only occurds in one of the records can be retrieved. This is fine, It\n> would be nice if we could however add another record 'r3' and ensure the\n> ref 'a' only occurs in 'r2', this would also ensure that merged tables\n> don't simply read the last record.\n> - t_default_write_opts: What is this test trying to achieve?\n\nThe test passes an empty 'reftable_write_options' parameter to the\n'reftable_new_writer()' function which creates a writer with default\noptions. The test then tests the merged table created from this writer.\n"},{"id":"498338","messageId":"20240709053847.4453-1-chandrapratap3519@gmail.com","threadId":"61730","inReplyTo":"20240703171131.3929-1-chandrapratap3519@gmail.com","subject":"[GSoC][PATCH v2 0/7] t: port reftable/merged_test.c to the unit testing framework","fromName":"Chandra Pratap","fromEmail":"chandrapratap3519@gmail.com","sentAt":"2024-07-09T05:28:39Z","receivedAt":"2024-07-09T05:39:20Z","isPatch":true,"sender":{"key":"chandrapratap3519@gmail.com","avatar":null},"body":"In the recent codebase update (commit 8bf6fbd, 2023-12-09), a new unit\ntesting framework written entirely in C was introduced to the Git project\naimed at simplifying testing and reducing test run times.\nCurrently, tests for the reftable refs-backend are performed by a custom\ntesting framework defined by reftable/test_framework.{c, h}. Port\nreftable/merged_test.c to the unit testing framework and improve upon\nthe ported test.\n\nThe first patch in the series moves the test to the unit testing framework,\nand the rest of the patches improve upon the ported test.\n\nMentored-by: Patrick Steinhardt \nMentored-by: Christian Couder \nSigned-off-by: Chandra Pratap \n\n---\nChanges in v2:\n- Rename test functions & description to be more explanatory\n- Remove a redundant comment in patch 1\n- Add the 3rd patch which makes improvements to a test\n- Add the 4th patch which makes apt function parameters 'const'\n- Remove a redundant check in patch 6\n\nCI/PR: https://github.com/gitgitgadget/git/pull/1755\n\nChandra Pratap (7):\n[PATCH 1/7] t: move reftable/merged_test.c to the unit testing framework\n[PATCH 2/7] t: harmonize t-reftable-merged.c with coding guidelines\n[PATCH 3/7] t-reftable-merged: improve the test for t_merged_single_record()\n[PATCH 4/7] t-reftable-merged: improve the const-correctness of helper functions\n[PATCH 5/7] t-reftable-merged: add tests for reftable_merged_table_max_update_index\n[PATCH 6/7] t-reftable-merged: use reftable_ref_record_equal to compare ref records\n[PATCH 7/7] t-reftable-merged: add test for REFTABLE_FORMAT_ERROR\n\nMakefile | 2 +-\nt/helper/test-reftable.c | 1 -\nreftable/merged_test.c => t/unit-tests/t-reftable-merged.c | 202 +++++++++++++++----------------\n3 files changed, 103 insertions(+), 102 deletions(-)\n\nRange-diff against v2:\n1: f4d5da52f5 ! 1: 0d71deffad t: move reftable/merged_test.c to the unit testing framework\n @@ t/unit-tests/t-reftable-merged.c: static void readers_destroy(struct reftable_re\n }\n \n -static void test_merged_between(void)\n -+static void t_merged_between(void)\n ++static void t_merged_single_record(void)\n {\n \tstruct reftable_ref_record r1[] = { {\n \t\t.refname = (char *) \"b\",\n @@ t/unit-tests/t-reftable-merged.c: static void test_merged_between(void)\n }\n \n -static void test_merged(void)\n -+static void t_merged(void)\n ++static void t_merged_refs(void)\n {\n \tstruct reftable_ref_record r1[] = {\n \t\t{\n @@ t/unit-tests/t-reftable-merged.c: static void test_default_write_opts(void)\n \n \treftable_reader_free(rd);\n \treftable_merged_table_free(merged);\n -@@ t/unit-tests/t-reftable-merged.c: static void test_default_write_opts(void)\n + \tstrbuf_release(&buf);\n + }\n \n - /* XXX test refs_for(oid) */\n +-/* XXX test refs_for(oid) */\n \n -int merged_test_main(int argc, const char *argv[])\n +int cmd_main(int argc, const char *argv[])\n @@ t/unit-tests/t-reftable-merged.c: static void test_default_write_opts(void)\n -\tRUN_TEST(test_merged);\n -\tRUN_TEST(test_default_write_opts);\n -\treturn 0;\n -+\tTEST(t_merged_logs(), \"merged table with log records\");\n -+\tTEST(t_merged_between(), \"seek ref in a merged table\");\n -+\tTEST(t_merged(), \"merged table with multiple updates to same ref\");\n +\tTEST(t_default_write_opts(), \"merged table with default write opts\");\n ++\tTEST(t_merged_logs(), \"merged table with multiple log updates for same ref\");\n ++\tTEST(t_merged_refs(), \"merged table with multiple updates to same ref\");\n ++\tTEST(t_merged_single_record(), \"ref ocurring in only one record can be fetched\");\n +\n +\treturn test_done();\n }\n2: fb0f0946b4 ! 2: a449e2edcf t: harmonize t-reftable-merged.c with coding guidelines\n @@ t/unit-tests/t-reftable-merged.c: merged_table_from_records(struct reftable_ref_\n \t\treftable_reader_free(readers[i]);\n \treftable_free(readers);\n }\n -@@ t/unit-tests/t-reftable-merged.c: static void t_merged_between(void)\n +@@ t/unit-tests/t-reftable-merged.c: static void t_merged_single_record(void)\n \tstruct reftable_reader **readers = NULL;\n \tstruct reftable_merged_table *mt =\n \t\tmerged_table_from_records(refs, &bs, &readers, sizes, bufs, 2);\n @@ t/unit-tests/t-reftable-merged.c: static void t_merged_between(void)\n \tint err;\n \n \tmerged_table_init_iter(mt, &it, BLOCK_TYPE_REF);\n -@@ t/unit-tests/t-reftable-merged.c: static void t_merged_between(void)\n +@@ t/unit-tests/t-reftable-merged.c: static void t_merged_single_record(void)\n \treftable_iterator_destroy(&it);\n \treaders_destroy(readers, 2);\n \treftable_merged_table_free(mt);\n @@ t/unit-tests/t-reftable-merged.c: static void t_merged_between(void)\n \treftable_free(bs);\n }\n \n -@@ t/unit-tests/t-reftable-merged.c: static void t_merged(void)\n +@@ t/unit-tests/t-reftable-merged.c: static void t_merged_refs(void)\n \tstruct reftable_reader **readers = NULL;\n \tstruct reftable_merged_table *mt =\n \t\tmerged_table_from_records(refs, &bs, &readers, sizes, bufs, 3);\n @@ t/unit-tests/t-reftable-merged.c: static void t_merged(void)\n \n \tmerged_table_init_iter(mt, &it, BLOCK_TYPE_REF);\n \terr = reftable_iterator_seek_ref(&it, \"a\");\n -@@ t/unit-tests/t-reftable-merged.c: static void t_merged(void)\n +@@ t/unit-tests/t-reftable-merged.c: static void t_merged_refs(void)\n \tcheck_int(reftable_merged_table_min_update_index(mt), ==, 1);\n \n \twhile (len < 100) { /* cap loops/recursion. */\n @@ t/unit-tests/t-reftable-merged.c: static void t_merged(void)\n \t\tint err = reftable_iterator_next_ref(&it, &ref);\n \t\tif (err > 0)\n \t\t\tbreak;\n -@@ t/unit-tests/t-reftable-merged.c: static void t_merged(void)\n +@@ t/unit-tests/t-reftable-merged.c: static void t_merged_refs(void)\n \treftable_iterator_destroy(&it);\n \n \tcheck_int(ARRAY_SIZE(want), ==, len);\n-: ---------- > 3: bba993bb26 t-reftable-merged: improve the test t_merged_single_record()\n-: ---------- > 4: 4d508eaa02 t-reftable-merged: improve the const-correctness of helper functions\n3: 85731b6358 ! 5: 1c9ba26c22 t-reftable-merged: add tests for reftable_merged_table_max_update_index\n @@ Commit message\n Signed-off-by: Chandra Pratap \n \n ## t/unit-tests/t-reftable-merged.c ##\n -@@ t/unit-tests/t-reftable-merged.c: static void t_merged(void)\n +@@ t/unit-tests/t-reftable-merged.c: static void t_merged_refs(void)\n \tcheck(!err);\n \tcheck_int(reftable_merged_table_hash_id(mt), ==, GIT_SHA1_FORMAT_ID);\n \tcheck_int(reftable_merged_table_min_update_index(mt), ==, 1);\n4: 5d6fd842ac < -: ---------- t-reftable-merged: use reftable_ref_record_equal to compare ref records\n-: ---------- > 6: 309c7f412e t-reftable-merged: use reftable_ref_record_equal to compare ref records\n5: d8cbb73533 = 7: 2cd7f5b0b4 t-reftable-merged: add test for REFTABLE_FORMAT_ERROR\n\n"},{"id":"498339","messageId":"20240709053847.4453-2-chandrapratap3519@gmail.com","threadId":"61730","inReplyTo":"20240709053847.4453-1-chandrapratap3519@gmail.com","subject":"[PATCH v2 1/7] t: move reftable/merged_test.c to the unit testing framework","fromName":"Chandra Pratap","fromEmail":"chandrapratap3519@gmail.com","sentAt":"2024-07-09T05:28:40Z","receivedAt":"2024-07-09T05:39:22Z","isPatch":true,"sender":{"key":"chandrapratap3519@gmail.com","avatar":null},"body":"reftable/merged_test.c exercises the functions defined in\nreftable/merged.{c, h}. Migrate reftable/merged_test.c to the unit\ntesting framework. Migration involves refactoring the tests\nto use the unit testing framework instead of reftable's test\nframework and renaming the tests according to unit-tests' naming\nconventions.\n\nAlso, move strbuf_add_void() and noop_flush() from\nreftable/test_framework.c to the ported test. This is because\nboth these functions are used in the merged tests and\nreftable/test_framework.{c, h} is not #included in the ported test.\n\nMentored-by: Patrick Steinhardt \nMentored-by: Christian Couder \nSigned-off-by: Chandra Pratap \n---\n Makefile | 2 +-\n t/helper/test-reftable.c | 1 -\n .../unit-tests/t-reftable-merged.c | 113 +++++++++---------\n 3 files changed, 60 insertions(+), 56 deletions(-)\n rename reftable/merged_test.c => t/unit-tests/t-reftable-merged.c (84%)\n\ndiff --git a/Makefile b/Makefile\nindex 3eab701b10..e5d1b53991 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1340,6 +1340,7 @@ UNIT_TEST_PROGRAMS += t-mem-pool\n UNIT_TEST_PROGRAMS += t-oidtree\n UNIT_TEST_PROGRAMS += t-prio-queue\n UNIT_TEST_PROGRAMS += t-reftable-basics\n+UNIT_TEST_PROGRAMS += t-reftable-merged\n UNIT_TEST_PROGRAMS += t-strbuf\n UNIT_TEST_PROGRAMS += t-strcmp-offset\n UNIT_TEST_PROGRAMS += t-strvec\n@@ -2679,7 +2680,6 @@ REFTABLE_OBJS += reftable/writer.o\n \n REFTABLE_TEST_OBJS += reftable/block_test.o\n REFTABLE_TEST_OBJS += reftable/dump.o\n-REFTABLE_TEST_OBJS += reftable/merged_test.o\n REFTABLE_TEST_OBJS += reftable/pq_test.o\n REFTABLE_TEST_OBJS += reftable/record_test.o\n REFTABLE_TEST_OBJS += reftable/readwrite_test.o\ndiff --git a/t/helper/test-reftable.c b/t/helper/test-reftable.c\nindex 9160bc5da6..0357718fa8 100644\n--- a/t/helper/test-reftable.c\n+++ b/t/helper/test-reftable.c\n@@ -10,7 +10,6 @@ int cmd__reftable(int argc, const char **argv)\n \ttree_test_main(argc, argv);\n \tpq_test_main(argc, argv);\n \treadwrite_test_main(argc, argv);\n-\tmerged_test_main(argc, argv);\n \tstack_test_main(argc, argv);\n \treturn 0;\n }\ndiff --git a/reftable/merged_test.c b/t/unit-tests/t-reftable-merged.c\nsimilarity index 84%\nrename from reftable/merged_test.c\nrename to t/unit-tests/t-reftable-merged.c\nindex a9d6661c13..78a864a54f 100644\n--- a/reftable/merged_test.c\n+++ b/t/unit-tests/t-reftable-merged.c\n@@ -6,20 +6,25 @@ license that can be found in the LICENSE file or at\n https://developers.google.com/open-source/licenses/bsd\n */\n \n-#include \"merged.h\"\n-\n-#include \"system.h\"\n+#include \"test-lib.h\"\n+#include \"reftable/blocksource.h\"\n+#include \"reftable/constants.h\"\n+#include \"reftable/merged.h\"\n+#include \"reftable/reader.h\"\n+#include \"reftable/reftable-generic.h\"\n+#include \"reftable/reftable-merged.h\"\n+#include \"reftable/reftable-writer.h\"\n+\n+static ssize_t strbuf_add_void(void *b, const void *data, size_t sz)\n+{\n+\tstrbuf_add(b, data, sz);\n+\treturn sz;\n+}\n \n-#include \"basics.h\"\n-#include \"blocksource.h\"\n-#include \"constants.h\"\n-#include \"reader.h\"\n-#include \"record.h\"\n-#include \"test_framework.h\"\n-#include \"reftable-merged.h\"\n-#include \"reftable-tests.h\"\n-#include \"reftable-generic.h\"\n-#include \"reftable-writer.h\"\n+static int noop_flush(void *arg)\n+{\n+\treturn 0;\n+}\n \n static void write_test_table(struct strbuf *buf,\n \t\t\t struct reftable_ref_record refs[], int n)\n@@ -49,12 +54,12 @@ static void write_test_table(struct strbuf *buf,\n \tfor (i = 0; i < n; i++) {\n \t\tuint64_t before = refs[i].update_index;\n \t\tint n = reftable_writer_add_ref(w, &refs[i]);\n-\t\tEXPECT(n == 0);\n-\t\tEXPECT(before == refs[i].update_index);\n+\t\tcheck_int(n, ==, 0);\n+\t\tcheck_int(before, ==, refs[i].update_index);\n \t}\n \n \terr = reftable_writer_close(w);\n-\tEXPECT_ERR(err);\n+\tcheck(!err);\n \n \treftable_writer_free(w);\n }\n@@ -76,11 +81,11 @@ static void write_test_log_table(struct strbuf *buf,\n \n \tfor (i = 0; i < n; i++) {\n \t\tint err = reftable_writer_add_log(w, &logs[i]);\n-\t\tEXPECT_ERR(err);\n+\t\tcheck(!err);\n \t}\n \n \terr = reftable_writer_close(w);\n-\tEXPECT_ERR(err);\n+\tcheck(!err);\n \n \treftable_writer_free(w);\n }\n@@ -105,12 +110,12 @@ merged_table_from_records(struct reftable_ref_record **refs,\n \n \t\terr = reftable_new_reader(&(*readers)[i], &(*source)[i],\n \t\t\t\t\t \"name\");\n-\t\tEXPECT_ERR(err);\n+\t\tcheck(!err);\n \t\treftable_table_from_reader(&tabs[i], (*readers)[i]);\n \t}\n \n \terr = reftable_new_merged_table(&mt, tabs, n, GIT_SHA1_FORMAT_ID);\n-\tEXPECT_ERR(err);\n+\tcheck(!err);\n \treturn mt;\n }\n \n@@ -122,7 +127,7 @@ static void readers_destroy(struct reftable_reader **readers, size_t n)\n \treftable_free(readers);\n }\n \n-static void test_merged_between(void)\n+static void t_merged_single_record(void)\n {\n \tstruct reftable_ref_record r1[] = { {\n \t\t.refname = (char *) \"b\",\n@@ -150,11 +155,11 @@ static void test_merged_between(void)\n \n \tmerged_table_init_iter(mt, &it, BLOCK_TYPE_REF);\n \terr = reftable_iterator_seek_ref(&it, \"a\");\n-\tEXPECT_ERR(err);\n+\tcheck(!err);\n \n \terr = reftable_iterator_next_ref(&it, &ref);\n-\tEXPECT_ERR(err);\n-\tEXPECT(ref.update_index == 2);\n+\tcheck(!err);\n+\tcheck_int(ref.update_index, ==, 2);\n \treftable_ref_record_release(&ref);\n \treftable_iterator_destroy(&it);\n \treaders_destroy(readers, 2);\n@@ -165,7 +170,7 @@ static void test_merged_between(void)\n \treftable_free(bs);\n }\n \n-static void test_merged(void)\n+static void t_merged_refs(void)\n {\n \tstruct reftable_ref_record r1[] = {\n \t\t{\n@@ -230,9 +235,9 @@ static void test_merged(void)\n \n \tmerged_table_init_iter(mt, &it, BLOCK_TYPE_REF);\n \terr = reftable_iterator_seek_ref(&it, \"a\");\n-\tEXPECT_ERR(err);\n-\tEXPECT(reftable_merged_table_hash_id(mt) == GIT_SHA1_FORMAT_ID);\n-\tEXPECT(reftable_merged_table_min_update_index(mt) == 1);\n+\tcheck(!err);\n+\tcheck_int(reftable_merged_table_hash_id(mt), ==, GIT_SHA1_FORMAT_ID);\n+\tcheck_int(reftable_merged_table_min_update_index(mt), ==, 1);\n \n \twhile (len < 100) { /* cap loops/recursion. */\n \t\tstruct reftable_ref_record ref = { NULL };\n@@ -245,9 +250,9 @@ static void test_merged(void)\n \t}\n \treftable_iterator_destroy(&it);\n \n-\tEXPECT(ARRAY_SIZE(want) == len);\n+\tcheck_int(ARRAY_SIZE(want), ==, len);\n \tfor (i = 0; i < len; i++) {\n-\t\tEXPECT(reftable_ref_record_equal(want[i], &out[i],\n+\t\tcheck(reftable_ref_record_equal(want[i], &out[i],\n \t\t\t\t\t\t GIT_SHA1_RAWSZ));\n \t}\n \tfor (i = 0; i < len; i++) {\n@@ -283,16 +288,16 @@ merged_table_from_log_records(struct reftable_log_record **logs,\n \n \t\terr = reftable_new_reader(&(*readers)[i], &(*source)[i],\n \t\t\t\t\t \"name\");\n-\t\tEXPECT_ERR(err);\n+\t\tcheck(!err);\n \t\treftable_table_from_reader(&tabs[i], (*readers)[i]);\n \t}\n \n \terr = reftable_new_merged_table(&mt, tabs, n, GIT_SHA1_FORMAT_ID);\n-\tEXPECT_ERR(err);\n+\tcheck(!err);\n \treturn mt;\n }\n \n-static void test_merged_logs(void)\n+static void t_merged_logs(void)\n {\n \tstruct reftable_log_record r1[] = {\n \t\t{\n@@ -362,9 +367,9 @@ static void test_merged_logs(void)\n \n \tmerged_table_init_iter(mt, &it, BLOCK_TYPE_LOG);\n \terr = reftable_iterator_seek_log(&it, \"a\");\n-\tEXPECT_ERR(err);\n-\tEXPECT(reftable_merged_table_hash_id(mt) == GIT_SHA1_FORMAT_ID);\n-\tEXPECT(reftable_merged_table_min_update_index(mt) == 1);\n+\tcheck(!err);\n+\tcheck_int(reftable_merged_table_hash_id(mt), ==, GIT_SHA1_FORMAT_ID);\n+\tcheck_int(reftable_merged_table_min_update_index(mt), ==, 1);\n \n \twhile (len < 100) { /* cap loops/recursion. */\n \t\tstruct reftable_log_record log = { NULL };\n@@ -377,19 +382,19 @@ static void test_merged_logs(void)\n \t}\n \treftable_iterator_destroy(&it);\n \n-\tEXPECT(ARRAY_SIZE(want) == len);\n+\tcheck_int(ARRAY_SIZE(want), ==, len);\n \tfor (i = 0; i < len; i++) {\n-\t\tEXPECT(reftable_log_record_equal(want[i], &out[i],\n+\t\tcheck(reftable_log_record_equal(want[i], &out[i],\n \t\t\t\t\t\t GIT_SHA1_RAWSZ));\n \t}\n \n \tmerged_table_init_iter(mt, &it, BLOCK_TYPE_LOG);\n \terr = reftable_iterator_seek_log_at(&it, \"a\", 2);\n-\tEXPECT_ERR(err);\n+\tcheck(!err);\n \treftable_log_record_release(&out[0]);\n \terr = reftable_iterator_next_log(&it, &out[0]);\n-\tEXPECT_ERR(err);\n-\tEXPECT(reftable_log_record_equal(&out[0], &r3[0], GIT_SHA1_RAWSZ));\n+\tcheck(!err);\n+\tcheck(reftable_log_record_equal(&out[0], &r3[0], GIT_SHA1_RAWSZ));\n \treftable_iterator_destroy(&it);\n \n \tfor (i = 0; i < len; i++) {\n@@ -405,7 +410,7 @@ static void test_merged_logs(void)\n \treftable_free(bs);\n }\n \n-static void test_default_write_opts(void)\n+static void t_default_write_opts(void)\n {\n \tstruct reftable_write_options opts = { 0 };\n \tstruct strbuf buf = STRBUF_INIT;\n@@ -426,36 +431,36 @@ static void test_default_write_opts(void)\n \treftable_writer_set_limits(w, 1, 1);\n \n \terr = reftable_writer_add_ref(w, &rec);\n-\tEXPECT_ERR(err);\n+\tcheck(!err);\n \n \terr = reftable_writer_close(w);\n-\tEXPECT_ERR(err);\n+\tcheck(!err);\n \treftable_writer_free(w);\n \n \tblock_source_from_strbuf(&source, &buf);\n \n \terr = reftable_new_reader(&rd, &source, \"filename\");\n-\tEXPECT_ERR(err);\n+\tcheck(!err);\n \n \thash_id = reftable_reader_hash_id(rd);\n-\tEXPECT(hash_id == GIT_SHA1_FORMAT_ID);\n+\tcheck_int(hash_id, ==, GIT_SHA1_FORMAT_ID);\n \n \treftable_table_from_reader(&tab[0], rd);\n \terr = reftable_new_merged_table(&merged, tab, 1, GIT_SHA1_FORMAT_ID);\n-\tEXPECT_ERR(err);\n+\tcheck(!err);\n \n \treftable_reader_free(rd);\n \treftable_merged_table_free(merged);\n \tstrbuf_release(&buf);\n }\n \n-/* XXX test refs_for(oid) */\n \n-int merged_test_main(int argc, const char *argv[])\n+int cmd_main(int argc, const char *argv[])\n {\n-\tRUN_TEST(test_merged_logs);\n-\tRUN_TEST(test_merged_between);\n-\tRUN_TEST(test_merged);\n-\tRUN_TEST(test_default_write_opts);\n-\treturn 0;\n+\tTEST(t_default_write_opts(), \"merged table with default write opts\");\n+\tTEST(t_merged_logs(), \"merged table with multiple log updates for same ref\");\n+\tTEST(t_merged_refs(), \"merged table with multiple updates to same ref\");\n+\tTEST(t_merged_single_record(), \"ref ocurring in only one record can be fetched\");\n+\n+\treturn test_done();\n }\n-- \n2.45.2.404.g9eaef5822c\n\n"},{"id":"498340","messageId":"20240709053847.4453-3-chandrapratap3519@gmail.com","threadId":"61730","inReplyTo":"20240709053847.4453-1-chandrapratap3519@gmail.com","subject":"[PATCH v2 2/7] t: harmonize t-reftable-merged.c with coding guidelines","fromName":"Chandra Pratap","fromEmail":"chandrapratap3519@gmail.com","sentAt":"2024-07-09T05:28:41Z","receivedAt":"2024-07-09T05:39:24Z","isPatch":true,"sender":{"key":"chandrapratap3519@gmail.com","avatar":null},"body":"Harmonize the newly ported test unit-tests/t-reftable-merged.c\nwith the following guidelines:\n- Single line control flow statements like 'for' and 'if'\n must omit curly braces.\n- Structs must be 0-initialized with '= { 0 }' instead of '= { NULL }'.\n- Array indices must be of type 'size_t', not 'int'.\n- It is fine to use C99 initial declaration in 'for' loop.\n\nMentored-by: Patrick Steinhardt \nMentored-by: Christian Couder \nSigned-off-by: Chandra Pratap \n---\n t/unit-tests/t-reftable-merged.c | 52 ++++++++++++--------------------\n 1 file changed, 20 insertions(+), 32 deletions(-)\n\ndiff --git a/t/unit-tests/t-reftable-merged.c b/t/unit-tests/t-reftable-merged.c\nindex 78a864a54f..a984116619 100644\n--- a/t/unit-tests/t-reftable-merged.c\n+++ b/t/unit-tests/t-reftable-merged.c\n@@ -40,12 +40,10 @@ static void write_test_table(struct strbuf *buf,\n \tstruct reftable_writer *w = NULL;\n \tfor (i = 0; i < n; i++) {\n \t\tuint64_t ui = refs[i].update_index;\n-\t\tif (ui > max) {\n+\t\tif (ui > max)\n \t\t\tmax = ui;\n-\t\t}\n-\t\tif (ui < min) {\n+\t\tif (ui < min)\n \t\t\tmin = ui;\n-\t\t}\n \t}\n \n \tw = reftable_new_writer(&strbuf_add_void, &noop_flush, buf, &opts);\n@@ -68,7 +66,6 @@ static void write_test_log_table(struct strbuf *buf,\n \t\t\t\t struct reftable_log_record logs[], int n,\n \t\t\t\t uint64_t update_index)\n {\n-\tint i = 0;\n \tint err;\n \n \tstruct reftable_write_options opts = {\n@@ -79,7 +76,7 @@ static void write_test_log_table(struct strbuf *buf,\n \tw = reftable_new_writer(&strbuf_add_void, &noop_flush, buf, &opts);\n \treftable_writer_set_limits(w, update_index, update_index);\n \n-\tfor (i = 0; i < n; i++) {\n+\tfor (int i = 0; i < n; i++) {\n \t\tint err = reftable_writer_add_log(w, &logs[i]);\n \t\tcheck(!err);\n \t}\n@@ -121,8 +118,7 @@ merged_table_from_records(struct reftable_ref_record **refs,\n \n static void readers_destroy(struct reftable_reader **readers, size_t n)\n {\n-\tint i = 0;\n-\tfor (; i < n; i++)\n+\tfor (size_t i = 0; i < n; i++)\n \t\treftable_reader_free(readers[i]);\n \treftable_free(readers);\n }\n@@ -148,9 +144,8 @@ static void t_merged_single_record(void)\n \tstruct reftable_reader **readers = NULL;\n \tstruct reftable_merged_table *mt =\n \t\tmerged_table_from_records(refs, &bs, &readers, sizes, bufs, 2);\n-\tint i;\n-\tstruct reftable_ref_record ref = { NULL };\n-\tstruct reftable_iterator it = { NULL };\n+\tstruct reftable_ref_record ref = { 0 };\n+\tstruct reftable_iterator it = { 0 };\n \tint err;\n \n \tmerged_table_init_iter(mt, &it, BLOCK_TYPE_REF);\n@@ -164,9 +159,8 @@ static void t_merged_single_record(void)\n \treftable_iterator_destroy(&it);\n \treaders_destroy(readers, 2);\n \treftable_merged_table_free(mt);\n-\tfor (i = 0; i < ARRAY_SIZE(bufs); i++) {\n+\tfor (size_t i = 0; i < ARRAY_SIZE(bufs); i++)\n \t\tstrbuf_release(&bufs[i]);\n-\t}\n \treftable_free(bs);\n }\n \n@@ -226,12 +220,12 @@ static void t_merged_refs(void)\n \tstruct reftable_reader **readers = NULL;\n \tstruct reftable_merged_table *mt =\n \t\tmerged_table_from_records(refs, &bs, &readers, sizes, bufs, 3);\n-\tstruct reftable_iterator it = { NULL };\n+\tstruct reftable_iterator it = { 0 };\n \tint err;\n \tstruct reftable_ref_record *out = NULL;\n \tsize_t len = 0;\n \tsize_t cap = 0;\n-\tint i = 0;\n+\tsize_t i;\n \n \tmerged_table_init_iter(mt, &it, BLOCK_TYPE_REF);\n \terr = reftable_iterator_seek_ref(&it, \"a\");\n@@ -240,7 +234,7 @@ static void t_merged_refs(void)\n \tcheck_int(reftable_merged_table_min_update_index(mt), ==, 1);\n \n \twhile (len < 100) { /* cap loops/recursion. */\n-\t\tstruct reftable_ref_record ref = { NULL };\n+\t\tstruct reftable_ref_record ref = { 0 };\n \t\tint err = reftable_iterator_next_ref(&it, &ref);\n \t\tif (err > 0)\n \t\t\tbreak;\n@@ -251,18 +245,15 @@ static void t_merged_refs(void)\n \treftable_iterator_destroy(&it);\n \n \tcheck_int(ARRAY_SIZE(want), ==, len);\n-\tfor (i = 0; i < len; i++) {\n+\tfor (i = 0; i < len; i++)\n \t\tcheck(reftable_ref_record_equal(want[i], &out[i],\n \t\t\t\t\t\t GIT_SHA1_RAWSZ));\n-\t}\n-\tfor (i = 0; i < len; i++) {\n+\tfor (i = 0; i < len; i++)\n \t\treftable_ref_record_release(&out[i]);\n-\t}\n \treftable_free(out);\n \n-\tfor (i = 0; i < 3; i++) {\n+\tfor (i = 0; i < 3; i++)\n \t\tstrbuf_release(&bufs[i]);\n-\t}\n \treaders_destroy(readers, 3);\n \treftable_merged_table_free(mt);\n \treftable_free(bs);\n@@ -358,12 +349,12 @@ static void t_merged_logs(void)\n \tstruct reftable_reader **readers = NULL;\n \tstruct reftable_merged_table *mt = merged_table_from_log_records(\n \t\tlogs, &bs, &readers, sizes, bufs, 3);\n-\tstruct reftable_iterator it = { NULL };\n+\tstruct reftable_iterator it = { 0 };\n \tint err;\n \tstruct reftable_log_record *out = NULL;\n \tsize_t len = 0;\n \tsize_t cap = 0;\n-\tint i = 0;\n+\tsize_t i = 0;\n \n \tmerged_table_init_iter(mt, &it, BLOCK_TYPE_LOG);\n \terr = reftable_iterator_seek_log(&it, \"a\");\n@@ -372,7 +363,7 @@ static void t_merged_logs(void)\n \tcheck_int(reftable_merged_table_min_update_index(mt), ==, 1);\n \n \twhile (len < 100) { /* cap loops/recursion. */\n-\t\tstruct reftable_log_record log = { NULL };\n+\t\tstruct reftable_log_record log = { 0 };\n \t\tint err = reftable_iterator_next_log(&it, &log);\n \t\tif (err > 0)\n \t\t\tbreak;\n@@ -383,10 +374,9 @@ static void t_merged_logs(void)\n \treftable_iterator_destroy(&it);\n \n \tcheck_int(ARRAY_SIZE(want), ==, len);\n-\tfor (i = 0; i < len; i++) {\n+\tfor (i = 0; i < len; i++)\n \t\tcheck(reftable_log_record_equal(want[i], &out[i],\n \t\t\t\t\t\t GIT_SHA1_RAWSZ));\n-\t}\n \n \tmerged_table_init_iter(mt, &it, BLOCK_TYPE_LOG);\n \terr = reftable_iterator_seek_log_at(&it, \"a\", 2);\n@@ -397,14 +387,12 @@ static void t_merged_logs(void)\n \tcheck(reftable_log_record_equal(&out[0], &r3[0], GIT_SHA1_RAWSZ));\n \treftable_iterator_destroy(&it);\n \n-\tfor (i = 0; i < len; i++) {\n+\tfor (i = 0; i < len; i++)\n \t\treftable_log_record_release(&out[i]);\n-\t}\n \treftable_free(out);\n \n-\tfor (i = 0; i < 3; i++) {\n+\tfor (i = 0; i < 3; i++)\n \t\tstrbuf_release(&bufs[i]);\n-\t}\n \treaders_destroy(readers, 3);\n \treftable_merged_table_free(mt);\n \treftable_free(bs);\n@@ -422,7 +410,7 @@ static void t_default_write_opts(void)\n \t\t.update_index = 1,\n \t};\n \tint err;\n-\tstruct reftable_block_source source = { NULL };\n+\tstruct reftable_block_source source = { 0 };\n \tstruct reftable_table *tab = reftable_calloc(1, sizeof(*tab));\n \tuint32_t hash_id;\n \tstruct reftable_reader *rd = NULL;\n-- \n2.45.2.404.g9eaef5822c\n\n"},{"id":"498341","messageId":"20240709053847.4453-4-chandrapratap3519@gmail.com","threadId":"61730","inReplyTo":"20240709053847.4453-1-chandrapratap3519@gmail.com","subject":"[PATCH v2 3/7] t-reftable-merged: improve the test t_merged_single_record()","fromName":"Chandra Pratap","fromEmail":"chandrapratap3519@gmail.com","sentAt":"2024-07-09T05:28:42Z","receivedAt":"2024-07-09T05:39:27Z","isPatch":true,"sender":{"key":"chandrapratap3519@gmail.com","avatar":null},"body":"In t-reftable-merged.c, the test t_merged_single_record() ensures\nthat a ref ('a') which occurs in only one of the records ('r2')\ncan be retrieved. Improve this test by adding another record 'r3'\nto ensure that ref 'a' only occurs in 'r2' and that merged tables\ndon't simply read the last record.\n\nMentored-by: Patrick Steinhardt \nMentored-by: Christian Couder \nSigned-off-by: Chandra Pratap \n---\n t/unit-tests/t-reftable-merged.c | 15 ++++++++++-----\n 1 file changed, 10 insertions(+), 5 deletions(-)\n\ndiff --git a/t/unit-tests/t-reftable-merged.c b/t/unit-tests/t-reftable-merged.c\nindex a984116619..85ebb96aaa 100644\n--- a/t/unit-tests/t-reftable-merged.c\n+++ b/t/unit-tests/t-reftable-merged.c\n@@ -136,14 +136,19 @@ static void t_merged_single_record(void)\n \t\t.update_index = 2,\n \t\t.value_type = REFTABLE_REF_DELETION,\n \t} };\n+\tstruct reftable_ref_record r3[] = { {\n+\t\t.refname = (char *) \"c\",\n+\t\t.update_index = 3,\n+\t\t.value_type = REFTABLE_REF_DELETION,\n+\t} };\n \n-\tstruct reftable_ref_record *refs[] = { r1, r2 };\n-\tint sizes[] = { 1, 1 };\n-\tstruct strbuf bufs[2] = { STRBUF_INIT, STRBUF_INIT };\n+\tstruct reftable_ref_record *refs[] = { r1, r2, r3 };\n+\tint sizes[] = { 1, 1, 1 };\n+\tstruct strbuf bufs[3] = { STRBUF_INIT, STRBUF_INIT, STRBUF_INIT };\n \tstruct reftable_block_source *bs = NULL;\n \tstruct reftable_reader **readers = NULL;\n \tstruct reftable_merged_table *mt =\n-\t\tmerged_table_from_records(refs, &bs, &readers, sizes, bufs, 2);\n+\t\tmerged_table_from_records(refs, &bs, &readers, sizes, bufs, 3);\n \tstruct reftable_ref_record ref = { 0 };\n \tstruct reftable_iterator it = { 0 };\n \tint err;\n@@ -157,7 +162,7 @@ static void t_merged_single_record(void)\n \tcheck_int(ref.update_index, ==, 2);\n \treftable_ref_record_release(&ref);\n \treftable_iterator_destroy(&it);\n-\treaders_destroy(readers, 2);\n+\treaders_destroy(readers, 3);\n \treftable_merged_table_free(mt);\n \tfor (size_t i = 0; i < ARRAY_SIZE(bufs); i++)\n \t\tstrbuf_release(&bufs[i]);\n-- \n2.45.2.404.g9eaef5822c\n\n"},{"id":"498342","messageId":"20240709053847.4453-5-chandrapratap3519@gmail.com","threadId":"61730","inReplyTo":"20240709053847.4453-1-chandrapratap3519@gmail.com","subject":"[PATCH v2 4/7] t-reftable-merged: improve the const-correctness of helper functions","fromName":"Chandra Pratap","fromEmail":"chandrapratap3519@gmail.com","sentAt":"2024-07-09T05:28:43Z","receivedAt":"2024-07-09T05:39:29Z","isPatch":true,"sender":{"key":"chandrapratap3519@gmail.com","avatar":null},"body":"In t-reftable-merged.c, a number of helper functions used by the\ntests can be re-defined with parameters made 'const' which makes\nit easier to understand if they're read-only or not. Re-define\nthese functions along these lines.\n\nMentored-by: Patrick Steinhardt \nMentored-by: Christian Couder \nSigned-off-by: Chandra Pratap \n---\n t/unit-tests/t-reftable-merged.c | 19 +++++++++----------\n 1 file changed, 9 insertions(+), 10 deletions(-)\n\ndiff --git a/t/unit-tests/t-reftable-merged.c b/t/unit-tests/t-reftable-merged.c\nindex 85ebb96aaa..d151d6557b 100644\n--- a/t/unit-tests/t-reftable-merged.c\n+++ b/t/unit-tests/t-reftable-merged.c\n@@ -15,7 +15,7 @@ license that can be found in the LICENSE file or at\n #include \"reftable/reftable-merged.h\"\n #include \"reftable/reftable-writer.h\"\n \n-static ssize_t strbuf_add_void(void *b, const void *data, size_t sz)\n+static ssize_t strbuf_add_void(void *b, const void *data, const size_t sz)\n {\n \tstrbuf_add(b, data, sz);\n \treturn sz;\n@@ -27,7 +27,7 @@ static int noop_flush(void *arg)\n }\n \n static void write_test_table(struct strbuf *buf,\n-\t\t\t struct reftable_ref_record refs[], int n)\n+\t\t\t struct reftable_ref_record refs[], const int n)\n {\n \tuint64_t min = 0xffffffff;\n \tuint64_t max = 0;\n@@ -62,9 +62,8 @@ static void write_test_table(struct strbuf *buf,\n \treftable_writer_free(w);\n }\n \n-static void write_test_log_table(struct strbuf *buf,\n-\t\t\t\t struct reftable_log_record logs[], int n,\n-\t\t\t\t uint64_t update_index)\n+static void write_test_log_table(struct strbuf *buf, struct reftable_log_record logs[],\n+\t\t\t\t const int n, const uint64_t update_index)\n {\n \tint err;\n \n@@ -90,8 +89,8 @@ static void write_test_log_table(struct strbuf *buf,\n static struct reftable_merged_table *\n merged_table_from_records(struct reftable_ref_record **refs,\n \t\t\t struct reftable_block_source **source,\n-\t\t\t struct reftable_reader ***readers, int *sizes,\n-\t\t\t struct strbuf *buf, size_t n)\n+\t\t\t struct reftable_reader ***readers, const int *sizes,\n+\t\t\t struct strbuf *buf, const size_t n)\n {\n \tstruct reftable_merged_table *mt = NULL;\n \tstruct reftable_table *tabs;\n@@ -116,7 +115,7 @@ merged_table_from_records(struct reftable_ref_record **refs,\n \treturn mt;\n }\n \n-static void readers_destroy(struct reftable_reader **readers, size_t n)\n+static void readers_destroy(struct reftable_reader **readers, const size_t n)\n {\n \tfor (size_t i = 0; i < n; i++)\n \t\treftable_reader_free(readers[i]);\n@@ -267,8 +266,8 @@ static void t_merged_refs(void)\n static struct reftable_merged_table *\n merged_table_from_log_records(struct reftable_log_record **logs,\n \t\t\t struct reftable_block_source **source,\n-\t\t\t struct reftable_reader ***readers, int *sizes,\n-\t\t\t struct strbuf *buf, size_t n)\n+\t\t\t struct reftable_reader ***readers, const int *sizes,\n+\t\t\t struct strbuf *buf, const size_t n)\n {\n \tstruct reftable_merged_table *mt = NULL;\n \tstruct reftable_table *tabs;\n-- \n2.45.2.404.g9eaef5822c\n\n"},{"id":"498343","messageId":"20240709053847.4453-6-chandrapratap3519@gmail.com","threadId":"61730","inReplyTo":"20240709053847.4453-1-chandrapratap3519@gmail.com","subject":"[PATCH v2 5/7] t-reftable-merged: add tests for reftable_merged_table_max_update_index","fromName":"Chandra Pratap","fromEmail":"chandrapratap3519@gmail.com","sentAt":"2024-07-09T05:28:44Z","receivedAt":"2024-07-09T05:39:33Z","isPatch":true,"sender":{"key":"chandrapratap3519@gmail.com","avatar":null},"body":"reftable_merged_table_max_update_index() as defined by reftable/\nmerged.{c, h} returns the maximum update index in a merged table.\nSince this function is currently unexercised, add tests for it.\n\nMentored-by: Patrick Steinhardt \nMentored-by: Christian Couder \nSigned-off-by: Chandra Pratap \n---\n t/unit-tests/t-reftable-merged.c | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/t/unit-tests/t-reftable-merged.c b/t/unit-tests/t-reftable-merged.c\nindex d151d6557b..ffc9bd25d2 100644\n--- a/t/unit-tests/t-reftable-merged.c\n+++ b/t/unit-tests/t-reftable-merged.c\n@@ -236,6 +236,7 @@ static void t_merged_refs(void)\n \tcheck(!err);\n \tcheck_int(reftable_merged_table_hash_id(mt), ==, GIT_SHA1_FORMAT_ID);\n \tcheck_int(reftable_merged_table_min_update_index(mt), ==, 1);\n+\tcheck_int(reftable_merged_table_max_update_index(mt), ==, 3);\n \n \twhile (len < 100) { /* cap loops/recursion. */\n \t\tstruct reftable_ref_record ref = { 0 };\n@@ -365,6 +366,7 @@ static void t_merged_logs(void)\n \tcheck(!err);\n \tcheck_int(reftable_merged_table_hash_id(mt), ==, GIT_SHA1_FORMAT_ID);\n \tcheck_int(reftable_merged_table_min_update_index(mt), ==, 1);\n+\tcheck_int(reftable_merged_table_max_update_index(mt), ==, 3);\n \n \twhile (len < 100) { /* cap loops/recursion. */\n \t\tstruct reftable_log_record log = { 0 };\n-- \n2.45.2.404.g9eaef5822c\n\n"},{"id":"498344","messageId":"20240709053847.4453-7-chandrapratap3519@gmail.com","threadId":"61730","inReplyTo":"20240709053847.4453-1-chandrapratap3519@gmail.com","subject":"[PATCH v2 6/7] t-reftable-merged: use reftable_ref_record_equal to compare ref records","fromName":"Chandra Pratap","fromEmail":"chandrapratap3519@gmail.com","sentAt":"2024-07-09T05:28:45Z","receivedAt":"2024-07-09T05:39:34Z","isPatch":true,"sender":{"key":"chandrapratap3519@gmail.com","avatar":null},"body":"In the test t_merged_single_record() defined in t-reftable-merged.c,\nthe 'input' and 'expected' ref records are checked for equality\nby comparing their update indices. It is very much possible for\ntwo different ref records to have the same update indices. Use\nreftable_ref_record_equal() instead for a stronger check.\n\nMentored-by: Patrick Steinhardt \nMentored-by: Christian Couder \nSigned-off-by: Chandra Pratap \n---\n t/unit-tests/t-reftable-merged.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/unit-tests/t-reftable-merged.c b/t/unit-tests/t-reftable-merged.c\nindex ffc9bd25d2..e0054e379e 100644\n--- a/t/unit-tests/t-reftable-merged.c\n+++ b/t/unit-tests/t-reftable-merged.c\n@@ -158,7 +158,7 @@ static void t_merged_single_record(void)\n \n \terr = reftable_iterator_next_ref(&it, &ref);\n \tcheck(!err);\n-\tcheck_int(ref.update_index, ==, 2);\n+\tcheck(reftable_ref_record_equal(&r2[0], &ref, GIT_SHA1_RAWSZ));\n \treftable_ref_record_release(&ref);\n \treftable_iterator_destroy(&it);\n \treaders_destroy(readers, 3);\n-- \n2.45.2.404.g9eaef5822c\n\n"},{"id":"498345","messageId":"20240709053847.4453-8-chandrapratap3519@gmail.com","threadId":"61730","inReplyTo":"20240709053847.4453-1-chandrapratap3519@gmail.com","subject":"[PATCH v2 7/7] t-reftable-merged: add test for REFTABLE_FORMAT_ERROR","fromName":"Chandra Pratap","fromEmail":"chandrapratap3519@gmail.com","sentAt":"2024-07-09T05:28:46Z","receivedAt":"2024-07-09T05:39:37Z","isPatch":true,"sender":{"key":"chandrapratap3519@gmail.com","avatar":null},"body":"When calling reftable_new_merged_table(), if the hash ID of the\npasssed reftable_table parameter doesn't match the passed hash_id\nparameter, a REFTABLE_FORMAT_ERROR is thrown. This case is\ncurrently left unexercised, so add a test for the same.\n\nMentored-by: Patrick Steinhardt \nMentored-by: Christian Couder \nSigned-off-by: Chandra Pratap \n---\n t/unit-tests/t-reftable-merged.c | 3 +++\n 1 file changed, 3 insertions(+)\n\ndiff --git a/t/unit-tests/t-reftable-merged.c b/t/unit-tests/t-reftable-merged.c\nindex e0054e379e..50047aa90b 100644\n--- a/t/unit-tests/t-reftable-merged.c\n+++ b/t/unit-tests/t-reftable-merged.c\n@@ -11,6 +11,7 @@ license that can be found in the LICENSE file or at\n #include \"reftable/constants.h\"\n #include \"reftable/merged.h\"\n #include \"reftable/reader.h\"\n+#include \"reftable/reftable-error.h\"\n #include \"reftable/reftable-generic.h\"\n #include \"reftable/reftable-merged.h\"\n #include \"reftable/reftable-writer.h\"\n@@ -440,6 +441,8 @@ static void t_default_write_opts(void)\n \tcheck_int(hash_id, ==, GIT_SHA1_FORMAT_ID);\n \n \treftable_table_from_reader(&tab[0], rd);\n+\terr = reftable_new_merged_table(&merged, tab, 1, GIT_SHA256_FORMAT_ID);\n+\tcheck_int(err, ==, REFTABLE_FORMAT_ERROR);\n \terr = reftable_new_merged_table(&merged, tab, 1, GIT_SHA1_FORMAT_ID);\n \tcheck(!err);\n \n-- \n2.45.2.404.g9eaef5822c\n\n"},{"id":"498387","messageId":"f5j7warzbamijaog6ur6uovr6i7fqwadrjbevnyyocz3orjux4@wugw42cut5oi","threadId":"61730","inReplyTo":"20240709053847.4453-2-chandrapratap3519@gmail.com","subject":"Re: [PATCH v2 1/7] t: move reftable/merged_test.c to the unit testing framework","fromName":"Justin Tobler","fromEmail":"jltobler@gmail.com","sentAt":"2024-07-09T23:05:06Z","receivedAt":"2024-07-09T23:05:37Z","isPatch":true,"sender":{"key":"jltobler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53454972?v=4"},"body":"On 24/07/09 10:58AM, Chandra Pratap wrote:\n> reftable/merged_test.c exercises the functions defined in\n> reftable/merged.{c, h}. Migrate reftable/merged_test.c to the unit\n> testing framework. Migration involves refactoring the tests\n> to use the unit testing framework instead of reftable's test\n> framework and renaming the tests according to unit-tests' naming\n> conventions.\n> \n> Also, move strbuf_add_void() and noop_flush() from\n> reftable/test_framework.c to the ported test. This is because\n> both these functions are used in the merged tests and\n> reftable/test_framework.{c, h} is not #included in the ported test.\n> \n[snip]\n> \n> -int merged_test_main(int argc, const char *argv[])\n\nSince we are removing this function definition, should we also remove\n`merged_test_main` from \"reftable/reftable-tests.h\"?\n\n> +int cmd_main(int argc, const char *argv[])\n> {\n> -\tRUN_TEST(test_merged_logs);\n> -\tRUN_TEST(test_merged_between);\n> -\tRUN_TEST(test_merged);\n> -\tRUN_TEST(test_default_write_opts);\n> -\treturn 0;\n> +\tTEST(t_default_write_opts(), \"merged table with default write opts\");\n> +\tTEST(t_merged_logs(), \"merged table with multiple log updates for same ref\");\n> +\tTEST(t_merged_refs(), \"merged table with multiple updates to same ref\");\n> +\tTEST(t_merged_single_record(), \"ref ocurring in only one record can be fetched\");\n> +\n> +\treturn test_done();\n> }\n> -- \n> 2.45.2.404.g9eaef5822c\n> \n> \n"},{"id":"498429","messageId":"CAOLa=ZQOdH4cTQ4iymZAZu0Q_WYTepF0y6zWKQAxtBVirfhsgw@mail.gmail.com","threadId":"61730","inReplyTo":"20240709053847.4453-8-chandrapratap3519@gmail.com","subject":"Re: [PATCH v2 7/7] t-reftable-merged: add test for REFTABLE_FORMAT_ERROR","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2024-07-10T09:18:31Z","receivedAt":"2024-07-10T09:18:33Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Chandra Pratap writes:\n\n> When calling reftable_new_merged_table(), if the hash ID of the\n> passsed reftable_table parameter doesn't match the passed hash_id\n\ns/passsed/passed\n\n> parameter, a REFTABLE_FORMAT_ERROR is thrown. This case is\n> currently left unexercised, so add a test for the same.\n>\n> Mentored-by: Patrick Steinhardt \n> Mentored-by: Christian Couder \n> Signed-off-by: Chandra Pratap \n> ---\n> t/unit-tests/t-reftable-merged.c | 3 +++\n> 1 file changed, 3 insertions(+)\n>\n> diff --git a/t/unit-tests/t-reftable-merged.c b/t/unit-tests/t-reftable-merged.c\n> index e0054e379e..50047aa90b 100644\n> --- a/t/unit-tests/t-reftable-merged.c\n> +++ b/t/unit-tests/t-reftable-merged.c\n> @@ -11,6 +11,7 @@ license that can be found in the LICENSE file or at\n> #include \"reftable/constants.h\"\n> #include \"reftable/merged.h\"\n> #include \"reftable/reader.h\"\n> +#include \"reftable/reftable-error.h\"\n> #include \"reftable/reftable-generic.h\"\n> #include \"reftable/reftable-merged.h\"\n> #include \"reftable/reftable-writer.h\"\n> @@ -440,6 +441,8 @@ static void t_default_write_opts(void)\n> \tcheck_int(hash_id, ==, GIT_SHA1_FORMAT_ID);\n>\n> \treftable_table_from_reader(&tab[0], rd);\n> +\terr = reftable_new_merged_table(&merged, tab, 1, GIT_SHA256_FORMAT_ID);\n> +\tcheck_int(err, ==, REFTABLE_FORMAT_ERROR);\n> \terr = reftable_new_merged_table(&merged, tab, 1, GIT_SHA1_FORMAT_ID);\n> \tcheck(!err);\n>\n> --\n> 2.45.2.404.g9eaef5822c\n"},{"id":"498430","messageId":"CAOLa=ZR4jZhwzyQtJ=zC0wYJ9=u2CPKDgps57e-zCgQ279mYVQ@mail.gmail.com","threadId":"61730","inReplyTo":"20240709053847.4453-1-chandrapratap3519@gmail.com","subject":"Re: [GSoC][PATCH v2 0/7] t: port reftable/merged_test.c to the unit testing framework","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2024-07-10T09:19:31Z","receivedAt":"2024-07-10T09:19:33Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Chandra Pratap writes:\n\n> In the recent codebase update (commit 8bf6fbd, 2023-12-09), a new unit\n> testing framework written entirely in C was introduced to the Git project\n> aimed at simplifying testing and reducing test run times.\n> Currently, tests for the reftable refs-backend are performed by a custom\n> testing framework defined by reftable/test_framework.{c, h}. Port\n> reftable/merged_test.c to the unit testing framework and improve upon\n> the ported test.\n>\n> The first patch in the series moves the test to the unit testing framework,\n> and the rest of the patches improve upon the ported test.\n>\n\nThis series looks good now, sans a typo and Justin's comment!\nThanks!\n\n> Mentored-by: Patrick Steinhardt \n> Mentored-by: Christian Couder \n> Signed-off-by: Chandra Pratap \n>\n> ---\n> Changes in v2:\n> - Rename test functions & description to be more explanatory\n> - Remove a redundant comment in patch 1\n> - Add the 3rd patch which makes improvements to a test\n> - Add the 4th patch which makes apt function parameters 'const'\n> - Remove a redundant check in patch 6\n>\n> CI/PR: https://github.com/gitgitgadget/git/pull/1755\n>\n> Chandra Pratap (7):\n> [PATCH 1/7] t: move reftable/merged_test.c to the unit testing framework\n> [PATCH 2/7] t: harmonize t-reftable-merged.c with coding guidelines\n> [PATCH 3/7] t-reftable-merged: improve the test for t_merged_single_record()\n> [PATCH 4/7] t-reftable-merged: improve the const-correctness of helper functions\n> [PATCH 5/7] t-reftable-merged: add tests for reftable_merged_table_max_update_index\n> [PATCH 6/7] t-reftable-merged: use reftable_ref_record_equal to compare ref records\n> [PATCH 7/7] t-reftable-merged: add test for REFTABLE_FORMAT_ERROR\n>\n> Makefile | 2 +-\n> t/helper/test-reftable.c | 1 -\n> reftable/merged_test.c => t/unit-tests/t-reftable-merged.c | 202 +++++++++++++++----------------\n> 3 files changed, 103 insertions(+), 102 deletions(-)\n>\n> Range-diff against v2:\n> 1: f4d5da52f5 ! 1: 0d71deffad t: move reftable/merged_test.c to the unit testing framework\n> @@ t/unit-tests/t-reftable-merged.c: static void readers_destroy(struct reftable_re\n> }\n>\n> -static void test_merged_between(void)\n> -+static void t_merged_between(void)\n> ++static void t_merged_single_record(void)\n> {\n> \tstruct reftable_ref_record r1[] = { {\n> \t\t.refname = (char *) \"b\",\n> @@ t/unit-tests/t-reftable-merged.c: static void test_merged_between(void)\n> }\n>\n> -static void test_merged(void)\n> -+static void t_merged(void)\n> ++static void t_merged_refs(void)\n> {\n> \tstruct reftable_ref_record r1[] = {\n> \t\t{\n> @@ t/unit-tests/t-reftable-merged.c: static void test_default_write_opts(void)\n>\n> \treftable_reader_free(rd);\n> \treftable_merged_table_free(merged);\n> -@@ t/unit-tests/t-reftable-merged.c: static void test_default_write_opts(void)\n> + \tstrbuf_release(&buf);\n> + }\n>\n> - /* XXX test refs_for(oid) */\n> +-/* XXX test refs_for(oid) */\n>\n> -int merged_test_main(int argc, const char *argv[])\n> +int cmd_main(int argc, const char *argv[])\n> @@ t/unit-tests/t-reftable-merged.c: static void test_default_write_opts(void)\n> -\tRUN_TEST(test_merged);\n> -\tRUN_TEST(test_default_write_opts);\n> -\treturn 0;\n> -+\tTEST(t_merged_logs(), \"merged table with log records\");\n> -+\tTEST(t_merged_between(), \"seek ref in a merged table\");\n> -+\tTEST(t_merged(), \"merged table with multiple updates to same ref\");\n> +\tTEST(t_default_write_opts(), \"merged table with default write opts\");\n> ++\tTEST(t_merged_logs(), \"merged table with multiple log updates for same ref\");\n> ++\tTEST(t_merged_refs(), \"merged table with multiple updates to same ref\");\n> ++\tTEST(t_merged_single_record(), \"ref ocurring in only one record can be fetched\");\n> +\n> +\treturn test_done();\n> }\n> 2: fb0f0946b4 ! 2: a449e2edcf t: harmonize t-reftable-merged.c with coding guidelines\n> @@ t/unit-tests/t-reftable-merged.c: merged_table_from_records(struct reftable_ref_\n> \t\treftable_reader_free(readers[i]);\n> \treftable_free(readers);\n> }\n> -@@ t/unit-tests/t-reftable-merged.c: static void t_merged_between(void)\n> +@@ t/unit-tests/t-reftable-merged.c: static void t_merged_single_record(void)\n> \tstruct reftable_reader **readers = NULL;\n> \tstruct reftable_merged_table *mt =\n> \t\tmerged_table_from_records(refs, &bs, &readers, sizes, bufs, 2);\n> @@ t/unit-tests/t-reftable-merged.c: static void t_merged_between(void)\n> \tint err;\n>\n> \tmerged_table_init_iter(mt, &it, BLOCK_TYPE_REF);\n> -@@ t/unit-tests/t-reftable-merged.c: static void t_merged_between(void)\n> +@@ t/unit-tests/t-reftable-merged.c: static void t_merged_single_record(void)\n> \treftable_iterator_destroy(&it);\n> \treaders_destroy(readers, 2);\n> \treftable_merged_table_free(mt);\n> @@ t/unit-tests/t-reftable-merged.c: static void t_merged_between(void)\n> \treftable_free(bs);\n> }\n>\n> -@@ t/unit-tests/t-reftable-merged.c: static void t_merged(void)\n> +@@ t/unit-tests/t-reftable-merged.c: static void t_merged_refs(void)\n> \tstruct reftable_reader **readers = NULL;\n> \tstruct reftable_merged_table *mt =\n> \t\tmerged_table_from_records(refs, &bs, &readers, sizes, bufs, 3);\n> @@ t/unit-tests/t-reftable-merged.c: static void t_merged(void)\n>\n> \tmerged_table_init_iter(mt, &it, BLOCK_TYPE_REF);\n> \terr = reftable_iterator_seek_ref(&it, \"a\");\n> -@@ t/unit-tests/t-reftable-merged.c: static void t_merged(void)\n> +@@ t/unit-tests/t-reftable-merged.c: static void t_merged_refs(void)\n> \tcheck_int(reftable_merged_table_min_update_index(mt), ==, 1);\n>\n> \twhile (len < 100) { /* cap loops/recursion. */\n> @@ t/unit-tests/t-reftable-merged.c: static void t_merged(void)\n> \t\tint err = reftable_iterator_next_ref(&it, &ref);\n> \t\tif (err > 0)\n> \t\t\tbreak;\n> -@@ t/unit-tests/t-reftable-merged.c: static void t_merged(void)\n> +@@ t/unit-tests/t-reftable-merged.c: static void t_merged_refs(void)\n> \treftable_iterator_destroy(&it);\n>\n> \tcheck_int(ARRAY_SIZE(want), ==, len);\n> -: ---------- > 3: bba993bb26 t-reftable-merged: improve the test t_merged_single_record()\n> -: ---------- > 4: 4d508eaa02 t-reftable-merged: improve the const-correctness of helper functions\n> 3: 85731b6358 ! 5: 1c9ba26c22 t-reftable-merged: add tests for reftable_merged_table_max_update_index\n> @@ Commit message\n> Signed-off-by: Chandra Pratap \n>\n> ## t/unit-tests/t-reftable-merged.c ##\n> -@@ t/unit-tests/t-reftable-merged.c: static void t_merged(void)\n> +@@ t/unit-tests/t-reftable-merged.c: static void t_merged_refs(void)\n> \tcheck(!err);\n> \tcheck_int(reftable_merged_table_hash_id(mt), ==, GIT_SHA1_FORMAT_ID);\n> \tcheck_int(reftable_merged_table_min_update_index(mt), ==, 1);\n> 4: 5d6fd842ac < -: ---------- t-reftable-merged: use reftable_ref_record_equal to compare ref records\n> -: ---------- > 6: 309c7f412e t-reftable-merged: use reftable_ref_record_equal to compare ref records\n> 5: d8cbb73533 = 7: 2cd7f5b0b4 t-reftable-merged: add test for REFTABLE_FORMAT_ERROR\n"},{"id":"498484","messageId":"20240711040854.4602-1-chandrapratap3519@gmail.com","threadId":"61730","inReplyTo":"20240709053847.4453-1-chandrapratap3519@gmail.com","subject":"[GSoC][PATCH v3 0/7] t: port reftable/merged_test.c to the unit testing framework","fromName":"Chandra Pratap","fromEmail":"chandrapratap3519@gmail.com","sentAt":"2024-07-11T03:58:29Z","receivedAt":"2024-07-11T04:09:34Z","isPatch":true,"sender":{"key":"chandrapratap3519@gmail.com","avatar":null},"body":"In the recent codebase update (commit 8bf6fbd, 2023-12-09), a new unit\ntesting framework written entirely in C was introduced to the Git project\naimed at simplifying testing and reducing test run times.\nCurrently, tests for the reftable refs-backend are performed by a custom\ntesting framework defined by reftable/test_framework.{c, h}. Port\nreftable/merged_test.c to the unit testing framework and improve upon\nthe ported test.\n\nThe first patch in the series moves the test to the unit testing framework,\nand the rest of the patches improve upon the ported test.\n\nMentored-by: Patrick Steinhardt \nMentored-by: Christian Couder \nSigned-off-by: Chandra Pratap \n\n---\nChanges in v3:\n- Remove merged_test_main() from reftable/reftable-tests.h\n- Fix a typo in patch 7\n\nCI/PR: https://github.com/gitgitgadget/git/pull/1755\n\nChandra Pratap (7):\n[PATCH 1/7] t: move reftable/merged_test.c to the unit testing framework\n[PATCH 2/7] t: harmonize t-reftable-merged.c with coding guidelines\n[PATCH 3/7] t-reftable-merged: improve the test for t_merged_single_record()\n[PATCH 4/7] t-reftable-merged: improve the const-correctness of helper functions\n[PATCH 5/7] t-reftable-merged: add tests for reftable_merged_table_max_update_index\n[PATCH 6/7] t-reftable-merged: use reftable_ref_record_equal to compare ref records\n[PATCH 7/7] t-reftable-merged: add test for REFTABLE_FORMAT_ERROR\n\nMakefile | 2 +-\nt/helper/test-reftable.c | 1 -\nreftable/reftable-tests.h\t\t\t\t | 1 -\nreftable/merged_test.c => t/unit-tests/t-reftable-merged.c | 202 +++++++++++++++----------------\n4 files changed, 103 insertions(+), 103 deletions(-)\n\nRange-diff against v2:\n1: 0d71deffad ! 1: 9c9fbaf75c t: move reftable/merged_test.c to the unit testing framework\n @@ Makefile: REFTABLE_OBJS += reftable/writer.o\n REFTABLE_TEST_OBJS += reftable/record_test.o\n REFTABLE_TEST_OBJS += reftable/readwrite_test.o\n\n + ## reftable/reftable-tests.h ##\n +@@ reftable/reftable-tests.h: license that can be found in the LICENSE file or at\n +\n + int basics_test_main(int argc, const char **argv);\n + int block_test_main(int argc, const char **argv);\n +-int merged_test_main(int argc, const char **argv);\n + int pq_test_main(int argc, const char **argv);\n + int record_test_main(int argc, const char **argv);\n + int readwrite_test_main(int argc, const char **argv);\n +\n ## t/helper/test-reftable.c ##\n @@ t/helper/test-reftable.c: int cmd__reftable(int argc, const char **argv)\n \ttree_test_main(argc, argv);\n2: a449e2edcf = 2: 08c993f5f6 t: harmonize t-reftable-merged.c with coding guidelines\n3: 20fb20bb59 = 3: fa3085bd9b t-reftable-merged: improve the test t_merged_single_record()\n4: 617f668b08 = 4: d491c1f383 t-reftable-merged: improve the const-correctness of helper functions\n5: bf51af687f = 5: ee9909f7ce t-reftable-merged: add tests for reftable_merged_table_max_update_index\n6: e2b8f6b3fe = 6: 5ce16e9cfc t-reftable-merged: use reftable_ref_record_equal to compare ref records\n7: b606d67ee7 ! 7: d090e9ca5b t-reftable-merged: add test for REFTABLE_FORMAT_ERROR\n @@ Commit message\n t-reftable-merged: add test for REFTABLE_FORMAT_ERROR\n\n When calling reftable_new_merged_table(), if the hash ID of the\n - passsed reftable_table parameter doesn't match the passed hash_id\n + passed reftable_table parameter doesn't match the passed hash_id\n parameter, a REFTABLE_FORMAT_ERROR is thrown. This case is\n currently left unexercised, so add a test for the same.\n\n"},{"id":"498485","messageId":"20240711040854.4602-2-chandrapratap3519@gmail.com","threadId":"61730","inReplyTo":"20240711040854.4602-1-chandrapratap3519@gmail.com","subject":"[PATCH v3 1/7] t: move reftable/merged_test.c to the unit testing framework","fromName":"Chandra Pratap","fromEmail":"chandrapratap3519@gmail.com","sentAt":"2024-07-11T03:58:30Z","receivedAt":"2024-07-11T04:09:36Z","isPatch":true,"sender":{"key":"chandrapratap3519@gmail.com","avatar":null},"body":"reftable/merged_test.c exercises the functions defined in\nreftable/merged.{c, h}. Migrate reftable/merged_test.c to the unit\ntesting framework. Migration involves refactoring the tests\nto use the unit testing framework instead of reftable's test\nframework and renaming the tests according to unit-tests' naming\nconventions.\n\nAlso, move strbuf_add_void() and noop_flush() from\nreftable/test_framework.c to the ported test. This is because\nboth these functions are used in the merged tests and\nreftable/test_framework.{c, h} is not #included in the ported test.\n\nMentored-by: Patrick Steinhardt \nMentored-by: Christian Couder \nSigned-off-by: Chandra Pratap \n---\n Makefile | 2 +-\n reftable/reftable-tests.h | 1 -\n t/helper/test-reftable.c | 1 -\n .../unit-tests/t-reftable-merged.c | 113 +++++++++---------\n 4 files changed, 60 insertions(+), 57 deletions(-)\n rename reftable/merged_test.c => t/unit-tests/t-reftable-merged.c (84%)\n\ndiff --git a/Makefile b/Makefile\nindex 3eab701b10..e5d1b53991 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1340,6 +1340,7 @@ UNIT_TEST_PROGRAMS += t-mem-pool\n UNIT_TEST_PROGRAMS += t-oidtree\n UNIT_TEST_PROGRAMS += t-prio-queue\n UNIT_TEST_PROGRAMS += t-reftable-basics\n+UNIT_TEST_PROGRAMS += t-reftable-merged\n UNIT_TEST_PROGRAMS += t-strbuf\n UNIT_TEST_PROGRAMS += t-strcmp-offset\n UNIT_TEST_PROGRAMS += t-strvec\n@@ -2679,7 +2680,6 @@ REFTABLE_OBJS += reftable/writer.o\n \n REFTABLE_TEST_OBJS += reftable/block_test.o\n REFTABLE_TEST_OBJS += reftable/dump.o\n-REFTABLE_TEST_OBJS += reftable/merged_test.o\n REFTABLE_TEST_OBJS += reftable/pq_test.o\n REFTABLE_TEST_OBJS += reftable/record_test.o\n REFTABLE_TEST_OBJS += reftable/readwrite_test.o\ndiff --git a/reftable/reftable-tests.h b/reftable/reftable-tests.h\nindex 114cc3d053..d5e03dcc1b 100644\n--- a/reftable/reftable-tests.h\n+++ b/reftable/reftable-tests.h\n@@ -11,7 +11,6 @@ license that can be found in the LICENSE file or at\n \n int basics_test_main(int argc, const char **argv);\n int block_test_main(int argc, const char **argv);\n-int merged_test_main(int argc, const char **argv);\n int pq_test_main(int argc, const char **argv);\n int record_test_main(int argc, const char **argv);\n int readwrite_test_main(int argc, const char **argv);\ndiff --git a/t/helper/test-reftable.c b/t/helper/test-reftable.c\nindex 9160bc5da6..0357718fa8 100644\n--- a/t/helper/test-reftable.c\n+++ b/t/helper/test-reftable.c\n@@ -10,7 +10,6 @@ int cmd__reftable(int argc, const char **argv)\n \ttree_test_main(argc, argv);\n \tpq_test_main(argc, argv);\n \treadwrite_test_main(argc, argv);\n-\tmerged_test_main(argc, argv);\n \tstack_test_main(argc, argv);\n \treturn 0;\n }\ndiff --git a/reftable/merged_test.c b/t/unit-tests/t-reftable-merged.c\nsimilarity index 84%\nrename from reftable/merged_test.c\nrename to t/unit-tests/t-reftable-merged.c\nindex a9d6661c13..78a864a54f 100644\n--- a/reftable/merged_test.c\n+++ b/t/unit-tests/t-reftable-merged.c\n@@ -6,20 +6,25 @@ license that can be found in the LICENSE file or at\n https://developers.google.com/open-source/licenses/bsd\n */\n \n-#include \"merged.h\"\n-\n-#include \"system.h\"\n+#include \"test-lib.h\"\n+#include \"reftable/blocksource.h\"\n+#include \"reftable/constants.h\"\n+#include \"reftable/merged.h\"\n+#include \"reftable/reader.h\"\n+#include \"reftable/reftable-generic.h\"\n+#include \"reftable/reftable-merged.h\"\n+#include \"reftable/reftable-writer.h\"\n+\n+static ssize_t strbuf_add_void(void *b, const void *data, size_t sz)\n+{\n+\tstrbuf_add(b, data, sz);\n+\treturn sz;\n+}\n \n-#include \"basics.h\"\n-#include \"blocksource.h\"\n-#include \"constants.h\"\n-#include \"reader.h\"\n-#include \"record.h\"\n-#include \"test_framework.h\"\n-#include \"reftable-merged.h\"\n-#include \"reftable-tests.h\"\n-#include \"reftable-generic.h\"\n-#include \"reftable-writer.h\"\n+static int noop_flush(void *arg)\n+{\n+\treturn 0;\n+}\n \n static void write_test_table(struct strbuf *buf,\n \t\t\t struct reftable_ref_record refs[], int n)\n@@ -49,12 +54,12 @@ static void write_test_table(struct strbuf *buf,\n \tfor (i = 0; i < n; i++) {\n \t\tuint64_t before = refs[i].update_index;\n \t\tint n = reftable_writer_add_ref(w, &refs[i]);\n-\t\tEXPECT(n == 0);\n-\t\tEXPECT(before == refs[i].update_index);\n+\t\tcheck_int(n, ==, 0);\n+\t\tcheck_int(before, ==, refs[i].update_index);\n \t}\n \n \terr = reftable_writer_close(w);\n-\tEXPECT_ERR(err);\n+\tcheck(!err);\n \n \treftable_writer_free(w);\n }\n@@ -76,11 +81,11 @@ static void write_test_log_table(struct strbuf *buf,\n \n \tfor (i = 0; i < n; i++) {\n \t\tint err = reftable_writer_add_log(w, &logs[i]);\n-\t\tEXPECT_ERR(err);\n+\t\tcheck(!err);\n \t}\n \n \terr = reftable_writer_close(w);\n-\tEXPECT_ERR(err);\n+\tcheck(!err);\n \n \treftable_writer_free(w);\n }\n@@ -105,12 +110,12 @@ merged_table_from_records(struct reftable_ref_record **refs,\n \n \t\terr = reftable_new_reader(&(*readers)[i], &(*source)[i],\n \t\t\t\t\t \"name\");\n-\t\tEXPECT_ERR(err);\n+\t\tcheck(!err);\n \t\treftable_table_from_reader(&tabs[i], (*readers)[i]);\n \t}\n \n \terr = reftable_new_merged_table(&mt, tabs, n, GIT_SHA1_FORMAT_ID);\n-\tEXPECT_ERR(err);\n+\tcheck(!err);\n \treturn mt;\n }\n \n@@ -122,7 +127,7 @@ static void readers_destroy(struct reftable_reader **readers, size_t n)\n \treftable_free(readers);\n }\n \n-static void test_merged_between(void)\n+static void t_merged_single_record(void)\n {\n \tstruct reftable_ref_record r1[] = { {\n \t\t.refname = (char *) \"b\",\n@@ -150,11 +155,11 @@ static void test_merged_between(void)\n \n \tmerged_table_init_iter(mt, &it, BLOCK_TYPE_REF);\n \terr = reftable_iterator_seek_ref(&it, \"a\");\n-\tEXPECT_ERR(err);\n+\tcheck(!err);\n \n \terr = reftable_iterator_next_ref(&it, &ref);\n-\tEXPECT_ERR(err);\n-\tEXPECT(ref.update_index == 2);\n+\tcheck(!err);\n+\tcheck_int(ref.update_index, ==, 2);\n \treftable_ref_record_release(&ref);\n \treftable_iterator_destroy(&it);\n \treaders_destroy(readers, 2);\n@@ -165,7 +170,7 @@ static void test_merged_between(void)\n \treftable_free(bs);\n }\n \n-static void test_merged(void)\n+static void t_merged_refs(void)\n {\n \tstruct reftable_ref_record r1[] = {\n \t\t{\n@@ -230,9 +235,9 @@ static void test_merged(void)\n \n \tmerged_table_init_iter(mt, &it, BLOCK_TYPE_REF);\n \terr = reftable_iterator_seek_ref(&it, \"a\");\n-\tEXPECT_ERR(err);\n-\tEXPECT(reftable_merged_table_hash_id(mt) == GIT_SHA1_FORMAT_ID);\n-\tEXPECT(reftable_merged_table_min_update_index(mt) == 1);\n+\tcheck(!err);\n+\tcheck_int(reftable_merged_table_hash_id(mt), ==, GIT_SHA1_FORMAT_ID);\n+\tcheck_int(reftable_merged_table_min_update_index(mt), ==, 1);\n \n \twhile (len < 100) { /* cap loops/recursion. */\n \t\tstruct reftable_ref_record ref = { NULL };\n@@ -245,9 +250,9 @@ static void test_merged(void)\n \t}\n \treftable_iterator_destroy(&it);\n \n-\tEXPECT(ARRAY_SIZE(want) == len);\n+\tcheck_int(ARRAY_SIZE(want), ==, len);\n \tfor (i = 0; i < len; i++) {\n-\t\tEXPECT(reftable_ref_record_equal(want[i], &out[i],\n+\t\tcheck(reftable_ref_record_equal(want[i], &out[i],\n \t\t\t\t\t\t GIT_SHA1_RAWSZ));\n \t}\n \tfor (i = 0; i < len; i++) {\n@@ -283,16 +288,16 @@ merged_table_from_log_records(struct reftable_log_record **logs,\n \n \t\terr = reftable_new_reader(&(*readers)[i], &(*source)[i],\n \t\t\t\t\t \"name\");\n-\t\tEXPECT_ERR(err);\n+\t\tcheck(!err);\n \t\treftable_table_from_reader(&tabs[i], (*readers)[i]);\n \t}\n \n \terr = reftable_new_merged_table(&mt, tabs, n, GIT_SHA1_FORMAT_ID);\n-\tEXPECT_ERR(err);\n+\tcheck(!err);\n \treturn mt;\n }\n \n-static void test_merged_logs(void)\n+static void t_merged_logs(void)\n {\n \tstruct reftable_log_record r1[] = {\n \t\t{\n@@ -362,9 +367,9 @@ static void test_merged_logs(void)\n \n \tmerged_table_init_iter(mt, &it, BLOCK_TYPE_LOG);\n \terr = reftable_iterator_seek_log(&it, \"a\");\n-\tEXPECT_ERR(err);\n-\tEXPECT(reftable_merged_table_hash_id(mt) == GIT_SHA1_FORMAT_ID);\n-\tEXPECT(reftable_merged_table_min_update_index(mt) == 1);\n+\tcheck(!err);\n+\tcheck_int(reftable_merged_table_hash_id(mt), ==, GIT_SHA1_FORMAT_ID);\n+\tcheck_int(reftable_merged_table_min_update_index(mt), ==, 1);\n \n \twhile (len < 100) { /* cap loops/recursion. */\n \t\tstruct reftable_log_record log = { NULL };\n@@ -377,19 +382,19 @@ static void test_merged_logs(void)\n \t}\n \treftable_iterator_destroy(&it);\n \n-\tEXPECT(ARRAY_SIZE(want) == len);\n+\tcheck_int(ARRAY_SIZE(want), ==, len);\n \tfor (i = 0; i < len; i++) {\n-\t\tEXPECT(reftable_log_record_equal(want[i], &out[i],\n+\t\tcheck(reftable_log_record_equal(want[i], &out[i],\n \t\t\t\t\t\t GIT_SHA1_RAWSZ));\n \t}\n \n \tmerged_table_init_iter(mt, &it, BLOCK_TYPE_LOG);\n \terr = reftable_iterator_seek_log_at(&it, \"a\", 2);\n-\tEXPECT_ERR(err);\n+\tcheck(!err);\n \treftable_log_record_release(&out[0]);\n \terr = reftable_iterator_next_log(&it, &out[0]);\n-\tEXPECT_ERR(err);\n-\tEXPECT(reftable_log_record_equal(&out[0], &r3[0], GIT_SHA1_RAWSZ));\n+\tcheck(!err);\n+\tcheck(reftable_log_record_equal(&out[0], &r3[0], GIT_SHA1_RAWSZ));\n \treftable_iterator_destroy(&it);\n \n \tfor (i = 0; i < len; i++) {\n@@ -405,7 +410,7 @@ static void test_merged_logs(void)\n \treftable_free(bs);\n }\n \n-static void test_default_write_opts(void)\n+static void t_default_write_opts(void)\n {\n \tstruct reftable_write_options opts = { 0 };\n \tstruct strbuf buf = STRBUF_INIT;\n@@ -426,36 +431,36 @@ static void test_default_write_opts(void)\n \treftable_writer_set_limits(w, 1, 1);\n \n \terr = reftable_writer_add_ref(w, &rec);\n-\tEXPECT_ERR(err);\n+\tcheck(!err);\n \n \terr = reftable_writer_close(w);\n-\tEXPECT_ERR(err);\n+\tcheck(!err);\n \treftable_writer_free(w);\n \n \tblock_source_from_strbuf(&source, &buf);\n \n \terr = reftable_new_reader(&rd, &source, \"filename\");\n-\tEXPECT_ERR(err);\n+\tcheck(!err);\n \n \thash_id = reftable_reader_hash_id(rd);\n-\tEXPECT(hash_id == GIT_SHA1_FORMAT_ID);\n+\tcheck_int(hash_id, ==, GIT_SHA1_FORMAT_ID);\n \n \treftable_table_from_reader(&tab[0], rd);\n \terr = reftable_new_merged_table(&merged, tab, 1, GIT_SHA1_FORMAT_ID);\n-\tEXPECT_ERR(err);\n+\tcheck(!err);\n \n \treftable_reader_free(rd);\n \treftable_merged_table_free(merged);\n \tstrbuf_release(&buf);\n }\n \n-/* XXX test refs_for(oid) */\n \n-int merged_test_main(int argc, const char *argv[])\n+int cmd_main(int argc, const char *argv[])\n {\n-\tRUN_TEST(test_merged_logs);\n-\tRUN_TEST(test_merged_between);\n-\tRUN_TEST(test_merged);\n-\tRUN_TEST(test_default_write_opts);\n-\treturn 0;\n+\tTEST(t_default_write_opts(), \"merged table with default write opts\");\n+\tTEST(t_merged_logs(), \"merged table with multiple log updates for same ref\");\n+\tTEST(t_merged_refs(), \"merged table with multiple updates to same ref\");\n+\tTEST(t_merged_single_record(), \"ref ocurring in only one record can be fetched\");\n+\n+\treturn test_done();\n }\n-- \n2.45.GIT\n\n"},{"id":"498486","messageId":"20240711040854.4602-3-chandrapratap3519@gmail.com","threadId":"61730","inReplyTo":"20240711040854.4602-1-chandrapratap3519@gmail.com","subject":"[PATCH v3 2/7] t: harmonize t-reftable-merged.c with coding guidelines","fromName":"Chandra Pratap","fromEmail":"chandrapratap3519@gmail.com","sentAt":"2024-07-11T03:58:31Z","receivedAt":"2024-07-11T04:09:39Z","isPatch":true,"sender":{"key":"chandrapratap3519@gmail.com","avatar":null},"body":"Harmonize the newly ported test unit-tests/t-reftable-merged.c\nwith the following guidelines:\n- Single line control flow statements like 'for' and 'if'\n must omit curly braces.\n- Structs must be 0-initialized with '= { 0 }' instead of '= { NULL }'.\n- Array indices must be of type 'size_t', not 'int'.\n- It is fine to use C99 initial declaration in 'for' loop.\n\nMentored-by: Patrick Steinhardt \nMentored-by: Christian Couder \nSigned-off-by: Chandra Pratap \n---\n t/unit-tests/t-reftable-merged.c | 52 ++++++++++++--------------------\n 1 file changed, 20 insertions(+), 32 deletions(-)\n\ndiff --git a/t/unit-tests/t-reftable-merged.c b/t/unit-tests/t-reftable-merged.c\nindex 78a864a54f..a984116619 100644\n--- a/t/unit-tests/t-reftable-merged.c\n+++ b/t/unit-tests/t-reftable-merged.c\n@@ -40,12 +40,10 @@ static void write_test_table(struct strbuf *buf,\n \tstruct reftable_writer *w = NULL;\n \tfor (i = 0; i < n; i++) {\n \t\tuint64_t ui = refs[i].update_index;\n-\t\tif (ui > max) {\n+\t\tif (ui > max)\n \t\t\tmax = ui;\n-\t\t}\n-\t\tif (ui < min) {\n+\t\tif (ui < min)\n \t\t\tmin = ui;\n-\t\t}\n \t}\n \n \tw = reftable_new_writer(&strbuf_add_void, &noop_flush, buf, &opts);\n@@ -68,7 +66,6 @@ static void write_test_log_table(struct strbuf *buf,\n \t\t\t\t struct reftable_log_record logs[], int n,\n \t\t\t\t uint64_t update_index)\n {\n-\tint i = 0;\n \tint err;\n \n \tstruct reftable_write_options opts = {\n@@ -79,7 +76,7 @@ static void write_test_log_table(struct strbuf *buf,\n \tw = reftable_new_writer(&strbuf_add_void, &noop_flush, buf, &opts);\n \treftable_writer_set_limits(w, update_index, update_index);\n \n-\tfor (i = 0; i < n; i++) {\n+\tfor (int i = 0; i < n; i++) {\n \t\tint err = reftable_writer_add_log(w, &logs[i]);\n \t\tcheck(!err);\n \t}\n@@ -121,8 +118,7 @@ merged_table_from_records(struct reftable_ref_record **refs,\n \n static void readers_destroy(struct reftable_reader **readers, size_t n)\n {\n-\tint i = 0;\n-\tfor (; i < n; i++)\n+\tfor (size_t i = 0; i < n; i++)\n \t\treftable_reader_free(readers[i]);\n \treftable_free(readers);\n }\n@@ -148,9 +144,8 @@ static void t_merged_single_record(void)\n \tstruct reftable_reader **readers = NULL;\n \tstruct reftable_merged_table *mt =\n \t\tmerged_table_from_records(refs, &bs, &readers, sizes, bufs, 2);\n-\tint i;\n-\tstruct reftable_ref_record ref = { NULL };\n-\tstruct reftable_iterator it = { NULL };\n+\tstruct reftable_ref_record ref = { 0 };\n+\tstruct reftable_iterator it = { 0 };\n \tint err;\n \n \tmerged_table_init_iter(mt, &it, BLOCK_TYPE_REF);\n@@ -164,9 +159,8 @@ static void t_merged_single_record(void)\n \treftable_iterator_destroy(&it);\n \treaders_destroy(readers, 2);\n \treftable_merged_table_free(mt);\n-\tfor (i = 0; i < ARRAY_SIZE(bufs); i++) {\n+\tfor (size_t i = 0; i < ARRAY_SIZE(bufs); i++)\n \t\tstrbuf_release(&bufs[i]);\n-\t}\n \treftable_free(bs);\n }\n \n@@ -226,12 +220,12 @@ static void t_merged_refs(void)\n \tstruct reftable_reader **readers = NULL;\n \tstruct reftable_merged_table *mt =\n \t\tmerged_table_from_records(refs, &bs, &readers, sizes, bufs, 3);\n-\tstruct reftable_iterator it = { NULL };\n+\tstruct reftable_iterator it = { 0 };\n \tint err;\n \tstruct reftable_ref_record *out = NULL;\n \tsize_t len = 0;\n \tsize_t cap = 0;\n-\tint i = 0;\n+\tsize_t i;\n \n \tmerged_table_init_iter(mt, &it, BLOCK_TYPE_REF);\n \terr = reftable_iterator_seek_ref(&it, \"a\");\n@@ -240,7 +234,7 @@ static void t_merged_refs(void)\n \tcheck_int(reftable_merged_table_min_update_index(mt), ==, 1);\n \n \twhile (len < 100) { /* cap loops/recursion. */\n-\t\tstruct reftable_ref_record ref = { NULL };\n+\t\tstruct reftable_ref_record ref = { 0 };\n \t\tint err = reftable_iterator_next_ref(&it, &ref);\n \t\tif (err > 0)\n \t\t\tbreak;\n@@ -251,18 +245,15 @@ static void t_merged_refs(void)\n \treftable_iterator_destroy(&it);\n \n \tcheck_int(ARRAY_SIZE(want), ==, len);\n-\tfor (i = 0; i < len; i++) {\n+\tfor (i = 0; i < len; i++)\n \t\tcheck(reftable_ref_record_equal(want[i], &out[i],\n \t\t\t\t\t\t GIT_SHA1_RAWSZ));\n-\t}\n-\tfor (i = 0; i < len; i++) {\n+\tfor (i = 0; i < len; i++)\n \t\treftable_ref_record_release(&out[i]);\n-\t}\n \treftable_free(out);\n \n-\tfor (i = 0; i < 3; i++) {\n+\tfor (i = 0; i < 3; i++)\n \t\tstrbuf_release(&bufs[i]);\n-\t}\n \treaders_destroy(readers, 3);\n \treftable_merged_table_free(mt);\n \treftable_free(bs);\n@@ -358,12 +349,12 @@ static void t_merged_logs(void)\n \tstruct reftable_reader **readers = NULL;\n \tstruct reftable_merged_table *mt = merged_table_from_log_records(\n \t\tlogs, &bs, &readers, sizes, bufs, 3);\n-\tstruct reftable_iterator it = { NULL };\n+\tstruct reftable_iterator it = { 0 };\n \tint err;\n \tstruct reftable_log_record *out = NULL;\n \tsize_t len = 0;\n \tsize_t cap = 0;\n-\tint i = 0;\n+\tsize_t i = 0;\n \n \tmerged_table_init_iter(mt, &it, BLOCK_TYPE_LOG);\n \terr = reftable_iterator_seek_log(&it, \"a\");\n@@ -372,7 +363,7 @@ static void t_merged_logs(void)\n \tcheck_int(reftable_merged_table_min_update_index(mt), ==, 1);\n \n \twhile (len < 100) { /* cap loops/recursion. */\n-\t\tstruct reftable_log_record log = { NULL };\n+\t\tstruct reftable_log_record log = { 0 };\n \t\tint err = reftable_iterator_next_log(&it, &log);\n \t\tif (err > 0)\n \t\t\tbreak;\n@@ -383,10 +374,9 @@ static void t_merged_logs(void)\n \treftable_iterator_destroy(&it);\n \n \tcheck_int(ARRAY_SIZE(want), ==, len);\n-\tfor (i = 0; i < len; i++) {\n+\tfor (i = 0; i < len; i++)\n \t\tcheck(reftable_log_record_equal(want[i], &out[i],\n \t\t\t\t\t\t GIT_SHA1_RAWSZ));\n-\t}\n \n \tmerged_table_init_iter(mt, &it, BLOCK_TYPE_LOG);\n \terr = reftable_iterator_seek_log_at(&it, \"a\", 2);\n@@ -397,14 +387,12 @@ static void t_merged_logs(void)\n \tcheck(reftable_log_record_equal(&out[0], &r3[0], GIT_SHA1_RAWSZ));\n \treftable_iterator_destroy(&it);\n \n-\tfor (i = 0; i < len; i++) {\n+\tfor (i = 0; i < len; i++)\n \t\treftable_log_record_release(&out[i]);\n-\t}\n \treftable_free(out);\n \n-\tfor (i = 0; i < 3; i++) {\n+\tfor (i = 0; i < 3; i++)\n \t\tstrbuf_release(&bufs[i]);\n-\t}\n \treaders_destroy(readers, 3);\n \treftable_merged_table_free(mt);\n \treftable_free(bs);\n@@ -422,7 +410,7 @@ static void t_default_write_opts(void)\n \t\t.update_index = 1,\n \t};\n \tint err;\n-\tstruct reftable_block_source source = { NULL };\n+\tstruct reftable_block_source source = { 0 };\n \tstruct reftable_table *tab = reftable_calloc(1, sizeof(*tab));\n \tuint32_t hash_id;\n \tstruct reftable_reader *rd = NULL;\n-- \n2.45.GIT\n\n"},{"id":"498487","messageId":"20240711040854.4602-4-chandrapratap3519@gmail.com","threadId":"61730","inReplyTo":"20240711040854.4602-1-chandrapratap3519@gmail.com","subject":"[PATCH v3 3/7] t-reftable-merged: improve the test t_merged_single_record()","fromName":"Chandra Pratap","fromEmail":"chandrapratap3519@gmail.com","sentAt":"2024-07-11T03:58:32Z","receivedAt":"2024-07-11T04:09:42Z","isPatch":true,"sender":{"key":"chandrapratap3519@gmail.com","avatar":null},"body":"In t-reftable-merged.c, the test t_merged_single_record() ensures\nthat a ref ('a') which occurs in only one of the records ('r2')\ncan be retrieved. Improve this test by adding another record 'r3'\nto ensure that ref 'a' only occurs in 'r2' and that merged tables\ndon't simply read the last record.\n\nMentored-by: Patrick Steinhardt \nMentored-by: Christian Couder \nSigned-off-by: Chandra Pratap \n---\n t/unit-tests/t-reftable-merged.c | 15 ++++++++++-----\n 1 file changed, 10 insertions(+), 5 deletions(-)\n\ndiff --git a/t/unit-tests/t-reftable-merged.c b/t/unit-tests/t-reftable-merged.c\nindex a984116619..85ebb96aaa 100644\n--- a/t/unit-tests/t-reftable-merged.c\n+++ b/t/unit-tests/t-reftable-merged.c\n@@ -136,14 +136,19 @@ static void t_merged_single_record(void)\n \t\t.update_index = 2,\n \t\t.value_type = REFTABLE_REF_DELETION,\n \t} };\n+\tstruct reftable_ref_record r3[] = { {\n+\t\t.refname = (char *) \"c\",\n+\t\t.update_index = 3,\n+\t\t.value_type = REFTABLE_REF_DELETION,\n+\t} };\n \n-\tstruct reftable_ref_record *refs[] = { r1, r2 };\n-\tint sizes[] = { 1, 1 };\n-\tstruct strbuf bufs[2] = { STRBUF_INIT, STRBUF_INIT };\n+\tstruct reftable_ref_record *refs[] = { r1, r2, r3 };\n+\tint sizes[] = { 1, 1, 1 };\n+\tstruct strbuf bufs[3] = { STRBUF_INIT, STRBUF_INIT, STRBUF_INIT };\n \tstruct reftable_block_source *bs = NULL;\n \tstruct reftable_reader **readers = NULL;\n \tstruct reftable_merged_table *mt =\n-\t\tmerged_table_from_records(refs, &bs, &readers, sizes, bufs, 2);\n+\t\tmerged_table_from_records(refs, &bs, &readers, sizes, bufs, 3);\n \tstruct reftable_ref_record ref = { 0 };\n \tstruct reftable_iterator it = { 0 };\n \tint err;\n@@ -157,7 +162,7 @@ static void t_merged_single_record(void)\n \tcheck_int(ref.update_index, ==, 2);\n \treftable_ref_record_release(&ref);\n \treftable_iterator_destroy(&it);\n-\treaders_destroy(readers, 2);\n+\treaders_destroy(readers, 3);\n \treftable_merged_table_free(mt);\n \tfor (size_t i = 0; i < ARRAY_SIZE(bufs); i++)\n \t\tstrbuf_release(&bufs[i]);\n-- \n2.45.GIT\n\n"},{"id":"498488","messageId":"20240711040854.4602-5-chandrapratap3519@gmail.com","threadId":"61730","inReplyTo":"20240711040854.4602-1-chandrapratap3519@gmail.com","subject":"[PATCH v3 4/7] t-reftable-merged: improve the const-correctness of helper functions","fromName":"Chandra Pratap","fromEmail":"chandrapratap3519@gmail.com","sentAt":"2024-07-11T03:58:33Z","receivedAt":"2024-07-11T04:09:44Z","isPatch":true,"sender":{"key":"chandrapratap3519@gmail.com","avatar":null},"body":"In t-reftable-merged.c, a number of helper functions used by the\ntests can be re-defined with parameters made 'const' which makes\nit easier to understand if they're read-only or not. Re-define\nthese functions along these lines.\n\nMentored-by: Patrick Steinhardt \nMentored-by: Christian Couder \nSigned-off-by: Chandra Pratap \n---\n t/unit-tests/t-reftable-merged.c | 19 +++++++++----------\n 1 file changed, 9 insertions(+), 10 deletions(-)\n\ndiff --git a/t/unit-tests/t-reftable-merged.c b/t/unit-tests/t-reftable-merged.c\nindex 85ebb96aaa..d151d6557b 100644\n--- a/t/unit-tests/t-reftable-merged.c\n+++ b/t/unit-tests/t-reftable-merged.c\n@@ -15,7 +15,7 @@ license that can be found in the LICENSE file or at\n #include \"reftable/reftable-merged.h\"\n #include \"reftable/reftable-writer.h\"\n \n-static ssize_t strbuf_add_void(void *b, const void *data, size_t sz)\n+static ssize_t strbuf_add_void(void *b, const void *data, const size_t sz)\n {\n \tstrbuf_add(b, data, sz);\n \treturn sz;\n@@ -27,7 +27,7 @@ static int noop_flush(void *arg)\n }\n \n static void write_test_table(struct strbuf *buf,\n-\t\t\t struct reftable_ref_record refs[], int n)\n+\t\t\t struct reftable_ref_record refs[], const int n)\n {\n \tuint64_t min = 0xffffffff;\n \tuint64_t max = 0;\n@@ -62,9 +62,8 @@ static void write_test_table(struct strbuf *buf,\n \treftable_writer_free(w);\n }\n \n-static void write_test_log_table(struct strbuf *buf,\n-\t\t\t\t struct reftable_log_record logs[], int n,\n-\t\t\t\t uint64_t update_index)\n+static void write_test_log_table(struct strbuf *buf, struct reftable_log_record logs[],\n+\t\t\t\t const int n, const uint64_t update_index)\n {\n \tint err;\n \n@@ -90,8 +89,8 @@ static void write_test_log_table(struct strbuf *buf,\n static struct reftable_merged_table *\n merged_table_from_records(struct reftable_ref_record **refs,\n \t\t\t struct reftable_block_source **source,\n-\t\t\t struct reftable_reader ***readers, int *sizes,\n-\t\t\t struct strbuf *buf, size_t n)\n+\t\t\t struct reftable_reader ***readers, const int *sizes,\n+\t\t\t struct strbuf *buf, const size_t n)\n {\n \tstruct reftable_merged_table *mt = NULL;\n \tstruct reftable_table *tabs;\n@@ -116,7 +115,7 @@ merged_table_from_records(struct reftable_ref_record **refs,\n \treturn mt;\n }\n \n-static void readers_destroy(struct reftable_reader **readers, size_t n)\n+static void readers_destroy(struct reftable_reader **readers, const size_t n)\n {\n \tfor (size_t i = 0; i < n; i++)\n \t\treftable_reader_free(readers[i]);\n@@ -267,8 +266,8 @@ static void t_merged_refs(void)\n static struct reftable_merged_table *\n merged_table_from_log_records(struct reftable_log_record **logs,\n \t\t\t struct reftable_block_source **source,\n-\t\t\t struct reftable_reader ***readers, int *sizes,\n-\t\t\t struct strbuf *buf, size_t n)\n+\t\t\t struct reftable_reader ***readers, const int *sizes,\n+\t\t\t struct strbuf *buf, const size_t n)\n {\n \tstruct reftable_merged_table *mt = NULL;\n \tstruct reftable_table *tabs;\n-- \n2.45.GIT\n\n"},{"id":"498489","messageId":"20240711040854.4602-6-chandrapratap3519@gmail.com","threadId":"61730","inReplyTo":"20240711040854.4602-1-chandrapratap3519@gmail.com","subject":"[PATCH v3 5/7] t-reftable-merged: add tests for reftable_merged_table_max_update_index","fromName":"Chandra Pratap","fromEmail":"chandrapratap3519@gmail.com","sentAt":"2024-07-11T03:58:34Z","receivedAt":"2024-07-11T04:09:47Z","isPatch":true,"sender":{"key":"chandrapratap3519@gmail.com","avatar":null},"body":"reftable_merged_table_max_update_index() as defined by reftable/\nmerged.{c, h} returns the maximum update index in a merged table.\nSince this function is currently unexercised, add tests for it.\n\nMentored-by: Patrick Steinhardt \nMentored-by: Christian Couder \nSigned-off-by: Chandra Pratap \n---\n t/unit-tests/t-reftable-merged.c | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/t/unit-tests/t-reftable-merged.c b/t/unit-tests/t-reftable-merged.c\nindex d151d6557b..ffc9bd25d2 100644\n--- a/t/unit-tests/t-reftable-merged.c\n+++ b/t/unit-tests/t-reftable-merged.c\n@@ -236,6 +236,7 @@ static void t_merged_refs(void)\n \tcheck(!err);\n \tcheck_int(reftable_merged_table_hash_id(mt), ==, GIT_SHA1_FORMAT_ID);\n \tcheck_int(reftable_merged_table_min_update_index(mt), ==, 1);\n+\tcheck_int(reftable_merged_table_max_update_index(mt), ==, 3);\n \n \twhile (len < 100) { /* cap loops/recursion. */\n \t\tstruct reftable_ref_record ref = { 0 };\n@@ -365,6 +366,7 @@ static void t_merged_logs(void)\n \tcheck(!err);\n \tcheck_int(reftable_merged_table_hash_id(mt), ==, GIT_SHA1_FORMAT_ID);\n \tcheck_int(reftable_merged_table_min_update_index(mt), ==, 1);\n+\tcheck_int(reftable_merged_table_max_update_index(mt), ==, 3);\n \n \twhile (len < 100) { /* cap loops/recursion. */\n \t\tstruct reftable_log_record log = { 0 };\n-- \n2.45.GIT\n\n"},{"id":"498490","messageId":"20240711040854.4602-7-chandrapratap3519@gmail.com","threadId":"61730","inReplyTo":"20240711040854.4602-1-chandrapratap3519@gmail.com","subject":"[PATCH v3 6/7] t-reftable-merged: use reftable_ref_record_equal to compare ref records","fromName":"Chandra Pratap","fromEmail":"chandrapratap3519@gmail.com","sentAt":"2024-07-11T03:58:35Z","receivedAt":"2024-07-11T04:09:50Z","isPatch":true,"sender":{"key":"chandrapratap3519@gmail.com","avatar":null},"body":"In the test t_merged_single_record() defined in t-reftable-merged.c,\nthe 'input' and 'expected' ref records are checked for equality\nby comparing their update indices. It is very much possible for\ntwo different ref records to have the same update indices. Use\nreftable_ref_record_equal() instead for a stronger check.\n\nMentored-by: Patrick Steinhardt \nMentored-by: Christian Couder \nSigned-off-by: Chandra Pratap \n---\n t/unit-tests/t-reftable-merged.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/unit-tests/t-reftable-merged.c b/t/unit-tests/t-reftable-merged.c\nindex ffc9bd25d2..e0054e379e 100644\n--- a/t/unit-tests/t-reftable-merged.c\n+++ b/t/unit-tests/t-reftable-merged.c\n@@ -158,7 +158,7 @@ static void t_merged_single_record(void)\n \n \terr = reftable_iterator_next_ref(&it, &ref);\n \tcheck(!err);\n-\tcheck_int(ref.update_index, ==, 2);\n+\tcheck(reftable_ref_record_equal(&r2[0], &ref, GIT_SHA1_RAWSZ));\n \treftable_ref_record_release(&ref);\n \treftable_iterator_destroy(&it);\n \treaders_destroy(readers, 3);\n-- \n2.45.GIT\n\n"},{"id":"498491","messageId":"20240711040854.4602-8-chandrapratap3519@gmail.com","threadId":"61730","inReplyTo":"20240711040854.4602-1-chandrapratap3519@gmail.com","subject":"[PATCH v3 7/7] t-reftable-merged: add test for REFTABLE_FORMAT_ERROR","fromName":"Chandra Pratap","fromEmail":"chandrapratap3519@gmail.com","sentAt":"2024-07-11T03:58:36Z","receivedAt":"2024-07-11T04:09:52Z","isPatch":true,"sender":{"key":"chandrapratap3519@gmail.com","avatar":null},"body":"When calling reftable_new_merged_table(), if the hash ID of the\npassed reftable_table parameter doesn't match the passed hash_id\nparameter, a REFTABLE_FORMAT_ERROR is thrown. This case is\ncurrently left unexercised, so add a test for the same.\n\nMentored-by: Patrick Steinhardt \nMentored-by: Christian Couder \nSigned-off-by: Chandra Pratap \n---\n t/unit-tests/t-reftable-merged.c | 3 +++\n 1 file changed, 3 insertions(+)\n\ndiff --git a/t/unit-tests/t-reftable-merged.c b/t/unit-tests/t-reftable-merged.c\nindex e0054e379e..50047aa90b 100644\n--- a/t/unit-tests/t-reftable-merged.c\n+++ b/t/unit-tests/t-reftable-merged.c\n@@ -11,6 +11,7 @@ license that can be found in the LICENSE file or at\n #include \"reftable/constants.h\"\n #include \"reftable/merged.h\"\n #include \"reftable/reader.h\"\n+#include \"reftable/reftable-error.h\"\n #include \"reftable/reftable-generic.h\"\n #include \"reftable/reftable-merged.h\"\n #include \"reftable/reftable-writer.h\"\n@@ -440,6 +441,8 @@ static void t_default_write_opts(void)\n \tcheck_int(hash_id, ==, GIT_SHA1_FORMAT_ID);\n \n \treftable_table_from_reader(&tab[0], rd);\n+\terr = reftable_new_merged_table(&merged, tab, 1, GIT_SHA256_FORMAT_ID);\n+\tcheck_int(err, ==, REFTABLE_FORMAT_ERROR);\n \terr = reftable_new_merged_table(&merged, tab, 1, GIT_SHA1_FORMAT_ID);\n \tcheck(!err);\n \n-- \n2.45.GIT\n\n"},{"id":"498513","messageId":"xmqqwmlsngdq.fsf@gitster.g","threadId":"61730","inReplyTo":"CAOLa=ZR4jZhwzyQtJ=zC0wYJ9=u2CPKDgps57e-zCgQ279mYVQ@mail.gmail.com","subject":"Re: [GSoC][PATCH v2 0/7] t: port reftable/merged_test.c to the unit testing framework","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-07-11T14:38:09Z","receivedAt":"2024-07-11T14:38:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Karthik Nayak writes:\n\n> This series looks good now, sans a typo and Justin's comment!\n> Thanks!\n\nThanks, all, for helping the series (and Chandra) along.\n\n"},{"id":"498537","messageId":"xmqq7cdrr7f2.fsf@gitster.g","threadId":"61730","inReplyTo":"20240711040854.4602-3-chandrapratap3519@gmail.com","subject":"Re: [PATCH v3 2/7] t: harmonize t-reftable-merged.c with coding guidelines","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-07-11T20:38:09Z","receivedAt":"2024-07-11T20:38:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Chandra Pratap writes:\n\nIt is very nice that the steps [1/7] and [2/7] are split this way.\n\n> Harmonize the newly ported test unit-tests/t-reftable-merged.c\n> with the following guidelines:\n> - Single line control flow statements like 'for' and 'if'\n> must omit curly braces.\n\nOK.\n\n> - Structs must be 0-initialized with '= { 0 }' instead of '= { NULL }'.\n\nCorrect.\n\n> - Array indices must be of type 'size_t', not 'int'.\n\nOK, but \"must be\" is probably a bit too strong (see below).\n\n> - It is fine to use C99 initial declaration in 'for' loop.\n\nYes, it is fine. You do not have to, but you can.\n\n> @@ -68,7 +66,6 @@ static void write_test_log_table(struct strbuf *buf,\n> \t\t\t\t struct reftable_log_record logs[], int n,\n> \t\t\t\t uint64_t update_index)\n> {\n> -\tint i = 0;\n> \tint err;\n> \n> \tstruct reftable_write_options opts = {\n> @@ -79,7 +76,7 @@ static void write_test_log_table(struct strbuf *buf,\n> \tw = reftable_new_writer(&strbuf_add_void, &noop_flush, buf, &opts);\n> \treftable_writer_set_limits(w, update_index, update_index);\n> \n> -\tfor (i = 0; i < n; i++) {\n> +\tfor (int i = 0; i < n; i++) {\n> \t\tint err = reftable_writer_add_log(w, &logs[i]);\n\nDid you mean size_t instead of int here? Probably not, because the\niteration goes up to \"int n\" that is supplied by the caller of this\ntest, so iterating with \"int\" is perfectly fine here.\n\nSo, \"must be size_t\" is already violated here. You could update the\ntype of the incoming parameter \"n\", but given that this is a test\nprogram that deals with a known logs[] array of a small bounded\nsize, that may be way overkill and \"int\" can be justified, too.\nOn the other hand, if it does not require too much investigation,\nyou may want to check the caller and if it can be updated to use\n\"size_t\" instead of \"int\".\n\nThe general rule is probably \"think twice before using 'int' as an\narray index; otherwise use 'size_t'\", which covers what I said in\nthe above paragraph.\n\n> @@ -121,8 +118,7 @@ merged_table_from_records(struct reftable_ref_record **refs,\n> \n> static void readers_destroy(struct reftable_reader **readers, size_t n)\n> {\n> -\tint i = 0;\n> -\tfor (; i < n; i++)\n> +\tfor (size_t i = 0; i < n; i++)\n> \t\treftable_reader_free(readers[i]);\n> \treftable_free(readers);\n> }\n\nMuch better.\n\n> @@ -148,9 +144,8 @@ static void t_merged_single_record(void)\n> \tstruct reftable_reader **readers = NULL;\n> \tstruct reftable_merged_table *mt =\n> \t\tmerged_table_from_records(refs, &bs, &readers, sizes, bufs, 2);\n> -\tint i;\n> -\tstruct reftable_ref_record ref = { NULL };\n> -\tstruct reftable_iterator it = { NULL };\n> +\tstruct reftable_ref_record ref = { 0 };\n> +\tstruct reftable_iterator it = { 0 };\n> \tint err;\n> \n> \tmerged_table_init_iter(mt, &it, BLOCK_TYPE_REF);\n> @@ -164,9 +159,8 @@ static void t_merged_single_record(void)\n> \treftable_iterator_destroy(&it);\n> \treaders_destroy(readers, 2);\n> \treftable_merged_table_free(mt);\n> -\tfor (i = 0; i < ARRAY_SIZE(bufs); i++) {\n> +\tfor (size_t i = 0; i < ARRAY_SIZE(bufs); i++)\n> \t\tstrbuf_release(&bufs[i]);\n> -\t}\n\nOK. size_t is overkill here because bufs[] is a function local\narray with only two elements in it, but once the patch to use\n\"size_t\" (i.e., this one) is written, it is not worth to go in and\nmake it use \"int\" again.\n\n> @@ -226,12 +220,12 @@ static void t_merged_refs(void)\n> \tstruct reftable_reader **readers = NULL;\n> \tstruct reftable_merged_table *mt =\n> \t\tmerged_table_from_records(refs, &bs, &readers, sizes, bufs, 3);\n> -\tstruct reftable_iterator it = { NULL };\n> +\tstruct reftable_iterator it = { 0 };\n> \tint err;\n> \tstruct reftable_ref_record *out = NULL;\n> \tsize_t len = 0;\n> \tsize_t cap = 0;\n> -\tint i = 0;\n> +\tsize_t i;\n\nOK. It is good that we got rid of useless initialization, as this\nis used to drive more than one loops below.\n\n> @@ -358,12 +349,12 @@ static void t_merged_logs(void)\n> \tstruct reftable_reader **readers = NULL;\n> \tstruct reftable_merged_table *mt = merged_table_from_log_records(\n> \t\tlogs, &bs, &readers, sizes, bufs, 3);\n> -\tstruct reftable_iterator it = { NULL };\n> +\tstruct reftable_iterator it = { 0 };\n> \tint err;\n> \tstruct reftable_log_record *out = NULL;\n> \tsize_t len = 0;\n> \tsize_t cap = 0;\n> -\tint i = 0;\n> +\tsize_t i = 0;\n\nLose the useless initialization here, too.\n"},{"id":"498563","messageId":"20240712055041.6476-2-chandrapratap3519@gmail.com","threadId":"61730","inReplyTo":"20240712055041.6476-1-chandrapratap3519@gmail.com","subject":"[PATCH v4 1/7] t: move reftable/merged_test.c to the unit testing framework","fromName":"Chandra Pratap","fromEmail":"chandrapratap3519@gmail.com","sentAt":"2024-07-12T05:38:57Z","receivedAt":"2024-07-12T05:51:19Z","isPatch":true,"sender":{"key":"chandrapratap3519@gmail.com","avatar":null},"body":"reftable/merged_test.c exercises the functions defined in\nreftable/merged.{c, h}. Migrate reftable/merged_test.c to the unit\ntesting framework. Migration involves refactoring the tests\nto use the unit testing framework instead of reftable's test\nframework and renaming the tests according to unit-tests' naming\nconventions.\n\nAlso, move strbuf_add_void() and noop_flush() from\nreftable/test_framework.c to the ported test. This is because\nboth these functions are used in the merged tests and\nreftable/test_framework.{c, h} is not #included in the ported test.\n\nMentored-by: Patrick Steinhardt \nMentored-by: Christian Couder \nSigned-off-by: Chandra Pratap \n---\n Makefile | 2 +-\n reftable/reftable-tests.h | 1 -\n t/helper/test-reftable.c | 1 -\n .../unit-tests/t-reftable-merged.c | 113 +++++++++---------\n 4 files changed, 60 insertions(+), 57 deletions(-)\n rename reftable/merged_test.c => t/unit-tests/t-reftable-merged.c (84%)\n\ndiff --git a/Makefile b/Makefile\nindex 3eab701b10..e5d1b53991 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1340,6 +1340,7 @@ UNIT_TEST_PROGRAMS += t-mem-pool\n UNIT_TEST_PROGRAMS += t-oidtree\n UNIT_TEST_PROGRAMS += t-prio-queue\n UNIT_TEST_PROGRAMS += t-reftable-basics\n+UNIT_TEST_PROGRAMS += t-reftable-merged\n UNIT_TEST_PROGRAMS += t-strbuf\n UNIT_TEST_PROGRAMS += t-strcmp-offset\n UNIT_TEST_PROGRAMS += t-strvec\n@@ -2679,7 +2680,6 @@ REFTABLE_OBJS += reftable/writer.o\n \n REFTABLE_TEST_OBJS += reftable/block_test.o\n REFTABLE_TEST_OBJS += reftable/dump.o\n-REFTABLE_TEST_OBJS += reftable/merged_test.o\n REFTABLE_TEST_OBJS += reftable/pq_test.o\n REFTABLE_TEST_OBJS += reftable/record_test.o\n REFTABLE_TEST_OBJS += reftable/readwrite_test.o\ndiff --git a/reftable/reftable-tests.h b/reftable/reftable-tests.h\nindex 114cc3d053..d5e03dcc1b 100644\n--- a/reftable/reftable-tests.h\n+++ b/reftable/reftable-tests.h\n@@ -11,7 +11,6 @@ license that can be found in the LICENSE file or at\n \n int basics_test_main(int argc, const char **argv);\n int block_test_main(int argc, const char **argv);\n-int merged_test_main(int argc, const char **argv);\n int pq_test_main(int argc, const char **argv);\n int record_test_main(int argc, const char **argv);\n int readwrite_test_main(int argc, const char **argv);\ndiff --git a/t/helper/test-reftable.c b/t/helper/test-reftable.c\nindex 9160bc5da6..0357718fa8 100644\n--- a/t/helper/test-reftable.c\n+++ b/t/helper/test-reftable.c\n@@ -10,7 +10,6 @@ int cmd__reftable(int argc, const char **argv)\n \ttree_test_main(argc, argv);\n \tpq_test_main(argc, argv);\n \treadwrite_test_main(argc, argv);\n-\tmerged_test_main(argc, argv);\n \tstack_test_main(argc, argv);\n \treturn 0;\n }\ndiff --git a/reftable/merged_test.c b/t/unit-tests/t-reftable-merged.c\nsimilarity index 84%\nrename from reftable/merged_test.c\nrename to t/unit-tests/t-reftable-merged.c\nindex a9d6661c13..78a864a54f 100644\n--- a/reftable/merged_test.c\n+++ b/t/unit-tests/t-reftable-merged.c\n@@ -6,20 +6,25 @@ license that can be found in the LICENSE file or at\n https://developers.google.com/open-source/licenses/bsd\n */\n \n-#include \"merged.h\"\n-\n-#include \"system.h\"\n+#include \"test-lib.h\"\n+#include \"reftable/blocksource.h\"\n+#include \"reftable/constants.h\"\n+#include \"reftable/merged.h\"\n+#include \"reftable/reader.h\"\n+#include \"reftable/reftable-generic.h\"\n+#include \"reftable/reftable-merged.h\"\n+#include \"reftable/reftable-writer.h\"\n+\n+static ssize_t strbuf_add_void(void *b, const void *data, size_t sz)\n+{\n+\tstrbuf_add(b, data, sz);\n+\treturn sz;\n+}\n \n-#include \"basics.h\"\n-#include \"blocksource.h\"\n-#include \"constants.h\"\n-#include \"reader.h\"\n-#include \"record.h\"\n-#include \"test_framework.h\"\n-#include \"reftable-merged.h\"\n-#include \"reftable-tests.h\"\n-#include \"reftable-generic.h\"\n-#include \"reftable-writer.h\"\n+static int noop_flush(void *arg)\n+{\n+\treturn 0;\n+}\n \n static void write_test_table(struct strbuf *buf,\n \t\t\t struct reftable_ref_record refs[], int n)\n@@ -49,12 +54,12 @@ static void write_test_table(struct strbuf *buf,\n \tfor (i = 0; i < n; i++) {\n \t\tuint64_t before = refs[i].update_index;\n \t\tint n = reftable_writer_add_ref(w, &refs[i]);\n-\t\tEXPECT(n == 0);\n-\t\tEXPECT(before == refs[i].update_index);\n+\t\tcheck_int(n, ==, 0);\n+\t\tcheck_int(before, ==, refs[i].update_index);\n \t}\n \n \terr = reftable_writer_close(w);\n-\tEXPECT_ERR(err);\n+\tcheck(!err);\n \n \treftable_writer_free(w);\n }\n@@ -76,11 +81,11 @@ static void write_test_log_table(struct strbuf *buf,\n \n \tfor (i = 0; i < n; i++) {\n \t\tint err = reftable_writer_add_log(w, &logs[i]);\n-\t\tEXPECT_ERR(err);\n+\t\tcheck(!err);\n \t}\n \n \terr = reftable_writer_close(w);\n-\tEXPECT_ERR(err);\n+\tcheck(!err);\n \n \treftable_writer_free(w);\n }\n@@ -105,12 +110,12 @@ merged_table_from_records(struct reftable_ref_record **refs,\n \n \t\terr = reftable_new_reader(&(*readers)[i], &(*source)[i],\n \t\t\t\t\t \"name\");\n-\t\tEXPECT_ERR(err);\n+\t\tcheck(!err);\n \t\treftable_table_from_reader(&tabs[i], (*readers)[i]);\n \t}\n \n \terr = reftable_new_merged_table(&mt, tabs, n, GIT_SHA1_FORMAT_ID);\n-\tEXPECT_ERR(err);\n+\tcheck(!err);\n \treturn mt;\n }\n \n@@ -122,7 +127,7 @@ static void readers_destroy(struct reftable_reader **readers, size_t n)\n \treftable_free(readers);\n }\n \n-static void test_merged_between(void)\n+static void t_merged_single_record(void)\n {\n \tstruct reftable_ref_record r1[] = { {\n \t\t.refname = (char *) \"b\",\n@@ -150,11 +155,11 @@ static void test_merged_between(void)\n \n \tmerged_table_init_iter(mt, &it, BLOCK_TYPE_REF);\n \terr = reftable_iterator_seek_ref(&it, \"a\");\n-\tEXPECT_ERR(err);\n+\tcheck(!err);\n \n \terr = reftable_iterator_next_ref(&it, &ref);\n-\tEXPECT_ERR(err);\n-\tEXPECT(ref.update_index == 2);\n+\tcheck(!err);\n+\tcheck_int(ref.update_index, ==, 2);\n \treftable_ref_record_release(&ref);\n \treftable_iterator_destroy(&it);\n \treaders_destroy(readers, 2);\n@@ -165,7 +170,7 @@ static void test_merged_between(void)\n \treftable_free(bs);\n }\n \n-static void test_merged(void)\n+static void t_merged_refs(void)\n {\n \tstruct reftable_ref_record r1[] = {\n \t\t{\n@@ -230,9 +235,9 @@ static void test_merged(void)\n \n \tmerged_table_init_iter(mt, &it, BLOCK_TYPE_REF);\n \terr = reftable_iterator_seek_ref(&it, \"a\");\n-\tEXPECT_ERR(err);\n-\tEXPECT(reftable_merged_table_hash_id(mt) == GIT_SHA1_FORMAT_ID);\n-\tEXPECT(reftable_merged_table_min_update_index(mt) == 1);\n+\tcheck(!err);\n+\tcheck_int(reftable_merged_table_hash_id(mt), ==, GIT_SHA1_FORMAT_ID);\n+\tcheck_int(reftable_merged_table_min_update_index(mt), ==, 1);\n \n \twhile (len < 100) { /* cap loops/recursion. */\n \t\tstruct reftable_ref_record ref = { NULL };\n@@ -245,9 +250,9 @@ static void test_merged(void)\n \t}\n \treftable_iterator_destroy(&it);\n \n-\tEXPECT(ARRAY_SIZE(want) == len);\n+\tcheck_int(ARRAY_SIZE(want), ==, len);\n \tfor (i = 0; i < len; i++) {\n-\t\tEXPECT(reftable_ref_record_equal(want[i], &out[i],\n+\t\tcheck(reftable_ref_record_equal(want[i], &out[i],\n \t\t\t\t\t\t GIT_SHA1_RAWSZ));\n \t}\n \tfor (i = 0; i < len; i++) {\n@@ -283,16 +288,16 @@ merged_table_from_log_records(struct reftable_log_record **logs,\n \n \t\terr = reftable_new_reader(&(*readers)[i], &(*source)[i],\n \t\t\t\t\t \"name\");\n-\t\tEXPECT_ERR(err);\n+\t\tcheck(!err);\n \t\treftable_table_from_reader(&tabs[i], (*readers)[i]);\n \t}\n \n \terr = reftable_new_merged_table(&mt, tabs, n, GIT_SHA1_FORMAT_ID);\n-\tEXPECT_ERR(err);\n+\tcheck(!err);\n \treturn mt;\n }\n \n-static void test_merged_logs(void)\n+static void t_merged_logs(void)\n {\n \tstruct reftable_log_record r1[] = {\n \t\t{\n@@ -362,9 +367,9 @@ static void test_merged_logs(void)\n \n \tmerged_table_init_iter(mt, &it, BLOCK_TYPE_LOG);\n \terr = reftable_iterator_seek_log(&it, \"a\");\n-\tEXPECT_ERR(err);\n-\tEXPECT(reftable_merged_table_hash_id(mt) == GIT_SHA1_FORMAT_ID);\n-\tEXPECT(reftable_merged_table_min_update_index(mt) == 1);\n+\tcheck(!err);\n+\tcheck_int(reftable_merged_table_hash_id(mt), ==, GIT_SHA1_FORMAT_ID);\n+\tcheck_int(reftable_merged_table_min_update_index(mt), ==, 1);\n \n \twhile (len < 100) { /* cap loops/recursion. */\n \t\tstruct reftable_log_record log = { NULL };\n@@ -377,19 +382,19 @@ static void test_merged_logs(void)\n \t}\n \treftable_iterator_destroy(&it);\n \n-\tEXPECT(ARRAY_SIZE(want) == len);\n+\tcheck_int(ARRAY_SIZE(want), ==, len);\n \tfor (i = 0; i < len; i++) {\n-\t\tEXPECT(reftable_log_record_equal(want[i], &out[i],\n+\t\tcheck(reftable_log_record_equal(want[i], &out[i],\n \t\t\t\t\t\t GIT_SHA1_RAWSZ));\n \t}\n \n \tmerged_table_init_iter(mt, &it, BLOCK_TYPE_LOG);\n \terr = reftable_iterator_seek_log_at(&it, \"a\", 2);\n-\tEXPECT_ERR(err);\n+\tcheck(!err);\n \treftable_log_record_release(&out[0]);\n \terr = reftable_iterator_next_log(&it, &out[0]);\n-\tEXPECT_ERR(err);\n-\tEXPECT(reftable_log_record_equal(&out[0], &r3[0], GIT_SHA1_RAWSZ));\n+\tcheck(!err);\n+\tcheck(reftable_log_record_equal(&out[0], &r3[0], GIT_SHA1_RAWSZ));\n \treftable_iterator_destroy(&it);\n \n \tfor (i = 0; i < len; i++) {\n@@ -405,7 +410,7 @@ static void test_merged_logs(void)\n \treftable_free(bs);\n }\n \n-static void test_default_write_opts(void)\n+static void t_default_write_opts(void)\n {\n \tstruct reftable_write_options opts = { 0 };\n \tstruct strbuf buf = STRBUF_INIT;\n@@ -426,36 +431,36 @@ static void test_default_write_opts(void)\n \treftable_writer_set_limits(w, 1, 1);\n \n \terr = reftable_writer_add_ref(w, &rec);\n-\tEXPECT_ERR(err);\n+\tcheck(!err);\n \n \terr = reftable_writer_close(w);\n-\tEXPECT_ERR(err);\n+\tcheck(!err);\n \treftable_writer_free(w);\n \n \tblock_source_from_strbuf(&source, &buf);\n \n \terr = reftable_new_reader(&rd, &source, \"filename\");\n-\tEXPECT_ERR(err);\n+\tcheck(!err);\n \n \thash_id = reftable_reader_hash_id(rd);\n-\tEXPECT(hash_id == GIT_SHA1_FORMAT_ID);\n+\tcheck_int(hash_id, ==, GIT_SHA1_FORMAT_ID);\n \n \treftable_table_from_reader(&tab[0], rd);\n \terr = reftable_new_merged_table(&merged, tab, 1, GIT_SHA1_FORMAT_ID);\n-\tEXPECT_ERR(err);\n+\tcheck(!err);\n \n \treftable_reader_free(rd);\n \treftable_merged_table_free(merged);\n \tstrbuf_release(&buf);\n }\n \n-/* XXX test refs_for(oid) */\n \n-int merged_test_main(int argc, const char *argv[])\n+int cmd_main(int argc, const char *argv[])\n {\n-\tRUN_TEST(test_merged_logs);\n-\tRUN_TEST(test_merged_between);\n-\tRUN_TEST(test_merged);\n-\tRUN_TEST(test_default_write_opts);\n-\treturn 0;\n+\tTEST(t_default_write_opts(), \"merged table with default write opts\");\n+\tTEST(t_merged_logs(), \"merged table with multiple log updates for same ref\");\n+\tTEST(t_merged_refs(), \"merged table with multiple updates to same ref\");\n+\tTEST(t_merged_single_record(), \"ref ocurring in only one record can be fetched\");\n+\n+\treturn test_done();\n }\n-- \n2.45.GIT\n\n"},{"id":"498564","messageId":"20240712055041.6476-3-chandrapratap3519@gmail.com","threadId":"61730","inReplyTo":"20240712055041.6476-1-chandrapratap3519@gmail.com","subject":"[PATCH v4 2/7] t: harmonize t-reftable-merged.c with coding guidelines","fromName":"Chandra Pratap","fromEmail":"chandrapratap3519@gmail.com","sentAt":"2024-07-12T05:38:58Z","receivedAt":"2024-07-12T05:51:21Z","isPatch":true,"sender":{"key":"chandrapratap3519@gmail.com","avatar":null},"body":"Harmonize the newly ported test unit-tests/t-reftable-merged.c\nwith the following guidelines:\n- Single line control flow statements like 'for' and 'if'\n must omit curly braces.\n- Structs must be 0-initialized with '= { 0 }' instead of '= { NULL }'.\n- Array indices should preferably be of type 'size_t', not 'int'.\n- It is fine to use C99 initial declaration in 'for' loop.\n\nWhile at it, use 'ARRAY_SIZE(x)' to store the number of elements\nin an array instead of hardcoding them.\n\nMentored-by: Patrick Steinhardt \nMentored-by: Christian Couder \nSigned-off-by: Chandra Pratap \n---\n t/unit-tests/t-reftable-merged.c | 68 +++++++++++++-------------------\n 1 file changed, 28 insertions(+), 40 deletions(-)\n\ndiff --git a/t/unit-tests/t-reftable-merged.c b/t/unit-tests/t-reftable-merged.c\nindex 78a864a54f..9791f53418 100644\n--- a/t/unit-tests/t-reftable-merged.c\n+++ b/t/unit-tests/t-reftable-merged.c\n@@ -27,11 +27,11 @@ static int noop_flush(void *arg)\n }\n \n static void write_test_table(struct strbuf *buf,\n-\t\t\t struct reftable_ref_record refs[], int n)\n+\t\t\t struct reftable_ref_record refs[], size_t n)\n {\n \tuint64_t min = 0xffffffff;\n \tuint64_t max = 0;\n-\tint i = 0;\n+\tsize_t i;\n \tint err;\n \n \tstruct reftable_write_options opts = {\n@@ -40,12 +40,10 @@ static void write_test_table(struct strbuf *buf,\n \tstruct reftable_writer *w = NULL;\n \tfor (i = 0; i < n; i++) {\n \t\tuint64_t ui = refs[i].update_index;\n-\t\tif (ui > max) {\n+\t\tif (ui > max)\n \t\t\tmax = ui;\n-\t\t}\n-\t\tif (ui < min) {\n+\t\tif (ui < min)\n \t\t\tmin = ui;\n-\t\t}\n \t}\n \n \tw = reftable_new_writer(&strbuf_add_void, &noop_flush, buf, &opts);\n@@ -65,10 +63,9 @@ static void write_test_table(struct strbuf *buf,\n }\n \n static void write_test_log_table(struct strbuf *buf,\n-\t\t\t\t struct reftable_log_record logs[], int n,\n+\t\t\t\t struct reftable_log_record logs[], size_t n,\n \t\t\t\t uint64_t update_index)\n {\n-\tint i = 0;\n \tint err;\n \n \tstruct reftable_write_options opts = {\n@@ -79,7 +76,7 @@ static void write_test_log_table(struct strbuf *buf,\n \tw = reftable_new_writer(&strbuf_add_void, &noop_flush, buf, &opts);\n \treftable_writer_set_limits(w, update_index, update_index);\n \n-\tfor (i = 0; i < n; i++) {\n+\tfor (size_t i = 0; i < n; i++) {\n \t\tint err = reftable_writer_add_log(w, &logs[i]);\n \t\tcheck(!err);\n \t}\n@@ -93,7 +90,7 @@ static void write_test_log_table(struct strbuf *buf,\n static struct reftable_merged_table *\n merged_table_from_records(struct reftable_ref_record **refs,\n \t\t\t struct reftable_block_source **source,\n-\t\t\t struct reftable_reader ***readers, int *sizes,\n+\t\t\t struct reftable_reader ***readers, size_t *sizes,\n \t\t\t struct strbuf *buf, size_t n)\n {\n \tstruct reftable_merged_table *mt = NULL;\n@@ -121,8 +118,7 @@ merged_table_from_records(struct reftable_ref_record **refs,\n \n static void readers_destroy(struct reftable_reader **readers, size_t n)\n {\n-\tint i = 0;\n-\tfor (; i < n; i++)\n+\tfor (size_t i = 0; i < n; i++)\n \t\treftable_reader_free(readers[i]);\n \treftable_free(readers);\n }\n@@ -142,15 +138,14 @@ static void t_merged_single_record(void)\n \t} };\n \n \tstruct reftable_ref_record *refs[] = { r1, r2 };\n-\tint sizes[] = { 1, 1 };\n+\tsize_t sizes[] = { ARRAY_SIZE(r1), ARRAY_SIZE(r2) };\n \tstruct strbuf bufs[2] = { STRBUF_INIT, STRBUF_INIT };\n \tstruct reftable_block_source *bs = NULL;\n \tstruct reftable_reader **readers = NULL;\n \tstruct reftable_merged_table *mt =\n \t\tmerged_table_from_records(refs, &bs, &readers, sizes, bufs, 2);\n-\tint i;\n-\tstruct reftable_ref_record ref = { NULL };\n-\tstruct reftable_iterator it = { NULL };\n+\tstruct reftable_ref_record ref = { 0 };\n+\tstruct reftable_iterator it = { 0 };\n \tint err;\n \n \tmerged_table_init_iter(mt, &it, BLOCK_TYPE_REF);\n@@ -164,9 +159,8 @@ static void t_merged_single_record(void)\n \treftable_iterator_destroy(&it);\n \treaders_destroy(readers, 2);\n \treftable_merged_table_free(mt);\n-\tfor (i = 0; i < ARRAY_SIZE(bufs); i++) {\n+\tfor (size_t i = 0; i < ARRAY_SIZE(bufs); i++)\n \t\tstrbuf_release(&bufs[i]);\n-\t}\n \treftable_free(bs);\n }\n \n@@ -220,18 +214,18 @@ static void t_merged_refs(void)\n \t};\n \n \tstruct reftable_ref_record *refs[] = { r1, r2, r3 };\n-\tint sizes[3] = { 3, 1, 2 };\n+\tsize_t sizes[3] = { ARRAY_SIZE(r1), ARRAY_SIZE(r2), ARRAY_SIZE(r3) };\n \tstruct strbuf bufs[3] = { STRBUF_INIT, STRBUF_INIT, STRBUF_INIT };\n \tstruct reftable_block_source *bs = NULL;\n \tstruct reftable_reader **readers = NULL;\n \tstruct reftable_merged_table *mt =\n \t\tmerged_table_from_records(refs, &bs, &readers, sizes, bufs, 3);\n-\tstruct reftable_iterator it = { NULL };\n+\tstruct reftable_iterator it = { 0 };\n \tint err;\n \tstruct reftable_ref_record *out = NULL;\n \tsize_t len = 0;\n \tsize_t cap = 0;\n-\tint i = 0;\n+\tsize_t i;\n \n \tmerged_table_init_iter(mt, &it, BLOCK_TYPE_REF);\n \terr = reftable_iterator_seek_ref(&it, \"a\");\n@@ -240,7 +234,7 @@ static void t_merged_refs(void)\n \tcheck_int(reftable_merged_table_min_update_index(mt), ==, 1);\n \n \twhile (len < 100) { /* cap loops/recursion. */\n-\t\tstruct reftable_ref_record ref = { NULL };\n+\t\tstruct reftable_ref_record ref = { 0 };\n \t\tint err = reftable_iterator_next_ref(&it, &ref);\n \t\tif (err > 0)\n \t\t\tbreak;\n@@ -251,18 +245,15 @@ static void t_merged_refs(void)\n \treftable_iterator_destroy(&it);\n \n \tcheck_int(ARRAY_SIZE(want), ==, len);\n-\tfor (i = 0; i < len; i++) {\n+\tfor (i = 0; i < len; i++)\n \t\tcheck(reftable_ref_record_equal(want[i], &out[i],\n \t\t\t\t\t\t GIT_SHA1_RAWSZ));\n-\t}\n-\tfor (i = 0; i < len; i++) {\n+\tfor (i = 0; i < len; i++)\n \t\treftable_ref_record_release(&out[i]);\n-\t}\n \treftable_free(out);\n \n-\tfor (i = 0; i < 3; i++) {\n+\tfor (i = 0; i < 3; i++)\n \t\tstrbuf_release(&bufs[i]);\n-\t}\n \treaders_destroy(readers, 3);\n \treftable_merged_table_free(mt);\n \treftable_free(bs);\n@@ -271,7 +262,7 @@ static void t_merged_refs(void)\n static struct reftable_merged_table *\n merged_table_from_log_records(struct reftable_log_record **logs,\n \t\t\t struct reftable_block_source **source,\n-\t\t\t struct reftable_reader ***readers, int *sizes,\n+\t\t\t struct reftable_reader ***readers, size_t *sizes,\n \t\t\t struct strbuf *buf, size_t n)\n {\n \tstruct reftable_merged_table *mt = NULL;\n@@ -352,18 +343,18 @@ static void t_merged_logs(void)\n \t};\n \n \tstruct reftable_log_record *logs[] = { r1, r2, r3 };\n-\tint sizes[3] = { 2, 1, 1 };\n+\tsize_t sizes[3] = { ARRAY_SIZE(r1), ARRAY_SIZE(r2), ARRAY_SIZE(r3) };\n \tstruct strbuf bufs[3] = { STRBUF_INIT, STRBUF_INIT, STRBUF_INIT };\n \tstruct reftable_block_source *bs = NULL;\n \tstruct reftable_reader **readers = NULL;\n \tstruct reftable_merged_table *mt = merged_table_from_log_records(\n \t\tlogs, &bs, &readers, sizes, bufs, 3);\n-\tstruct reftable_iterator it = { NULL };\n+\tstruct reftable_iterator it = { 0 };\n \tint err;\n \tstruct reftable_log_record *out = NULL;\n \tsize_t len = 0;\n \tsize_t cap = 0;\n-\tint i = 0;\n+\tsize_t i;\n \n \tmerged_table_init_iter(mt, &it, BLOCK_TYPE_LOG);\n \terr = reftable_iterator_seek_log(&it, \"a\");\n@@ -372,7 +363,7 @@ static void t_merged_logs(void)\n \tcheck_int(reftable_merged_table_min_update_index(mt), ==, 1);\n \n \twhile (len < 100) { /* cap loops/recursion. */\n-\t\tstruct reftable_log_record log = { NULL };\n+\t\tstruct reftable_log_record log = { 0 };\n \t\tint err = reftable_iterator_next_log(&it, &log);\n \t\tif (err > 0)\n \t\t\tbreak;\n@@ -383,10 +374,9 @@ static void t_merged_logs(void)\n \treftable_iterator_destroy(&it);\n \n \tcheck_int(ARRAY_SIZE(want), ==, len);\n-\tfor (i = 0; i < len; i++) {\n+\tfor (i = 0; i < len; i++)\n \t\tcheck(reftable_log_record_equal(want[i], &out[i],\n \t\t\t\t\t\t GIT_SHA1_RAWSZ));\n-\t}\n \n \tmerged_table_init_iter(mt, &it, BLOCK_TYPE_LOG);\n \terr = reftable_iterator_seek_log_at(&it, \"a\", 2);\n@@ -397,14 +387,12 @@ static void t_merged_logs(void)\n \tcheck(reftable_log_record_equal(&out[0], &r3[0], GIT_SHA1_RAWSZ));\n \treftable_iterator_destroy(&it);\n \n-\tfor (i = 0; i < len; i++) {\n+\tfor (i = 0; i < len; i++)\n \t\treftable_log_record_release(&out[i]);\n-\t}\n \treftable_free(out);\n \n-\tfor (i = 0; i < 3; i++) {\n+\tfor (i = 0; i < 3; i++)\n \t\tstrbuf_release(&bufs[i]);\n-\t}\n \treaders_destroy(readers, 3);\n \treftable_merged_table_free(mt);\n \treftable_free(bs);\n@@ -422,7 +410,7 @@ static void t_default_write_opts(void)\n \t\t.update_index = 1,\n \t};\n \tint err;\n-\tstruct reftable_block_source source = { NULL };\n+\tstruct reftable_block_source source = { 0 };\n \tstruct reftable_table *tab = reftable_calloc(1, sizeof(*tab));\n \tuint32_t hash_id;\n \tstruct reftable_reader *rd = NULL;\n-- \n2.45.GIT\n\n"},{"id":"498565","messageId":"20240712055041.6476-1-chandrapratap3519@gmail.com","threadId":"61730","inReplyTo":"20240711040854.4602-1-chandrapratap3519@gmail.com","subject":"[GSoC][PATCH v4 0/7] t: port reftable/merged_test.c to the unit testing framework","fromName":"Chandra Pratap","fromEmail":"chandrapratap3519@gmail.com","sentAt":"2024-07-12T05:38:56Z","receivedAt":"2024-07-12T05:51:21Z","isPatch":true,"sender":{"key":"chandrapratap3519@gmail.com","avatar":null},"body":"In the recent codebase update (commit 8bf6fbd, 2023-12-09), a new unit\ntesting framework written entirely in C was introduced to the Git project\naimed at simplifying testing and reducing test run times.\nCurrently, tests for the reftable refs-backend are performed by a custom\ntesting framework defined by reftable/test_framework.{c, h}. Port\nreftable/merged_test.c to the unit testing framework and improve upon\nthe ported test.\n\nThe first patch in the series moves the test to the unit testing framework,\nand the rest of the patches improve upon the ported test.\n\nMentored-by: Patrick Steinhardt \nMentored-by: Christian Couder \nSigned-off-by: Chandra Pratap \n\n---\nChanges in v4:\n- Make more variables 'size_t' and remove redundant initialization\n in patch 2 in-line with Junio's comments on v3.\n- Use ARRAY_SIZE in patch 2 instead of hardcoding arrays' element counts.\n\nCI/PR: https://github.com/gitgitgadget/git/pull/1755\n\nChandra Pratap (7):\n[PATCH 1/7] t: move reftable/merged_test.c to the unit testing framework\n[PATCH 2/7] t: harmonize t-reftable-merged.c with coding guidelines\n[PATCH 3/7] t-reftable-merged: improve the test for t_merged_single_record()\n[PATCH 4/7] t-reftable-merged: improve the const-correctness of helper functions\n[PATCH 5/7] t-reftable-merged: add tests for reftable_merged_table_max_update_index\n[PATCH 6/7] t-reftable-merged: use reftable_ref_record_equal to compare ref records\n[PATCH 7/7] t-reftable-merged: add test for REFTABLE_FORMAT_ERROR\n\nMakefile | 2 +-\nt/helper/test-reftable.c | 1 -\nreftable/reftable-tests.h\t\t\t\t | 1 -\nreftable/merged_test.c => t/unit-tests/t-reftable-merged.c | 208 +++++++++++++++----------------\n4 files changed, 106 insertions(+), 106 deletions(-)\n\nRange-diff against v3:\n1: 08c993f5f6 ! 1: 963b9397b2 t: harmonize t-reftable-merged.c with coding guidelines\n @@ Commit message\n - Single line control flow statements like 'for' and 'if'\n must omit curly braces.\n - Structs must be 0-initialized with '= { 0 }' instead of '= { NULL }'.\n - - Array indices must be of type 'size_t', not 'int'.\n + - Array indices should preferably be of type 'size_t', not 'int'.\n - It is fine to use C99 initial declaration in 'for' loop.\n \n + While at it, use 'ARRAY_SIZE(x)' to store the number of elements\n + in an array instead of hardcoding them.\n +\n Mentored-by: Patrick Steinhardt \n Mentored-by: Christian Couder \n Signed-off-by: Chandra Pratap \n \n ## t/unit-tests/t-reftable-merged.c ##\n +@@ t/unit-tests/t-reftable-merged.c: static int noop_flush(void *arg)\n + }\n + \n + static void write_test_table(struct strbuf *buf,\n +-\t\t\t struct reftable_ref_record refs[], int n)\n ++\t\t\t struct reftable_ref_record refs[], size_t n)\n + {\n + \tuint64_t min = 0xffffffff;\n + \tuint64_t max = 0;\n +-\tint i = 0;\n ++\tsize_t i;\n + \tint err;\n + \n + \tstruct reftable_write_options opts = {\n @@ t/unit-tests/t-reftable-merged.c: static void write_test_table(struct strbuf *buf,\n \tstruct reftable_writer *w = NULL;\n \tfor (i = 0; i < n; i++) {\n @@ t/unit-tests/t-reftable-merged.c: static void write_test_table(struct strbuf *bu\n \t}\n \n \tw = reftable_new_writer(&strbuf_add_void, &noop_flush, buf, &opts);\n -@@ t/unit-tests/t-reftable-merged.c: static void write_test_log_table(struct strbuf *buf,\n - \t\t\t\t struct reftable_log_record logs[], int n,\n +@@ t/unit-tests/t-reftable-merged.c: static void write_test_table(struct strbuf *buf,\n + }\n + \n + static void write_test_log_table(struct strbuf *buf,\n +-\t\t\t\t struct reftable_log_record logs[], int n,\n ++\t\t\t\t struct reftable_log_record logs[], size_t n,\n \t\t\t\t uint64_t update_index)\n {\n -\tint i = 0;\n @@ t/unit-tests/t-reftable-merged.c: static void write_test_log_table(struct strbuf\n \treftable_writer_set_limits(w, update_index, update_index);\n \n -\tfor (i = 0; i < n; i++) {\n -+\tfor (int i = 0; i < n; i++) {\n ++\tfor (size_t i = 0; i < n; i++) {\n \t\tint err = reftable_writer_add_log(w, &logs[i]);\n \t\tcheck(!err);\n \t}\n +@@ t/unit-tests/t-reftable-merged.c: static void write_test_log_table(struct strbuf *buf,\n + static struct reftable_merged_table *\n + merged_table_from_records(struct reftable_ref_record **refs,\n + \t\t\t struct reftable_block_source **source,\n +-\t\t\t struct reftable_reader ***readers, int *sizes,\n ++\t\t\t struct reftable_reader ***readers, size_t *sizes,\n + \t\t\t struct strbuf *buf, size_t n)\n + {\n + \tstruct reftable_merged_table *mt = NULL;\n @@ t/unit-tests/t-reftable-merged.c: merged_table_from_records(struct reftable_ref_record **refs,\n \n static void readers_destroy(struct reftable_reader **readers, size_t n)\n @@ t/unit-tests/t-reftable-merged.c: merged_table_from_records(struct reftable_ref_\n \treftable_free(readers);\n }\n @@ t/unit-tests/t-reftable-merged.c: static void t_merged_single_record(void)\n + \t} };\n + \n + \tstruct reftable_ref_record *refs[] = { r1, r2 };\n +-\tint sizes[] = { 1, 1 };\n ++\tsize_t sizes[] = { ARRAY_SIZE(r1), ARRAY_SIZE(r2) };\n + \tstruct strbuf bufs[2] = { STRBUF_INIT, STRBUF_INIT };\n + \tstruct reftable_block_source *bs = NULL;\n \tstruct reftable_reader **readers = NULL;\n \tstruct reftable_merged_table *mt =\n \t\tmerged_table_from_records(refs, &bs, &readers, sizes, bufs, 2);\n @@ t/unit-tests/t-reftable-merged.c: static void t_merged_single_record(void)\n }\n \n @@ t/unit-tests/t-reftable-merged.c: static void t_merged_refs(void)\n + \t};\n + \n + \tstruct reftable_ref_record *refs[] = { r1, r2, r3 };\n +-\tint sizes[3] = { 3, 1, 2 };\n ++\tsize_t sizes[3] = { ARRAY_SIZE(r1), ARRAY_SIZE(r2), ARRAY_SIZE(r3) };\n + \tstruct strbuf bufs[3] = { STRBUF_INIT, STRBUF_INIT, STRBUF_INIT };\n + \tstruct reftable_block_source *bs = NULL;\n \tstruct reftable_reader **readers = NULL;\n \tstruct reftable_merged_table *mt =\n \t\tmerged_table_from_records(refs, &bs, &readers, sizes, bufs, 3);\n @@ t/unit-tests/t-reftable-merged.c: static void t_merged_refs(void)\n \treaders_destroy(readers, 3);\n \treftable_merged_table_free(mt);\n \treftable_free(bs);\n +@@ t/unit-tests/t-reftable-merged.c: static void t_merged_refs(void)\n + static struct reftable_merged_table *\n + merged_table_from_log_records(struct reftable_log_record **logs,\n + \t\t\t struct reftable_block_source **source,\n +-\t\t\t struct reftable_reader ***readers, int *sizes,\n ++\t\t\t struct reftable_reader ***readers, size_t *sizes,\n + \t\t\t struct strbuf *buf, size_t n)\n + {\n + \tstruct reftable_merged_table *mt = NULL;\n @@ t/unit-tests/t-reftable-merged.c: static void t_merged_logs(void)\n + \t};\n + \n + \tstruct reftable_log_record *logs[] = { r1, r2, r3 };\n +-\tint sizes[3] = { 2, 1, 1 };\n ++\tsize_t sizes[3] = { ARRAY_SIZE(r1), ARRAY_SIZE(r2), ARRAY_SIZE(r3) };\n + \tstruct strbuf bufs[3] = { STRBUF_INIT, STRBUF_INIT, STRBUF_INIT };\n + \tstruct reftable_block_source *bs = NULL;\n \tstruct reftable_reader **readers = NULL;\n \tstruct reftable_merged_table *mt = merged_table_from_log_records(\n \t\tlogs, &bs, &readers, sizes, bufs, 3);\n @@ t/unit-tests/t-reftable-merged.c: static void t_merged_logs(void)\n \tsize_t len = 0;\n \tsize_t cap = 0;\n -\tint i = 0;\n -+\tsize_t i = 0;\n ++\tsize_t i;\n \n \tmerged_table_init_iter(mt, &it, BLOCK_TYPE_LOG);\n \terr = reftable_iterator_seek_log(&it, \"a\");\n2: fa3085bd9b ! 2: b7a0bd8165 t-reftable-merged: improve the test t_merged_single_record()\n @@ t/unit-tests/t-reftable-merged.c: static void t_merged_single_record(void)\n +\t} };\n \n -\tstruct reftable_ref_record *refs[] = { r1, r2 };\n --\tint sizes[] = { 1, 1 };\n +-\tsize_t sizes[] = { ARRAY_SIZE(r1), ARRAY_SIZE(r2) };\n -\tstruct strbuf bufs[2] = { STRBUF_INIT, STRBUF_INIT };\n +\tstruct reftable_ref_record *refs[] = { r1, r2, r3 };\n -+\tint sizes[] = { 1, 1, 1 };\n ++\tsize_t sizes[] = { ARRAY_SIZE(r1), ARRAY_SIZE(r2), ARRAY_SIZE(r3) };\n +\tstruct strbuf bufs[3] = { STRBUF_INIT, STRBUF_INIT, STRBUF_INIT };\n \tstruct reftable_block_source *bs = NULL;\n \tstruct reftable_reader **readers = NULL;\n3: d491c1f383 ! 3: e86d19a1c8 t-reftable-merged: improve the const-correctness of helper functions\n @@ t/unit-tests/t-reftable-merged.c: static int noop_flush(void *arg)\n }\n \n static void write_test_table(struct strbuf *buf,\n --\t\t\t struct reftable_ref_record refs[], int n)\n -+\t\t\t struct reftable_ref_record refs[], const int n)\n +-\t\t\t struct reftable_ref_record refs[], size_t n)\n ++\t\t\t struct reftable_ref_record refs[], const size_t n)\n {\n \tuint64_t min = 0xffffffff;\n \tuint64_t max = 0;\n @@ t/unit-tests/t-reftable-merged.c: static void write_test_table(struct strbuf *bu\n }\n \n -static void write_test_log_table(struct strbuf *buf,\n --\t\t\t\t struct reftable_log_record logs[], int n,\n +-\t\t\t\t struct reftable_log_record logs[], size_t n,\n -\t\t\t\t uint64_t update_index)\n +static void write_test_log_table(struct strbuf *buf, struct reftable_log_record logs[],\n -+\t\t\t\t const int n, const uint64_t update_index)\n ++\t\t\t\t const size_t n, const uint64_t update_index)\n {\n \tint err;\n \n @@ t/unit-tests/t-reftable-merged.c: static void write_test_log_table(struct strbuf\n static struct reftable_merged_table *\n merged_table_from_records(struct reftable_ref_record **refs,\n \t\t\t struct reftable_block_source **source,\n --\t\t\t struct reftable_reader ***readers, int *sizes,\n +-\t\t\t struct reftable_reader ***readers, size_t *sizes,\n -\t\t\t struct strbuf *buf, size_t n)\n -+\t\t\t struct reftable_reader ***readers, const int *sizes,\n ++\t\t\t struct reftable_reader ***readers, const size_t *sizes,\n +\t\t\t struct strbuf *buf, const size_t n)\n {\n \tstruct reftable_merged_table *mt = NULL;\n @@ t/unit-tests/t-reftable-merged.c: static void t_merged_refs(void)\n static struct reftable_merged_table *\n merged_table_from_log_records(struct reftable_log_record **logs,\n \t\t\t struct reftable_block_source **source,\n --\t\t\t struct reftable_reader ***readers, int *sizes,\n +-\t\t\t struct reftable_reader ***readers, size_t *sizes,\n -\t\t\t struct strbuf *buf, size_t n)\n -+\t\t\t struct reftable_reader ***readers, const int *sizes,\n ++\t\t\t struct reftable_reader ***readers, const size_t *sizes,\n +\t\t\t struct strbuf *buf, const size_t n)\n {\n \tstruct reftable_merged_table *mt = NULL;\n4: ee9909f7ce = 4: 561ea9c5e9 t-reftable-merged: add tests for reftable_merged_table_max_update_index\n5: 5ce16e9cfc = 5: 7f1a329e94 t-reftable-merged: use reftable_ref_record_equal to compare ref records\n6: d090e9ca5b = 6: feaf46b765 t-reftable-merged: add test for REFTABLE_FORMAT_ERROR\n\n"},{"id":"498566","messageId":"20240712055041.6476-4-chandrapratap3519@gmail.com","threadId":"61730","inReplyTo":"20240712055041.6476-1-chandrapratap3519@gmail.com","subject":"[PATCH v4 3/7] t-reftable-merged: improve the test t_merged_single_record()","fromName":"Chandra Pratap","fromEmail":"chandrapratap3519@gmail.com","sentAt":"2024-07-12T05:38:59Z","receivedAt":"2024-07-12T05:51:23Z","isPatch":true,"sender":{"key":"chandrapratap3519@gmail.com","avatar":null},"body":"In t-reftable-merged.c, the test t_merged_single_record() ensures\nthat a ref ('a') which occurs in only one of the records ('r2')\ncan be retrieved. Improve this test by adding another record 'r3'\nto ensure that ref 'a' only occurs in 'r2' and that merged tables\ndon't simply read the last record.\n\nMentored-by: Patrick Steinhardt \nMentored-by: Christian Couder \nSigned-off-by: Chandra Pratap \n---\n t/unit-tests/t-reftable-merged.c | 15 ++++++++++-----\n 1 file changed, 10 insertions(+), 5 deletions(-)\n\ndiff --git a/t/unit-tests/t-reftable-merged.c b/t/unit-tests/t-reftable-merged.c\nindex 9791f53418..f4c14c5d47 100644\n--- a/t/unit-tests/t-reftable-merged.c\n+++ b/t/unit-tests/t-reftable-merged.c\n@@ -136,14 +136,19 @@ static void t_merged_single_record(void)\n \t\t.update_index = 2,\n \t\t.value_type = REFTABLE_REF_DELETION,\n \t} };\n+\tstruct reftable_ref_record r3[] = { {\n+\t\t.refname = (char *) \"c\",\n+\t\t.update_index = 3,\n+\t\t.value_type = REFTABLE_REF_DELETION,\n+\t} };\n \n-\tstruct reftable_ref_record *refs[] = { r1, r2 };\n-\tsize_t sizes[] = { ARRAY_SIZE(r1), ARRAY_SIZE(r2) };\n-\tstruct strbuf bufs[2] = { STRBUF_INIT, STRBUF_INIT };\n+\tstruct reftable_ref_record *refs[] = { r1, r2, r3 };\n+\tsize_t sizes[] = { ARRAY_SIZE(r1), ARRAY_SIZE(r2), ARRAY_SIZE(r3) };\n+\tstruct strbuf bufs[3] = { STRBUF_INIT, STRBUF_INIT, STRBUF_INIT };\n \tstruct reftable_block_source *bs = NULL;\n \tstruct reftable_reader **readers = NULL;\n \tstruct reftable_merged_table *mt =\n-\t\tmerged_table_from_records(refs, &bs, &readers, sizes, bufs, 2);\n+\t\tmerged_table_from_records(refs, &bs, &readers, sizes, bufs, 3);\n \tstruct reftable_ref_record ref = { 0 };\n \tstruct reftable_iterator it = { 0 };\n \tint err;\n@@ -157,7 +162,7 @@ static void t_merged_single_record(void)\n \tcheck_int(ref.update_index, ==, 2);\n \treftable_ref_record_release(&ref);\n \treftable_iterator_destroy(&it);\n-\treaders_destroy(readers, 2);\n+\treaders_destroy(readers, 3);\n \treftable_merged_table_free(mt);\n \tfor (size_t i = 0; i < ARRAY_SIZE(bufs); i++)\n \t\tstrbuf_release(&bufs[i]);\n-- \n2.45.GIT\n\n"},{"id":"498567","messageId":"20240712055041.6476-5-chandrapratap3519@gmail.com","threadId":"61730","inReplyTo":"20240712055041.6476-1-chandrapratap3519@gmail.com","subject":"[PATCH v4 4/7] t-reftable-merged: improve the const-correctness of helper functions","fromName":"Chandra Pratap","fromEmail":"chandrapratap3519@gmail.com","sentAt":"2024-07-12T05:39:00Z","receivedAt":"2024-07-12T05:51:26Z","isPatch":true,"sender":{"key":"chandrapratap3519@gmail.com","avatar":null},"body":"In t-reftable-merged.c, a number of helper functions used by the\ntests can be re-defined with parameters made 'const' which makes\nit easier to understand if they're read-only or not. Re-define\nthese functions along these lines.\n\nMentored-by: Patrick Steinhardt \nMentored-by: Christian Couder \nSigned-off-by: Chandra Pratap \n---\n t/unit-tests/t-reftable-merged.c | 19 +++++++++----------\n 1 file changed, 9 insertions(+), 10 deletions(-)\n\ndiff --git a/t/unit-tests/t-reftable-merged.c b/t/unit-tests/t-reftable-merged.c\nindex f4c14c5d47..ff2f448bb6 100644\n--- a/t/unit-tests/t-reftable-merged.c\n+++ b/t/unit-tests/t-reftable-merged.c\n@@ -15,7 +15,7 @@ license that can be found in the LICENSE file or at\n #include \"reftable/reftable-merged.h\"\n #include \"reftable/reftable-writer.h\"\n \n-static ssize_t strbuf_add_void(void *b, const void *data, size_t sz)\n+static ssize_t strbuf_add_void(void *b, const void *data, const size_t sz)\n {\n \tstrbuf_add(b, data, sz);\n \treturn sz;\n@@ -27,7 +27,7 @@ static int noop_flush(void *arg)\n }\n \n static void write_test_table(struct strbuf *buf,\n-\t\t\t struct reftable_ref_record refs[], size_t n)\n+\t\t\t struct reftable_ref_record refs[], const size_t n)\n {\n \tuint64_t min = 0xffffffff;\n \tuint64_t max = 0;\n@@ -62,9 +62,8 @@ static void write_test_table(struct strbuf *buf,\n \treftable_writer_free(w);\n }\n \n-static void write_test_log_table(struct strbuf *buf,\n-\t\t\t\t struct reftable_log_record logs[], size_t n,\n-\t\t\t\t uint64_t update_index)\n+static void write_test_log_table(struct strbuf *buf, struct reftable_log_record logs[],\n+\t\t\t\t const size_t n, const uint64_t update_index)\n {\n \tint err;\n \n@@ -90,8 +89,8 @@ static void write_test_log_table(struct strbuf *buf,\n static struct reftable_merged_table *\n merged_table_from_records(struct reftable_ref_record **refs,\n \t\t\t struct reftable_block_source **source,\n-\t\t\t struct reftable_reader ***readers, size_t *sizes,\n-\t\t\t struct strbuf *buf, size_t n)\n+\t\t\t struct reftable_reader ***readers, const size_t *sizes,\n+\t\t\t struct strbuf *buf, const size_t n)\n {\n \tstruct reftable_merged_table *mt = NULL;\n \tstruct reftable_table *tabs;\n@@ -116,7 +115,7 @@ merged_table_from_records(struct reftable_ref_record **refs,\n \treturn mt;\n }\n \n-static void readers_destroy(struct reftable_reader **readers, size_t n)\n+static void readers_destroy(struct reftable_reader **readers, const size_t n)\n {\n \tfor (size_t i = 0; i < n; i++)\n \t\treftable_reader_free(readers[i]);\n@@ -267,8 +266,8 @@ static void t_merged_refs(void)\n static struct reftable_merged_table *\n merged_table_from_log_records(struct reftable_log_record **logs,\n \t\t\t struct reftable_block_source **source,\n-\t\t\t struct reftable_reader ***readers, size_t *sizes,\n-\t\t\t struct strbuf *buf, size_t n)\n+\t\t\t struct reftable_reader ***readers, const size_t *sizes,\n+\t\t\t struct strbuf *buf, const size_t n)\n {\n \tstruct reftable_merged_table *mt = NULL;\n \tstruct reftable_table *tabs;\n-- \n2.45.GIT\n\n"},{"id":"498568","messageId":"20240712055041.6476-6-chandrapratap3519@gmail.com","threadId":"61730","inReplyTo":"20240712055041.6476-1-chandrapratap3519@gmail.com","subject":"[PATCH v4 5/7] t-reftable-merged: add tests for reftable_merged_table_max_update_index","fromName":"Chandra Pratap","fromEmail":"chandrapratap3519@gmail.com","sentAt":"2024-07-12T05:39:01Z","receivedAt":"2024-07-12T05:51:28Z","isPatch":true,"sender":{"key":"chandrapratap3519@gmail.com","avatar":null},"body":"reftable_merged_table_max_update_index() as defined by reftable/\nmerged.{c, h} returns the maximum update index in a merged table.\nSince this function is currently unexercised, add tests for it.\n\nMentored-by: Patrick Steinhardt \nMentored-by: Christian Couder \nSigned-off-by: Chandra Pratap \n---\n t/unit-tests/t-reftable-merged.c | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/t/unit-tests/t-reftable-merged.c b/t/unit-tests/t-reftable-merged.c\nindex ff2f448bb6..065b359200 100644\n--- a/t/unit-tests/t-reftable-merged.c\n+++ b/t/unit-tests/t-reftable-merged.c\n@@ -236,6 +236,7 @@ static void t_merged_refs(void)\n \tcheck(!err);\n \tcheck_int(reftable_merged_table_hash_id(mt), ==, GIT_SHA1_FORMAT_ID);\n \tcheck_int(reftable_merged_table_min_update_index(mt), ==, 1);\n+\tcheck_int(reftable_merged_table_max_update_index(mt), ==, 3);\n \n \twhile (len < 100) { /* cap loops/recursion. */\n \t\tstruct reftable_ref_record ref = { 0 };\n@@ -365,6 +366,7 @@ static void t_merged_logs(void)\n \tcheck(!err);\n \tcheck_int(reftable_merged_table_hash_id(mt), ==, GIT_SHA1_FORMAT_ID);\n \tcheck_int(reftable_merged_table_min_update_index(mt), ==, 1);\n+\tcheck_int(reftable_merged_table_max_update_index(mt), ==, 3);\n \n \twhile (len < 100) { /* cap loops/recursion. */\n \t\tstruct reftable_log_record log = { 0 };\n-- \n2.45.GIT\n\n"},{"id":"498569","messageId":"20240712055041.6476-7-chandrapratap3519@gmail.com","threadId":"61730","inReplyTo":"20240712055041.6476-1-chandrapratap3519@gmail.com","subject":"[PATCH v4 6/7] t-reftable-merged: use reftable_ref_record_equal to compare ref records","fromName":"Chandra Pratap","fromEmail":"chandrapratap3519@gmail.com","sentAt":"2024-07-12T05:39:02Z","receivedAt":"2024-07-12T05:51:31Z","isPatch":true,"sender":{"key":"chandrapratap3519@gmail.com","avatar":null},"body":"In the test t_merged_single_record() defined in t-reftable-merged.c,\nthe 'input' and 'expected' ref records are checked for equality\nby comparing their update indices. It is very much possible for\ntwo different ref records to have the same update indices. Use\nreftable_ref_record_equal() instead for a stronger check.\n\nMentored-by: Patrick Steinhardt \nMentored-by: Christian Couder \nSigned-off-by: Chandra Pratap \n---\n t/unit-tests/t-reftable-merged.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/unit-tests/t-reftable-merged.c b/t/unit-tests/t-reftable-merged.c\nindex 065b359200..9f9275f871 100644\n--- a/t/unit-tests/t-reftable-merged.c\n+++ b/t/unit-tests/t-reftable-merged.c\n@@ -158,7 +158,7 @@ static void t_merged_single_record(void)\n \n \terr = reftable_iterator_next_ref(&it, &ref);\n \tcheck(!err);\n-\tcheck_int(ref.update_index, ==, 2);\n+\tcheck(reftable_ref_record_equal(&r2[0], &ref, GIT_SHA1_RAWSZ));\n \treftable_ref_record_release(&ref);\n \treftable_iterator_destroy(&it);\n \treaders_destroy(readers, 3);\n-- \n2.45.GIT\n\n"},{"id":"498570","messageId":"20240712055041.6476-8-chandrapratap3519@gmail.com","threadId":"61730","inReplyTo":"20240712055041.6476-1-chandrapratap3519@gmail.com","subject":"[PATCH v4 7/7] t-reftable-merged: add test for REFTABLE_FORMAT_ERROR","fromName":"Chandra Pratap","fromEmail":"chandrapratap3519@gmail.com","sentAt":"2024-07-12T05:39:03Z","receivedAt":"2024-07-12T05:51:33Z","isPatch":true,"sender":{"key":"chandrapratap3519@gmail.com","avatar":null},"body":"When calling reftable_new_merged_table(), if the hash ID of the\npassed reftable_table parameter doesn't match the passed hash_id\nparameter, a REFTABLE_FORMAT_ERROR is thrown. This case is\ncurrently left unexercised, so add a test for the same.\n\nMentored-by: Patrick Steinhardt \nMentored-by: Christian Couder \nSigned-off-by: Chandra Pratap \n---\n t/unit-tests/t-reftable-merged.c | 3 +++\n 1 file changed, 3 insertions(+)\n\ndiff --git a/t/unit-tests/t-reftable-merged.c b/t/unit-tests/t-reftable-merged.c\nindex 9f9275f871..b6263ee8b5 100644\n--- a/t/unit-tests/t-reftable-merged.c\n+++ b/t/unit-tests/t-reftable-merged.c\n@@ -11,6 +11,7 @@ license that can be found in the LICENSE file or at\n #include \"reftable/constants.h\"\n #include \"reftable/merged.h\"\n #include \"reftable/reader.h\"\n+#include \"reftable/reftable-error.h\"\n #include \"reftable/reftable-generic.h\"\n #include \"reftable/reftable-merged.h\"\n #include \"reftable/reftable-writer.h\"\n@@ -440,6 +441,8 @@ static void t_default_write_opts(void)\n \tcheck_int(hash_id, ==, GIT_SHA1_FORMAT_ID);\n \n \treftable_table_from_reader(&tab[0], rd);\n+\terr = reftable_new_merged_table(&merged, tab, 1, GIT_SHA256_FORMAT_ID);\n+\tcheck_int(err, ==, REFTABLE_FORMAT_ERROR);\n \terr = reftable_new_merged_table(&merged, tab, 1, GIT_SHA1_FORMAT_ID);\n \tcheck(!err);\n \n-- \n2.45.GIT\n\n"},{"id":"499241","messageId":"ZqDFZAWBlW39Q25T@tanuki","threadId":"61730","inReplyTo":"20240712055041.6476-5-chandrapratap3519@gmail.com","subject":"Re: [PATCH v4 4/7] t-reftable-merged: improve the const-correctness of helper functions","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-07-24T09:12:04Z","receivedAt":"2024-07-24T09:12:10Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Fri, Jul 12, 2024 at 11:09:00AM +0530, Chandra Pratap wrote:\n> In t-reftable-merged.c, a number of helper functions used by the\n> tests can be re-defined with parameters made 'const' which makes\n> it easier to understand if they're read-only or not. Re-define\n> these functions along these lines.\n> \n> Mentored-by: Patrick Steinhardt \n> Mentored-by: Christian Couder \n> Signed-off-by: Chandra Pratap \n> ---\n> t/unit-tests/t-reftable-merged.c | 19 +++++++++----------\n> 1 file changed, 9 insertions(+), 10 deletions(-)\n> \n> diff --git a/t/unit-tests/t-reftable-merged.c b/t/unit-tests/t-reftable-merged.c\n> index f4c14c5d47..ff2f448bb6 100644\n> --- a/t/unit-tests/t-reftable-merged.c\n> +++ b/t/unit-tests/t-reftable-merged.c\n> @@ -15,7 +15,7 @@ license that can be found in the LICENSE file or at\n> #include \"reftable/reftable-merged.h\"\n> #include \"reftable/reftable-writer.h\"\n> \n> -static ssize_t strbuf_add_void(void *b, const void *data, size_t sz)\n> +static ssize_t strbuf_add_void(void *b, const void *data, const size_t sz)\n\nIt is quite uncustomary for the Git codebase to mark such plain values\nas `const`. While there is value in marking pointers as constant such\nthat the caller knows that the data it points to won't get modified,\nthere isn't really any value in marking pass-by-value parameters.\n\nAs far as I can see all changes relate to pass-by-value parameters, so\nI'd rather drop this patch.\n\nPatrick\n"}]}