{"thread":{"id":"61555","subject":"[GSoC][PATCH] t: port reftable/basics_test.c to the unit testing","startedAt":"2024-05-28T11:39:37Z","lastAt":"2024-05-29T06:00:26Z","messageCount":9,"participants":["Chandra Pratap","Christian Couder","Patrick Steinhardt"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"495739","messageId":"20240528113856.8348-1-chandrapratap3519@gmail.com","threadId":"61555","inReplyTo":null,"subject":"[GSoC][PATCH] t: port reftable/basics_test.c to the unit testing","fromName":"Chandra Pratap","fromEmail":"chandrapratap3519@gmail.com","sentAt":"2024-05-28T11:30:01Z","receivedAt":"2024-05-28T11:39:37Z","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.\n\nCurrently, tests for the raftable 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"},{"id":"495740","messageId":"20240528113856.8348-2-chandrapratap3519@gmail.com","threadId":"61555","inReplyTo":"20240528113856.8348-1-chandrapratap3519@gmail.com","subject":"[PATCH 1/2] t: move reftable/basics_test.c to the unit testing framework","fromName":"Chandra Pratap","fromEmail":"chandrapratap3519@gmail.com","sentAt":"2024-05-28T11:30:02Z","receivedAt":"2024-05-28T11:39:40Z","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            | 50 ++++++++-----------\n 3 files changed, 21 insertions(+), 32 deletions(-)\n rename reftable/basics_test.c => t/unit-tests/t-reftable-basics.c (60%)\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 60%\nrename from reftable/basics_test.c\nrename to t/unit-tests/t-reftable-basics.c\nindex 997c4d9e01..b6088e1ddd 100644\n--- a/reftable/basics_test.c\n+++ b/t/unit-tests/t-reftable-basics.c\n@@ -1,16 +1,6 @@\n-/*\n-Copyright 2020 Google LLC\n-\n-Use of this source code is governed by a BSD-style\n-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/system.h\"\n+#include \"reftable/basics.h\"\n \n struct integer_needle_lesseq_args {\n \tint needle;\n@@ -42,9 +32,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 +41,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 +56,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 +67,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 +78,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":"495741","messageId":"20240528113856.8348-3-chandrapratap3519@gmail.com","threadId":"61555","inReplyTo":"20240528113856.8348-1-chandrapratap3519@gmail.com","subject":"[PATCH 2/2] t: improve upon reftable/basics_test.c in the unit testing framework","fromName":"Chandra Pratap","fromEmail":"chandrapratap3519@gmail.com","sentAt":"2024-05-28T11:30:03Z","receivedAt":"2024-05-28T11:39:43Z","isPatch":true,"sender":{"key":"chandrapratap3519@gmail.com","avatar":null},"body":"Enhance the new test for reftable/basics.{c, h} in the unit testing\nframework. The enhancements include:\n- Move tests for functions in reftable/basics.{c, h} from\nreftable/record_test.c and reftable/stack_test.c to the new unit test.\n- Add tests for functions that are not currently tested, like put_be16.\n- Improve the test-cases for the already existing tests.\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 reftable/stack_test.c            | 25 -----------\n t/unit-tests/t-reftable-basics.c | 77 +++++++++++++++++++++++++++-----\n 3 files changed, 65 insertions(+), 74 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/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 b6088e1ddd..53fce33d53 100644\n--- a/t/unit-tests/t-reftable-basics.c\n+++ b/t/unit-tests/t-reftable-basics.c\n@@ -51,36 +51,87 @@ 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 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 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_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@@ -90,6 +141,8 @@ 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+\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":"495746","messageId":"CAP8UFD02+bDsCxZACSfuSuHWhEeKBH4boxu+ic6xnDo8hMZJqA@mail.gmail.com","threadId":"61555","inReplyTo":"20240528113856.8348-1-chandrapratap3519@gmail.com","subject":"Re: [GSoC][PATCH] t: port reftable/basics_test.c to the unit testing","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2024-05-28T12:29:16Z","receivedAt":"2024-05-28T12:29:30Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Tue, May 28, 2024 at 1:46 PM Chandra Pratap\n<chandrapratap3519@gmail.com> wrote:\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>\n> Currently, tests for the raftable refs-backend are performed by a custom\n\ns/raftable/reftable/\n\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"},{"id":"495747","messageId":"CAP8UFD2ObwdQVE9Ta+th6O-FC9cTR8sNxsrSQWBXosjP4ccqSQ@mail.gmail.com","threadId":"61555","inReplyTo":"20240528113856.8348-2-chandrapratap3519@gmail.com","subject":"Re: [PATCH 1/2] t: move reftable/basics_test.c to the unit testing framework","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2024-05-28T12:34:05Z","receivedAt":"2024-05-28T12:34:18Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Tue, May 28, 2024 at 1:45 PM Chandra Pratap\n<chandrapratap3519@gmail.com> wrote:\n\n> diff --git a/reftable/basics_test.c b/t/unit-tests/t-reftable-basics.c\n> similarity index 60%\n> rename from reftable/basics_test.c\n> rename to t/unit-tests/t-reftable-basics.c\n> index 997c4d9e01..b6088e1ddd 100644\n> --- a/reftable/basics_test.c\n> +++ b/t/unit-tests/t-reftable-basics.c\n> @@ -1,16 +1,6 @@\n> -/*\n> -Copyright 2020 Google LLC\n> -\n> -Use of this source code is governed by a BSD-style\n> -license that can be found in the LICENSE file or at\n> -https://developers.google.com/open-source/licenses/bsd\n> -*/\n\nAre we sure it's Ok to remove the above Copyright? Or is it safer to\njust move it over (and perhaps add our own)?\n"},{"id":"495748","messageId":"CAP8UFD02Ck0M2AOmTtJO3gHr3w+3Of7BAAa0Rbc=CtMms60nyA@mail.gmail.com","threadId":"61555","inReplyTo":"20240528113856.8348-3-chandrapratap3519@gmail.com","subject":"Re: [PATCH 2/2] t: improve upon reftable/basics_test.c in the unit testing framework","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2024-05-28T12:46:17Z","receivedAt":"2024-05-28T12:46:31Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Tue, May 28, 2024 at 1:40 PM Chandra Pratap\n<chandrapratap3519@gmail.com> wrote:\n>\n> Enhance the new test for reftable/basics.{c, h} in the unit testing\n> framework. The enhancements include:\n> - Move tests for functions in reftable/basics.{c, h} from\n> reftable/record_test.c and reftable/stack_test.c to the new unit test.\n\nIt might be easier for readers if one patch moved tests from\nreftable/record_test.c and a separate patch moved tests from\nreftable/stack_test.c.\n\n> - Add tests for functions that are not currently tested, like put_be16.\n\nThis could perhaps be done in a separate patch.\n\n> - Improve the test-cases for the already existing tests.\n\nAre these improvements to tests that were moved in patch 1/2 of this\nseries? If that's the case, these improvements could be in a separate\npatch too.\n"},{"id":"495778","messageId":"ZlbEanuHHlwyaRfT@tanuki","threadId":"61555","inReplyTo":"CAP8UFD02Ck0M2AOmTtJO3gHr3w+3Of7BAAa0Rbc=CtMms60nyA@mail.gmail.com","subject":"Re: [PATCH 2/2] t: improve upon reftable/basics_test.c in the unit testing framework","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-05-29T06:00:10Z","receivedAt":"2024-05-29T06:00:16Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Tue, May 28, 2024 at 02:46:17PM +0200, Christian Couder wrote:\n> On Tue, May 28, 2024 at 1:40 PM Chandra Pratap\n> <chandrapratap3519@gmail.com> wrote:\n> >\n> > Enhance the new test for reftable/basics.{c, h} in the unit testing\n> > framework. The enhancements include:\n> > - Move tests for functions in reftable/basics.{c, h} from\n> > reftable/record_test.c and reftable/stack_test.c to the new unit test.\n> \n> It might be easier for readers if one patch moved tests from\n> reftable/record_test.c and a separate patch moved tests from\n> reftable/stack_test.c.\n> \n> > - Add tests for functions that are not currently tested, like put_be16.\n> \n> This could perhaps be done in a separate patch.\n> \n> > - Improve the test-cases for the already existing tests.\n> \n> Are these improvements to tests that were moved in patch 1/2 of this\n> series? If that's the case, these improvements could be in a separate\n> patch too.\n\nSome of the test cases are extended to also cover additional edge cases.\nBut I agree that this is easy to miss, and also think that this patch\nshould be split up further.\n\nPatrick\n"},{"id":"495779","messageId":"ZlbEcHL3fEFXbt-H@tanuki","threadId":"61555","inReplyTo":"20240528113856.8348-2-chandrapratap3519@gmail.com","subject":"Re: [PATCH 1/2] t: move reftable/basics_test.c to the unit testing framework","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-05-29T06:00:16Z","receivedAt":"2024-05-29T06:00:20Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Tue, May 28, 2024 at 05:00:02PM +0530, Chandra Pratap wrote:\n> --- a/reftable/basics_test.c\n> +++ b/t/unit-tests/t-reftable-basics.c\n> @@ -1,16 +1,6 @@\n> -/*\n> -Copyright 2020 Google LLC\n> -\n> -Use of this source code is governed by a BSD-style\n> -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/system.h\"\n\nThere is no need to include \"reftable/system.h\" explicitly as it is\nalready transitively included via \"reftable/basics.h\". The same should\nbe true for all other reftable headers.\n\nPatrick\n"},{"id":"495780","messageId":"ZlbEdZNbrVNEHFLw@tanuki","threadId":"61555","inReplyTo":"CAP8UFD2ObwdQVE9Ta+th6O-FC9cTR8sNxsrSQWBXosjP4ccqSQ@mail.gmail.com","subject":"Re: [PATCH 1/2] t: move reftable/basics_test.c to the unit testing framework","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-05-29T06:00:21Z","receivedAt":"2024-05-29T06:00:26Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Tue, May 28, 2024 at 02:34:05PM +0200, Christian Couder wrote:\n> On Tue, May 28, 2024 at 1:45 PM Chandra Pratap\n> <chandrapratap3519@gmail.com> wrote:\n> \n> > diff --git a/reftable/basics_test.c b/t/unit-tests/t-reftable-basics.c\n> > similarity index 60%\n> > rename from reftable/basics_test.c\n> > rename to t/unit-tests/t-reftable-basics.c\n> > index 997c4d9e01..b6088e1ddd 100644\n> > --- a/reftable/basics_test.c\n> > +++ b/t/unit-tests/t-reftable-basics.c\n> > @@ -1,16 +1,6 @@\n> > -/*\n> > -Copyright 2020 Google LLC\n> > -\n> > -Use of this source code is governed by a BSD-style\n> > -license that can be found in the LICENSE file or at\n> > -https://developers.google.com/open-source/licenses/bsd\n> > -*/\n> \n> Are we sure it's Ok to remove the above Copyright? Or is it safer to\n> just move it over (and perhaps add our own)?\n\nGood question indeed. We should retain the copyright header given that\nthese source files have a different license compared to the rest of the\ncodebase.\n\nPatrick\n"}]}