{"thread":{"id":"61559","subject":"[GSoC][PATCH v2 0/4] t: port reftable/basics_test.c to the unit testing","startedAt":"2024-05-29T07:04:31Z","lastAt":"2024-05-30T14:33:37Z","messageCount":18,"participants":["Chandra Pratap","Patrick Steinhardt","Junio C Hamano","Christian Couder"],"isPatch":true,"patchVersion":2,"patchTotal":4},"messages":[{"id":"495782","messageId":"20240529070341.4248-1-chandrapratap3519@gmail.com","threadId":"61559","inReplyTo":"--in-reply-to=20240528113856.8348-1-chandrapratap3519@gmail.com","subject":"[GSoC][PATCH v2 0/4] t: port reftable/basics_test.c to the unit testing","fromName":"Chandra Pratap","fromEmail":"chandrapratap3519@gmail.com","sentAt":"2024-05-29T06:55:08Z","receivedAt":"2024-05-29T07:04:31Z","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/basics_test.c to the unit testing framework and improve upon\nthe ported test.\n\nMentored-by: Patrick Steinhardt <ps@pks.im>\nMentored-by: Christian Couder <chriscool@tuxfamily.org>\nSigned-off-by: Chandra Pratap <chandrapratap3519@gmail.com>\n\n---\nChanges in v2:\n- Split up the second patch of the previous series into sub-patches\n\nCI for v2: https://github.com/gitgitgadget/git/pull/1736\n"},{"id":"495783","messageId":"20240529070341.4248-2-chandrapratap3519@gmail.com","threadId":"61559","inReplyTo":"20240529070341.4248-1-chandrapratap3519@gmail.com","subject":"[GSoC][PATCH v2 1/4] t: move reftable/basics_test.c to the unit testing framework","fromName":"Chandra Pratap","fromEmail":"chandrapratap3519@gmail.com","sentAt":"2024-05-29T06:55:09Z","receivedAt":"2024-05-29T07:04:34Z","isPatch":true,"sender":{"key":"chandrapratap3519@gmail.com","avatar":null},"body":"reftable/basics_test.c exercise the functions defined in\nreftable/basics.{c, h}. Migrate reftable/basics_test.c to the\nunit testing framework. Migration involves refactoring the tests\nto use the unit testing framework instead of reftable's test\nframework.\n\nMentored-by: Patrick Steinhardt <ps@pks.im>\nMentored-by: Christian Couder <chriscool@tuxfamily.org>\nSigned-off-by: Chandra Pratap <chandrapratap3519@gmail.com>\n---\n Makefile                                      |  2 +-\n t/helper/test-reftable.c                      |  1 -\n .../unit-tests/t-reftable-basics.c            | 41 +++++++++----------\n 3 files changed, 20 insertions(+), 24 deletions(-)\n rename reftable/basics_test.c => t/unit-tests/t-reftable-basics.c (65%)\n\ndiff --git a/Makefile b/Makefile\nindex 8f4432ae57..36188ca256 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1337,6 +1337,7 @@ THIRD_PARTY_SOURCES += sha1dc/%\n UNIT_TEST_PROGRAMS += t-ctype\n UNIT_TEST_PROGRAMS += t-mem-pool\n UNIT_TEST_PROGRAMS += t-prio-queue\n+UNIT_TEST_PROGRAMS += t-reftable-basics\n UNIT_TEST_PROGRAMS += t-strbuf\n UNIT_TEST_PROGRAMS += t-trailer\n UNIT_TEST_PROGS = $(patsubst %,$(UNIT_TEST_BIN)/%$X,$(UNIT_TEST_PROGRAMS))\n@@ -2671,7 +2672,6 @@ REFTABLE_OBJS += reftable/stack.o\n REFTABLE_OBJS += reftable/tree.o\n REFTABLE_OBJS += reftable/writer.o\n \n-REFTABLE_TEST_OBJS += reftable/basics_test.o\n REFTABLE_TEST_OBJS += reftable/block_test.o\n REFTABLE_TEST_OBJS += reftable/dump.o\n REFTABLE_TEST_OBJS += reftable/merged_test.o\ndiff --git a/t/helper/test-reftable.c b/t/helper/test-reftable.c\nindex bae731669c..9160bc5da6 100644\n--- a/t/helper/test-reftable.c\n+++ b/t/helper/test-reftable.c\n@@ -5,7 +5,6 @@\n int cmd__reftable(int argc, const char **argv)\n {\n \t/* test from simple to complex. */\n-\tbasics_test_main(argc, argv);\n \trecord_test_main(argc, argv);\n \tblock_test_main(argc, argv);\n \ttree_test_main(argc, argv);\ndiff --git a/reftable/basics_test.c b/t/unit-tests/t-reftable-basics.c\nsimilarity index 65%\nrename from reftable/basics_test.c\nrename to t/unit-tests/t-reftable-basics.c\nindex 997c4d9e01..99e6c89120 100644\n--- a/reftable/basics_test.c\n+++ b/t/unit-tests/t-reftable-basics.c\n@@ -6,11 +6,8 @@ license that can be found in the LICENSE file or at\n https://developers.google.com/open-source/licenses/bsd\n */\n \n-#include \"system.h\"\n-\n-#include \"basics.h\"\n-#include \"test_framework.h\"\n-#include \"reftable-tests.h\"\n+#include \"test-lib.h\"\n+#include \"reftable/basics.h\"\n \n struct integer_needle_lesseq_args {\n \tint needle;\n@@ -42,9 +39,8 @@ static void test_binsearch(void)\n \t\t{11, 5},\n \t\t{9000, 5},\n \t};\n-\tsize_t i = 0;\n \n-\tfor (i = 0; i < ARRAY_SIZE(testcases); i++) {\n+\tfor (size_t i = 0; i < ARRAY_SIZE(testcases); i++) {\n \t\tstruct integer_needle_lesseq_args args = {\n \t\t\t.haystack = haystack,\n \t\t\t.needle = testcases[i].needle,\n@@ -52,14 +48,14 @@ static void test_binsearch(void)\n \t\tsize_t idx;\n \n \t\tidx = binsearch(ARRAY_SIZE(haystack), &integer_needle_lesseq, &args);\n-\t\tEXPECT(idx == testcases[i].expected_idx);\n+\t\tcheck_int(idx, ==, testcases[i].expected_idx);\n \t}\n }\n \n static void test_names_length(void)\n {\n \tchar *a[] = { \"a\", \"b\", NULL };\n-\tEXPECT(names_length(a) == 2);\n+\tcheck_int(names_length(a), ==, 2);\n }\n \n static void test_parse_names_normal(void)\n@@ -67,9 +63,9 @@ static void test_parse_names_normal(void)\n \tchar in[] = \"a\\nb\\n\";\n \tchar **out = NULL;\n \tparse_names(in, strlen(in), &out);\n-\tEXPECT(!strcmp(out[0], \"a\"));\n-\tEXPECT(!strcmp(out[1], \"b\"));\n-\tEXPECT(!out[2]);\n+\tcheck_str(out[0], \"a\");\n+\tcheck_str(out[1], \"b\");\n+\tcheck(!out[2]);\n \tfree_names(out);\n }\n \n@@ -78,8 +74,8 @@ static void test_parse_names_drop_empty(void)\n \tchar in[] = \"a\\n\\n\";\n \tchar **out = NULL;\n \tparse_names(in, strlen(in), &out);\n-\tEXPECT(!strcmp(out[0], \"a\"));\n-\tEXPECT(!out[1]);\n+\tcheck_str(out[0], \"a\");\n+\tcheck(!out[1]);\n \tfree_names(out);\n }\n \n@@ -89,17 +85,18 @@ static void test_common_prefix(void)\n \tstruct strbuf s2 = STRBUF_INIT;\n \tstrbuf_addstr(&s1, \"abcdef\");\n \tstrbuf_addstr(&s2, \"abc\");\n-\tEXPECT(common_prefix_size(&s1, &s2) == 3);\n+\tcheck_int(common_prefix_size(&s1, &s2), ==, 3);\n \tstrbuf_release(&s1);\n \tstrbuf_release(&s2);\n }\n \n-int basics_test_main(int argc, const char *argv[])\n+int cmd_main(int argc, const char *argv[])\n {\n-\tRUN_TEST(test_common_prefix);\n-\tRUN_TEST(test_parse_names_normal);\n-\tRUN_TEST(test_parse_names_drop_empty);\n-\tRUN_TEST(test_binsearch);\n-\tRUN_TEST(test_names_length);\n-\treturn 0;\n+\tTEST(test_common_prefix(), \"common_prefix_size works\");\n+\tTEST(test_parse_names_normal(), \"parse_names works for basic input\");\n+\tTEST(test_parse_names_drop_empty(), \"parse_names drops empty string\");\n+\tTEST(test_binsearch(), \"binary search with binsearch works\");\n+\tTEST(test_names_length(), \"names_length retuns size of a NULL-terminated string array\");\n+\n+\treturn test_done();\n }\n-- \n2.45.GIT\n\n"},{"id":"495784","messageId":"20240529070341.4248-3-chandrapratap3519@gmail.com","threadId":"61559","inReplyTo":"20240529070341.4248-1-chandrapratap3519@gmail.com","subject":"[GSoC][PATCH v2 2/4] t: move tests from reftable/stack_test.c to the new unit test","fromName":"Chandra Pratap","fromEmail":"chandrapratap3519@gmail.com","sentAt":"2024-05-29T06:55:10Z","receivedAt":"2024-05-29T07:04:36Z","isPatch":true,"sender":{"key":"chandrapratap3519@gmail.com","avatar":null},"body":"parse_names() and names_equal() are functions defined in\nreftable/basics.{c, h}. Move the tests for these functions from\nreftable/stack_test.c to the newly ported test.\n\nMentored-by: Patrick Steinhardt <ps@pks.im>\nMentored-by: Christian Couder <chriscool@tuxfamily.org>\nSigned-off-by: Chandra Pratap <chandrapratap3519@gmail.com>\n---\n reftable/stack_test.c            | 25 -------------------------\n t/unit-tests/t-reftable-basics.c | 25 ++++++++++++++++++++++---\n 2 files changed, 22 insertions(+), 28 deletions(-)\n\ndiff --git a/reftable/stack_test.c b/reftable/stack_test.c\nindex 7889f818d1..6f6af11e53 100644\n--- a/reftable/stack_test.c\n+++ b/reftable/stack_test.c\n@@ -102,29 +102,6 @@ static void test_read_file(void)\n \t(void) remove(fn);\n }\n \n-static void test_parse_names(void)\n-{\n-\tchar buf[] = \"line\\n\";\n-\tchar **names = NULL;\n-\tparse_names(buf, strlen(buf), &names);\n-\n-\tEXPECT(NULL != names[0]);\n-\tEXPECT(0 == strcmp(names[0], \"line\"));\n-\tEXPECT(NULL == names[1]);\n-\tfree_names(names);\n-}\n-\n-static void test_names_equal(void)\n-{\n-\tchar *a[] = { \"a\", \"b\", \"c\", NULL };\n-\tchar *b[] = { \"a\", \"b\", \"d\", NULL };\n-\tchar *c[] = { \"a\", \"b\", NULL };\n-\n-\tEXPECT(names_equal(a, a));\n-\tEXPECT(!names_equal(a, b));\n-\tEXPECT(!names_equal(a, c));\n-}\n-\n static int write_test_ref(struct reftable_writer *wr, void *arg)\n {\n \tstruct reftable_ref_record *ref = arg;\n@@ -1048,8 +1025,6 @@ static void test_reftable_stack_compaction_concurrent_clean(void)\n int stack_test_main(int argc, const char *argv[])\n {\n \tRUN_TEST(test_empty_add);\n-\tRUN_TEST(test_names_equal);\n-\tRUN_TEST(test_parse_names);\n \tRUN_TEST(test_read_file);\n \tRUN_TEST(test_reflog_expire);\n \tRUN_TEST(test_reftable_stack_add);\ndiff --git a/t/unit-tests/t-reftable-basics.c b/t/unit-tests/t-reftable-basics.c\nindex 99e6c89120..55fcff12d9 100644\n--- a/t/unit-tests/t-reftable-basics.c\n+++ b/t/unit-tests/t-reftable-basics.c\n@@ -58,14 +58,32 @@ static void test_names_length(void)\n \tcheck_int(names_length(a), ==, 2);\n }\n \n+static void test_names_equal(void)\n+{\n+\tchar *a[] = { \"a\", \"b\", \"c\", NULL };\n+\tchar *b[] = { \"a\", \"b\", \"d\", NULL };\n+\tchar *c[] = { \"a\", \"b\", NULL };\n+\n+\tcheck(names_equal(a, a));\n+\tcheck(!names_equal(a, b));\n+\tcheck(!names_equal(a, c));\n+}\n+\n static void test_parse_names_normal(void)\n {\n-\tchar in[] = \"a\\nb\\n\";\n+\tchar in1[] = \"line\\n\";\n+\tchar in2[] = \"a\\nb\\nc\";\n \tchar **out = NULL;\n-\tparse_names(in, strlen(in), &out);\n+\tparse_names(in1, strlen(in1), &out);\n+\tcheck_str(out[0], \"line\");\n+\tcheck(!out[1]);\n+\tfree_names(out);\n+\n+\tparse_names(in2, strlen(in2), &out);\n \tcheck_str(out[0], \"a\");\n \tcheck_str(out[1], \"b\");\n-\tcheck(!out[2]);\n+\tcheck_str(out[2], \"c\");\n+\tcheck(!out[3]);\n \tfree_names(out);\n }\n \n@@ -97,6 +115,7 @@ int cmd_main(int argc, const char *argv[])\n \tTEST(test_parse_names_drop_empty(), \"parse_names drops empty string\");\n \tTEST(test_binsearch(), \"binary search with binsearch works\");\n \tTEST(test_names_length(), \"names_length retuns size of a NULL-terminated string array\");\n+\tTEST(test_names_equal(), \"names_equal compares NULL-terminated string arrays\");\n \n \treturn test_done();\n }\n-- \n2.45.GIT\n\n"},{"id":"495785","messageId":"20240529070341.4248-4-chandrapratap3519@gmail.com","threadId":"61559","inReplyTo":"20240529070341.4248-1-chandrapratap3519@gmail.com","subject":"[GSoC][PATCH v2 3/4] t: move tests from reftable/record_test.c to the new unit test","fromName":"Chandra Pratap","fromEmail":"chandrapratap3519@gmail.com","sentAt":"2024-05-29T06:55:11Z","receivedAt":"2024-05-29T07:04:39Z","isPatch":true,"sender":{"key":"chandrapratap3519@gmail.com","avatar":null},"body":"common_prefix_size(), get_be24() and put_be24() are functions defined\nin reftable/basics.{c, h}. Move the tests for these functions from\nreftable/record_test.c to the newly ported test.\n\nMentored-by: Patrick Steinhardt <ps@pks.im>\nMentored-by: Christian Couder <chriscool@tuxfamily.org>\nSigned-off-by: Chandra Pratap <chandrapratap3519@gmail.com>\n---\n reftable/record_test.c           | 37 -----------------------------\n t/unit-tests/t-reftable-basics.c | 40 ++++++++++++++++++++++++++------\n 2 files changed, 33 insertions(+), 44 deletions(-)\n\ndiff --git a/reftable/record_test.c b/reftable/record_test.c\nindex c158ee79ff..58290bdba3 100644\n--- a/reftable/record_test.c\n+++ b/reftable/record_test.c\n@@ -64,31 +64,6 @@ static void test_varint_roundtrip(void)\n \t}\n }\n \n-static void test_common_prefix(void)\n-{\n-\tstruct {\n-\t\tconst char *a, *b;\n-\t\tint want;\n-\t} cases[] = {\n-\t\t{ \"abc\", \"ab\", 2 },\n-\t\t{ \"\", \"abc\", 0 },\n-\t\t{ \"abc\", \"abd\", 2 },\n-\t\t{ \"abc\", \"pqr\", 0 },\n-\t};\n-\n-\tint i = 0;\n-\tfor (i = 0; i < ARRAY_SIZE(cases); i++) {\n-\t\tstruct strbuf a = STRBUF_INIT;\n-\t\tstruct strbuf b = STRBUF_INIT;\n-\t\tstrbuf_addstr(&a, cases[i].a);\n-\t\tstrbuf_addstr(&b, cases[i].b);\n-\t\tEXPECT(common_prefix_size(&a, &b) == cases[i].want);\n-\n-\t\tstrbuf_release(&a);\n-\t\tstrbuf_release(&b);\n-\t}\n-}\n-\n static void set_hash(uint8_t *h, int j)\n {\n \tint i = 0;\n@@ -258,16 +233,6 @@ static void test_reftable_log_record_roundtrip(void)\n \tstrbuf_release(&scratch);\n }\n \n-static void test_u24_roundtrip(void)\n-{\n-\tuint32_t in = 0x112233;\n-\tuint8_t dest[3];\n-\tuint32_t out;\n-\tput_be24(dest, in);\n-\tout = get_be24(dest);\n-\tEXPECT(in == out);\n-}\n-\n static void test_key_roundtrip(void)\n {\n \tuint8_t buffer[1024] = { 0 };\n@@ -411,9 +376,7 @@ int record_test_main(int argc, const char *argv[])\n \tRUN_TEST(test_reftable_ref_record_roundtrip);\n \tRUN_TEST(test_varint_roundtrip);\n \tRUN_TEST(test_key_roundtrip);\n-\tRUN_TEST(test_common_prefix);\n \tRUN_TEST(test_reftable_obj_record_roundtrip);\n \tRUN_TEST(test_reftable_index_record_roundtrip);\n-\tRUN_TEST(test_u24_roundtrip);\n \treturn 0;\n }\ndiff --git a/t/unit-tests/t-reftable-basics.c b/t/unit-tests/t-reftable-basics.c\nindex 55fcff12d9..b02ca02040 100644\n--- a/t/unit-tests/t-reftable-basics.c\n+++ b/t/unit-tests/t-reftable-basics.c\n@@ -99,13 +99,38 @@ static void test_parse_names_drop_empty(void)\n \n static void test_common_prefix(void)\n {\n-\tstruct strbuf s1 = STRBUF_INIT;\n-\tstruct strbuf s2 = STRBUF_INIT;\n-\tstrbuf_addstr(&s1, \"abcdef\");\n-\tstrbuf_addstr(&s2, \"abc\");\n-\tcheck_int(common_prefix_size(&s1, &s2), ==, 3);\n-\tstrbuf_release(&s1);\n-\tstrbuf_release(&s2);\n+\tstruct strbuf a = STRBUF_INIT;\n+\tstruct strbuf b = STRBUF_INIT;\n+\tstruct {\n+\t\tconst char *a, *b;\n+\t\tint want;\n+\t} cases[] = {\n+\t\t{\"abcdef\", \"abc\", 3},\n+\t\t{ \"abc\", \"ab\", 2 },\n+\t\t{ \"\", \"abc\", 0 },\n+\t\t{ \"abc\", \"abd\", 2 },\n+\t\t{ \"abc\", \"pqr\", 0 },\n+\t};\n+\n+\tfor (size_t i = 0; i < ARRAY_SIZE(cases); i++) {\n+\t\tstrbuf_addstr(&a, cases[i].a);\n+\t\tstrbuf_addstr(&b, cases[i].b);\n+\t\tcheck_int(common_prefix_size(&a, &b), ==, cases[i].want);\n+\t\tstrbuf_reset(&a);\n+\t\tstrbuf_reset(&b);\n+\t}\n+\tstrbuf_release(&a);\n+\tstrbuf_release(&b);\n+}\n+\n+static void test_u24_roundtrip(void)\n+{\n+\tuint32_t in = 0x112233;\n+\tuint8_t dest[3];\n+\tuint32_t out;\n+\tput_be24(dest, in);\n+\tout = get_be24(dest);\n+\tcheck_int(in, ==, out);\n }\n \n int cmd_main(int argc, const char *argv[])\n@@ -116,6 +141,7 @@ int cmd_main(int argc, const char *argv[])\n \tTEST(test_binsearch(), \"binary search with binsearch works\");\n \tTEST(test_names_length(), \"names_length retuns size of a NULL-terminated string array\");\n \tTEST(test_names_equal(), \"names_equal compares NULL-terminated string arrays\");\n+\tTEST(test_u24_roundtrip(), \"put_be24 and get_be24 work\");\n \n \treturn test_done();\n }\n-- \n2.45.GIT\n\n"},{"id":"495786","messageId":"20240529070341.4248-5-chandrapratap3519@gmail.com","threadId":"61559","inReplyTo":"20240529070341.4248-1-chandrapratap3519@gmail.com","subject":"[GSoC][PATCH v2 4/4] t: add test for put_be16() and improve test-case for parse_names()","fromName":"Chandra Pratap","fromEmail":"chandrapratap3519@gmail.com","sentAt":"2024-05-29T06:55:12Z","receivedAt":"2024-05-29T07:04:42Z","isPatch":true,"sender":{"key":"chandrapratap3519@gmail.com","avatar":null},"body":"put_be16() is a function defined in reftable/basics.{c, h} for which\nthere are no tests in the current setup. Add a test for the same and\nimprove the existing test-case for parse_names().\n\nMentored-by: Patrick Steinhardt <ps@pks.im>\nMentored-by: Christian Couder <chriscool@tuxfamily.org>\nSigned-off-by: Chandra Pratap <chandrapratap3519@gmail.com>\n---\n t/unit-tests/t-reftable-basics.c | 16 ++++++++++++----\n 1 file changed, 12 insertions(+), 4 deletions(-)\n\ndiff --git a/t/unit-tests/t-reftable-basics.c b/t/unit-tests/t-reftable-basics.c\nindex b02ca02040..8372faec8c 100644\n--- a/t/unit-tests/t-reftable-basics.c\n+++ b/t/unit-tests/t-reftable-basics.c\n@@ -89,11 +89,13 @@ static void test_parse_names_normal(void)\n \n static void test_parse_names_drop_empty(void)\n {\n-\tchar in[] = \"a\\n\\n\";\n+\tchar in[] = \"a\\n\\nb\\n\";\n \tchar **out = NULL;\n \tparse_names(in, strlen(in), &out);\n \tcheck_str(out[0], \"a\");\n-\tcheck(!out[1]);\n+\t/* simply '\\n' should be dropped as empty string */\n+\tcheck_str(out[1], \"b\");\n+\tcheck(!out[2]);\n \tfree_names(out);\n }\n \n@@ -123,14 +125,20 @@ static void test_common_prefix(void)\n \tstrbuf_release(&b);\n }\n \n-static void test_u24_roundtrip(void)\n+static void test_be_roundtrip(void)\n {\n \tuint32_t in = 0x112233;\n \tuint8_t dest[3];\n \tuint32_t out;\n+\t/* test put_be24 and get_be24 roundtrip */\n \tput_be24(dest, in);\n \tout = get_be24(dest);\n \tcheck_int(in, ==, out);\n+\t/* test put_be16 and get_be16 roundtrip */\n+\tin = 0xfef1;\n+\tput_be16(dest, in);\n+\tout = get_be16(dest);\n+\tcheck_int(in, ==, out);\n }\n \n int cmd_main(int argc, const char *argv[])\n@@ -141,7 +149,7 @@ int cmd_main(int argc, const char *argv[])\n \tTEST(test_binsearch(), \"binary search with binsearch works\");\n \tTEST(test_names_length(), \"names_length retuns size of a NULL-terminated string array\");\n \tTEST(test_names_equal(), \"names_equal compares NULL-terminated string arrays\");\n-\tTEST(test_u24_roundtrip(), \"put_be24 and get_be24 work\");\n+\tTEST(test_be_roundtrip(), \"put_be24, get_be24 and put_be16 work\");\n \n \treturn test_done();\n }\n-- \n2.45.GIT\n\n"},{"id":"495805","messageId":"Zlb1lLh-DcsOO2La@tanuki","threadId":"61559","inReplyTo":"20240529070341.4248-1-chandrapratap3519@gmail.com","subject":"Re: [GSoC][PATCH v2 0/4] t: port reftable/basics_test.c to the unit testing","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-05-29T09:29:56Z","receivedAt":"2024-05-29T09:30:02Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Wed, May 29, 2024 at 12:25:08PM +0530, Chandra Pratap wrote:\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/basics_test.c to the unit testing framework and improve upon\n> the ported test.\n> \n> Mentored-by: Patrick Steinhardt <ps@pks.im>\n> Mentored-by: Christian Couder <chriscool@tuxfamily.org>\n> Signed-off-by: Chandra Pratap <chandrapratap3519@gmail.com>\n\nThe evolution of a patch series can be followed a bit easier if\nsubsequent versions are attached to the initial thread. You can do this\nby passing e.g. `--in-reply-to=<message-id>` to git-format-patch(1),\nwhere the message ID is the one of the cover letter of the first\nversion.\n\nI'd also recommend to attach a range diff to your cover letter via the\n`--range-diff=` parameter. This range diff helps the reviewer to spot\nwhat has changed between your preceding version and this one.\n\nPatrick\n"},{"id":"495806","messageId":"Zlb1m5cwhW_R5EzP@tanuki","threadId":"61559","inReplyTo":"20240529070341.4248-4-chandrapratap3519@gmail.com","subject":"Re: [GSoC][PATCH v2 3/4] t: move tests from reftable/record_test.c to the new unit test","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-05-29T09:30:03Z","receivedAt":"2024-05-29T09:30:08Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Wed, May 29, 2024 at 12:25:11PM +0530, Chandra Pratap wrote:\n[snip]\n> diff --git a/t/unit-tests/t-reftable-basics.c b/t/unit-tests/t-reftable-basics.c\n> index 55fcff12d9..b02ca02040 100644\n> --- a/t/unit-tests/t-reftable-basics.c\n> +++ b/t/unit-tests/t-reftable-basics.c\n> @@ -99,13 +99,38 @@ static void test_parse_names_drop_empty(void)\n>  \n>  static void test_common_prefix(void)\n>  {\n> -\tstruct strbuf s1 = STRBUF_INIT;\n> -\tstruct strbuf s2 = STRBUF_INIT;\n> -\tstrbuf_addstr(&s1, \"abcdef\");\n> -\tstrbuf_addstr(&s2, \"abc\");\n> -\tcheck_int(common_prefix_size(&s1, &s2), ==, 3);\n> -\tstrbuf_release(&s1);\n> -\tstrbuf_release(&s2);\n> +\tstruct strbuf a = STRBUF_INIT;\n> +\tstruct strbuf b = STRBUF_INIT;\n> +\tstruct {\n> +\t\tconst char *a, *b;\n> +\t\tint want;\n> +\t} cases[] = {\n> +\t\t{\"abcdef\", \"abc\", 3},\n> +\t\t{ \"abc\", \"ab\", 2 },\n> +\t\t{ \"\", \"abc\", 0 },\n> +\t\t{ \"abc\", \"abd\", 2 },\n> +\t\t{ \"abc\", \"pqr\", 0 },\n> +\t};\n> +\n> +\tfor (size_t i = 0; i < ARRAY_SIZE(cases); i++) {\n> +\t\tstrbuf_addstr(&a, cases[i].a);\n> +\t\tstrbuf_addstr(&b, cases[i].b);\n> +\t\tcheck_int(common_prefix_size(&a, &b), ==, cases[i].want);\n> +\t\tstrbuf_reset(&a);\n> +\t\tstrbuf_reset(&b);\n> +\t}\n> +\tstrbuf_release(&a);\n> +\tstrbuf_release(&b);\n> +}\n\nOh, so this test was even duplicated. It may make sense to point out\ndetails like this in the commit message to prepare the reader. But\nthat's probably not worth a reroll.\n\nPatrick\n"},{"id":"495807","messageId":"Zlb1oiN6E4Isrnmg@tanuki","threadId":"61559","inReplyTo":"20240529070341.4248-5-chandrapratap3519@gmail.com","subject":"Re: [GSoC][PATCH v2 4/4] t: add test for put_be16() and improve test-case for parse_names()","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-05-29T09:30:10Z","receivedAt":"2024-05-29T09:30:15Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Wed, May 29, 2024 at 12:25:12PM +0530, Chandra Pratap wrote:\n> put_be16() is a function defined in reftable/basics.{c, h} for which\n> there are no tests in the current setup. Add a test for the same and\n> improve the existing test-case for parse_names().\n> \n> Mentored-by: Patrick Steinhardt <ps@pks.im>\n> Mentored-by: Christian Couder <chriscool@tuxfamily.org>\n> Signed-off-by: Chandra Pratap <chandrapratap3519@gmail.com>\n> ---\n>  t/unit-tests/t-reftable-basics.c | 16 ++++++++++++----\n>  1 file changed, 12 insertions(+), 4 deletions(-)\n> \n> diff --git a/t/unit-tests/t-reftable-basics.c b/t/unit-tests/t-reftable-basics.c\n> index b02ca02040..8372faec8c 100644\n> --- a/t/unit-tests/t-reftable-basics.c\n> +++ b/t/unit-tests/t-reftable-basics.c\n> @@ -89,11 +89,13 @@ static void test_parse_names_normal(void)\n>  \n>  static void test_parse_names_drop_empty(void)\n>  {\n> -\tchar in[] = \"a\\n\\n\";\n> +\tchar in[] = \"a\\n\\nb\\n\";\n>  \tchar **out = NULL;\n>  \tparse_names(in, strlen(in), &out);\n>  \tcheck_str(out[0], \"a\");\n> -\tcheck(!out[1]);\n> +\t/* simply '\\n' should be dropped as empty string */\n> +\tcheck_str(out[1], \"b\");\n> +\tcheck(!out[2]);\n>  \tfree_names(out);\n>  }\n\nI'd split out this change into yet another commit. Also, you say that\nthe test case is being \"improved\", but without mentioning what the\nimprovement actually is.\n\n> @@ -123,14 +125,20 @@ static void test_common_prefix(void)\n>  \tstrbuf_release(&b);\n>  }\n>  \n> -static void test_u24_roundtrip(void)\n> +static void test_be_roundtrip(void)\n>  {\n>  \tuint32_t in = 0x112233;\n>  \tuint8_t dest[3];\n>  \tuint32_t out;\n> +\t/* test put_be24 and get_be24 roundtrip */\n>  \tput_be24(dest, in);\n>  \tout = get_be24(dest);\n>  \tcheck_int(in, ==, out);\n> +\t/* test put_be16 and get_be16 roundtrip */\n> +\tin = 0xfef1;\n> +\tput_be16(dest, in);\n> +\tout = get_be16(dest);\n> +\tcheck_int(in, ==, out);\n>  }\n\nWould it make sense to have separate tests for each of the variants\ninstead of one test for all of these? Might make things a bit easier to\nfollow.\n\nPatrick\n"},{"id":"495843","messageId":"20240529171439.18271-1-chandrapratap3519@gmail.com","threadId":"61559","inReplyTo":"20240529070341.4248-1-chandrapratap3519@gmail.com","subject":"[GSoC][PATCH v3 0/4] t: port reftable/basics_test.c to the unit testing","fromName":"Chandra Pratap","fromEmail":"chandrapratap3519@gmail.com","sentAt":"2024-05-29T16:59:26Z","receivedAt":"2024-05-29T17:15:47Z","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/basics_test.c to the unit testing framework and improve upon\nthe ported test.\n\nMentored-by: Patrick Steinhardt <ps@pks.im>\nMentored-by: Christian Couder <chriscool@tuxfamily.org>\nSigned-off-by: Chandra Pratap <chandrapratap3519@gmail.com>\n\n---\nChanges in v3:\n- Split up the 4th patch of the previous series into 2 sub-patches\n\nCI/PR for v3: https://github.com/gitgitgadget/git/pull/1736\n\nrange-diff against v2:\n\n1:  3ab1b415f4 = 1:  3ab1b415f4 t: move reftable/basics_test.c to the unit testing framework\n2:  51fec8a376 = 2:  0143bbd2e4 t: move tests from reftable/stack_test.c to the new unit test\n3:  e0e9adfdf6 = 3:  2f1d02e945 t: move tests from reftable/record_test.c to the new unit test\n-:  ---------- > 4:  14606ac8db t: add test for put_be16()\n4:  81b8975b4c ! 5:  2e741bab6d t: add test for put_be16() and improve test-case for parse_names()\n    @@ Metadata\n     Author: Chandra Pratap <chandrapratap3519@gmail.com>\n\n      ## Commit message ##\n    -    t: add test for put_be16() and improve test-case for parse_names()\n    +    t: improve the test-case for parse_names()\n\n    -    put_be16() is a function defined in reftable/basics.{c, h} for which\n    -    there are no tests in the current setup. Add a test for the same and\n    -    improve the existing test-case for parse_names().\n    +    In the existing test-case for parse_names(), the fact that empty\n    +    lines should be ignored is not obvious because the empty line is\n    +    immediately followed by end-of-string. This can be mistaken as the\n    +    empty line getting replaced by NULL. Improve this by adding a\n    +    non-empty line after the empty one to demonstrate the intended behavior.\n\n         Mentored-by: Patrick Steinhardt <ps@pks.im>\n         Mentored-by: Christian Couder <chriscool@tuxfamily.org>\n    @@ t/unit-tests/t-reftable-basics.c: static void test_parse_names_normal(void)\n      \tfree_names(out);\n      }\n\n    -@@ t/unit-tests/t-reftable-basics.c: static void test_common_prefix(void)\n    - \tstrbuf_release(&b);\n    - }\n    -\n    --static void test_u24_roundtrip(void)\n    -+static void test_be_roundtrip(void)\n    - {\n    - \tuint32_t in = 0x112233;\n    - \tuint8_t dest[3];\n    - \tuint32_t out;\n    -+\t/* test put_be24 and get_be24 roundtrip */\n    - \tput_be24(dest, in);\n    - \tout = get_be24(dest);\n    - \tcheck_int(in, ==, out);\n    -+\t/* test put_be16 and get_be16 roundtrip */\n    -+\tin = 0xfef1;\n    -+\tput_be16(dest, in);\n    -+\tout = get_be16(dest);\n    -+\tcheck_int(in, ==, out);\n    - }\n    -\n    - int cmd_main(int argc, const char *argv[])\n    -@@ t/unit-tests/t-reftable-basics.c: int cmd_main(int argc, const char *argv[])\n    - \tTEST(test_binsearch(), \"binary search with binsearch works\");\n    - \tTEST(test_names_length(), \"names_length retuns size of a NULL-terminated string array\");\n    - \tTEST(test_names_equal(), \"names_equal compares NULL-terminated string arrays\");\n    --\tTEST(test_u24_roundtrip(), \"put_be24 and get_be24 work\");\n    -+\tTEST(test_be_roundtrip(), \"put_be24, get_be24 and put_be16 work\");\n    -\n    - \treturn test_done();\n    - }\n\n"},{"id":"495845","messageId":"20240529171439.18271-2-chandrapratap3519@gmail.com","threadId":"61559","inReplyTo":"20240529171439.18271-1-chandrapratap3519@gmail.com","subject":"[GSoC][PATCH v3 1/5] t: move reftable/basics_test.c to the unit testing framework","fromName":"Chandra Pratap","fromEmail":"chandrapratap3519@gmail.com","sentAt":"2024-05-29T16:59:27Z","receivedAt":"2024-05-29T17:15:49Z","isPatch":true,"sender":{"key":"chandrapratap3519@gmail.com","avatar":null},"body":"reftable/basics_test.c exercise the functions defined in\nreftable/basics.{c, h}. Migrate reftable/basics_test.c to the\nunit testing framework. Migration involves refactoring the tests\nto use the unit testing framework instead of reftable's test\nframework.\n\nMentored-by: Patrick Steinhardt <ps@pks.im>\nMentored-by: Christian Couder <chriscool@tuxfamily.org>\nSigned-off-by: Chandra Pratap <chandrapratap3519@gmail.com>\n---\n Makefile                                      |  2 +-\n t/helper/test-reftable.c                      |  1 -\n .../unit-tests/t-reftable-basics.c            | 41 +++++++++----------\n 3 files changed, 20 insertions(+), 24 deletions(-)\n rename reftable/basics_test.c => t/unit-tests/t-reftable-basics.c (65%)\n\ndiff --git a/Makefile b/Makefile\nindex 8f4432ae57..36188ca256 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1337,6 +1337,7 @@ THIRD_PARTY_SOURCES += sha1dc/%\n UNIT_TEST_PROGRAMS += t-ctype\n UNIT_TEST_PROGRAMS += t-mem-pool\n UNIT_TEST_PROGRAMS += t-prio-queue\n+UNIT_TEST_PROGRAMS += t-reftable-basics\n UNIT_TEST_PROGRAMS += t-strbuf\n UNIT_TEST_PROGRAMS += t-trailer\n UNIT_TEST_PROGS = $(patsubst %,$(UNIT_TEST_BIN)/%$X,$(UNIT_TEST_PROGRAMS))\n@@ -2671,7 +2672,6 @@ REFTABLE_OBJS += reftable/stack.o\n REFTABLE_OBJS += reftable/tree.o\n REFTABLE_OBJS += reftable/writer.o\n \n-REFTABLE_TEST_OBJS += reftable/basics_test.o\n REFTABLE_TEST_OBJS += reftable/block_test.o\n REFTABLE_TEST_OBJS += reftable/dump.o\n REFTABLE_TEST_OBJS += reftable/merged_test.o\ndiff --git a/t/helper/test-reftable.c b/t/helper/test-reftable.c\nindex bae731669c..9160bc5da6 100644\n--- a/t/helper/test-reftable.c\n+++ b/t/helper/test-reftable.c\n@@ -5,7 +5,6 @@\n int cmd__reftable(int argc, const char **argv)\n {\n \t/* test from simple to complex. */\n-\tbasics_test_main(argc, argv);\n \trecord_test_main(argc, argv);\n \tblock_test_main(argc, argv);\n \ttree_test_main(argc, argv);\ndiff --git a/reftable/basics_test.c b/t/unit-tests/t-reftable-basics.c\nsimilarity index 65%\nrename from reftable/basics_test.c\nrename to t/unit-tests/t-reftable-basics.c\nindex 997c4d9e01..99e6c89120 100644\n--- a/reftable/basics_test.c\n+++ b/t/unit-tests/t-reftable-basics.c\n@@ -6,11 +6,8 @@ license that can be found in the LICENSE file or at\n https://developers.google.com/open-source/licenses/bsd\n */\n \n-#include \"system.h\"\n-\n-#include \"basics.h\"\n-#include \"test_framework.h\"\n-#include \"reftable-tests.h\"\n+#include \"test-lib.h\"\n+#include \"reftable/basics.h\"\n \n struct integer_needle_lesseq_args {\n \tint needle;\n@@ -42,9 +39,8 @@ static void test_binsearch(void)\n \t\t{11, 5},\n \t\t{9000, 5},\n \t};\n-\tsize_t i = 0;\n \n-\tfor (i = 0; i < ARRAY_SIZE(testcases); i++) {\n+\tfor (size_t i = 0; i < ARRAY_SIZE(testcases); i++) {\n \t\tstruct integer_needle_lesseq_args args = {\n \t\t\t.haystack = haystack,\n \t\t\t.needle = testcases[i].needle,\n@@ -52,14 +48,14 @@ static void test_binsearch(void)\n \t\tsize_t idx;\n \n \t\tidx = binsearch(ARRAY_SIZE(haystack), &integer_needle_lesseq, &args);\n-\t\tEXPECT(idx == testcases[i].expected_idx);\n+\t\tcheck_int(idx, ==, testcases[i].expected_idx);\n \t}\n }\n \n static void test_names_length(void)\n {\n \tchar *a[] = { \"a\", \"b\", NULL };\n-\tEXPECT(names_length(a) == 2);\n+\tcheck_int(names_length(a), ==, 2);\n }\n \n static void test_parse_names_normal(void)\n@@ -67,9 +63,9 @@ static void test_parse_names_normal(void)\n \tchar in[] = \"a\\nb\\n\";\n \tchar **out = NULL;\n \tparse_names(in, strlen(in), &out);\n-\tEXPECT(!strcmp(out[0], \"a\"));\n-\tEXPECT(!strcmp(out[1], \"b\"));\n-\tEXPECT(!out[2]);\n+\tcheck_str(out[0], \"a\");\n+\tcheck_str(out[1], \"b\");\n+\tcheck(!out[2]);\n \tfree_names(out);\n }\n \n@@ -78,8 +74,8 @@ static void test_parse_names_drop_empty(void)\n \tchar in[] = \"a\\n\\n\";\n \tchar **out = NULL;\n \tparse_names(in, strlen(in), &out);\n-\tEXPECT(!strcmp(out[0], \"a\"));\n-\tEXPECT(!out[1]);\n+\tcheck_str(out[0], \"a\");\n+\tcheck(!out[1]);\n \tfree_names(out);\n }\n \n@@ -89,17 +85,18 @@ static void test_common_prefix(void)\n \tstruct strbuf s2 = STRBUF_INIT;\n \tstrbuf_addstr(&s1, \"abcdef\");\n \tstrbuf_addstr(&s2, \"abc\");\n-\tEXPECT(common_prefix_size(&s1, &s2) == 3);\n+\tcheck_int(common_prefix_size(&s1, &s2), ==, 3);\n \tstrbuf_release(&s1);\n \tstrbuf_release(&s2);\n }\n \n-int basics_test_main(int argc, const char *argv[])\n+int cmd_main(int argc, const char *argv[])\n {\n-\tRUN_TEST(test_common_prefix);\n-\tRUN_TEST(test_parse_names_normal);\n-\tRUN_TEST(test_parse_names_drop_empty);\n-\tRUN_TEST(test_binsearch);\n-\tRUN_TEST(test_names_length);\n-\treturn 0;\n+\tTEST(test_common_prefix(), \"common_prefix_size works\");\n+\tTEST(test_parse_names_normal(), \"parse_names works for basic input\");\n+\tTEST(test_parse_names_drop_empty(), \"parse_names drops empty string\");\n+\tTEST(test_binsearch(), \"binary search with binsearch works\");\n+\tTEST(test_names_length(), \"names_length retuns size of a NULL-terminated string array\");\n+\n+\treturn test_done();\n }\n-- \n2.45.GIT\n\n"},{"id":"495844","messageId":"20240529171439.18271-3-chandrapratap3519@gmail.com","threadId":"61559","inReplyTo":"20240529171439.18271-1-chandrapratap3519@gmail.com","subject":"[GSoC][PATCH v3 2/5] t: move tests from reftable/stack_test.c to the new unit test","fromName":"Chandra Pratap","fromEmail":"chandrapratap3519@gmail.com","sentAt":"2024-05-29T16:59:28Z","receivedAt":"2024-05-29T17:15:52Z","isPatch":true,"sender":{"key":"chandrapratap3519@gmail.com","avatar":null},"body":"parse_names() and names_equal() are functions defined in\nreftable/basics.{c, h}. Move the tests for these functions from\nreftable/stack_test.c to the newly ported test.\n\nMentored-by: Patrick Steinhardt <ps@pks.im>\nMentored-by: Christian Couder <chriscool@tuxfamily.org>\nSigned-off-by: Chandra Pratap <chandrapratap3519@gmail.com>\n---\n reftable/stack_test.c            | 25 -------------------------\n t/unit-tests/t-reftable-basics.c | 25 ++++++++++++++++++++++---\n 2 files changed, 22 insertions(+), 28 deletions(-)\n\ndiff --git a/reftable/stack_test.c b/reftable/stack_test.c\nindex 7889f818d1..6f6af11e53 100644\n--- a/reftable/stack_test.c\n+++ b/reftable/stack_test.c\n@@ -102,29 +102,6 @@ static void test_read_file(void)\n \t(void) remove(fn);\n }\n \n-static void test_parse_names(void)\n-{\n-\tchar buf[] = \"line\\n\";\n-\tchar **names = NULL;\n-\tparse_names(buf, strlen(buf), &names);\n-\n-\tEXPECT(NULL != names[0]);\n-\tEXPECT(0 == strcmp(names[0], \"line\"));\n-\tEXPECT(NULL == names[1]);\n-\tfree_names(names);\n-}\n-\n-static void test_names_equal(void)\n-{\n-\tchar *a[] = { \"a\", \"b\", \"c\", NULL };\n-\tchar *b[] = { \"a\", \"b\", \"d\", NULL };\n-\tchar *c[] = { \"a\", \"b\", NULL };\n-\n-\tEXPECT(names_equal(a, a));\n-\tEXPECT(!names_equal(a, b));\n-\tEXPECT(!names_equal(a, c));\n-}\n-\n static int write_test_ref(struct reftable_writer *wr, void *arg)\n {\n \tstruct reftable_ref_record *ref = arg;\n@@ -1048,8 +1025,6 @@ static void test_reftable_stack_compaction_concurrent_clean(void)\n int stack_test_main(int argc, const char *argv[])\n {\n \tRUN_TEST(test_empty_add);\n-\tRUN_TEST(test_names_equal);\n-\tRUN_TEST(test_parse_names);\n \tRUN_TEST(test_read_file);\n \tRUN_TEST(test_reflog_expire);\n \tRUN_TEST(test_reftable_stack_add);\ndiff --git a/t/unit-tests/t-reftable-basics.c b/t/unit-tests/t-reftable-basics.c\nindex 99e6c89120..55fcff12d9 100644\n--- a/t/unit-tests/t-reftable-basics.c\n+++ b/t/unit-tests/t-reftable-basics.c\n@@ -58,14 +58,32 @@ static void test_names_length(void)\n \tcheck_int(names_length(a), ==, 2);\n }\n \n+static void test_names_equal(void)\n+{\n+\tchar *a[] = { \"a\", \"b\", \"c\", NULL };\n+\tchar *b[] = { \"a\", \"b\", \"d\", NULL };\n+\tchar *c[] = { \"a\", \"b\", NULL };\n+\n+\tcheck(names_equal(a, a));\n+\tcheck(!names_equal(a, b));\n+\tcheck(!names_equal(a, c));\n+}\n+\n static void test_parse_names_normal(void)\n {\n-\tchar in[] = \"a\\nb\\n\";\n+\tchar in1[] = \"line\\n\";\n+\tchar in2[] = \"a\\nb\\nc\";\n \tchar **out = NULL;\n-\tparse_names(in, strlen(in), &out);\n+\tparse_names(in1, strlen(in1), &out);\n+\tcheck_str(out[0], \"line\");\n+\tcheck(!out[1]);\n+\tfree_names(out);\n+\n+\tparse_names(in2, strlen(in2), &out);\n \tcheck_str(out[0], \"a\");\n \tcheck_str(out[1], \"b\");\n-\tcheck(!out[2]);\n+\tcheck_str(out[2], \"c\");\n+\tcheck(!out[3]);\n \tfree_names(out);\n }\n \n@@ -97,6 +115,7 @@ int cmd_main(int argc, const char *argv[])\n \tTEST(test_parse_names_drop_empty(), \"parse_names drops empty string\");\n \tTEST(test_binsearch(), \"binary search with binsearch works\");\n \tTEST(test_names_length(), \"names_length retuns size of a NULL-terminated string array\");\n+\tTEST(test_names_equal(), \"names_equal compares NULL-terminated string arrays\");\n \n \treturn test_done();\n }\n-- \n2.45.GIT\n\n"},{"id":"495846","messageId":"20240529171439.18271-4-chandrapratap3519@gmail.com","threadId":"61559","inReplyTo":"20240529171439.18271-1-chandrapratap3519@gmail.com","subject":"[GSoC][PATCH v3 3/5] t: move tests from reftable/record_test.c to the new unit test","fromName":"Chandra Pratap","fromEmail":"chandrapratap3519@gmail.com","sentAt":"2024-05-29T16:59:29Z","receivedAt":"2024-05-29T17:15:54Z","isPatch":true,"sender":{"key":"chandrapratap3519@gmail.com","avatar":null},"body":"common_prefix_size(), get_be24() and put_be24() are functions defined\nin reftable/basics.{c, h}. Move the tests for these functions from\nreftable/record_test.c to the newly ported test.\n\nMentored-by: Patrick Steinhardt <ps@pks.im>\nMentored-by: Christian Couder <chriscool@tuxfamily.org>\nSigned-off-by: Chandra Pratap <chandrapratap3519@gmail.com>\n---\n reftable/record_test.c           | 37 -----------------------------\n t/unit-tests/t-reftable-basics.c | 40 ++++++++++++++++++++++++++------\n 2 files changed, 33 insertions(+), 44 deletions(-)\n\ndiff --git a/reftable/record_test.c b/reftable/record_test.c\nindex c158ee79ff..58290bdba3 100644\n--- a/reftable/record_test.c\n+++ b/reftable/record_test.c\n@@ -64,31 +64,6 @@ static void test_varint_roundtrip(void)\n \t}\n }\n \n-static void test_common_prefix(void)\n-{\n-\tstruct {\n-\t\tconst char *a, *b;\n-\t\tint want;\n-\t} cases[] = {\n-\t\t{ \"abc\", \"ab\", 2 },\n-\t\t{ \"\", \"abc\", 0 },\n-\t\t{ \"abc\", \"abd\", 2 },\n-\t\t{ \"abc\", \"pqr\", 0 },\n-\t};\n-\n-\tint i = 0;\n-\tfor (i = 0; i < ARRAY_SIZE(cases); i++) {\n-\t\tstruct strbuf a = STRBUF_INIT;\n-\t\tstruct strbuf b = STRBUF_INIT;\n-\t\tstrbuf_addstr(&a, cases[i].a);\n-\t\tstrbuf_addstr(&b, cases[i].b);\n-\t\tEXPECT(common_prefix_size(&a, &b) == cases[i].want);\n-\n-\t\tstrbuf_release(&a);\n-\t\tstrbuf_release(&b);\n-\t}\n-}\n-\n static void set_hash(uint8_t *h, int j)\n {\n \tint i = 0;\n@@ -258,16 +233,6 @@ static void test_reftable_log_record_roundtrip(void)\n \tstrbuf_release(&scratch);\n }\n \n-static void test_u24_roundtrip(void)\n-{\n-\tuint32_t in = 0x112233;\n-\tuint8_t dest[3];\n-\tuint32_t out;\n-\tput_be24(dest, in);\n-\tout = get_be24(dest);\n-\tEXPECT(in == out);\n-}\n-\n static void test_key_roundtrip(void)\n {\n \tuint8_t buffer[1024] = { 0 };\n@@ -411,9 +376,7 @@ int record_test_main(int argc, const char *argv[])\n \tRUN_TEST(test_reftable_ref_record_roundtrip);\n \tRUN_TEST(test_varint_roundtrip);\n \tRUN_TEST(test_key_roundtrip);\n-\tRUN_TEST(test_common_prefix);\n \tRUN_TEST(test_reftable_obj_record_roundtrip);\n \tRUN_TEST(test_reftable_index_record_roundtrip);\n-\tRUN_TEST(test_u24_roundtrip);\n \treturn 0;\n }\ndiff --git a/t/unit-tests/t-reftable-basics.c b/t/unit-tests/t-reftable-basics.c\nindex 55fcff12d9..b02ca02040 100644\n--- a/t/unit-tests/t-reftable-basics.c\n+++ b/t/unit-tests/t-reftable-basics.c\n@@ -99,13 +99,38 @@ static void test_parse_names_drop_empty(void)\n \n static void test_common_prefix(void)\n {\n-\tstruct strbuf s1 = STRBUF_INIT;\n-\tstruct strbuf s2 = STRBUF_INIT;\n-\tstrbuf_addstr(&s1, \"abcdef\");\n-\tstrbuf_addstr(&s2, \"abc\");\n-\tcheck_int(common_prefix_size(&s1, &s2), ==, 3);\n-\tstrbuf_release(&s1);\n-\tstrbuf_release(&s2);\n+\tstruct strbuf a = STRBUF_INIT;\n+\tstruct strbuf b = STRBUF_INIT;\n+\tstruct {\n+\t\tconst char *a, *b;\n+\t\tint want;\n+\t} cases[] = {\n+\t\t{\"abcdef\", \"abc\", 3},\n+\t\t{ \"abc\", \"ab\", 2 },\n+\t\t{ \"\", \"abc\", 0 },\n+\t\t{ \"abc\", \"abd\", 2 },\n+\t\t{ \"abc\", \"pqr\", 0 },\n+\t};\n+\n+\tfor (size_t i = 0; i < ARRAY_SIZE(cases); i++) {\n+\t\tstrbuf_addstr(&a, cases[i].a);\n+\t\tstrbuf_addstr(&b, cases[i].b);\n+\t\tcheck_int(common_prefix_size(&a, &b), ==, cases[i].want);\n+\t\tstrbuf_reset(&a);\n+\t\tstrbuf_reset(&b);\n+\t}\n+\tstrbuf_release(&a);\n+\tstrbuf_release(&b);\n+}\n+\n+static void test_u24_roundtrip(void)\n+{\n+\tuint32_t in = 0x112233;\n+\tuint8_t dest[3];\n+\tuint32_t out;\n+\tput_be24(dest, in);\n+\tout = get_be24(dest);\n+\tcheck_int(in, ==, out);\n }\n \n int cmd_main(int argc, const char *argv[])\n@@ -116,6 +141,7 @@ int cmd_main(int argc, const char *argv[])\n \tTEST(test_binsearch(), \"binary search with binsearch works\");\n \tTEST(test_names_length(), \"names_length retuns size of a NULL-terminated string array\");\n \tTEST(test_names_equal(), \"names_equal compares NULL-terminated string arrays\");\n+\tTEST(test_u24_roundtrip(), \"put_be24 and get_be24 work\");\n \n \treturn test_done();\n }\n-- \n2.45.GIT\n\n"},{"id":"495847","messageId":"20240529171439.18271-5-chandrapratap3519@gmail.com","threadId":"61559","inReplyTo":"20240529171439.18271-1-chandrapratap3519@gmail.com","subject":"[GSoC][PATCH v3 4/5] t: add test for put_be16()","fromName":"Chandra Pratap","fromEmail":"chandrapratap3519@gmail.com","sentAt":"2024-05-29T16:59:30Z","receivedAt":"2024-05-29T17:15:57Z","isPatch":true,"sender":{"key":"chandrapratap3519@gmail.com","avatar":null},"body":"put_be16() is a function defined in reftable/basics.{c, h} for which\nthere are no tests in the current setup. Add a test for the same.\n\nMentored-by: Patrick Steinhardt <ps@pks.im>\nMentored-by: Christian Couder <chriscool@tuxfamily.org>\nSigned-off-by: Chandra Pratap <chandrapratap3519@gmail.com>\n---\n t/unit-tests/t-reftable-basics.c | 11 +++++++++++\n 1 file changed, 11 insertions(+)\n\ndiff --git a/t/unit-tests/t-reftable-basics.c b/t/unit-tests/t-reftable-basics.c\nindex b02ca02040..3c08218257 100644\n--- a/t/unit-tests/t-reftable-basics.c\n+++ b/t/unit-tests/t-reftable-basics.c\n@@ -133,6 +133,16 @@ static void test_u24_roundtrip(void)\n \tcheck_int(in, ==, out);\n }\n \n+static void test_u16_roundtrip(void)\n+{\n+\tuint32_t in = 0xfef1;\n+\tuint8_t dest[3];\n+\tuint32_t out;\n+\tput_be16(dest, in);\n+\tout = get_be16(dest);\n+\tcheck_int(in, ==, out);\n+}\n+\n int cmd_main(int argc, const char *argv[])\n {\n \tTEST(test_common_prefix(), \"common_prefix_size works\");\n@@ -142,6 +152,7 @@ int cmd_main(int argc, const char *argv[])\n \tTEST(test_names_length(), \"names_length retuns size of a NULL-terminated string array\");\n \tTEST(test_names_equal(), \"names_equal compares NULL-terminated string arrays\");\n \tTEST(test_u24_roundtrip(), \"put_be24 and get_be24 work\");\n+\tTEST(test_u16_roundtrip(), \"put_be16 and get_be16 work\");\n \n \treturn test_done();\n }\n-- \n2.45.GIT\n\n"},{"id":"495848","messageId":"20240529171439.18271-6-chandrapratap3519@gmail.com","threadId":"61559","inReplyTo":"20240529171439.18271-1-chandrapratap3519@gmail.com","subject":"[GSoC][PATCH v3 5/5] t: improve the test-case for parse_names()","fromName":"Chandra Pratap","fromEmail":"chandrapratap3519@gmail.com","sentAt":"2024-05-29T16:59:31Z","receivedAt":"2024-05-29T17:15:59Z","isPatch":true,"sender":{"key":"chandrapratap3519@gmail.com","avatar":null},"body":"In the existing test-case for parse_names(), the fact that empty\nlines should be ignored is not obvious because the empty line is\nimmediately followed by end-of-string. This can be mistaken as the\nempty line getting replaced by NULL. Improve this by adding a\nnon-empty line after the empty one to demonstrate the intended behavior.\n\nMentored-by: Patrick Steinhardt <ps@pks.im>\nMentored-by: Christian Couder <chriscool@tuxfamily.org>\nSigned-off-by: Chandra Pratap <chandrapratap3519@gmail.com>\n---\n t/unit-tests/t-reftable-basics.c | 6 ++++--\n 1 file changed, 4 insertions(+), 2 deletions(-)\n\ndiff --git a/t/unit-tests/t-reftable-basics.c b/t/unit-tests/t-reftable-basics.c\nindex 3c08218257..529049af12 100644\n--- a/t/unit-tests/t-reftable-basics.c\n+++ b/t/unit-tests/t-reftable-basics.c\n@@ -89,11 +89,13 @@ static void test_parse_names_normal(void)\n \n static void test_parse_names_drop_empty(void)\n {\n-\tchar in[] = \"a\\n\\n\";\n+\tchar in[] = \"a\\n\\nb\\n\";\n \tchar **out = NULL;\n \tparse_names(in, strlen(in), &out);\n \tcheck_str(out[0], \"a\");\n-\tcheck(!out[1]);\n+\t/* simply '\\n' should be dropped as empty string */\n+\tcheck_str(out[1], \"b\");\n+\tcheck(!out[2]);\n \tfree_names(out);\n }\n \n-- \n2.45.GIT\n\n"},{"id":"495868","messageId":"xmqqbk4otfae.fsf@gitster.g","threadId":"61559","inReplyTo":"Zlb1m5cwhW_R5EzP@tanuki","subject":"Re: [GSoC][PATCH v2 3/4] t: move tests from reftable/record_test.c to the new unit test","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-05-29T22:38:17Z","receivedAt":"2024-05-29T22:38:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n>> +\tstrbuf_release(&a);\n>> +\tstrbuf_release(&b);\n>> +}\n>\n> Oh, so this test was even duplicated. It may make sense to point out\n> details like this in the commit message to prepare the reader. But\n> that's probably not worth a reroll.\n\nProbably.  But if you are sending out another round anyway, then it\nis a good opportunity to update the proposed log message.\n\n;-)\n"},{"id":"495883","messageId":"ZlgCKsawq54QNe6h@tanuki","threadId":"61559","inReplyTo":"20240529171439.18271-1-chandrapratap3519@gmail.com","subject":"Re: [GSoC][PATCH v3 0/4] t: port reftable/basics_test.c to the unit testing","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-05-30T04:35:54Z","receivedAt":"2024-05-30T04:36:01Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Wed, May 29, 2024 at 10:29:26PM +0530, Chandra Pratap wrote:\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/basics_test.c to the unit testing framework and improve upon\n> the ported test.\n> \n> Mentored-by: Patrick Steinhardt <ps@pks.im>\n> Mentored-by: Christian Couder <chriscool@tuxfamily.org>\n> Signed-off-by: Chandra Pratap <chandrapratap3519@gmail.com>\n\nThis version looks good to me, thanks!\n\nPatrick\n"},{"id":"495901","messageId":"CAP8UFD2UrestEMsh=q0WOrJjk3DwhOz8XpweggWQ+VTwCpDtsw@mail.gmail.com","threadId":"61559","inReplyTo":"ZlgCKsawq54QNe6h@tanuki","subject":"Re: [GSoC][PATCH v3 0/4] t: port reftable/basics_test.c to the unit testing","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2024-05-30T07:52:27Z","receivedAt":"2024-05-30T07:52:41Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Thu, May 30, 2024 at 6:36 AM Patrick Steinhardt <ps@pks.im> wrote:\n>\n> On Wed, May 29, 2024 at 10:29:26PM +0530, Chandra Pratap wrote:\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/basics_test.c to the unit testing framework and improve upon\n> > the ported test.\n> >\n> > Mentored-by: Patrick Steinhardt <ps@pks.im>\n> > Mentored-by: Christian Couder <chriscool@tuxfamily.org>\n> > Signed-off-by: Chandra Pratap <chandrapratap3519@gmail.com>\n>\n> This version looks good to me, thanks!\n\nIt looks good to me too.\n"},{"id":"495959","messageId":"xmqqy17rqsht.fsf@gitster.g","threadId":"61559","inReplyTo":"CAP8UFD2UrestEMsh=q0WOrJjk3DwhOz8XpweggWQ+VTwCpDtsw@mail.gmail.com","subject":"Re: [GSoC][PATCH v3 0/4] t: port reftable/basics_test.c to the unit testing","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-05-30T14:33:34Z","receivedAt":"2024-05-30T14:33:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder <christian.couder@gmail.com> writes:\n\n> On Thu, May 30, 2024 at 6:36 AM Patrick Steinhardt <ps@pks.im> wrote:\n>>\n>> On Wed, May 29, 2024 at 10:29:26PM +0530, Chandra Pratap wrote:\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/basics_test.c to the unit testing framework and improve upon\n>> > the ported test.\n>> >\n>> > Mentored-by: Patrick Steinhardt <ps@pks.im>\n>> > Mentored-by: Christian Couder <chriscool@tuxfamily.org>\n>> > Signed-off-by: Chandra Pratap <chandrapratap3519@gmail.com>\n>>\n>> This version looks good to me, thanks!\n>\n> It looks good to me too.\n\nThanks, all.  Queued.\n"}]}