{"thread":{"id":"62979","subject":"[PATCH 0/5] t/unit-tests: convert unit-tests to use clar","startedAt":"2025-02-20T08:30:07Z","lastAt":"2025-02-25T10:13:01Z","messageCount":30,"participants":["Seyi Kuforiji","Phillip Wood","Patrick Steinhardt","phillip.wood123@gmail.com","Seyi Chamber","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":5},"messages":[{"id":"512759","messageId":"20250220082959.10854-1-kuforiji98@gmail.com","threadId":"62979","inReplyTo":null,"subject":"[PATCH 0/5] t/unit-tests: convert unit-tests to use clar","fromName":"Seyi Kuforiji","fromEmail":"kuforiji98@gmail.com","sentAt":"2025-02-20T08:29:54Z","receivedAt":"2025-02-20T08:30:07Z","isPatch":true,"sender":{"key":"kuforiji98@gmail.com","avatar":"https://avatars.githubusercontent.com/u/94905626?v=4"},"body":"Hello,\n\nThis small patch series transitions a couple more of our existing unit\ntest files to the Clar testing framework. This change is part of our\nongoing effort to standardize our testing framework to enhance\nmaintainability.\n\nThanks\nSeyi\n\nMentored-by: Patrick Steinhardt <ps@pks.im>\nSigned-off-by: Seyi Kuforiji <kuforiji98@gmail.com>\n\nSeyi Kuforiji (5):\n  t/unit-tests: implement oid helper functions in unit-tests.{c,h}\n  t/unit-tests: convert oid-array test to use clar test framework\n  t/unit-tests: convert oidmap test to use clar test framework\n  t/unit-tests: convert oidtree test to use clar test framework\n  t/unit-tests: remove lib-oid.{c,h,o}\n\n Makefile                                      |   7 +-\n t/meson.build                                 |   7 +-\n t/unit-tests/lib-oid.c                        |  52 ------\n t/unit-tests/lib-oid.h                        |  25 ---\n t/unit-tests/{t-oid-array.c => u-oid-array.c} | 123 +++++++-------\n t/unit-tests/{t-oidmap.c => u-oidmap.c}       | 153 ++++++------------\n t/unit-tests/{t-oidtree.c => u-oidtree.c}     |  78 ++++-----\n t/unit-tests/unit-test.c                      |  42 +++++\n t/unit-tests/unit-test.h                      |  19 +++\n 9 files changed, 212 insertions(+), 294 deletions(-)\n delete mode 100644 t/unit-tests/lib-oid.c\n delete mode 100644 t/unit-tests/lib-oid.h\n rename t/unit-tests/{t-oid-array.c => u-oid-array.c} (35%)\n rename t/unit-tests/{t-oidmap.c => u-oidmap.c} (32%)\n rename t/unit-tests/{t-oidtree.c => u-oidtree.c} (44%)\n\n-- \n2.47.0.86.g15030f9556\n\n"},{"id":"512760","messageId":"20250220082959.10854-2-kuforiji98@gmail.com","threadId":"62979","inReplyTo":"20250220082959.10854-1-kuforiji98@gmail.com","subject":"[PATCH 1/5] t/unit-tests: implement oid helper functions in unit-tests.{c,h}","fromName":"Seyi Kuforiji","fromEmail":"kuforiji98@gmail.com","sentAt":"2025-02-20T08:29:55Z","receivedAt":"2025-02-20T08:30:11Z","isPatch":true,"sender":{"key":"kuforiji98@gmail.com","avatar":"https://avatars.githubusercontent.com/u/94905626?v=4"},"body":"`get_oid_arbitrary_hex()` and `init_hash_algo()` are both required for\noid-related tests to run without errors. In the current implementation,\nboth functions are defined and declared in the\n`t/unit-tests/lib-oid.{c,h}` which is utilized by oid-related tests in\nthe homegrown unit tests structure.\n\nImplement equivalent functions in unit-tests.{c,h}. Both these functions\nbecome available for oid-related test files implemented using the clar\ntesting framework, which requires them. This will be used by subsequent\ncommits.\n\nMentored-by: Patrick Steinhardt <ps@pks.im>\nSigned-off-by: Seyi Kuforiji <kuforiji98@gmail.com>\n---\n t/unit-tests/unit-test.c | 42 ++++++++++++++++++++++++++++++++++++++++\n t/unit-tests/unit-test.h | 19 ++++++++++++++++++\n 2 files changed, 61 insertions(+)\n\ndiff --git a/t/unit-tests/unit-test.c b/t/unit-tests/unit-test.c\nindex fa8818842a..13d54f192a 100644\n--- a/t/unit-tests/unit-test.c\n+++ b/t/unit-tests/unit-test.c\n@@ -1,5 +1,7 @@\n #include \"unit-test.h\"\n+#include \"hex.h\"\n #include \"parse-options.h\"\n+#include \"strbuf.h\"\n #include \"string-list.h\"\n #include \"strvec.h\"\n \n@@ -62,3 +64,43 @@ int cmd_main(int argc, const char **argv)\n \tstrvec_clear(&args);\n \treturn ret;\n }\n+\n+int init_hash_algo(void)\n+{\n+\tstatic int algo = -1;\n+\n+\tif (algo < 0) {\n+\t\tconst char *algo_name = getenv(\"GIT_TEST_DEFAULT_HASH\");\n+\t\talgo = algo_name ? hash_algo_by_name(algo_name) : GIT_HASH_SHA1;\n+\n+\t\tcl_assert(algo != GIT_HASH_UNKNOWN);\n+\t}\n+\treturn algo;\n+}\n+\n+static void cl_parse_oid(const char *hex, struct object_id *oid,\n+\t\t\t\t       const struct git_hash_algo *algop)\n+{\n+\tint ret;\n+\tsize_t sz = strlen(hex);\n+\tstruct strbuf buf = STRBUF_INIT;\n+\n+\tcl_assert(sz <= algop->hexsz);\n+\n+\tstrbuf_add(&buf, hex, sz);\n+\tstrbuf_addchars(&buf, '0', algop->hexsz - sz);\n+\n+\tret = get_oid_hex_algop(buf.buf, oid, algop);\n+\tcl_assert_equal_i(ret, 0);\n+\n+\tstrbuf_release(&buf);\n+}\n+\n+\n+void cl_parse_any_oid(const char *hex, struct object_id *oid)\n+{\n+\tint hash_algo = init_hash_algo();\n+\n+\tcl_assert(hash_algo != GIT_HASH_UNKNOWN);\n+\tcl_parse_oid(hex, oid, &hash_algos[hash_algo]);\n+}\ndiff --git a/t/unit-tests/unit-test.h b/t/unit-tests/unit-test.h\nindex 85e5d6a948..ebed51212f 100644\n--- a/t/unit-tests/unit-test.h\n+++ b/t/unit-tests/unit-test.h\n@@ -8,3 +8,22 @@\n \tsnprintf(desc, sizeof(desc), fmt, __VA_ARGS__); \\\n \tclar__fail(__FILE__, __func__, __LINE__, \"Test failed.\", desc, 1); \\\n } while (0)\n+\n+/*\n+ * Convert arbitrary hex string to object_id.\n+ * For example, passing \"abc12\" will generate\n+ * \"abc1200000000000000000000000000000000000\" hex of length 40 for SHA-1 and\n+ * create object_id with that.\n+ * WARNING: passing a string of length more than the hexsz of respective hash\n+ * algo is not allowed. The hash algo is decided based on GIT_TEST_DEFAULT_HASH\n+ * environment variable.\n+ */\n+void cl_parse_any_oid(const char *s, struct object_id *oid);\n+/*\n+ * Returns one of GIT_HASH_{SHA1, SHA256, UNKNOWN} based on the value of\n+ * GIT_TEST_DEFAULT_HASH environment variable. The fallback value in the\n+ * absence of GIT_TEST_DEFAULT_HASH is GIT_HASH_SHA1. It also uses\n+ * cl_assert(algo != GIT_HASH_UNKNOWN) before returning to verify if the\n+ * GIT_TEST_DEFAULT_HASH's value is valid or not.\n+ */\n+int init_hash_algo(void);\n-- \n2.47.0.86.g15030f9556\n\n"},{"id":"512761","messageId":"20250220082959.10854-3-kuforiji98@gmail.com","threadId":"62979","inReplyTo":"20250220082959.10854-1-kuforiji98@gmail.com","subject":"[PATCH 2/5] t/unit-tests: convert oid-array test to use clar","fromName":"Seyi Kuforiji","fromEmail":"kuforiji98@gmail.com","sentAt":"2025-02-20T08:29:56Z","receivedAt":"2025-02-20T08:30:15Z","isPatch":true,"sender":{"key":"kuforiji98@gmail.com","avatar":"https://avatars.githubusercontent.com/u/94905626?v=4"},"body":"Adapt oid-array test script to clar framework by using clar assertions\nwhere necessary. Remove descriptions from macros to reduce\nredundancy, and move test input arrays to global scope for reuse across\nmultiple test functions. Introduce `test_oid_array__initialize()` to\nexplicitly initialize the hash algorithm.\n\nThese changes streamline the test suite, making individual tests\nself-contained and reducing redundant code.\n\nMentored-by: Patrick Steinhardt <ps@pks.im>\nSigned-off-by: Seyi Kuforiji <kuforiji98@gmail.com>\n---\n Makefile                                      |   2 +-\n t/meson.build                                 |   2 +-\n t/unit-tests/{t-oid-array.c => u-oid-array.c} | 123 +++++++++---------\n 3 files changed, 65 insertions(+), 62 deletions(-)\n rename t/unit-tests/{t-oid-array.c => u-oid-array.c} (35%)\n\ndiff --git a/Makefile b/Makefile\nindex bcf5ed3f85..f8e061365b 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1356,6 +1356,7 @@ CLAR_TEST_SUITES += u-example-decorate\n CLAR_TEST_SUITES += u-hash\n CLAR_TEST_SUITES += u-hashmap\n CLAR_TEST_SUITES += u-mem-pool\n+CLAR_TEST_SUITES += u-oid-array\n CLAR_TEST_SUITES += u-prio-queue\n CLAR_TEST_SUITES += u-reftable-tree\n CLAR_TEST_SUITES += u-strbuf\n@@ -1366,7 +1367,6 @@ CLAR_TEST_OBJS = $(patsubst %,$(UNIT_TEST_DIR)/%.o,$(CLAR_TEST_SUITES))\n CLAR_TEST_OBJS += $(UNIT_TEST_DIR)/clar/clar.o\n CLAR_TEST_OBJS += $(UNIT_TEST_DIR)/unit-test.o\n \n-UNIT_TEST_PROGRAMS += t-oid-array\n UNIT_TEST_PROGRAMS += t-oidmap\n UNIT_TEST_PROGRAMS += t-oidtree\n UNIT_TEST_PROGRAMS += t-reftable-basics\ndiff --git a/t/meson.build b/t/meson.build\nindex 780939d49f..93410a8545 100644\n--- a/t/meson.build\n+++ b/t/meson.build\n@@ -4,6 +4,7 @@ clar_test_suites = [\n   'unit-tests/u-hash.c',\n   'unit-tests/u-hashmap.c',\n   'unit-tests/u-mem-pool.c',\n+  'unit-tests/u-oid-array.c',\n   'unit-tests/u-prio-queue.c',\n   'unit-tests/u-reftable-tree.c',\n   'unit-tests/u-strbuf.c',\n@@ -48,7 +49,6 @@ clar_unit_tests = executable('unit-tests',\n test('unit-tests', clar_unit_tests)\n \n unit_test_programs = [\n-  'unit-tests/t-oid-array.c',\n   'unit-tests/t-oidmap.c',\n   'unit-tests/t-oidtree.c',\n   'unit-tests/t-reftable-basics.c',\ndiff --git a/t/unit-tests/t-oid-array.c b/t/unit-tests/u-oid-array.c\nsimilarity index 35%\nrename from t/unit-tests/t-oid-array.c\nrename to t/unit-tests/u-oid-array.c\nindex 45b59a2a51..6d173dc004 100644\n--- a/t/unit-tests/t-oid-array.c\n+++ b/t/unit-tests/u-oid-array.c\n@@ -1,22 +1,18 @@\n #define USE_THE_REPOSITORY_VARIABLE\n \n-#include \"test-lib.h\"\n-#include \"lib-oid.h\"\n+#include \"unit-test.h\"\n #include \"oid-array.h\"\n #include \"hex.h\"\n \n-static int fill_array(struct oid_array *array, const char *hexes[], size_t n)\n+static void fill_array(struct oid_array *array, const char *hexes[], size_t n)\n {\n \tfor (size_t i = 0; i < n; i++) {\n \t\tstruct object_id oid;\n \n-\t\tif (!check_int(get_oid_arbitrary_hex(hexes[i], &oid), ==, 0))\n-\t\t\treturn -1;\n+\t\tcl_parse_any_oid(hexes[i], &oid);\n \t\toid_array_append(array, &oid);\n \t}\n-\tif (!check_uint(array->nr, ==, n))\n-\t\treturn -1;\n-\treturn 0;\n+\tcl_assert_equal_i(array->nr, n);\n }\n \n static int add_to_oid_array(const struct object_id *oid, void *data)\n@@ -34,30 +30,22 @@ static void t_enumeration(const char **input_args, size_t input_sz,\n \t\t\t actual = OID_ARRAY_INIT;\n \tsize_t i;\n \n-\tif (fill_array(&input, input_args, input_sz))\n-\t\treturn;\n-\tif (fill_array(&expect, expect_args, expect_sz))\n-\t\treturn;\n+\tfill_array(&input, input_args, input_sz);\n+\tfill_array(&expect, expect_args, expect_sz);\n \n \toid_array_for_each_unique(&input, add_to_oid_array, &actual);\n-\tif (!check_uint(actual.nr, ==, expect.nr))\n-\t\treturn;\n-\n-\tfor (i = 0; i < actual.nr; i++) {\n-\t\tif (!check(oideq(&actual.oid[i], &expect.oid[i])))\n-\t\t\ttest_msg(\"expected: %s\\n       got: %s\\n     index: %\" PRIuMAX,\n-\t\t\t\t oid_to_hex(&expect.oid[i]), oid_to_hex(&actual.oid[i]),\n-\t\t\t\t (uintmax_t)i);\n-\t}\n+\tcl_assert_equal_i(actual.nr, expect.nr);\n+\n+\tfor (i = 0; i < actual.nr; i++)\n+\t\tcl_assert(oideq(&actual.oid[i], &expect.oid[i]));\n \n \toid_array_clear(&actual);\n \toid_array_clear(&input);\n \toid_array_clear(&expect);\n }\n \n-#define TEST_ENUMERATION(input, expect, desc)                                     \\\n-\tTEST(t_enumeration(input, ARRAY_SIZE(input), expect, ARRAY_SIZE(expect)), \\\n-\t\t\t   desc \" works\")\n+#define TEST_ENUMERATION(input, expect)                                     \\\n+\tt_enumeration(input, ARRAY_SIZE(input), expect, ARRAY_SIZE(expect));\n \n static void t_lookup(const char **input_hexes, size_t n, const char *query_hex,\n \t\t     int lower_bound, int upper_bound)\n@@ -66,61 +54,76 @@ static void t_lookup(const char **input_hexes, size_t n, const char *query_hex,\n \tstruct object_id oid_query;\n \tint ret;\n \n-\tif (!check_int(get_oid_arbitrary_hex(query_hex, &oid_query), ==, 0))\n-\t\treturn;\n-\tif (fill_array(&array, input_hexes, n))\n-\t\treturn;\n+\tcl_parse_any_oid(query_hex, &oid_query);\n+\tfill_array(&array, input_hexes, n);\n \tret = oid_array_lookup(&array, &oid_query);\n \n-\tif (!check_int(ret, <=, upper_bound) ||\n-\t    !check_int(ret, >=, lower_bound))\n-\t\ttest_msg(\"oid query for lookup: %s\", oid_to_hex(&oid_query));\n+\tcl_assert(ret <= upper_bound);\n+\tcl_assert(ret >= lower_bound);\n \n \toid_array_clear(&array);\n }\n \n-#define TEST_LOOKUP(input_hexes, query, lower_bound, upper_bound, desc) \\\n-\tTEST(t_lookup(input_hexes, ARRAY_SIZE(input_hexes), query,      \\\n-\t\t      lower_bound, upper_bound),                        \\\n-\t     desc \" works\")\n+#define TEST_LOOKUP(input_hexes, query, lower_bound, upper_bound) \\\n+\tt_lookup(input_hexes, ARRAY_SIZE(input_hexes), query,      \\\n+\t\t      lower_bound, upper_bound);\n \n-static void setup(void)\n+void test_oid_array__initialize(void)\n {\n \t/* The hash algo is used by oid_array_lookup() internally */\n \tint algo = init_hash_algo();\n-\tif (check_int(algo, !=, GIT_HASH_UNKNOWN))\n-\t\trepo_set_hash_algo(the_repository, algo);\n+\tcl_assert(algo != GIT_HASH_UNKNOWN);\n+\trepo_set_hash_algo(the_repository, algo);\n }\n \n-int cmd_main(int argc UNUSED, const char **argv UNUSED)\n+static const char *arr_input[] = { \"88\", \"44\", \"aa\", \"55\" };\n+static const char *arr_input_dup[] = { \"88\", \"44\", \"aa\", \"55\",\n+\t\t\t\t       \"88\", \"44\", \"aa\", \"55\",\n+\t\t\t\t       \"88\", \"44\", \"aa\", \"55\" };\n+static const char *res_sorted[] = { \"44\", \"55\", \"88\", \"aa\" };\n+\n+void test_oid_array__enumerate_unique(void)\n {\n-\tconst char *arr_input[] = { \"88\", \"44\", \"aa\", \"55\" };\n-\tconst char *arr_input_dup[] = { \"88\", \"44\", \"aa\", \"55\",\n-\t\t\t\t\t\"88\", \"44\", \"aa\", \"55\",\n-\t\t\t\t\t\"88\", \"44\", \"aa\", \"55\" };\n-\tconst char *res_sorted[] = { \"44\", \"55\", \"88\", \"aa\" };\n-\tconst char *nearly_55;\n+\tTEST_ENUMERATION(arr_input, res_sorted);\n+}\n+\n+void test_oid_array__enumerate_duplicate(void)\n+{\n+\tTEST_ENUMERATION(arr_input_dup, res_sorted);\n+}\n+\n+void test_oid_array__lookup(void)\n+{\n+\tTEST_LOOKUP(arr_input, \"55\", 1, 1);\n+}\n \n-\tif (!TEST(setup(), \"setup\"))\n-\t\ttest_skip_all(\"hash algo initialization failed\");\n+void test_oid_array__lookup_non_existent(void)\n+{\n+\tTEST_LOOKUP(arr_input, \"33\", INT_MIN, -1);\n+}\n+\n+void test_oid_array__lookup_duplicates(void)\n+{\n+\tTEST_LOOKUP(arr_input_dup, \"55\", 3, 5);\n+}\n \n-\tTEST_ENUMERATION(arr_input, res_sorted, \"ordered enumeration\");\n-\tTEST_ENUMERATION(arr_input_dup, res_sorted,\n-\t\t\t \"ordered enumeration with duplicate suppression\");\n+void test_oid_array__lookup_non_existent_dup(void)\n+{\n+\tTEST_LOOKUP(arr_input_dup, \"66\", INT_MIN, -1);\n+}\n \n-\tTEST_LOOKUP(arr_input, \"55\", 1, 1, \"lookup\");\n-\tTEST_LOOKUP(arr_input, \"33\", INT_MIN, -1, \"lookup non-existent entry\");\n-\tTEST_LOOKUP(arr_input_dup, \"55\", 3, 5, \"lookup with duplicates\");\n-\tTEST_LOOKUP(arr_input_dup, \"66\", INT_MIN, -1,\n-\t\t    \"lookup non-existent entry with duplicates\");\n+void test_oid_array__lookup_almost_dup(void)\n+{\n+\tconst char *nearly_55;\n \n \tnearly_55 = init_hash_algo() == GIT_HASH_SHA1 ?\n \t\t\t\"5500000000000000000000000000000000000001\" :\n \t\t\t\"5500000000000000000000000000000000000000000000000000000000000001\";\n-\tTEST_LOOKUP(((const char *[]){ \"55\", nearly_55 }), \"55\", 0, 0,\n-\t\t    \"lookup with almost duplicate values\");\n-\tTEST_LOOKUP(((const char *[]){ \"55\", \"55\" }), \"55\", 0, 1,\n-\t\t    \"lookup with single duplicate value\");\n \n-\treturn test_done();\n+\tTEST_LOOKUP(((const char *[]){ \"55\", nearly_55 }), \"55\", 0, 0);\n+}\n+\n+void test_oid_array__lookup_single_dup(void)\n+{\n+\tTEST_LOOKUP(((const char *[]){ \"55\", \"55\" }), \"55\", 0, 1);\n }\n-- \n2.47.0.86.g15030f9556\n\n"},{"id":"512762","messageId":"20250220082959.10854-4-kuforiji98@gmail.com","threadId":"62979","inReplyTo":"20250220082959.10854-1-kuforiji98@gmail.com","subject":"[PATCH 3/5] t/unit-tests: convert oidmap test to use clar","fromName":"Seyi Kuforiji","fromEmail":"kuforiji98@gmail.com","sentAt":"2025-02-20T08:29:57Z","receivedAt":"2025-02-20T08:30:18Z","isPatch":true,"sender":{"key":"kuforiji98@gmail.com","avatar":"https://avatars.githubusercontent.com/u/94905626?v=4"},"body":"Adapt oidmap test script to clar framework by using clar assertions\nwhere necessary. `cl_parse_any_oid` handles the necessary checks needed\nfor the test to run smoothly.\n\nIntroduce 'test_oidmap__initialize` handles the to set up of the global\noidmap map with predefined key-value pairs, and `test_oidmap__cleanup`\nfrees the oidmap and its entries when all tests are completed.\n\nThis streamlines the test suite, making individual tests self-contained\nand reducing redundant code.\n\nMentored-by: Patrick Steinhardt <ps@pks.im>\nSigned-off-by: Seyi Kuforiji <kuforiji98@gmail.com>\n---\n Makefile                                |   2 +-\n t/meson.build                           |   2 +-\n t/unit-tests/{t-oidmap.c => u-oidmap.c} | 153 ++++++++----------------\n 3 files changed, 54 insertions(+), 103 deletions(-)\n rename t/unit-tests/{t-oidmap.c => u-oidmap.c} (32%)\n\ndiff --git a/Makefile b/Makefile\nindex f8e061365b..58a6af1eb0 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1357,6 +1357,7 @@ CLAR_TEST_SUITES += u-hash\n CLAR_TEST_SUITES += u-hashmap\n CLAR_TEST_SUITES += u-mem-pool\n CLAR_TEST_SUITES += u-oid-array\n+CLAR_TEST_SUITES += u-oidmap\n CLAR_TEST_SUITES += u-prio-queue\n CLAR_TEST_SUITES += u-reftable-tree\n CLAR_TEST_SUITES += u-strbuf\n@@ -1367,7 +1368,6 @@ CLAR_TEST_OBJS = $(patsubst %,$(UNIT_TEST_DIR)/%.o,$(CLAR_TEST_SUITES))\n CLAR_TEST_OBJS += $(UNIT_TEST_DIR)/clar/clar.o\n CLAR_TEST_OBJS += $(UNIT_TEST_DIR)/unit-test.o\n \n-UNIT_TEST_PROGRAMS += t-oidmap\n UNIT_TEST_PROGRAMS += t-oidtree\n UNIT_TEST_PROGRAMS += t-reftable-basics\n UNIT_TEST_PROGRAMS += t-reftable-block\ndiff --git a/t/meson.build b/t/meson.build\nindex 93410a8545..f9e0ae15df 100644\n--- a/t/meson.build\n+++ b/t/meson.build\n@@ -5,6 +5,7 @@ clar_test_suites = [\n   'unit-tests/u-hashmap.c',\n   'unit-tests/u-mem-pool.c',\n   'unit-tests/u-oid-array.c',\n+  'unit-tests/u-oidmap.c',\n   'unit-tests/u-prio-queue.c',\n   'unit-tests/u-reftable-tree.c',\n   'unit-tests/u-strbuf.c',\n@@ -49,7 +50,6 @@ clar_unit_tests = executable('unit-tests',\n test('unit-tests', clar_unit_tests)\n \n unit_test_programs = [\n-  'unit-tests/t-oidmap.c',\n   'unit-tests/t-oidtree.c',\n   'unit-tests/t-reftable-basics.c',\n   'unit-tests/t-reftable-block.c',\ndiff --git a/t/unit-tests/t-oidmap.c b/t/unit-tests/u-oidmap.c\nsimilarity index 32%\nrename from t/unit-tests/t-oidmap.c\nrename to t/unit-tests/u-oidmap.c\nindex b22e52d08b..e40740db5c 100644\n--- a/t/unit-tests/t-oidmap.c\n+++ b/t/unit-tests/u-oidmap.c\n@@ -1,5 +1,4 @@\n-#include \"test-lib.h\"\n-#include \"lib-oid.h\"\n+#include \"unit-test.h\"\n #include \"oidmap.h\"\n #include \"hash.h\"\n #include \"hex.h\"\n@@ -18,102 +17,85 @@ static const char *const key_val[][2] = { { \"11\", \"one\" },\n \t\t\t\t\t  { \"22\", \"two\" },\n \t\t\t\t\t  { \"33\", \"three\" } };\n \n-static void setup(void (*f)(struct oidmap *map))\n+static struct oidmap map;\n+\n+void test_oidmap__initialize(void)\n {\n-\tstruct oidmap map = OIDMAP_INIT;\n-\tint ret = 0;\n+\toidmap_init(&map, 0);\n \n \tfor (size_t i = 0; i < ARRAY_SIZE(key_val); i++){\n \t\tstruct test_entry *entry;\n \n \t\tFLEX_ALLOC_STR(entry, name, key_val[i][1]);\n-\t\tif ((ret = get_oid_arbitrary_hex(key_val[i][0], &entry->entry.oid))) {\n-\t\t\tfree(entry);\n-\t\t\tbreak;\n-\t\t}\n-\t\tentry = oidmap_put(&map, entry);\n-\t\tif (!check(entry == NULL))\n-\t\t\tfree(entry);\n+\t\tcl_parse_any_oid(key_val[i][0], &entry->entry.oid);\n+\t\tcl_assert(oidmap_put(&map, entry) == NULL);\n \t}\n+}\n \n-\tif (!ret)\n-\t\tf(&map);\n+void test_oidmap__cleanup(void)\n+{\n \toidmap_free(&map, 1);\n }\n \n-static void t_replace(struct oidmap *map)\n+void test_oidmap__replace(void)\n {\n \tstruct test_entry *entry, *prev;\n \n \tFLEX_ALLOC_STR(entry, name, \"un\");\n-\tif (get_oid_arbitrary_hex(\"11\", &entry->entry.oid))\n-\t\treturn;\n-\tprev = oidmap_put(map, entry);\n-\tif (!check(prev != NULL))\n-\t\treturn;\n-\tcheck_str(prev->name, \"one\");\n+\tcl_parse_any_oid(\"11\", &entry->entry.oid);\n+\tprev = oidmap_put(&map, entry);\n+\tcl_assert(prev != NULL);\n+\tcl_assert_equal_s(prev->name, \"one\");\n \tfree(prev);\n \n \tFLEX_ALLOC_STR(entry, name, \"deux\");\n-\tif (get_oid_arbitrary_hex(\"22\", &entry->entry.oid))\n-\t\treturn;\n-\tprev = oidmap_put(map, entry);\n-\tif (!check(prev != NULL))\n-\t\treturn;\n-\tcheck_str(prev->name, \"two\");\n+\tcl_parse_any_oid(\"22\", &entry->entry.oid);\n+\tprev = oidmap_put(&map, entry);\n+\tcl_assert(prev != NULL);\n+\tcl_assert_equal_s(prev->name, \"two\");\n \tfree(prev);\n }\n \n-static void t_get(struct oidmap *map)\n+void test_oidmap__get(void)\n {\n \tstruct test_entry *entry;\n \tstruct object_id oid;\n \n-\tif (get_oid_arbitrary_hex(\"22\", &oid))\n-\t\treturn;\n-\tentry = oidmap_get(map, &oid);\n-\tif (!check(entry != NULL))\n-\t\treturn;\n-\tcheck_str(entry->name, \"two\");\n-\n-\tif (get_oid_arbitrary_hex(\"44\", &oid))\n-\t\treturn;\n-\tcheck(oidmap_get(map, &oid) == NULL);\n-\n-\tif (get_oid_arbitrary_hex(\"11\", &oid))\n-\t\treturn;\n-\tentry = oidmap_get(map, &oid);\n-\tif (!check(entry != NULL))\n-\t\treturn;\n-\tcheck_str(entry->name, \"one\");\n+\tcl_parse_any_oid(\"22\", &oid);\n+\tentry = oidmap_get(&map, &oid);\n+\tcl_assert(entry != NULL);\n+\tcl_assert_equal_s(entry->name, \"two\");\n+\n+\tcl_parse_any_oid(\"44\", &oid);\n+\tcl_assert(oidmap_get(&map, &oid) == NULL);\n+\n+\tcl_parse_any_oid(\"11\", &oid);\n+\tentry = oidmap_get(&map, &oid);\n+\tcl_assert(entry != NULL);\n+\tcl_assert_equal_s(entry->name, \"one\");\n }\n \n-static void t_remove(struct oidmap *map)\n+void test_oidmap__remove(void)\n {\n \tstruct test_entry *entry;\n \tstruct object_id oid;\n \n-\tif (get_oid_arbitrary_hex(\"11\", &oid))\n-\t\treturn;\n-\tentry = oidmap_remove(map, &oid);\n-\tif (!check(entry != NULL))\n-\t\treturn;\n-\tcheck_str(entry->name, \"one\");\n-\tcheck(oidmap_get(map, &oid) == NULL);\n+\tcl_parse_any_oid(\"11\", &oid);\n+\tentry = oidmap_remove(&map, &oid);\n+\tcl_assert(entry != NULL);\n+\tcl_assert_equal_s(entry->name, \"one\");\n+\tcl_assert(oidmap_get(&map, &oid) == NULL);\n \tfree(entry);\n \n-\tif (get_oid_arbitrary_hex(\"22\", &oid))\n-\t\treturn;\n-\tentry = oidmap_remove(map, &oid);\n-\tif (!check(entry != NULL))\n-\t\treturn;\n-\tcheck_str(entry->name, \"two\");\n-\tcheck(oidmap_get(map, &oid) == NULL);\n+\tcl_parse_any_oid(\"22\", &oid);\n+\tentry = oidmap_remove(&map, &oid);\n+\tcl_assert(entry != NULL);\n+\tcl_assert_equal_s(entry->name, \"two\");\n+\tcl_assert(oidmap_get(&map, &oid) == NULL);\n \tfree(entry);\n \n-\tif (get_oid_arbitrary_hex(\"44\", &oid))\n-\t\treturn;\n-\tcheck(oidmap_remove(map, &oid) == NULL);\n+\tcl_parse_any_oid(\"44\", &oid);\n+\tcl_assert(oidmap_remove(&map, &oid) == NULL);\n }\n \n static int key_val_contains(struct test_entry *entry, char seen[])\n@@ -121,8 +103,7 @@ static int key_val_contains(struct test_entry *entry, char seen[])\n \tfor (size_t i = 0; i < ARRAY_SIZE(key_val); i++) {\n \t\tstruct object_id oid;\n \n-\t\tif (get_oid_arbitrary_hex(key_val[i][0], &oid))\n-\t\t\treturn -1;\n+\t\tcl_parse_any_oid(key_val[i][0], &oid);\n \n \t\tif (oideq(&entry->entry.oid, &oid)) {\n \t\t\tif (seen[i])\n@@ -134,48 +115,18 @@ static int key_val_contains(struct test_entry *entry, char seen[])\n \treturn 1;\n }\n \n-static void t_iterate(struct oidmap *map)\n+void test_oidmap__iterate(void)\n {\n \tstruct oidmap_iter iter;\n \tstruct test_entry *entry;\n \tchar seen[ARRAY_SIZE(key_val)] = { 0 };\n \tint count = 0;\n \n-\toidmap_iter_init(map, &iter);\n+\toidmap_iter_init(&map, &iter);\n \twhile ((entry = oidmap_iter_next(&iter))) {\n-\t\tint ret;\n-\t\tif (!check_int((ret = key_val_contains(entry, seen)), ==, 0)) {\n-\t\t\tswitch (ret) {\n-\t\t\tcase -1:\n-\t\t\t\tbreak; /* error message handled by get_oid_arbitrary_hex() */\n-\t\t\tcase 1:\n-\t\t\t\ttest_msg(\"obtained entry was not given in the input\\n\"\n-\t\t\t\t\t \"  name: %s\\n   oid: %s\\n\",\n-\t\t\t\t\t entry->name, oid_to_hex(&entry->entry.oid));\n-\t\t\t\tbreak;\n-\t\t\tcase 2:\n-\t\t\t\ttest_msg(\"duplicate entry detected\\n\"\n-\t\t\t\t\t \"  name: %s\\n   oid: %s\\n\",\n-\t\t\t\t\t entry->name, oid_to_hex(&entry->entry.oid));\n-\t\t\t\tbreak;\n-\t\t\tdefault:\n-\t\t\t\ttest_msg(\"BUG: invalid return value (%d) from key_val_contains()\",\n-\t\t\t\t\t ret);\n-\t\t\t\tbreak;\n-\t\t\t}\n-\t\t} else {\n-\t\t\tcount++;\n-\t\t}\n+\t\tcl_assert_equal_i(key_val_contains(entry, seen), 0);\n+\t\tcount++;\n \t}\n-\tcheck_int(count, ==, ARRAY_SIZE(key_val));\n-\tcheck_int(hashmap_get_size(&map->map), ==, ARRAY_SIZE(key_val));\n-}\n-\n-int cmd_main(int argc UNUSED, const char **argv UNUSED)\n-{\n-\tTEST(setup(t_replace), \"replace works\");\n-\tTEST(setup(t_get), \"get works\");\n-\tTEST(setup(t_remove), \"remove works\");\n-\tTEST(setup(t_iterate), \"iterate works\");\n-\treturn test_done();\n+\tcl_assert_equal_i(count, ARRAY_SIZE(key_val));\n+\tcl_assert_equal_i(hashmap_get_size(&map.map), ARRAY_SIZE(key_val));\n }\n-- \n2.47.0.86.g15030f9556\n\n"},{"id":"512763","messageId":"20250220082959.10854-5-kuforiji98@gmail.com","threadId":"62979","inReplyTo":"20250220082959.10854-1-kuforiji98@gmail.com","subject":"[PATCH 4/5] t/unit-tests: convert oidtree test to use clar","fromName":"Seyi Kuforiji","fromEmail":"kuforiji98@gmail.com","sentAt":"2025-02-20T08:29:58Z","receivedAt":"2025-02-20T08:30:22Z","isPatch":true,"sender":{"key":"kuforiji98@gmail.com","avatar":"https://avatars.githubusercontent.com/u/94905626?v=4"},"body":"Adapt oidtree test script to clar framework by using clar assertions\nwhere necessary. `cl_parse_any_oid` handles the necessary checks needed\nfor the test to run smoothly.\n\nIntroduce 'test_oidtree__initialize` handles the to set up of the global\noidtree variable and `test_oidtree__cleanup` frees the oidtree when all\ntests are completed.\n\nThis streamlines the test suite, making individual tests self-contained\nand reducing redundant code.\n\nMentored-by: Patrick Steinhardt <ps@pks.im>\nSigned-off-by: Seyi Kuforiji <kuforiji98@gmail.com>\n---\n Makefile                                  |  2 +-\n t/meson.build                             |  2 +-\n t/unit-tests/{t-oidtree.c => u-oidtree.c} | 78 +++++++++--------------\n 3 files changed, 32 insertions(+), 50 deletions(-)\n rename t/unit-tests/{t-oidtree.c => u-oidtree.c} (44%)\n\ndiff --git a/Makefile b/Makefile\nindex 58a6af1eb0..feb01702c7 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1358,6 +1358,7 @@ CLAR_TEST_SUITES += u-hashmap\n CLAR_TEST_SUITES += u-mem-pool\n CLAR_TEST_SUITES += u-oid-array\n CLAR_TEST_SUITES += u-oidmap\n+CLAR_TEST_SUITES += u-oidtree\n CLAR_TEST_SUITES += u-prio-queue\n CLAR_TEST_SUITES += u-reftable-tree\n CLAR_TEST_SUITES += u-strbuf\n@@ -1368,7 +1369,6 @@ CLAR_TEST_OBJS = $(patsubst %,$(UNIT_TEST_DIR)/%.o,$(CLAR_TEST_SUITES))\n CLAR_TEST_OBJS += $(UNIT_TEST_DIR)/clar/clar.o\n CLAR_TEST_OBJS += $(UNIT_TEST_DIR)/unit-test.o\n \n-UNIT_TEST_PROGRAMS += t-oidtree\n UNIT_TEST_PROGRAMS += t-reftable-basics\n UNIT_TEST_PROGRAMS += t-reftable-block\n UNIT_TEST_PROGRAMS += t-reftable-merged\ndiff --git a/t/meson.build b/t/meson.build\nindex f9e0ae15df..0b412a7c16 100644\n--- a/t/meson.build\n+++ b/t/meson.build\n@@ -6,6 +6,7 @@ clar_test_suites = [\n   'unit-tests/u-mem-pool.c',\n   'unit-tests/u-oid-array.c',\n   'unit-tests/u-oidmap.c',\n+  'unit-tests/u-oidtree.c',\n   'unit-tests/u-prio-queue.c',\n   'unit-tests/u-reftable-tree.c',\n   'unit-tests/u-strbuf.c',\n@@ -50,7 +51,6 @@ clar_unit_tests = executable('unit-tests',\n test('unit-tests', clar_unit_tests)\n \n unit_test_programs = [\n-  'unit-tests/t-oidtree.c',\n   'unit-tests/t-reftable-basics.c',\n   'unit-tests/t-reftable-block.c',\n   'unit-tests/t-reftable-merged.c',\ndiff --git a/t/unit-tests/t-oidtree.c b/t/unit-tests/u-oidtree.c\nsimilarity index 44%\nrename from t/unit-tests/t-oidtree.c\nrename to t/unit-tests/u-oidtree.c\nindex a38754b066..de6f6bd292 100644\n--- a/t/unit-tests/t-oidtree.c\n+++ b/t/unit-tests/u-oidtree.c\n@@ -1,10 +1,11 @@\n-#include \"test-lib.h\"\n-#include \"lib-oid.h\"\n+#include \"unit-test.h\"\n #include \"oidtree.h\"\n #include \"hash.h\"\n #include \"hex.h\"\n #include \"strvec.h\"\n \n+static struct oidtree ot;\n+\n #define FILL_TREE(tree, ...)                                       \\\n \tdo {                                                       \\\n \t\tconst char *hexes[] = { __VA_ARGS__ };             \\\n@@ -16,8 +17,7 @@ static int fill_tree_loc(struct oidtree *ot, const char *hexes[], size_t n)\n {\n \tfor (size_t i = 0; i < n; i++) {\n \t\tstruct object_id oid;\n-\t\tif (!check_int(get_oid_arbitrary_hex(hexes[i], &oid), ==, 0))\n-\t\t\treturn -1;\n+\t\tcl_parse_any_oid(hexes[i], &oid);\n \t\toidtree_insert(ot, &oid);\n \t}\n \treturn 0;\n@@ -27,10 +27,8 @@ static void check_contains(struct oidtree *ot, const char *hex, int expected)\n {\n \tstruct object_id oid;\n \n-\tif (!check_int(get_oid_arbitrary_hex(hex, &oid), ==, 0))\n-\t\treturn;\n-\tif (!check_int(oidtree_contains(ot, &oid), ==, expected))\n-\t\ttest_msg(\"oid: %s\", oid_to_hex(&oid));\n+\tcl_parse_any_oid(hex, &oid);\n+\tcl_assert_equal_i(oidtree_contains(ot, &oid), expected);\n }\n \n struct expected_hex_iter {\n@@ -44,19 +42,11 @@ static enum cb_next check_each_cb(const struct object_id *oid, void *data)\n \tstruct expected_hex_iter *hex_iter = data;\n \tstruct object_id expected;\n \n-\tif (!check_int(hex_iter->i, <, hex_iter->expected_hexes.nr)) {\n-\t\ttest_msg(\"error: extraneous callback for query: ('%s'), object_id: ('%s')\",\n-\t\t\t hex_iter->query, oid_to_hex(oid));\n-\t\treturn CB_BREAK;\n-\t}\n-\n-\tif (!check_int(get_oid_arbitrary_hex(hex_iter->expected_hexes.v[hex_iter->i],\n-\t\t\t\t\t     &expected), ==, 0))\n-\t\t; /* the data is bogus and cannot be used */\n-\telse if (!check(oideq(oid, &expected)))\n-\t\ttest_msg(\"expected: %s\\n       got: %s\\n     query: %s\",\n-\t\t\t oid_to_hex(&expected), oid_to_hex(oid), hex_iter->query);\n+\tcl_assert(hex_iter->i < hex_iter->expected_hexes.nr);\n \n+\tcl_parse_any_oid(hex_iter->expected_hexes.v[hex_iter->i],\n+\t\t\t &expected);\n+\tcl_assert_equal_s(oid_to_hex(oid), oid_to_hex(&expected));\n \thex_iter->i += 1;\n \treturn CB_CONTINUE;\n }\n@@ -75,48 +65,40 @@ static void check_each(struct oidtree *ot, const char *query, ...)\n \t\tstrvec_push(&hex_iter.expected_hexes, arg);\n \tva_end(hex_args);\n \n-\tif (!check_int(get_oid_arbitrary_hex(query, &oid), ==, 0))\n-\t\treturn;\n+\tcl_parse_any_oid(query, &oid);\n \toidtree_each(ot, &oid, strlen(query), check_each_cb, &hex_iter);\n \n-\tif (!check_int(hex_iter.i, ==, hex_iter.expected_hexes.nr))\n-\t\ttest_msg(\"error: could not find some 'object_id's for query ('%s')\", query);\n+\tcl_assert_equal_i(hex_iter.i, hex_iter.expected_hexes.nr);\n \tstrvec_clear(&hex_iter.expected_hexes);\n }\n \n-static void setup(void (*f)(struct oidtree *ot))\n+void test_oidtree__initialize(void)\n {\n-\tstruct oidtree ot;\n-\n \toidtree_init(&ot);\n-\tf(&ot);\n-\toidtree_clear(&ot);\n }\n \n-static void t_contains(struct oidtree *ot)\n+void test_oidtree__cleanup(void)\n {\n-\tFILL_TREE(ot, \"444\", \"1\", \"2\", \"3\", \"4\", \"5\", \"a\", \"b\", \"c\", \"d\", \"e\");\n-\tcheck_contains(ot, \"44\", 0);\n-\tcheck_contains(ot, \"441\", 0);\n-\tcheck_contains(ot, \"440\", 0);\n-\tcheck_contains(ot, \"444\", 1);\n-\tcheck_contains(ot, \"4440\", 1);\n-\tcheck_contains(ot, \"4444\", 0);\n+\toidtree_clear(&ot);\n }\n \n-static void t_each(struct oidtree *ot)\n+void test_oidtree__contains(void)\n {\n-\tFILL_TREE(ot, \"f\", \"9\", \"8\", \"123\", \"321\", \"320\", \"a\", \"b\", \"c\", \"d\", \"e\");\n-\tcheck_each(ot, \"12300\", \"123\", NULL);\n-\tcheck_each(ot, \"3211\", NULL); /* should not reach callback */\n-\tcheck_each(ot, \"3210\", \"321\", NULL);\n-\tcheck_each(ot, \"32100\", \"321\", NULL);\n-\tcheck_each(ot, \"32\", \"320\", \"321\", NULL);\n+\tFILL_TREE(&ot, \"444\", \"1\", \"2\", \"3\", \"4\", \"5\", \"a\", \"b\", \"c\", \"d\", \"e\");\n+\tcheck_contains(&ot, \"44\", 0);\n+\tcheck_contains(&ot, \"441\", 0);\n+\tcheck_contains(&ot, \"440\", 0);\n+\tcheck_contains(&ot, \"444\", 1);\n+\tcheck_contains(&ot, \"4440\", 1);\n+\tcheck_contains(&ot, \"4444\", 0);\n }\n \n-int cmd_main(int argc UNUSED, const char **argv UNUSED)\n+void test_oidtree__each(void)\n {\n-\tTEST(setup(t_contains), \"oidtree insert and contains works\");\n-\tTEST(setup(t_each), \"oidtree each works\");\n-\treturn test_done();\n+\tFILL_TREE(&ot, \"f\", \"9\", \"8\", \"123\", \"321\", \"320\", \"a\", \"b\", \"c\", \"d\", \"e\");\n+\tcheck_each(&ot, \"12300\", \"123\", NULL);\n+\tcheck_each(&ot, \"3211\", NULL); /* should not reach callback */\n+\tcheck_each(&ot, \"3210\", \"321\", NULL);\n+\tcheck_each(&ot, \"32100\", \"321\", NULL);\n+\tcheck_each(&ot, \"32\", \"320\", \"321\", NULL);\n }\n-- \n2.47.0.86.g15030f9556\n\n"},{"id":"512764","messageId":"20250220082959.10854-6-kuforiji98@gmail.com","threadId":"62979","inReplyTo":"20250220082959.10854-1-kuforiji98@gmail.com","subject":"[PATCH 5/5] t/unit-tests: remove lib-oid.{c,h,o}","fromName":"Seyi Kuforiji","fromEmail":"kuforiji98@gmail.com","sentAt":"2025-02-20T08:29:59Z","receivedAt":"2025-02-20T08:30:25Z","isPatch":true,"sender":{"key":"kuforiji98@gmail.com","avatar":"https://avatars.githubusercontent.com/u/94905626?v=4"},"body":"The `lib-oid.c`, `lib-oid.h`, and `lib-oid.o files` are no longer needed\nsince their equivalent functions have been implemented in unit-test.c\nand unit-test.h. This removes redundant code and ensures all unit\ntest-related functionality is consolidated in a single location.\n\nDrop references to lib-oid from our `Makefile`, and `meson.build` files\nto prevent build errors due to missing files.\n\nMentored-by: Patrick Steinhardt <ps@pks.im>\nSigned-off-by: Seyi Kuforiji <kuforiji98@gmail.com>\n---\n Makefile               |  1 -\n t/meson.build          |  1 -\n t/unit-tests/lib-oid.c | 52 ------------------------------------------\n t/unit-tests/lib-oid.h | 25 --------------------\n 4 files changed, 79 deletions(-)\n delete mode 100644 t/unit-tests/lib-oid.c\n delete mode 100644 t/unit-tests/lib-oid.h\n\ndiff --git a/Makefile b/Makefile\nindex feb01702c7..6afa6587ba 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1381,7 +1381,6 @@ UNIT_TEST_PROGRAMS += t-trailer\n UNIT_TEST_PROGRAMS += t-urlmatch-normalization\n UNIT_TEST_PROGS = $(patsubst %,$(UNIT_TEST_BIN)/%$X,$(UNIT_TEST_PROGRAMS))\n UNIT_TEST_OBJS += $(UNIT_TEST_DIR)/test-lib.o\n-UNIT_TEST_OBJS += $(UNIT_TEST_DIR)/lib-oid.o\n UNIT_TEST_OBJS += $(UNIT_TEST_DIR)/lib-reftable.o\n \n # xdiff and reftable libs may in turn depend on what is in libgit.a\ndiff --git a/t/meson.build b/t/meson.build\nindex 0b412a7c16..c1c4aa32aa 100644\n--- a/t/meson.build\n+++ b/t/meson.build\n@@ -68,7 +68,6 @@ foreach unit_test_program : unit_test_programs\n   unit_test = executable(unit_test_name,\n     sources: [\n       'unit-tests/test-lib.c',\n-      'unit-tests/lib-oid.c',\n       'unit-tests/lib-reftable.c',\n       unit_test_program,\n     ],\ndiff --git a/t/unit-tests/lib-oid.c b/t/unit-tests/lib-oid.c\ndeleted file mode 100644\nindex 8f0ccac532..0000000000\n--- a/t/unit-tests/lib-oid.c\n+++ /dev/null\n@@ -1,52 +0,0 @@\n-#include \"test-lib.h\"\n-#include \"lib-oid.h\"\n-#include \"strbuf.h\"\n-#include \"hex.h\"\n-\n-int init_hash_algo(void)\n-{\n-\tstatic int algo = -1;\n-\n-\tif (algo < 0) {\n-\t\tconst char *algo_name = getenv(\"GIT_TEST_DEFAULT_HASH\");\n-\t\talgo = algo_name ? hash_algo_by_name(algo_name) : GIT_HASH_SHA1;\n-\n-\t\tif (!check(algo != GIT_HASH_UNKNOWN))\n-\t\t\ttest_msg(\"BUG: invalid GIT_TEST_DEFAULT_HASH value ('%s')\",\n-\t\t\t\t algo_name);\n-\t}\n-\treturn algo;\n-}\n-\n-static int get_oid_arbitrary_hex_algop(const char *hex, struct object_id *oid,\n-\t\t\t\t       const struct git_hash_algo *algop)\n-{\n-\tint ret;\n-\tsize_t sz = strlen(hex);\n-\tstruct strbuf buf = STRBUF_INIT;\n-\n-\tif (!check(sz <= algop->hexsz)) {\n-\t\ttest_msg(\"BUG: hex string (%s) bigger than maximum allowed (%lu)\",\n-\t\t\t hex, (unsigned long)algop->hexsz);\n-\t\treturn -1;\n-\t}\n-\n-\tstrbuf_add(&buf, hex, sz);\n-\tstrbuf_addchars(&buf, '0', algop->hexsz - sz);\n-\n-\tret = get_oid_hex_algop(buf.buf, oid, algop);\n-\tif (!check_int(ret, ==, 0))\n-\t\ttest_msg(\"BUG: invalid hex input (%s) provided\", hex);\n-\n-\tstrbuf_release(&buf);\n-\treturn ret;\n-}\n-\n-int get_oid_arbitrary_hex(const char *hex, struct object_id *oid)\n-{\n-\tint hash_algo = init_hash_algo();\n-\n-\tif (!check_int(hash_algo, !=, GIT_HASH_UNKNOWN))\n-\t\treturn -1;\n-\treturn get_oid_arbitrary_hex_algop(hex, oid, &hash_algos[hash_algo]);\n-}\ndiff --git a/t/unit-tests/lib-oid.h b/t/unit-tests/lib-oid.h\ndeleted file mode 100644\nindex 4e77c04bd2..0000000000\n--- a/t/unit-tests/lib-oid.h\n+++ /dev/null\n@@ -1,25 +0,0 @@\n-#ifndef LIB_OID_H\n-#define LIB_OID_H\n-\n-#include \"hash.h\"\n-\n-/*\n- * Convert arbitrary hex string to object_id.\n- * For example, passing \"abc12\" will generate\n- * \"abc1200000000000000000000000000000000000\" hex of length 40 for SHA-1 and\n- * create object_id with that.\n- * WARNING: passing a string of length more than the hexsz of respective hash\n- * algo is not allowed. The hash algo is decided based on GIT_TEST_DEFAULT_HASH\n- * environment variable.\n- */\n-int get_oid_arbitrary_hex(const char *s, struct object_id *oid);\n-/*\n- * Returns one of GIT_HASH_{SHA1, SHA256, UNKNOWN} based on the value of\n- * GIT_TEST_DEFAULT_HASH environment variable. The fallback value in the\n- * absence of GIT_TEST_DEFAULT_HASH is GIT_HASH_SHA1. It also uses\n- * check(algo != GIT_HASH_UNKNOWN) before returning to verify if the\n- * GIT_TEST_DEFAULT_HASH's value is valid or not.\n- */\n-int init_hash_algo(void);\n-\n-#endif /* LIB_OID_H */\n-- \n2.47.0.86.g15030f9556\n\n"},{"id":"512771","messageId":"c9e8a5f8-fda6-4383-bbb3-0c960a4cf86e@gmail.com","threadId":"62979","inReplyTo":"20250220082959.10854-2-kuforiji98@gmail.com","subject":"Re: [PATCH 1/5] t/unit-tests: implement oid helper functions in unit-tests.{c,h}","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2025-02-20T14:38:21Z","receivedAt":"2025-02-20T14:38:24Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Seyi\n\nOn 20/02/2025 08:29, Seyi Kuforiji wrote:\n> `get_oid_arbitrary_hex()` and `init_hash_algo()` are both required for\n> oid-related tests to run without errors. In the current implementation,\n> both functions are defined and declared in the\n> `t/unit-tests/lib-oid.{c,h}` which is utilized by oid-related tests in\n> the homegrown unit tests structure.\n> \n> Implement equivalent functions in unit-tests.{c,h}. Both these functions\n> become available for oid-related test files implemented using the clar\n> testing framework, which requires them. This will be used by subsequent\n> commits.\n\nIt is nice to see these tests being moved over to clar but I'm not sure \nthat moving these functions into this file is good idea. All the unit \ntests need to link against unit-tests.o but only a subset will want \naccess to these functions. Putting them in this file means that all the \ntests will now depend on code from strbuf.o and hex.o. I think we could \nadd the new functions to lib-oid.c and then remove the old ones when \nthere are not needed any more.\n\nBest Wishes\n\nPhillip\n\n> Mentored-by: Patrick Steinhardt <ps@pks.im>\n> Signed-off-by: Seyi Kuforiji <kuforiji98@gmail.com>\n> ---\n>   t/unit-tests/unit-test.c | 42 ++++++++++++++++++++++++++++++++++++++++\n>   t/unit-tests/unit-test.h | 19 ++++++++++++++++++\n>   2 files changed, 61 insertions(+)\n> \n> diff --git a/t/unit-tests/unit-test.c b/t/unit-tests/unit-test.c\n> index fa8818842a..13d54f192a 100644\n> --- a/t/unit-tests/unit-test.c\n> +++ b/t/unit-tests/unit-test.c\n> @@ -1,5 +1,7 @@\n>   #include \"unit-test.h\"\n> +#include \"hex.h\"\n>   #include \"parse-options.h\"\n> +#include \"strbuf.h\"\n>   #include \"string-list.h\"\n>   #include \"strvec.h\"\n>   \n> @@ -62,3 +64,43 @@ int cmd_main(int argc, const char **argv)\n>   \tstrvec_clear(&args);\n>   \treturn ret;\n>   }\n> +\n> +int init_hash_algo(void)\n> +{\n> +\tstatic int algo = -1;\n> +\n> +\tif (algo < 0) {\n> +\t\tconst char *algo_name = getenv(\"GIT_TEST_DEFAULT_HASH\");\n> +\t\talgo = algo_name ? hash_algo_by_name(algo_name) : GIT_HASH_SHA1;\n> +\n> +\t\tcl_assert(algo != GIT_HASH_UNKNOWN);\n> +\t}\n> +\treturn algo;\n> +}\n> +\n> +static void cl_parse_oid(const char *hex, struct object_id *oid,\n> +\t\t\t\t       const struct git_hash_algo *algop)\n> +{\n> +\tint ret;\n> +\tsize_t sz = strlen(hex);\n> +\tstruct strbuf buf = STRBUF_INIT;\n> +\n> +\tcl_assert(sz <= algop->hexsz);\n> +\n> +\tstrbuf_add(&buf, hex, sz);\n> +\tstrbuf_addchars(&buf, '0', algop->hexsz - sz);\n> +\n> +\tret = get_oid_hex_algop(buf.buf, oid, algop);\n> +\tcl_assert_equal_i(ret, 0);\n> +\n> +\tstrbuf_release(&buf);\n> +}\n> +\n> +\n> +void cl_parse_any_oid(const char *hex, struct object_id *oid)\n> +{\n> +\tint hash_algo = init_hash_algo();\n> +\n> +\tcl_assert(hash_algo != GIT_HASH_UNKNOWN);\n> +\tcl_parse_oid(hex, oid, &hash_algos[hash_algo]);\n> +}\n> diff --git a/t/unit-tests/unit-test.h b/t/unit-tests/unit-test.h\n> index 85e5d6a948..ebed51212f 100644\n> --- a/t/unit-tests/unit-test.h\n> +++ b/t/unit-tests/unit-test.h\n> @@ -8,3 +8,22 @@\n>   \tsnprintf(desc, sizeof(desc), fmt, __VA_ARGS__); \\\n>   \tclar__fail(__FILE__, __func__, __LINE__, \"Test failed.\", desc, 1); \\\n>   } while (0)\n> +\n> +/*\n> + * Convert arbitrary hex string to object_id.\n> + * For example, passing \"abc12\" will generate\n> + * \"abc1200000000000000000000000000000000000\" hex of length 40 for SHA-1 and\n> + * create object_id with that.\n> + * WARNING: passing a string of length more than the hexsz of respective hash\n> + * algo is not allowed. The hash algo is decided based on GIT_TEST_DEFAULT_HASH\n> + * environment variable.\n> + */\n> +void cl_parse_any_oid(const char *s, struct object_id *oid);\n> +/*\n> + * Returns one of GIT_HASH_{SHA1, SHA256, UNKNOWN} based on the value of\n> + * GIT_TEST_DEFAULT_HASH environment variable. The fallback value in the\n> + * absence of GIT_TEST_DEFAULT_HASH is GIT_HASH_SHA1. It also uses\n> + * cl_assert(algo != GIT_HASH_UNKNOWN) before returning to verify if the\n> + * GIT_TEST_DEFAULT_HASH's value is valid or not.\n> + */\n> +int init_hash_algo(void);\n\n"},{"id":"512772","messageId":"cf9d44d0-20ed-4d8f-84a3-1bf284075605@gmail.com","threadId":"62979","inReplyTo":"20250220082959.10854-3-kuforiji98@gmail.com","subject":"Re: [PATCH 2/5] t/unit-tests: convert oid-array test to use clar","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2025-02-20T14:38:30Z","receivedAt":"2025-02-20T14:38:33Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Seyi\n\nOn 20/02/2025 08:29, Seyi Kuforiji wrote:\n> Adapt oid-array test script to clar framework by using clar assertions\n> where necessary. Remove descriptions from macros to reduce\n> redundancy, and move test input arrays to global scope for reuse across\n> multiple test functions. Introduce `test_oid_array__initialize()` to\n> explicitly initialize the hash algorithm.\n> \n> These changes streamline the test suite, making individual tests\n> self-contained and reducing redundant code.\n\nI think these conversion look correct but once again we're losing \nvaluable debugging information because we haven't added better \nassertions to clar.\n\n>   \toid_array_for_each_unique(&input, add_to_oid_array, &actual);\n> -\tif (!check_uint(actual.nr, ==, expect.nr))\n> -\t\treturn;\n> -\n> -\tfor (i = 0; i < actual.nr; i++) {\n> -\t\tif (!check(oideq(&actual.oid[i], &expect.oid[i])))\n> -\t\t\ttest_msg(\"expected: %s\\n       got: %s\\n     index: %\" PRIuMAX,\n> -\t\t\t\t oid_to_hex(&expect.oid[i]), oid_to_hex(&actual.oid[i]),\n> -\t\t\t\t (uintmax_t)i);\n> -\t}\n> +\tcl_assert_equal_i(actual.nr, expect.nr);\n> +\n> +\tfor (i = 0; i < actual.nr; i++)\n> +\t\tcl_assert(oideq(&actual.oid[i], &expect.oid[i]));\n\nIf this fails the poor person debugging it will have no idea why as \nthere is now no indication of which two oids were being compared.\n\n> -\tif (!check_int(ret, <=, upper_bound) ||\n> -\t    !check_int(ret, >=, lower_bound))\n> -\t\ttest_msg(\"oid query for lookup: %s\", oid_to_hex(&oid_query));\n> +\tcl_assert(ret <= upper_bound);\n> +\tcl_assert(ret >= lower_bound);\n\nThis is another case where we could do with better assertions in clar\n\n> -static void setup(void)\n> +void test_oid_array__initialize(void)\n>   {\n>   \t/* The hash algo is used by oid_array_lookup() internally */\n>   \tint algo = init_hash_algo();\n> -\tif (check_int(algo, !=, GIT_HASH_UNKNOWN))\n> -\t\trepo_set_hash_algo(the_repository, algo);\n> +\tcl_assert(algo != GIT_HASH_UNKNOWN);\n\ninit_has_algo() in unit-test.c already does this.\n\nBest Wishes\n\nPhillip\n\n\n> +\trepo_set_hash_algo(the_repository, algo);\n>   }\n>   \n> -int cmd_main(int argc UNUSED, const char **argv UNUSED)\n> +static const char *arr_input[] = { \"88\", \"44\", \"aa\", \"55\" };\n> +static const char *arr_input_dup[] = { \"88\", \"44\", \"aa\", \"55\",\n> +\t\t\t\t       \"88\", \"44\", \"aa\", \"55\",\n> +\t\t\t\t       \"88\", \"44\", \"aa\", \"55\" };\n> +static const char *res_sorted[] = { \"44\", \"55\", \"88\", \"aa\" };\n> +\n> +void test_oid_array__enumerate_unique(void)\n>   {\n> -\tconst char *arr_input[] = { \"88\", \"44\", \"aa\", \"55\" };\n> -\tconst char *arr_input_dup[] = { \"88\", \"44\", \"aa\", \"55\",\n> -\t\t\t\t\t\"88\", \"44\", \"aa\", \"55\",\n> -\t\t\t\t\t\"88\", \"44\", \"aa\", \"55\" };\n> -\tconst char *res_sorted[] = { \"44\", \"55\", \"88\", \"aa\" };\n> -\tconst char *nearly_55;\n> +\tTEST_ENUMERATION(arr_input, res_sorted);\n> +}\n> +\n> +void test_oid_array__enumerate_duplicate(void)\n> +{\n> +\tTEST_ENUMERATION(arr_input_dup, res_sorted);\n> +}\n> +\n> +void test_oid_array__lookup(void)\n> +{\n> +\tTEST_LOOKUP(arr_input, \"55\", 1, 1);\n> +}\n>   \n> -\tif (!TEST(setup(), \"setup\"))\n> -\t\ttest_skip_all(\"hash algo initialization failed\");\n> +void test_oid_array__lookup_non_existent(void)\n> +{\n> +\tTEST_LOOKUP(arr_input, \"33\", INT_MIN, -1);\n> +}\n> +\n> +void test_oid_array__lookup_duplicates(void)\n> +{\n> +\tTEST_LOOKUP(arr_input_dup, \"55\", 3, 5);\n> +}\n>   \n> -\tTEST_ENUMERATION(arr_input, res_sorted, \"ordered enumeration\");\n> -\tTEST_ENUMERATION(arr_input_dup, res_sorted,\n> -\t\t\t \"ordered enumeration with duplicate suppression\");\n> +void test_oid_array__lookup_non_existent_dup(void)\n> +{\n> +\tTEST_LOOKUP(arr_input_dup, \"66\", INT_MIN, -1);\n> +}\n>   \n> -\tTEST_LOOKUP(arr_input, \"55\", 1, 1, \"lookup\");\n> -\tTEST_LOOKUP(arr_input, \"33\", INT_MIN, -1, \"lookup non-existent entry\");\n> -\tTEST_LOOKUP(arr_input_dup, \"55\", 3, 5, \"lookup with duplicates\");\n> -\tTEST_LOOKUP(arr_input_dup, \"66\", INT_MIN, -1,\n> -\t\t    \"lookup non-existent entry with duplicates\");\n> +void test_oid_array__lookup_almost_dup(void)\n> +{\n> +\tconst char *nearly_55;\n>   \n>   \tnearly_55 = init_hash_algo() == GIT_HASH_SHA1 ?\n>   \t\t\t\"5500000000000000000000000000000000000001\" :\n>   \t\t\t\"5500000000000000000000000000000000000000000000000000000000000001\";\n> -\tTEST_LOOKUP(((const char *[]){ \"55\", nearly_55 }), \"55\", 0, 0,\n> -\t\t    \"lookup with almost duplicate values\");\n> -\tTEST_LOOKUP(((const char *[]){ \"55\", \"55\" }), \"55\", 0, 1,\n> -\t\t    \"lookup with single duplicate value\");\n>   \n> -\treturn test_done();\n> +\tTEST_LOOKUP(((const char *[]){ \"55\", nearly_55 }), \"55\", 0, 0);\n> +}\n> +\n> +void test_oid_array__lookup_single_dup(void)\n> +{\n> +\tTEST_LOOKUP(((const char *[]){ \"55\", \"55\" }), \"55\", 0, 1);\n>   }\n\n"},{"id":"512804","messageId":"Z7gySOeGuicIOWZS@pks.im","threadId":"62979","inReplyTo":"c9e8a5f8-fda6-4383-bbb3-0c960a4cf86e@gmail.com","subject":"Re: [PATCH 1/5] t/unit-tests: implement oid helper functions in unit-tests.{c,h}","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-02-21T07:59:04Z","receivedAt":"2025-02-21T07:59:13Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Thu, Feb 20, 2025 at 02:38:21PM +0000, Phillip Wood wrote:\n> Hi Seyi\n> \n> On 20/02/2025 08:29, Seyi Kuforiji wrote:\n> > `get_oid_arbitrary_hex()` and `init_hash_algo()` are both required for\n> > oid-related tests to run without errors. In the current implementation,\n> > both functions are defined and declared in the\n> > `t/unit-tests/lib-oid.{c,h}` which is utilized by oid-related tests in\n> > the homegrown unit tests structure.\n> > \n> > Implement equivalent functions in unit-tests.{c,h}. Both these functions\n> > become available for oid-related test files implemented using the clar\n> > testing framework, which requires them. This will be used by subsequent\n> > commits.\n> \n> It is nice to see these tests being moved over to clar but I'm not sure that\n> moving these functions into this file is good idea. All the unit tests need\n> to link against unit-tests.o but only a subset will want access to these\n> functions. Putting them in this file means that all the tests will now\n> depend on code from strbuf.o and hex.o. I think we could add the new\n> functions to lib-oid.c and then remove the old ones when there are not\n> needed any more.\n\nThat should probably work, yeah. In that case we'd have to rename\n`init_hash_algo()`, e.g. to something like `cl_setup_hash_algo()`, to\nclearly show that it is part of the clar testing framework. I think\nhaving a common prefix for such helper functions would be good anyway so\nthat the symbols never conflict with symbols we have in libgit.\n\nPatrick\n"},{"id":"512805","messageId":"Z7gyUBVi6PTtoQul@pks.im","threadId":"62979","inReplyTo":"20250220082959.10854-2-kuforiji98@gmail.com","subject":"Re: [PATCH 1/5] t/unit-tests: implement oid helper functions in unit-tests.{c,h}","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-02-21T07:59:12Z","receivedAt":"2025-02-21T07:59:16Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Thu, Feb 20, 2025 at 09:29:55AM +0100, Seyi Kuforiji wrote:\n> diff --git a/t/unit-tests/unit-test.h b/t/unit-tests/unit-test.h\n> index 85e5d6a948..ebed51212f 100644\n> --- a/t/unit-tests/unit-test.h\n> +++ b/t/unit-tests/unit-test.h\n> @@ -8,3 +8,22 @@\n>  \tsnprintf(desc, sizeof(desc), fmt, __VA_ARGS__); \\\n>  \tclar__fail(__FILE__, __func__, __LINE__, \"Test failed.\", desc, 1); \\\n>  } while (0)\n> +\n> +/*\n> + * Convert arbitrary hex string to object_id.\n> + * For example, passing \"abc12\" will generate\n> + * \"abc1200000000000000000000000000000000000\" hex of length 40 for SHA-1 and\n> + * create object_id with that.\n> + * WARNING: passing a string of length more than the hexsz of respective hash\n> + * algo is not allowed. The hash algo is decided based on GIT_TEST_DEFAULT_HASH\n> + * environment variable.\n> + */\n> +void cl_parse_any_oid(const char *s, struct object_id *oid);\n\nNit: let's add a space between the function decarations. I'd also add a\nblank line into the comments before the \"For example\" paragraph and\nafter it to make it easier to parse.\n\nPatrick\n"},{"id":"512818","messageId":"efa58be8-e958-429a-a5bd-8df442f28bee@gmail.com","threadId":"62979","inReplyTo":"20250220082959.10854-4-kuforiji98@gmail.com","subject":"Re: [PATCH 3/5] t/unit-tests: convert oidmap test to use clar","fromName":"","fromEmail":"phillip.wood123@gmail.com","sentAt":"2025-02-21T10:04:38Z","receivedAt":"2025-02-21T10:04:41Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Seyi\n\nOn 20/02/2025 08:29, Seyi Kuforiji wrote:\n> Adapt oidmap test script to clar framework by using clar assertions\n> where necessary. `cl_parse_any_oid` handles the necessary checks needed\n> for the test to run smoothly.\n\nI'm not sure what the last half of this sentence means. What checks are \nperformed and how does that lead to the test running smoothly?\n\n> Introduce 'test_oidmap__initialize` handles the to set up of the global\n> oidmap map with predefined key-value pairs, and `test_oidmap__cleanup`\n> frees the oidmap and its entries when all tests are completed.\n> \n> This streamlines the test suite, making individual tests self-contained\n> and reducing redundant code.\n\nThis seems to be saying that by sharing global state we're making the \ntests self-contained - I'm not sure how that can be true. We need to \nmove to sharing a single oidmap between all the tests because clar's \nsetup and teardown functions don't take a context pointer. That's fine \nbut I don't see how it makes the tests self-contained.\n\nEverything up to this point looks good.\n\n>   \twhile ((entry = oidmap_iter_next(&iter))) {\n> -\t\tint ret;\n> -\t\tif (!check_int((ret = key_val_contains(entry, seen)), ==, 0)) {\n> -\t\t\tswitch (ret) {\n> -\t\t\tcase -1:\n> -\t\t\t\tbreak; /* error message handled by get_oid_arbitrary_hex() */\n> -\t\t\tcase 1:\n> -\t\t\t\ttest_msg(\"obtained entry was not given in the input\\n\"\n> -\t\t\t\t\t \"  name: %s\\n   oid: %s\\n\",\n> -\t\t\t\t\t entry->name, oid_to_hex(&entry->entry.oid));\n> -\t\t\t\tbreak;\n> -\t\t\tcase 2:\n> -\t\t\t\ttest_msg(\"duplicate entry detected\\n\"\n> -\t\t\t\t\t \"  name: %s\\n   oid: %s\\n\",\n> -\t\t\t\t\t entry->name, oid_to_hex(&entry->entry.oid));\n> -\t\t\t\tbreak;\n> -\t\t\tdefault:\n> -\t\t\t\ttest_msg(\"BUG: invalid return value (%d) from key_val_contains()\",\n> -\t\t\t\t\t ret);\n> -\t\t\t\tbreak;\n> -\t\t\t}\n> -\t\t} else {\n> -\t\t\tcount++;\n> -\t\t}\n> +\t\tcl_assert_equal_i(key_val_contains(entry, seen), 0);\n\nI think wed' be better to use clar_fail_f() so that we can keep the \nhelpful error messages. Using cl_assert_equal_i() isn't terrible as if \nthe test fails at least we know the error code but as we already have \nthe logic in place to provide better messages lets adapt it.\n\nThere is a change of behavior here as before we'd loop through the whole \nlist of entries detecting all the errors. Now we quit on the first \nerror. I don't think that matters but it would be good to point out the \nchange in the commit message.\n\nBest Wishes\n\nPhillip\n"},{"id":"512824","messageId":"2d60f5ae-7985-4400-a4d1-0931dac0653e@gmail.com","threadId":"62979","inReplyTo":"20250220082959.10854-5-kuforiji98@gmail.com","subject":"Re: [PATCH 4/5] t/unit-tests: convert oidtree test to use clar","fromName":"","fromEmail":"phillip.wood123@gmail.com","sentAt":"2025-02-21T14:48:26Z","receivedAt":"2025-02-21T14:48:29Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Seyi\n\nOn 20/02/2025 08:29, Seyi Kuforiji wrote:\n> Adapt oidtree test script to clar framework by using clar assertions\n> where necessary. `cl_parse_any_oid` handles the necessary checks needed\n> for the test to run smoothly.\n> \n> Introduce 'test_oidtree__initialize` handles the to set up of the global\n> oidtree variable and `test_oidtree__cleanup` frees the oidtree when all\n> tests are completed.\n> \n> This streamlines the test suite, making individual tests self-contained\n> and reducing redundant code.\n\nMy comments on the commit message for the last patch apply here as well.\n\n> -\tif (!check_int(get_oid_arbitrary_hex(hex_iter->expected_hexes.v[hex_iter->i],\n> -\t\t\t\t\t     &expected), ==, 0))\n> -\t\t; /* the data is bogus and cannot be used */\n> -\telse if (!check(oideq(oid, &expected)))\n> -\t\ttest_msg(\"expected: %s\\n       got: %s\\n     query: %s\",\n> -\t\t\t oid_to_hex(&expected), oid_to_hex(oid), hex_iter->query);\n> +\tcl_assert(hex_iter->i < hex_iter->expected_hexes.nr);\n>   \n> +\tcl_parse_any_oid(hex_iter->expected_hexes.v[hex_iter->i],\n> +\t\t\t &expected);\n\nusing cl_parse_any_oid() means that it is safe to dispense with the \ncheck on the return value because we now do that check in cl_parse_oid().\n\n> +\tcl_assert_equal_s(oid_to_hex(oid), oid_to_hex(&expected));\n\nThis is nice - we'll get a message containing the two oid's if they \ndon't match. We should use the same trick in patch 2.\n\nAs with the last patch we now bail out after the first error, we should \nmention that in the commit message.\n\n> -\tif (!check_int(hex_iter.i, ==, hex_iter.expected_hexes.nr))\n> -\t\ttest_msg(\"error: could not find some 'object_id's for query ('%s')\", query);\n> +\tcl_assert_equal_i(hex_iter.i, hex_iter.expected_hexes.nr);\n\nUsing cl_failf() here would enable us to keep the message so that if the \ntest fails we can see which query it was that failed.\n\nApart from that last point this is looking good\n\nBest Wishes\n\nPhillip\n\n"},{"id":"512825","messageId":"cf6c2567-2aae-4539-bc39-0cb9efd310cc@gmail.com","threadId":"62979","inReplyTo":"20250220082959.10854-2-kuforiji98@gmail.com","subject":"Re: [PATCH 1/5] t/unit-tests: implement oid helper functions in unit-tests.{c,h}","fromName":"","fromEmail":"phillip.wood123@gmail.com","sentAt":"2025-02-21T14:50:51Z","receivedAt":"2025-02-21T14:50:54Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Seyi\n\nOn 20/02/2025 08:29, Seyi Kuforiji wrote:\n\n> +static void cl_parse_oid(const char *hex, struct object_id *oid,\n> +\t\t\t\t       const struct git_hash_algo *algop)\n> +{\n> +\tint ret;\n> +\tsize_t sz = strlen(hex);\n> +\tstruct strbuf buf = STRBUF_INIT;\n> +\n> +\tcl_assert(sz <= algop->hexsz);\n> +\n> +\tstrbuf_add(&buf, hex, sz);\n> +\tstrbuf_addchars(&buf, '0', algop->hexsz - sz);\n> +\n> +\tret = get_oid_hex_algop(buf.buf, oid, algop);\n> +\tcl_assert_equal_i(ret, 0);\n\nThese last two lines would be better written as\n\n\tcl_assert_equal_i(get_oid_hex_algop(buf.buf, oid, algop), 0);\n\nSo that if it fails the message shows which function was being called\n\nBest Wishes\n\nPhillip\n\n"},{"id":"512826","messageId":"443ecd89-785f-4985-8666-e8caae33306b@gmail.com","threadId":"62979","inReplyTo":"20250220082959.10854-1-kuforiji98@gmail.com","subject":"Re: [PATCH 0/5] t/unit-tests: convert unit-tests to use clar","fromName":"","fromEmail":"phillip.wood123@gmail.com","sentAt":"2025-02-21T14:52:12Z","receivedAt":"2025-02-21T14:52:15Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Seyi\n\nOn 20/02/2025 08:29, Seyi Kuforiji wrote:\n> Hello,\n> \n> This small patch series transitions a couple more of our existing unit\n> test files to the Clar testing framework. This change is part of our\n> ongoing effort to standardize our testing framework to enhance\n> maintainability.\n\nOverall I think the conversions are correct, but we'd be better to keep \nthe diagnostic messages by using cl_failf().\n\nBest Wishes\n\nPhillip\n\n> Thanks\n> Seyi\n> \n> Mentored-by: Patrick Steinhardt <ps@pks.im>\n> Signed-off-by: Seyi Kuforiji <kuforiji98@gmail.com>\n> \n> Seyi Kuforiji (5):\n>    t/unit-tests: implement oid helper functions in unit-tests.{c,h}\n>    t/unit-tests: convert oid-array test to use clar test framework\n>    t/unit-tests: convert oidmap test to use clar test framework\n>    t/unit-tests: convert oidtree test to use clar test framework\n>    t/unit-tests: remove lib-oid.{c,h,o}\n> \n>   Makefile                                      |   7 +-\n>   t/meson.build                                 |   7 +-\n>   t/unit-tests/lib-oid.c                        |  52 ------\n>   t/unit-tests/lib-oid.h                        |  25 ---\n>   t/unit-tests/{t-oid-array.c => u-oid-array.c} | 123 +++++++-------\n>   t/unit-tests/{t-oidmap.c => u-oidmap.c}       | 153 ++++++------------\n>   t/unit-tests/{t-oidtree.c => u-oidtree.c}     |  78 ++++-----\n>   t/unit-tests/unit-test.c                      |  42 +++++\n>   t/unit-tests/unit-test.h                      |  19 +++\n>   9 files changed, 212 insertions(+), 294 deletions(-)\n>   delete mode 100644 t/unit-tests/lib-oid.c\n>   delete mode 100644 t/unit-tests/lib-oid.h\n>   rename t/unit-tests/{t-oid-array.c => u-oid-array.c} (35%)\n>   rename t/unit-tests/{t-oidmap.c => u-oidmap.c} (32%)\n>   rename t/unit-tests/{t-oidtree.c => u-oidtree.c} (44%)\n> \n\n"},{"id":"512887","messageId":"CAGedMtfgYzEBFe7_jghhJRHt5MXAesc+GfJXDDgP=BLg8LeRyg@mail.gmail.com","threadId":"62979","inReplyTo":"cf9d44d0-20ed-4d8f-84a3-1bf284075605@gmail.com","subject":"Re: [PATCH 2/5] t/unit-tests: convert oid-array test to use clar","fromName":"Seyi Chamber","fromEmail":"kuforiji98@gmail.com","sentAt":"2025-02-24T09:11:32Z","receivedAt":"2025-02-24T09:11:44Z","isPatch":true,"sender":{"key":"kuforiji98@gmail.com","avatar":"https://avatars.githubusercontent.com/u/94905626?v=4"},"body":"On Thu, 20 Feb 2025 at 15:38, Phillip Wood <phillip.wood123@gmail.com> wrote:\n>\n> Hi Seyi\n>\n> On 20/02/2025 08:29, Seyi Kuforiji wrote:\n> > Adapt oid-array test script to clar framework by using clar assertions\n> > where necessary. Remove descriptions from macros to reduce\n> > redundancy, and move test input arrays to global scope for reuse across\n> > multiple test functions. Introduce `test_oid_array__initialize()` to\n> > explicitly initialize the hash algorithm.\n> >\n> > These changes streamline the test suite, making individual tests\n> > self-contained and reducing redundant code.\n>\n> I think these conversion look correct but once again we're losing\n> valuable debugging information because we haven't added better\n> assertions to clear.\n>\n\nI understand your concern about losing debugging information, but it\nis more beneficial to prioritize clarity and simplicity in unit tests.\nUnit tests should be short and easy, and adding extra debugging\nmessages increases complexity, making them harder to maintain and\nread. The assertion failures already indicate where an issue occurs,\nallowing whoever is debugging to inspect the test inputs directly if\nneeded.\n\nAssertion failures are rarely hit in real-world scenarios, and when\nthey do occur, one can manually print values or use a debugger to\ninvestigate. This makes the additional debugging messages unnecessary\nin most cases. The lack of explicit debugging output is not a\nsignificant issue in practice. Furthermore, there are plans to\ncollaborate with Clar upstream to equip common assertions with the\nability to print custom messages in a formatted string where an error\noccurs. This would allow our test to be simple and easy to read and\nalso maintain some of our custom debug messages.\n\n\n\n> >       oid_array_for_each_unique(&input, add_to_oid_array, &actual);\n> > -     if (!check_uint(actual.nr, ==, expect.nr))\n> > -             return;\n> > -\n> > -     for (i = 0; i < actual.nr; i++) {\n> > -             if (!check(oideq(&actual.oid[i], &expect.oid[i])))\n> > -                     test_msg(\"expected: %s\\n       got: %s\\n     index: %\" PRIuMAX,\n> > -                              oid_to_hex(&expect.oid[i]), oid_to_hex(&actual.oid[i]),\n> > -                              (uintmax_t)i);\n> > -     }\n> > +     cl_assert_equal_i(actual.nr, expect.nr);\n> > +\n> > +     for (i = 0; i < actual.nr; i++)\n> > +             cl_assert(oideq(&actual.oid[i], &expect.oid[i]));\n>\n> If this fails the poor person debugging it will have no idea why as\n> there is now no indication of which two oids were being compared.\n>\n> > -     if (!check_int(ret, <=, upper_bound) ||\n> > -         !check_int(ret, >=, lower_bound))\n> > -             test_msg(\"oid query for lookup: %s\", oid_to_hex(&oid_query));\n> > +     cl_assert(ret <= upper_bound);\n> > +     cl_assert(ret >= lower_bound);\n>\n> This is another case where we could do with better assertions in clar\n>\n> > -static void setup(void)\n> > +void test_oid_array__initialize(void)\n> >   {\n> >       /* The hash algo is used by oid_array_lookup() internally */\n> >       int algo = init_hash_algo();\n> > -     if (check_int(algo, !=, GIT_HASH_UNKNOWN))\n> > -             repo_set_hash_algo(the_repository, algo);\n> > +     cl_assert(algo != GIT_HASH_UNKNOWN);\n>\n> init_has_algo() in unit-test.c already does this.\n>\n\nThanks for spotting this! Will fix this in an updated patch.\n> Best Wishes\n>\n> Phillip\n>\n>\n> > +     repo_set_hash_algo(the_repository, algo);\n> >   }\n> >\n> > -int cmd_main(int argc UNUSED, const char **argv UNUSED)\n> > +static const char *arr_input[] = { \"88\", \"44\", \"aa\", \"55\" };\n> > +static const char *arr_input_dup[] = { \"88\", \"44\", \"aa\", \"55\",\n> > +                                    \"88\", \"44\", \"aa\", \"55\",\n> > +                                    \"88\", \"44\", \"aa\", \"55\" };\n> > +static const char *res_sorted[] = { \"44\", \"55\", \"88\", \"aa\" };\n> > +\n> > +void test_oid_array__enumerate_unique(void)\n> >   {\n> > -     const char *arr_input[] = { \"88\", \"44\", \"aa\", \"55\" };\n> > -     const char *arr_input_dup[] = { \"88\", \"44\", \"aa\", \"55\",\n> > -                                     \"88\", \"44\", \"aa\", \"55\",\n> > -                                     \"88\", \"44\", \"aa\", \"55\" };\n> > -     const char *res_sorted[] = { \"44\", \"55\", \"88\", \"aa\" };\n> > -     const char *nearly_55;\n> > +     TEST_ENUMERATION(arr_input, res_sorted);\n> > +}\n> > +\n> > +void test_oid_array__enumerate_duplicate(void)\n> > +{\n> > +     TEST_ENUMERATION(arr_input_dup, res_sorted);\n> > +}\n> > +\n> > +void test_oid_array__lookup(void)\n> > +{\n> > +     TEST_LOOKUP(arr_input, \"55\", 1, 1);\n> > +}\n> >\n> > -     if (!TEST(setup(), \"setup\"))\n> > -             test_skip_all(\"hash algo initialization failed\");\n> > +void test_oid_array__lookup_non_existent(void)\n> > +{\n> > +     TEST_LOOKUP(arr_input, \"33\", INT_MIN, -1);\n> > +}\n> > +\n> > +void test_oid_array__lookup_duplicates(void)\n> > +{\n> > +     TEST_LOOKUP(arr_input_dup, \"55\", 3, 5);\n> > +}\n> >\n> > -     TEST_ENUMERATION(arr_input, res_sorted, \"ordered enumeration\");\n> > -     TEST_ENUMERATION(arr_input_dup, res_sorted,\n> > -                      \"ordered enumeration with duplicate suppression\");\n> > +void test_oid_array__lookup_non_existent_dup(void)\n> > +{\n> > +     TEST_LOOKUP(arr_input_dup, \"66\", INT_MIN, -1);\n> > +}\n> >\n> > -     TEST_LOOKUP(arr_input, \"55\", 1, 1, \"lookup\");\n> > -     TEST_LOOKUP(arr_input, \"33\", INT_MIN, -1, \"lookup non-existent entry\");\n> > -     TEST_LOOKUP(arr_input_dup, \"55\", 3, 5, \"lookup with duplicates\");\n> > -     TEST_LOOKUP(arr_input_dup, \"66\", INT_MIN, -1,\n> > -                 \"lookup non-existent entry with duplicates\");\n> > +void test_oid_array__lookup_almost_dup(void)\n> > +{\n> > +     const char *nearly_55;\n> >\n> >       nearly_55 = init_hash_algo() == GIT_HASH_SHA1 ?\n> >                       \"5500000000000000000000000000000000000001\" :\n> >                       \"5500000000000000000000000000000000000000000000000000000000000001\";\n> > -     TEST_LOOKUP(((const char *[]){ \"55\", nearly_55 }), \"55\", 0, 0,\n> > -                 \"lookup with almost duplicate values\");\n> > -     TEST_LOOKUP(((const char *[]){ \"55\", \"55\" }), \"55\", 0, 1,\n> > -                 \"lookup with single duplicate value\");\n> >\n> > -     return test_done();\n> > +     TEST_LOOKUP(((const char *[]){ \"55\", nearly_55 }), \"55\", 0, 0);\n> > +}\n> > +\n> > +void test_oid_array__lookup_single_dup(void)\n> > +{\n> > +     TEST_LOOKUP(((const char *[]){ \"55\", \"55\" }), \"55\", 0, 1);\n> >   }\n>\n\nThanks\nSeyi\n"},{"id":"512888","messageId":"1c34ccb8-fb6c-4101-a00f-29fc2fb89934@gmail.com","threadId":"62979","inReplyTo":"CAGedMtfgYzEBFe7_jghhJRHt5MXAesc+GfJXDDgP=BLg8LeRyg@mail.gmail.com","subject":"Re: [PATCH 2/5] t/unit-tests: convert oid-array test to use clar","fromName":"","fromEmail":"phillip.wood123@gmail.com","sentAt":"2025-02-24T10:12:24Z","receivedAt":"2025-02-24T10:12:31Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Seyi\n\nOn 24/02/2025 09:11, Seyi Chamber wrote:\n> On Thu, 20 Feb 2025 at 15:38, Phillip Wood <phillip.wood123@gmail.com> wrote:\n>>\n>> Hi Seyi\n>>\n>> On 20/02/2025 08:29, Seyi Kuforiji wrote:\n>>> Adapt oid-array test script to clar framework by using clar assertions\n>>> where necessary. Remove descriptions from macros to reduce\n>>> redundancy, and move test input arrays to global scope for reuse across\n>>> multiple test functions. Introduce `test_oid_array__initialize()` to\n>>> explicitly initialize the hash algorithm.\n>>>\n>>> These changes streamline the test suite, making individual tests\n>>> self-contained and reducing redundant code.\n>>\n>> I think these conversion look correct but once again we're losing\n>> valuable debugging information because we haven't added better\n>> assertions to clear.\n>>\n> \n> I understand your concern about losing debugging information, but it\n> is more beneficial to prioritize clarity and simplicity in unit tests.\n> Unit tests should be short and easy, and adding extra debugging\n> messages increases complexity, making them harder to maintain and\n> read. The assertion failures already indicate where an issue occurs,\n> allowing whoever is debugging to inspect the test inputs directly if\n> needed.\n\nIf the test failure is on a CI run for a platform that the person \ndebugging the failure does not have access to how are they going to do \nthat? This is not a hypothetical concern as our CI runs the test suite \non MacOs, Linux and Windows. Individual developers often only have \naccess to one or to of those platforms. My experience of debugging CI \ntest failures is that without decent diagnostic messages it is extremely \ndifficult to figure out what went wrong if one does not have access to \nthe platform where the test is failing.\n\n> Assertion failures are rarely hit in real-world scenarios, and when\n> they do occur, one can manually print values or use a debugger to\n> investigate. This makes the additional debugging messages unnecessary\n> in most cases.\n\nThis seems to be arguing that because we expect the tests to pass we \ndon't need to worry about how difficult it is to debug them when they \nfail. I do not agree with that line of argument.\n\n> The lack of explicit debugging output is not a\n> significant issue in practice. Furthermore, there are plans to\n> collaborate with Clar upstream to equip common assertions with the\n> ability to print custom messages in a formatted string where an error\n> occurs. This would allow our test to be simple and easy to read and\n> also maintain some of our custom debug messages.\n\nHave you got any more details about this please? We already have \ncl_failf() in our codebase.\n\nBest Wishes\n\nPhillip\n\n> \n> \n>>>        oid_array_for_each_unique(&input, add_to_oid_array, &actual);\n>>> -     if (!check_uint(actual.nr, ==, expect.nr))\n>>> -             return;\n>>> -\n>>> -     for (i = 0; i < actual.nr; i++) {\n>>> -             if (!check(oideq(&actual.oid[i], &expect.oid[i])))\n>>> -                     test_msg(\"expected: %s\\n       got: %s\\n     index: %\" PRIuMAX,\n>>> -                              oid_to_hex(&expect.oid[i]), oid_to_hex(&actual.oid[i]),\n>>> -                              (uintmax_t)i);\n>>> -     }\n>>> +     cl_assert_equal_i(actual.nr, expect.nr);\n>>> +\n>>> +     for (i = 0; i < actual.nr; i++)\n>>> +             cl_assert(oideq(&actual.oid[i], &expect.oid[i]));\n>>\n>> If this fails the poor person debugging it will have no idea why as\n>> there is now no indication of which two oids were being compared.\n>>\n>>> -     if (!check_int(ret, <=, upper_bound) ||\n>>> -         !check_int(ret, >=, lower_bound))\n>>> -             test_msg(\"oid query for lookup: %s\", oid_to_hex(&oid_query));\n>>> +     cl_assert(ret <= upper_bound);\n>>> +     cl_assert(ret >= lower_bound);\n>>\n>> This is another case where we could do with better assertions in clar\n>>\n>>> -static void setup(void)\n>>> +void test_oid_array__initialize(void)\n>>>    {\n>>>        /* The hash algo is used by oid_array_lookup() internally */\n>>>        int algo = init_hash_algo();\n>>> -     if (check_int(algo, !=, GIT_HASH_UNKNOWN))\n>>> -             repo_set_hash_algo(the_repository, algo);\n>>> +     cl_assert(algo != GIT_HASH_UNKNOWN);\n>>\n>> init_has_algo() in unit-test.c already does this.\n>>\n> \n> Thanks for spotting this! Will fix this in an updated patch.\n>> Best Wishes\n>>\n>> Phillip\n>>\n>>\n>>> +     repo_set_hash_algo(the_repository, algo);\n>>>    }\n>>>\n>>> -int cmd_main(int argc UNUSED, const char **argv UNUSED)\n>>> +static const char *arr_input[] = { \"88\", \"44\", \"aa\", \"55\" };\n>>> +static const char *arr_input_dup[] = { \"88\", \"44\", \"aa\", \"55\",\n>>> +                                    \"88\", \"44\", \"aa\", \"55\",\n>>> +                                    \"88\", \"44\", \"aa\", \"55\" };\n>>> +static const char *res_sorted[] = { \"44\", \"55\", \"88\", \"aa\" };\n>>> +\n>>> +void test_oid_array__enumerate_unique(void)\n>>>    {\n>>> -     const char *arr_input[] = { \"88\", \"44\", \"aa\", \"55\" };\n>>> -     const char *arr_input_dup[] = { \"88\", \"44\", \"aa\", \"55\",\n>>> -                                     \"88\", \"44\", \"aa\", \"55\",\n>>> -                                     \"88\", \"44\", \"aa\", \"55\" };\n>>> -     const char *res_sorted[] = { \"44\", \"55\", \"88\", \"aa\" };\n>>> -     const char *nearly_55;\n>>> +     TEST_ENUMERATION(arr_input, res_sorted);\n>>> +}\n>>> +\n>>> +void test_oid_array__enumerate_duplicate(void)\n>>> +{\n>>> +     TEST_ENUMERATION(arr_input_dup, res_sorted);\n>>> +}\n>>> +\n>>> +void test_oid_array__lookup(void)\n>>> +{\n>>> +     TEST_LOOKUP(arr_input, \"55\", 1, 1);\n>>> +}\n>>>\n>>> -     if (!TEST(setup(), \"setup\"))\n>>> -             test_skip_all(\"hash algo initialization failed\");\n>>> +void test_oid_array__lookup_non_existent(void)\n>>> +{\n>>> +     TEST_LOOKUP(arr_input, \"33\", INT_MIN, -1);\n>>> +}\n>>> +\n>>> +void test_oid_array__lookup_duplicates(void)\n>>> +{\n>>> +     TEST_LOOKUP(arr_input_dup, \"55\", 3, 5);\n>>> +}\n>>>\n>>> -     TEST_ENUMERATION(arr_input, res_sorted, \"ordered enumeration\");\n>>> -     TEST_ENUMERATION(arr_input_dup, res_sorted,\n>>> -                      \"ordered enumeration with duplicate suppression\");\n>>> +void test_oid_array__lookup_non_existent_dup(void)\n>>> +{\n>>> +     TEST_LOOKUP(arr_input_dup, \"66\", INT_MIN, -1);\n>>> +}\n>>>\n>>> -     TEST_LOOKUP(arr_input, \"55\", 1, 1, \"lookup\");\n>>> -     TEST_LOOKUP(arr_input, \"33\", INT_MIN, -1, \"lookup non-existent entry\");\n>>> -     TEST_LOOKUP(arr_input_dup, \"55\", 3, 5, \"lookup with duplicates\");\n>>> -     TEST_LOOKUP(arr_input_dup, \"66\", INT_MIN, -1,\n>>> -                 \"lookup non-existent entry with duplicates\");\n>>> +void test_oid_array__lookup_almost_dup(void)\n>>> +{\n>>> +     const char *nearly_55;\n>>>\n>>>        nearly_55 = init_hash_algo() == GIT_HASH_SHA1 ?\n>>>                        \"5500000000000000000000000000000000000001\" :\n>>>                        \"5500000000000000000000000000000000000000000000000000000000000001\";\n>>> -     TEST_LOOKUP(((const char *[]){ \"55\", nearly_55 }), \"55\", 0, 0,\n>>> -                 \"lookup with almost duplicate values\");\n>>> -     TEST_LOOKUP(((const char *[]){ \"55\", \"55\" }), \"55\", 0, 1,\n>>> -                 \"lookup with single duplicate value\");\n>>>\n>>> -     return test_done();\n>>> +     TEST_LOOKUP(((const char *[]){ \"55\", nearly_55 }), \"55\", 0, 0);\n>>> +}\n>>> +\n>>> +void test_oid_array__lookup_single_dup(void)\n>>> +{\n>>> +     TEST_LOOKUP(((const char *[]){ \"55\", \"55\" }), \"55\", 0, 1);\n>>>    }\n>>\n> \n> Thanks\n> Seyi\n\n"},{"id":"512889","messageId":"CAGedMtfEMkA80NkozBucNKeN-wNqeqk2PqJVcvNg9obyZ7Ei2w@mail.gmail.com","threadId":"62979","inReplyTo":"efa58be8-e958-429a-a5bd-8df442f28bee@gmail.com","subject":"Re: [PATCH 3/5] t/unit-tests: convert oidmap test to use clar","fromName":"Seyi Chamber","fromEmail":"kuforiji98@gmail.com","sentAt":"2025-02-24T10:56:33Z","receivedAt":"2025-02-24T10:56:45Z","isPatch":true,"sender":{"key":"kuforiji98@gmail.com","avatar":"https://avatars.githubusercontent.com/u/94905626?v=4"},"body":"On Fri, 21 Feb 2025 at 11:04, <phillip.wood123@gmail.com> wrote:\n>\n> Hi Seyi\n>\n> On 20/02/2025 08:29, Seyi Kuforiji wrote:\n> > Adapt oidmap test script to clar framework by using clar assertions\n> > where necessary. `cl_parse_any_oid` handles the necessary checks needed\n> > for the test to run smoothly.\n>\n> I'm not sure what the last half of this sentence means. What checks are\n> performed and how does that lead to the test running smoothly?\n>\n\n`cl_parse_any_oid()` ensures the hash algorithm is set before parsing.\nThis prevents issues from an uninitialized or invalid hash algorithm.\nWithout these checks, the test could behave unpredictably. I’ll update\nthe commit message to make this clearer.\n\n> > Introduce 'test_oidmap__initialize` handles the to set up of the global\n> > oidmap map with predefined key-value pairs, and `test_oidmap__cleanup`\n> > frees the oidmap and its entries when all tests are completed.\n> >\n> > This streamlines the test suite, making individual tests self-contained\n> > and reducing redundant code.\n>\n> This seems to be saying that by sharing global state we're making the\n> tests self-contained - I'm not sure how that can be true. We need to\n> move to sharing a single oidmap between all the tests because clar's\n> setup and teardown functions don't take a context pointer. That's fine\n> but I don't see how it makes the tests self-contained.\n>\n\nRight! Thank you. I'll adjust the commit message accordingly.\n\n> Everything up to this point looks good.\n>\n> >       while ((entry = oidmap_iter_next(&iter))) {\n> > -             int ret;\n> > -             if (!check_int((ret = key_val_contains(entry, seen)), ==, 0)) {\n> > -                     switch (ret) {\n> > -                     case -1:\n> > -                             break; /* error message handled by get_oid_arbitrary_hex() */\n> > -                     case 1:\n> > -                             test_msg(\"obtained entry was not given in the input\\n\"\n> > -                                      \"  name: %s\\n   oid: %s\\n\",\n> > -                                      entry->name, oid_to_hex(&entry->entry.oid));\n> > -                             break;\n> > -                     case 2:\n> > -                             test_msg(\"duplicate entry detected\\n\"\n> > -                                      \"  name: %s\\n   oid: %s\\n\",\n> > -                                      entry->name, oid_to_hex(&entry->entry.oid));\n> > -                             break;\n> > -                     default:\n> > -                             test_msg(\"BUG: invalid return value (%d) from key_val_contains()\",\n> > -                                      ret);\n> > -                             break;\n> > -                     }\n> > -             } else {\n> > -                     count++;\n> > -             }\n> > +             cl_assert_equal_i(key_val_contains(entry, seen), 0);\n>\n> I think wed' be better to use clar_fail_f() so that we can keep the\n> helpful error messages. Using cl_assert_equal_i() isn't terrible as if\n> the test fails at least we know the error code but as we already have\n> the logic in place to provide better messages lets adapt it.\n>\n> There is a change of behavior here as before we'd loop through the whole\n> list of entries detecting all the errors. Now we quit on the first\n> error. I don't think that matters but it would be good to point out the\n> change in the commit message.\n>\n> Best Wishes\n>\n> Phillip\nNoted! thank you.\nSeyi\n"},{"id":"512909","messageId":"20250224152704.70289-1-kuforiji98@gmail.com","threadId":"62979","inReplyTo":"20250220082959.10854-1-kuforiji98@gmail.com","subject":"[PATCH v2 0/4] t/unit-tests: convert unit-tests to use clar","fromName":"Seyi Kuforiji","fromEmail":"kuforiji98@gmail.com","sentAt":"2025-02-24T15:27:00Z","receivedAt":"2025-02-24T15:27:42Z","isPatch":true,"sender":{"key":"kuforiji98@gmail.com","avatar":"https://avatars.githubusercontent.com/u/94905626?v=4"},"body":"Hello,\n\nThis small patch series transitions a couple more of our existing unit\ntest files to the Clar testing framework. This change is part of our\nongoing effort to standardize our testing framework to enhance\nmaintainability.\n\nChanges in v2:\n - fixes to the commit messages and how they read\n - some code refactoring based on review\n\nThanks\nSeyi\n\nMentored-by: Patrick Steinhardt <ps@pks.im>\nMentored-by: Philip Wood <phillip.wood@dunelm.org.uk>\nSigned-off-by: Seyi Kuforiji <kuforiji98@gmail.com>\n\nSeyi Kuforiji (4):\n  t/unit-tests: implement clar specific oid helper functions\n  t/unit-tests: convert oid-array test to use clar test framework\n  t/unit-tests: convert oidmap test to use clar test framework\n  t/unit-tests: convert oidtree test to use clar test framework\n\n Makefile                                      |   8 +-\n t/meson.build                                 |   8 +-\n t/unit-tests/lib-oid.c                        |  31 ++--\n t/unit-tests/lib-oid.h                        |   9 +-\n t/unit-tests/{t-oid-array.c => u-oid-array.c} | 125 +++++++-------\n t/unit-tests/{t-oidmap.c => u-oidmap.c}       | 153 +++++++-----------\n t/unit-tests/{t-oidtree.c => u-oidtree.c}     |  79 ++++-----\n t/unit-tests/unit-test.c                      |   2 +\n 8 files changed, 177 insertions(+), 238 deletions(-)\n rename t/unit-tests/{t-oid-array.c => u-oid-array.c} (34%)\n rename t/unit-tests/{t-oidmap.c => u-oidmap.c} (32%)\n rename t/unit-tests/{t-oidtree.c => u-oidtree.c} (45%)\n\nRange-diff against v1:\n1:  19192c6c89 < -:  ---------- t/unit-tests: implement oid helper functions in unit-tests.{c,h}\n5:  e81ec73f27 ! 1:  7f14d0d574 t/unit-tests: remove lib-oid.{c,h,o}\n    @@ Metadata\n     Author: Seyi Kuforiji <kuforiji98@gmail.com>\n     \n      ## Commit message ##\n    -    t/unit-tests: remove lib-oid.{c,h,o}\n    +    t/unit-tests: implement clar specific oid helper functions\n     \n    -    The `lib-oid.c`, `lib-oid.h`, and `lib-oid.o files` are no longer needed\n    -    since their equivalent functions have been implemented in unit-test.c\n    -    and unit-test.h. This removes redundant code and ensures all unit\n    -    test-related functionality is consolidated in a single location.\n    +    `get_oid_arbitrary_hex()` and `init_hash_algo()` are both required for\n    +    oid-related tests to run without errors. In the current implementation,\n    +    both functions are defined and declared in the\n    +    `t/unit-tests/lib-oid.{c,h}` which is utilized by oid-related tests in\n    +    the homegrown unit tests structure.\n     \n    -    Drop references to lib-oid from our `Makefile`, and `meson.build` files\n    -    to prevent build errors due to missing files.\n    +    Adapt functions in lib-oid.{c,h} to use clar. Both these functions\n    +    become available for oid-related test files implemented using the clar\n    +    testing framework, which requires them. This will be used by subsequent\n    +    commits.\n     \n         Mentored-by: Patrick Steinhardt <ps@pks.im>\n         Signed-off-by: Seyi Kuforiji <kuforiji98@gmail.com>\n     \n      ## Makefile ##\n    +@@ Makefile: CLAR_TEST_PROG = $(UNIT_TEST_BIN)/unit-tests$(X)\n    + CLAR_TEST_OBJS = $(patsubst %,$(UNIT_TEST_DIR)/%.o,$(CLAR_TEST_SUITES))\n    + CLAR_TEST_OBJS += $(UNIT_TEST_DIR)/clar/clar.o\n    + CLAR_TEST_OBJS += $(UNIT_TEST_DIR)/unit-test.o\n    ++CLAR_TEST_OBJS += $(UNIT_TEST_DIR)/lib-oid.o\n    + \n    + UNIT_TEST_PROGRAMS += t-oid-array\n    + UNIT_TEST_PROGRAMS += t-oidmap\n     @@ Makefile: UNIT_TEST_PROGRAMS += t-trailer\n      UNIT_TEST_PROGRAMS += t-urlmatch-normalization\n      UNIT_TEST_PROGS = $(patsubst %,$(UNIT_TEST_BIN)/%$X,$(UNIT_TEST_PROGRAMS))\n    @@ Makefile: UNIT_TEST_PROGRAMS += t-trailer\n      # xdiff and reftable libs may in turn depend on what is in libgit.a\n     \n      ## t/meson.build ##\n    +@@ t/meson.build: clar_test_suites = [\n    + clar_sources = [\n    +   'unit-tests/clar/clar.c',\n    +   'unit-tests/unit-test.c',\n    ++  'unit-tests/lib-oid.c'\n    + ]\n    + \n    + clar_decls_h = custom_target(\n     @@ t/meson.build: foreach unit_test_program : unit_test_programs\n        unit_test = executable(unit_test_name,\n          sources: [\n    @@ t/meson.build: foreach unit_test_program : unit_test_programs\n            unit_test_program,\n          ],\n     \n    - ## t/unit-tests/lib-oid.c (deleted) ##\n    + ## t/unit-tests/lib-oid.c ##\n     @@\n     -#include \"test-lib.h\"\n    --#include \"lib-oid.h\"\n    --#include \"strbuf.h\"\n    --#include \"hex.h\"\n    --\n    ++#include \"unit-test.h\"\n    + #include \"lib-oid.h\"\n    + #include \"strbuf.h\"\n    + #include \"hex.h\"\n    + \n     -int init_hash_algo(void)\n    --{\n    --\tstatic int algo = -1;\n    --\n    --\tif (algo < 0) {\n    --\t\tconst char *algo_name = getenv(\"GIT_TEST_DEFAULT_HASH\");\n    --\t\talgo = algo_name ? hash_algo_by_name(algo_name) : GIT_HASH_SHA1;\n    --\n    ++int cl_setup_hash_algo(void)\n    + {\n    + \tstatic int algo = -1;\n    + \n    +@@ t/unit-tests/lib-oid.c: int init_hash_algo(void)\n    + \t\tconst char *algo_name = getenv(\"GIT_TEST_DEFAULT_HASH\");\n    + \t\talgo = algo_name ? hash_algo_by_name(algo_name) : GIT_HASH_SHA1;\n    + \n     -\t\tif (!check(algo != GIT_HASH_UNKNOWN))\n     -\t\t\ttest_msg(\"BUG: invalid GIT_TEST_DEFAULT_HASH value ('%s')\",\n     -\t\t\t\t algo_name);\n    --\t}\n    --\treturn algo;\n    --}\n    --\n    ++\t\tcl_assert(algo != GIT_HASH_UNKNOWN);\n    + \t}\n    + \treturn algo;\n    + }\n    + \n     -static int get_oid_arbitrary_hex_algop(const char *hex, struct object_id *oid,\n    --\t\t\t\t       const struct git_hash_algo *algop)\n    --{\n    --\tint ret;\n    --\tsize_t sz = strlen(hex);\n    --\tstruct strbuf buf = STRBUF_INIT;\n    --\n    ++static void cl_parse_oid(const char *hex, struct object_id *oid,\n    + \t\t\t\t       const struct git_hash_algo *algop)\n    + {\n    + \tint ret;\n    + \tsize_t sz = strlen(hex);\n    + \tstruct strbuf buf = STRBUF_INIT;\n    + \n     -\tif (!check(sz <= algop->hexsz)) {\n     -\t\ttest_msg(\"BUG: hex string (%s) bigger than maximum allowed (%lu)\",\n     -\t\t\t hex, (unsigned long)algop->hexsz);\n     -\t\treturn -1;\n     -\t}\n    --\n    --\tstrbuf_add(&buf, hex, sz);\n    --\tstrbuf_addchars(&buf, '0', algop->hexsz - sz);\n    --\n    ++\tcl_assert(sz <= algop->hexsz);\n    + \n    + \tstrbuf_add(&buf, hex, sz);\n    + \tstrbuf_addchars(&buf, '0', algop->hexsz - sz);\n    + \n     -\tret = get_oid_hex_algop(buf.buf, oid, algop);\n     -\tif (!check_int(ret, ==, 0))\n     -\t\ttest_msg(\"BUG: invalid hex input (%s) provided\", hex);\n    --\n    --\tstrbuf_release(&buf);\n    ++\tcl_assert_equal_i(get_oid_hex_algop(buf.buf, oid, algop), 0);\n    + \n    + \tstrbuf_release(&buf);\n     -\treturn ret;\n    --}\n    --\n    + }\n    + \n     -int get_oid_arbitrary_hex(const char *hex, struct object_id *oid)\n    --{\n    ++\n    ++void cl_parse_any_oid(const char *hex, struct object_id *oid)\n    + {\n     -\tint hash_algo = init_hash_algo();\n    --\n    ++\tint hash_algo = cl_setup_hash_algo();\n    + \n     -\tif (!check_int(hash_algo, !=, GIT_HASH_UNKNOWN))\n     -\t\treturn -1;\n     -\treturn get_oid_arbitrary_hex_algop(hex, oid, &hash_algos[hash_algo]);\n    --}\n    ++\tcl_assert(hash_algo != GIT_HASH_UNKNOWN);\n    ++\tcl_parse_oid(hex, oid, &hash_algos[hash_algo]);\n    + }\n     \n    - ## t/unit-tests/lib-oid.h (deleted) ##\n    + ## t/unit-tests/lib-oid.h ##\n    +@@\n    + \n    + /*\n    +  * Convert arbitrary hex string to object_id.\n    ++ *\n    +  * For example, passing \"abc12\" will generate\n    +  * \"abc1200000000000000000000000000000000000\" hex of length 40 for SHA-1 and\n    +  * create object_id with that.\n     @@\n    --#ifndef LIB_OID_H\n    --#define LIB_OID_H\n    --\n    --#include \"hash.h\"\n    --\n    --/*\n    -- * Convert arbitrary hex string to object_id.\n    -- * For example, passing \"abc12\" will generate\n    -- * \"abc1200000000000000000000000000000000000\" hex of length 40 for SHA-1 and\n    -- * create object_id with that.\n    -- * WARNING: passing a string of length more than the hexsz of respective hash\n    -- * algo is not allowed. The hash algo is decided based on GIT_TEST_DEFAULT_HASH\n    -- * environment variable.\n    -- */\n    +  * algo is not allowed. The hash algo is decided based on GIT_TEST_DEFAULT_HASH\n    +  * environment variable.\n    +  */\n     -int get_oid_arbitrary_hex(const char *s, struct object_id *oid);\n    --/*\n    -- * Returns one of GIT_HASH_{SHA1, SHA256, UNKNOWN} based on the value of\n    -- * GIT_TEST_DEFAULT_HASH environment variable. The fallback value in the\n    -- * absence of GIT_TEST_DEFAULT_HASH is GIT_HASH_SHA1. It also uses\n    ++\n    ++void cl_parse_any_oid (const char *s, struct object_id *oid);\n    + /*\n    +  * Returns one of GIT_HASH_{SHA1, SHA256, UNKNOWN} based on the value of\n    +  * GIT_TEST_DEFAULT_HASH environment variable. The fallback value in the\n    +  * absence of GIT_TEST_DEFAULT_HASH is GIT_HASH_SHA1. It also uses\n     - * check(algo != GIT_HASH_UNKNOWN) before returning to verify if the\n    -- * GIT_TEST_DEFAULT_HASH's value is valid or not.\n    -- */\n    ++ * cl_assert(algo != GIT_HASH_UNKNOWN) before returning to verify if the\n    +  * GIT_TEST_DEFAULT_HASH's value is valid or not.\n    +  */\n     -int init_hash_algo(void);\n    --\n    --#endif /* LIB_OID_H */\n    ++\n    ++int cl_setup_hash_algo(void);\n    + \n    + #endif /* LIB_OID_H */\n    +\n    + ## t/unit-tests/unit-test.c ##\n    +@@\n    + #include \"unit-test.h\"\n    ++#include \"hex.h\"\n    + #include \"parse-options.h\"\n    ++#include \"strbuf.h\"\n    + #include \"string-list.h\"\n    + #include \"strvec.h\"\n    + \n2:  8a99bbdc31 ! 2:  430f5c5007 t/unit-tests: convert oid-array test to use clar test framework\n    @@ Makefile: CLAR_TEST_SUITES += u-example-decorate\n      CLAR_TEST_SUITES += u-prio-queue\n      CLAR_TEST_SUITES += u-reftable-tree\n      CLAR_TEST_SUITES += u-strbuf\n    -@@ Makefile: CLAR_TEST_OBJS = $(patsubst %,$(UNIT_TEST_DIR)/%.o,$(CLAR_TEST_SUITES))\n    - CLAR_TEST_OBJS += $(UNIT_TEST_DIR)/clar/clar.o\n    +@@ Makefile: CLAR_TEST_OBJS += $(UNIT_TEST_DIR)/clar/clar.o\n      CLAR_TEST_OBJS += $(UNIT_TEST_DIR)/unit-test.o\n    + CLAR_TEST_OBJS += $(UNIT_TEST_DIR)/lib-oid.o\n      \n     -UNIT_TEST_PROGRAMS += t-oid-array\n      UNIT_TEST_PROGRAMS += t-oidmap\n    @@ t/unit-tests/u-oid-array.c (new)\n     +#define USE_THE_REPOSITORY_VARIABLE\n     +\n     +#include \"unit-test.h\"\n    ++#include \"lib-oid.h\"\n     +#include \"oid-array.h\"\n     +#include \"hex.h\"\n     +\n    @@ t/unit-tests/u-oid-array.c (new)\n     +void test_oid_array__initialize(void)\n     +{\n     +\t/* The hash algo is used by oid_array_lookup() internally */\n    -+\tint algo = init_hash_algo();\n    -+\tcl_assert(algo != GIT_HASH_UNKNOWN);\n    ++\tint algo = cl_setup_hash_algo();\n     +\trepo_set_hash_algo(the_repository, algo);\n     +}\n     +\n    @@ t/unit-tests/u-oid-array.c (new)\n     +{\n     +\tconst char *nearly_55;\n     +\n    -+\tnearly_55 = init_hash_algo() == GIT_HASH_SHA1 ?\n    ++\tnearly_55 = cl_setup_hash_algo() == GIT_HASH_SHA1 ?\n     +\t\t\t\"5500000000000000000000000000000000000001\" :\n     +\t\t\t\"5500000000000000000000000000000000000000000000000000000000000001\";\n     +\n3:  c19545e2bc ! 3:  319cea1265 t/unit-tests: convert oidmap test to use clar test framework\n    @@ Commit message\n         t/unit-tests: convert oidmap test to use clar test framework\n     \n         Adapt oidmap test script to clar framework by using clar assertions\n    -    where necessary. `cl_parse_any_oid` handles the necessary checks needed\n    -    for the test to run smoothly.\n    +    where necessary. `cl_parse_any_oid()` ensures the hash algorithm is set\n    +    before parsing. This prevents issues from an uninitialized or invalid\n    +    hash algorithm.\n     \n         Introduce 'test_oidmap__initialize` handles the to set up of the global\n         oidmap map with predefined key-value pairs, and `test_oidmap__cleanup`\n         frees the oidmap and its entries when all tests are completed.\n     \n    -    This streamlines the test suite, making individual tests self-contained\n    -    and reducing redundant code.\n    +    The test loops through all entries to detect multiple errors. With this\n    +    change, it stops at the first error encountered, making it easier to\n    +    address it.\n     \n         Mentored-by: Patrick Steinhardt <ps@pks.im>\n         Signed-off-by: Seyi Kuforiji <kuforiji98@gmail.com>\n    @@ Makefile: CLAR_TEST_SUITES += u-hash\n      CLAR_TEST_SUITES += u-prio-queue\n      CLAR_TEST_SUITES += u-reftable-tree\n      CLAR_TEST_SUITES += u-strbuf\n    -@@ Makefile: CLAR_TEST_OBJS = $(patsubst %,$(UNIT_TEST_DIR)/%.o,$(CLAR_TEST_SUITES))\n    - CLAR_TEST_OBJS += $(UNIT_TEST_DIR)/clar/clar.o\n    +@@ Makefile: CLAR_TEST_OBJS += $(UNIT_TEST_DIR)/clar/clar.o\n      CLAR_TEST_OBJS += $(UNIT_TEST_DIR)/unit-test.o\n    + CLAR_TEST_OBJS += $(UNIT_TEST_DIR)/lib-oid.o\n      \n     -UNIT_TEST_PROGRAMS += t-oidmap\n      UNIT_TEST_PROGRAMS += t-oidtree\n    @@ t/unit-tests/t-oidmap.c (deleted)\n      ## t/unit-tests/u-oidmap.c (new) ##\n     @@\n     +#include \"unit-test.h\"\n    ++#include \"lib-oid.h\"\n     +#include \"oidmap.h\"\n     +#include \"hash.h\"\n     +#include \"hex.h\"\n    @@ t/unit-tests/u-oidmap.c (new)\n     +\n     +\toidmap_iter_init(&map, &iter);\n     +\twhile ((entry = oidmap_iter_next(&iter))) {\n    -+\t\tcl_assert_equal_i(key_val_contains(entry, seen), 0);\n    ++\t\tif (key_val_contains(entry, seen) != 0) {\n    ++\t\t\tcl_failf(\"Unexpected entry: name = %s, oid = %s\",\n    ++\t\t\t\t entry->name, oid_to_hex(&entry->entry.oid));\n    ++\t\t}\n     +\t\tcount++;\n     +\t}\n     +\tcl_assert_equal_i(count, ARRAY_SIZE(key_val));\n4:  733b53cd05 ! 4:  ea63a5c9f1 t/unit-tests: convert oidtree test to use clar test framework\n    @@ Commit message\n         t/unit-tests: convert oidtree test to use clar test framework\n     \n         Adapt oidtree test script to clar framework by using clar assertions\n    -    where necessary. `cl_parse_any_oid` handles the necessary checks needed\n    -    for the test to run smoothly.\n    +    where necessary. `cl_parse_any_oid()` ensures the hash algorithm is set\n    +    before parsing. This prevents issues from an uninitialized or invalid\n    +    hash algorithm.\n     \n         Introduce 'test_oidtree__initialize` handles the to set up of the global\n         oidtree variable and `test_oidtree__cleanup` frees the oidtree when all\n         tests are completed.\n     \n    -    This streamlines the test suite, making individual tests self-contained\n    -    and reducing redundant code.\n    +    With this change, `check_each` stops at the first error encountered,\n    +    making it easier to address it.\n     \n         Mentored-by: Patrick Steinhardt <ps@pks.im>\n         Signed-off-by: Seyi Kuforiji <kuforiji98@gmail.com>\n    @@ Makefile: CLAR_TEST_SUITES += u-hashmap\n      CLAR_TEST_SUITES += u-prio-queue\n      CLAR_TEST_SUITES += u-reftable-tree\n      CLAR_TEST_SUITES += u-strbuf\n    -@@ Makefile: CLAR_TEST_OBJS = $(patsubst %,$(UNIT_TEST_DIR)/%.o,$(CLAR_TEST_SUITES))\n    - CLAR_TEST_OBJS += $(UNIT_TEST_DIR)/clar/clar.o\n    +@@ Makefile: CLAR_TEST_OBJS += $(UNIT_TEST_DIR)/clar/clar.o\n      CLAR_TEST_OBJS += $(UNIT_TEST_DIR)/unit-test.o\n    + CLAR_TEST_OBJS += $(UNIT_TEST_DIR)/lib-oid.o\n      \n     -UNIT_TEST_PROGRAMS += t-oidtree\n      UNIT_TEST_PROGRAMS += t-reftable-basics\n    @@ t/unit-tests/t-oidtree.c (deleted)\n      ## t/unit-tests/u-oidtree.c (new) ##\n     @@\n     +#include \"unit-test.h\"\n    ++#include \"lib-oid.h\"\n     +#include \"oidtree.h\"\n     +#include \"hash.h\"\n     +#include \"hex.h\"\n    @@ t/unit-tests/u-oidtree.c (new)\n     +\tcl_parse_any_oid(query, &oid);\n     +\toidtree_each(ot, &oid, strlen(query), check_each_cb, &hex_iter);\n     +\n    -+\tcl_assert_equal_i(hex_iter.i, hex_iter.expected_hexes.nr);\n    ++\tif (hex_iter.i != hex_iter.expected_hexes.nr)\n    ++\t\tcl_failf(\"error: could not find some 'object_id's for query ('%s')\", query);\n    ++\n     +\tstrvec_clear(&hex_iter.expected_hexes);\n     +}\n     +\n-- \n2.47.0.86.g15030f9556\n\n"},{"id":"512910","messageId":"20250224152704.70289-2-kuforiji98@gmail.com","threadId":"62979","inReplyTo":"20250224152704.70289-1-kuforiji98@gmail.com","subject":"[PATCH v2 1/4] t/unit-tests: implement clar specific oid helper functions","fromName":"Seyi Kuforiji","fromEmail":"kuforiji98@gmail.com","sentAt":"2025-02-24T15:27:01Z","receivedAt":"2025-02-24T15:27:53Z","isPatch":true,"sender":{"key":"kuforiji98@gmail.com","avatar":"https://avatars.githubusercontent.com/u/94905626?v=4"},"body":"`get_oid_arbitrary_hex()` and `init_hash_algo()` are both required for\noid-related tests to run without errors. In the current implementation,\nboth functions are defined and declared in the\n`t/unit-tests/lib-oid.{c,h}` which is utilized by oid-related tests in\nthe homegrown unit tests structure.\n\nAdapt functions in lib-oid.{c,h} to use clar. Both these functions\nbecome available for oid-related test files implemented using the clar\ntesting framework, which requires them. This will be used by subsequent\ncommits.\n\nMentored-by: Patrick Steinhardt <ps@pks.im>\nSigned-off-by: Seyi Kuforiji <kuforiji98@gmail.com>\n---\n Makefile                 |  2 +-\n t/meson.build            |  2 +-\n t/unit-tests/lib-oid.c   | 31 +++++++++++--------------------\n t/unit-tests/lib-oid.h   |  9 ++++++---\n t/unit-tests/unit-test.c |  2 ++\n 5 files changed, 21 insertions(+), 25 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex bcf5ed3f85..81799488f0 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1365,6 +1365,7 @@ CLAR_TEST_PROG = $(UNIT_TEST_BIN)/unit-tests$(X)\n CLAR_TEST_OBJS = $(patsubst %,$(UNIT_TEST_DIR)/%.o,$(CLAR_TEST_SUITES))\n CLAR_TEST_OBJS += $(UNIT_TEST_DIR)/clar/clar.o\n CLAR_TEST_OBJS += $(UNIT_TEST_DIR)/unit-test.o\n+CLAR_TEST_OBJS += $(UNIT_TEST_DIR)/lib-oid.o\n \n UNIT_TEST_PROGRAMS += t-oid-array\n UNIT_TEST_PROGRAMS += t-oidmap\n@@ -1381,7 +1382,6 @@ UNIT_TEST_PROGRAMS += t-trailer\n UNIT_TEST_PROGRAMS += t-urlmatch-normalization\n UNIT_TEST_PROGS = $(patsubst %,$(UNIT_TEST_BIN)/%$X,$(UNIT_TEST_PROGRAMS))\n UNIT_TEST_OBJS += $(UNIT_TEST_DIR)/test-lib.o\n-UNIT_TEST_OBJS += $(UNIT_TEST_DIR)/lib-oid.o\n UNIT_TEST_OBJS += $(UNIT_TEST_DIR)/lib-reftable.o\n \n # xdiff and reftable libs may in turn depend on what is in libgit.a\ndiff --git a/t/meson.build b/t/meson.build\nindex 780939d49f..862cf1cfd4 100644\n--- a/t/meson.build\n+++ b/t/meson.build\n@@ -14,6 +14,7 @@ clar_test_suites = [\n clar_sources = [\n   'unit-tests/clar/clar.c',\n   'unit-tests/unit-test.c',\n+  'unit-tests/lib-oid.c'\n ]\n \n clar_decls_h = custom_target(\n@@ -68,7 +69,6 @@ foreach unit_test_program : unit_test_programs\n   unit_test = executable(unit_test_name,\n     sources: [\n       'unit-tests/test-lib.c',\n-      'unit-tests/lib-oid.c',\n       'unit-tests/lib-reftable.c',\n       unit_test_program,\n     ],\ndiff --git a/t/unit-tests/lib-oid.c b/t/unit-tests/lib-oid.c\nindex 8f0ccac532..8bf09ea1cb 100644\n--- a/t/unit-tests/lib-oid.c\n+++ b/t/unit-tests/lib-oid.c\n@@ -1,9 +1,9 @@\n-#include \"test-lib.h\"\n+#include \"unit-test.h\"\n #include \"lib-oid.h\"\n #include \"strbuf.h\"\n #include \"hex.h\"\n \n-int init_hash_algo(void)\n+int cl_setup_hash_algo(void)\n {\n \tstatic int algo = -1;\n \n@@ -11,42 +11,33 @@ int init_hash_algo(void)\n \t\tconst char *algo_name = getenv(\"GIT_TEST_DEFAULT_HASH\");\n \t\talgo = algo_name ? hash_algo_by_name(algo_name) : GIT_HASH_SHA1;\n \n-\t\tif (!check(algo != GIT_HASH_UNKNOWN))\n-\t\t\ttest_msg(\"BUG: invalid GIT_TEST_DEFAULT_HASH value ('%s')\",\n-\t\t\t\t algo_name);\n+\t\tcl_assert(algo != GIT_HASH_UNKNOWN);\n \t}\n \treturn algo;\n }\n \n-static int get_oid_arbitrary_hex_algop(const char *hex, struct object_id *oid,\n+static void cl_parse_oid(const char *hex, struct object_id *oid,\n \t\t\t\t       const struct git_hash_algo *algop)\n {\n \tint ret;\n \tsize_t sz = strlen(hex);\n \tstruct strbuf buf = STRBUF_INIT;\n \n-\tif (!check(sz <= algop->hexsz)) {\n-\t\ttest_msg(\"BUG: hex string (%s) bigger than maximum allowed (%lu)\",\n-\t\t\t hex, (unsigned long)algop->hexsz);\n-\t\treturn -1;\n-\t}\n+\tcl_assert(sz <= algop->hexsz);\n \n \tstrbuf_add(&buf, hex, sz);\n \tstrbuf_addchars(&buf, '0', algop->hexsz - sz);\n \n-\tret = get_oid_hex_algop(buf.buf, oid, algop);\n-\tif (!check_int(ret, ==, 0))\n-\t\ttest_msg(\"BUG: invalid hex input (%s) provided\", hex);\n+\tcl_assert_equal_i(get_oid_hex_algop(buf.buf, oid, algop), 0);\n \n \tstrbuf_release(&buf);\n-\treturn ret;\n }\n \n-int get_oid_arbitrary_hex(const char *hex, struct object_id *oid)\n+\n+void cl_parse_any_oid(const char *hex, struct object_id *oid)\n {\n-\tint hash_algo = init_hash_algo();\n+\tint hash_algo = cl_setup_hash_algo();\n \n-\tif (!check_int(hash_algo, !=, GIT_HASH_UNKNOWN))\n-\t\treturn -1;\n-\treturn get_oid_arbitrary_hex_algop(hex, oid, &hash_algos[hash_algo]);\n+\tcl_assert(hash_algo != GIT_HASH_UNKNOWN);\n+\tcl_parse_oid(hex, oid, &hash_algos[hash_algo]);\n }\ndiff --git a/t/unit-tests/lib-oid.h b/t/unit-tests/lib-oid.h\nindex 4e77c04bd2..4031775104 100644\n--- a/t/unit-tests/lib-oid.h\n+++ b/t/unit-tests/lib-oid.h\n@@ -5,6 +5,7 @@\n \n /*\n  * Convert arbitrary hex string to object_id.\n+ *\n  * For example, passing \"abc12\" will generate\n  * \"abc1200000000000000000000000000000000000\" hex of length 40 for SHA-1 and\n  * create object_id with that.\n@@ -12,14 +13,16 @@\n  * algo is not allowed. The hash algo is decided based on GIT_TEST_DEFAULT_HASH\n  * environment variable.\n  */\n-int get_oid_arbitrary_hex(const char *s, struct object_id *oid);\n+\n+void cl_parse_any_oid (const char *s, struct object_id *oid);\n /*\n  * Returns one of GIT_HASH_{SHA1, SHA256, UNKNOWN} based on the value of\n  * GIT_TEST_DEFAULT_HASH environment variable. The fallback value in the\n  * absence of GIT_TEST_DEFAULT_HASH is GIT_HASH_SHA1. It also uses\n- * check(algo != GIT_HASH_UNKNOWN) before returning to verify if the\n+ * cl_assert(algo != GIT_HASH_UNKNOWN) before returning to verify if the\n  * GIT_TEST_DEFAULT_HASH's value is valid or not.\n  */\n-int init_hash_algo(void);\n+\n+int cl_setup_hash_algo(void);\n \n #endif /* LIB_OID_H */\ndiff --git a/t/unit-tests/unit-test.c b/t/unit-tests/unit-test.c\nindex fa8818842a..5af645048a 100644\n--- a/t/unit-tests/unit-test.c\n+++ b/t/unit-tests/unit-test.c\n@@ -1,5 +1,7 @@\n #include \"unit-test.h\"\n+#include \"hex.h\"\n #include \"parse-options.h\"\n+#include \"strbuf.h\"\n #include \"string-list.h\"\n #include \"strvec.h\"\n \n-- \n2.47.0.86.g15030f9556\n\n"},{"id":"512911","messageId":"20250224152704.70289-3-kuforiji98@gmail.com","threadId":"62979","inReplyTo":"20250224152704.70289-1-kuforiji98@gmail.com","subject":"[PATCH v2 2/4] t/unit-tests: convert oid-array test to use clar test framework","fromName":"Seyi Kuforiji","fromEmail":"kuforiji98@gmail.com","sentAt":"2025-02-24T15:27:02Z","receivedAt":"2025-02-24T15:27:59Z","isPatch":true,"sender":{"key":"kuforiji98@gmail.com","avatar":"https://avatars.githubusercontent.com/u/94905626?v=4"},"body":"Adapt oid-array test script to clar framework by using clar assertions\nwhere necessary. Remove descriptions from macros to reduce\nredundancy, and move test input arrays to global scope for reuse across\nmultiple test functions. Introduce `test_oid_array__initialize()` to\nexplicitly initialize the hash algorithm.\n\nThese changes streamline the test suite, making individual tests\nself-contained and reducing redundant code.\n\nMentored-by: Patrick Steinhardt <ps@pks.im>\nSigned-off-by: Seyi Kuforiji <kuforiji98@gmail.com>\n---\n Makefile                                      |   2 +-\n t/meson.build                                 |   2 +-\n t/unit-tests/{t-oid-array.c => u-oid-array.c} | 125 +++++++++---------\n 3 files changed, 66 insertions(+), 63 deletions(-)\n rename t/unit-tests/{t-oid-array.c => u-oid-array.c} (34%)\n\ndiff --git a/Makefile b/Makefile\nindex 81799488f0..c9b0320fcb 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1356,6 +1356,7 @@ CLAR_TEST_SUITES += u-example-decorate\n CLAR_TEST_SUITES += u-hash\n CLAR_TEST_SUITES += u-hashmap\n CLAR_TEST_SUITES += u-mem-pool\n+CLAR_TEST_SUITES += u-oid-array\n CLAR_TEST_SUITES += u-prio-queue\n CLAR_TEST_SUITES += u-reftable-tree\n CLAR_TEST_SUITES += u-strbuf\n@@ -1367,7 +1368,6 @@ CLAR_TEST_OBJS += $(UNIT_TEST_DIR)/clar/clar.o\n CLAR_TEST_OBJS += $(UNIT_TEST_DIR)/unit-test.o\n CLAR_TEST_OBJS += $(UNIT_TEST_DIR)/lib-oid.o\n \n-UNIT_TEST_PROGRAMS += t-oid-array\n UNIT_TEST_PROGRAMS += t-oidmap\n UNIT_TEST_PROGRAMS += t-oidtree\n UNIT_TEST_PROGRAMS += t-reftable-basics\ndiff --git a/t/meson.build b/t/meson.build\nindex 862cf1cfd4..1b17a53acc 100644\n--- a/t/meson.build\n+++ b/t/meson.build\n@@ -4,6 +4,7 @@ clar_test_suites = [\n   'unit-tests/u-hash.c',\n   'unit-tests/u-hashmap.c',\n   'unit-tests/u-mem-pool.c',\n+  'unit-tests/u-oid-array.c',\n   'unit-tests/u-prio-queue.c',\n   'unit-tests/u-reftable-tree.c',\n   'unit-tests/u-strbuf.c',\n@@ -49,7 +50,6 @@ clar_unit_tests = executable('unit-tests',\n test('unit-tests', clar_unit_tests)\n \n unit_test_programs = [\n-  'unit-tests/t-oid-array.c',\n   'unit-tests/t-oidmap.c',\n   'unit-tests/t-oidtree.c',\n   'unit-tests/t-reftable-basics.c',\ndiff --git a/t/unit-tests/t-oid-array.c b/t/unit-tests/u-oid-array.c\nsimilarity index 34%\nrename from t/unit-tests/t-oid-array.c\nrename to t/unit-tests/u-oid-array.c\nindex 45b59a2a51..e48a433f21 100644\n--- a/t/unit-tests/t-oid-array.c\n+++ b/t/unit-tests/u-oid-array.c\n@@ -1,22 +1,19 @@\n #define USE_THE_REPOSITORY_VARIABLE\n \n-#include \"test-lib.h\"\n+#include \"unit-test.h\"\n #include \"lib-oid.h\"\n #include \"oid-array.h\"\n #include \"hex.h\"\n \n-static int fill_array(struct oid_array *array, const char *hexes[], size_t n)\n+static void fill_array(struct oid_array *array, const char *hexes[], size_t n)\n {\n \tfor (size_t i = 0; i < n; i++) {\n \t\tstruct object_id oid;\n \n-\t\tif (!check_int(get_oid_arbitrary_hex(hexes[i], &oid), ==, 0))\n-\t\t\treturn -1;\n+\t\tcl_parse_any_oid(hexes[i], &oid);\n \t\toid_array_append(array, &oid);\n \t}\n-\tif (!check_uint(array->nr, ==, n))\n-\t\treturn -1;\n-\treturn 0;\n+\tcl_assert_equal_i(array->nr, n);\n }\n \n static int add_to_oid_array(const struct object_id *oid, void *data)\n@@ -34,30 +31,22 @@ static void t_enumeration(const char **input_args, size_t input_sz,\n \t\t\t actual = OID_ARRAY_INIT;\n \tsize_t i;\n \n-\tif (fill_array(&input, input_args, input_sz))\n-\t\treturn;\n-\tif (fill_array(&expect, expect_args, expect_sz))\n-\t\treturn;\n+\tfill_array(&input, input_args, input_sz);\n+\tfill_array(&expect, expect_args, expect_sz);\n \n \toid_array_for_each_unique(&input, add_to_oid_array, &actual);\n-\tif (!check_uint(actual.nr, ==, expect.nr))\n-\t\treturn;\n-\n-\tfor (i = 0; i < actual.nr; i++) {\n-\t\tif (!check(oideq(&actual.oid[i], &expect.oid[i])))\n-\t\t\ttest_msg(\"expected: %s\\n       got: %s\\n     index: %\" PRIuMAX,\n-\t\t\t\t oid_to_hex(&expect.oid[i]), oid_to_hex(&actual.oid[i]),\n-\t\t\t\t (uintmax_t)i);\n-\t}\n+\tcl_assert_equal_i(actual.nr, expect.nr);\n+\n+\tfor (i = 0; i < actual.nr; i++)\n+\t\tcl_assert(oideq(&actual.oid[i], &expect.oid[i]));\n \n \toid_array_clear(&actual);\n \toid_array_clear(&input);\n \toid_array_clear(&expect);\n }\n \n-#define TEST_ENUMERATION(input, expect, desc)                                     \\\n-\tTEST(t_enumeration(input, ARRAY_SIZE(input), expect, ARRAY_SIZE(expect)), \\\n-\t\t\t   desc \" works\")\n+#define TEST_ENUMERATION(input, expect)                                     \\\n+\tt_enumeration(input, ARRAY_SIZE(input), expect, ARRAY_SIZE(expect));\n \n static void t_lookup(const char **input_hexes, size_t n, const char *query_hex,\n \t\t     int lower_bound, int upper_bound)\n@@ -66,61 +55,75 @@ static void t_lookup(const char **input_hexes, size_t n, const char *query_hex,\n \tstruct object_id oid_query;\n \tint ret;\n \n-\tif (!check_int(get_oid_arbitrary_hex(query_hex, &oid_query), ==, 0))\n-\t\treturn;\n-\tif (fill_array(&array, input_hexes, n))\n-\t\treturn;\n+\tcl_parse_any_oid(query_hex, &oid_query);\n+\tfill_array(&array, input_hexes, n);\n \tret = oid_array_lookup(&array, &oid_query);\n \n-\tif (!check_int(ret, <=, upper_bound) ||\n-\t    !check_int(ret, >=, lower_bound))\n-\t\ttest_msg(\"oid query for lookup: %s\", oid_to_hex(&oid_query));\n+\tcl_assert(ret <= upper_bound);\n+\tcl_assert(ret >= lower_bound);\n \n \toid_array_clear(&array);\n }\n \n-#define TEST_LOOKUP(input_hexes, query, lower_bound, upper_bound, desc) \\\n-\tTEST(t_lookup(input_hexes, ARRAY_SIZE(input_hexes), query,      \\\n-\t\t      lower_bound, upper_bound),                        \\\n-\t     desc \" works\")\n+#define TEST_LOOKUP(input_hexes, query, lower_bound, upper_bound) \\\n+\tt_lookup(input_hexes, ARRAY_SIZE(input_hexes), query,      \\\n+\t\t      lower_bound, upper_bound);\n \n-static void setup(void)\n+void test_oid_array__initialize(void)\n {\n \t/* The hash algo is used by oid_array_lookup() internally */\n-\tint algo = init_hash_algo();\n-\tif (check_int(algo, !=, GIT_HASH_UNKNOWN))\n-\t\trepo_set_hash_algo(the_repository, algo);\n+\tint algo = cl_setup_hash_algo();\n+\trepo_set_hash_algo(the_repository, algo);\n }\n \n-int cmd_main(int argc UNUSED, const char **argv UNUSED)\n+static const char *arr_input[] = { \"88\", \"44\", \"aa\", \"55\" };\n+static const char *arr_input_dup[] = { \"88\", \"44\", \"aa\", \"55\",\n+\t\t\t\t       \"88\", \"44\", \"aa\", \"55\",\n+\t\t\t\t       \"88\", \"44\", \"aa\", \"55\" };\n+static const char *res_sorted[] = { \"44\", \"55\", \"88\", \"aa\" };\n+\n+void test_oid_array__enumerate_unique(void)\n {\n-\tconst char *arr_input[] = { \"88\", \"44\", \"aa\", \"55\" };\n-\tconst char *arr_input_dup[] = { \"88\", \"44\", \"aa\", \"55\",\n-\t\t\t\t\t\"88\", \"44\", \"aa\", \"55\",\n-\t\t\t\t\t\"88\", \"44\", \"aa\", \"55\" };\n-\tconst char *res_sorted[] = { \"44\", \"55\", \"88\", \"aa\" };\n-\tconst char *nearly_55;\n+\tTEST_ENUMERATION(arr_input, res_sorted);\n+}\n+\n+void test_oid_array__enumerate_duplicate(void)\n+{\n+\tTEST_ENUMERATION(arr_input_dup, res_sorted);\n+}\n+\n+void test_oid_array__lookup(void)\n+{\n+\tTEST_LOOKUP(arr_input, \"55\", 1, 1);\n+}\n \n-\tif (!TEST(setup(), \"setup\"))\n-\t\ttest_skip_all(\"hash algo initialization failed\");\n+void test_oid_array__lookup_non_existent(void)\n+{\n+\tTEST_LOOKUP(arr_input, \"33\", INT_MIN, -1);\n+}\n+\n+void test_oid_array__lookup_duplicates(void)\n+{\n+\tTEST_LOOKUP(arr_input_dup, \"55\", 3, 5);\n+}\n \n-\tTEST_ENUMERATION(arr_input, res_sorted, \"ordered enumeration\");\n-\tTEST_ENUMERATION(arr_input_dup, res_sorted,\n-\t\t\t \"ordered enumeration with duplicate suppression\");\n+void test_oid_array__lookup_non_existent_dup(void)\n+{\n+\tTEST_LOOKUP(arr_input_dup, \"66\", INT_MIN, -1);\n+}\n \n-\tTEST_LOOKUP(arr_input, \"55\", 1, 1, \"lookup\");\n-\tTEST_LOOKUP(arr_input, \"33\", INT_MIN, -1, \"lookup non-existent entry\");\n-\tTEST_LOOKUP(arr_input_dup, \"55\", 3, 5, \"lookup with duplicates\");\n-\tTEST_LOOKUP(arr_input_dup, \"66\", INT_MIN, -1,\n-\t\t    \"lookup non-existent entry with duplicates\");\n+void test_oid_array__lookup_almost_dup(void)\n+{\n+\tconst char *nearly_55;\n \n-\tnearly_55 = init_hash_algo() == GIT_HASH_SHA1 ?\n+\tnearly_55 = cl_setup_hash_algo() == GIT_HASH_SHA1 ?\n \t\t\t\"5500000000000000000000000000000000000001\" :\n \t\t\t\"5500000000000000000000000000000000000000000000000000000000000001\";\n-\tTEST_LOOKUP(((const char *[]){ \"55\", nearly_55 }), \"55\", 0, 0,\n-\t\t    \"lookup with almost duplicate values\");\n-\tTEST_LOOKUP(((const char *[]){ \"55\", \"55\" }), \"55\", 0, 1,\n-\t\t    \"lookup with single duplicate value\");\n \n-\treturn test_done();\n+\tTEST_LOOKUP(((const char *[]){ \"55\", nearly_55 }), \"55\", 0, 0);\n+}\n+\n+void test_oid_array__lookup_single_dup(void)\n+{\n+\tTEST_LOOKUP(((const char *[]){ \"55\", \"55\" }), \"55\", 0, 1);\n }\n-- \n2.47.0.86.g15030f9556\n\n"},{"id":"512912","messageId":"20250224152704.70289-4-kuforiji98@gmail.com","threadId":"62979","inReplyTo":"20250224152704.70289-1-kuforiji98@gmail.com","subject":"[PATCH v2 3/4] t/unit-tests: convert oidmap test to use clar test framework","fromName":"Seyi Kuforiji","fromEmail":"kuforiji98@gmail.com","sentAt":"2025-02-24T15:27:03Z","receivedAt":"2025-02-24T15:28:04Z","isPatch":true,"sender":{"key":"kuforiji98@gmail.com","avatar":"https://avatars.githubusercontent.com/u/94905626?v=4"},"body":"Adapt oidmap test script to clar framework by using clar assertions\nwhere necessary. `cl_parse_any_oid()` ensures the hash algorithm is set\nbefore parsing. This prevents issues from an uninitialized or invalid\nhash algorithm.\n\nIntroduce 'test_oidmap__initialize` handles the to set up of the global\noidmap map with predefined key-value pairs, and `test_oidmap__cleanup`\nfrees the oidmap and its entries when all tests are completed.\n\nThe test loops through all entries to detect multiple errors. With this\nchange, it stops at the first error encountered, making it easier to\naddress it.\n\nMentored-by: Patrick Steinhardt <ps@pks.im>\nSigned-off-by: Seyi Kuforiji <kuforiji98@gmail.com>\n---\n Makefile                                |   2 +-\n t/meson.build                           |   2 +-\n t/unit-tests/{t-oidmap.c => u-oidmap.c} | 153 +++++++++---------------\n 3 files changed, 56 insertions(+), 101 deletions(-)\n rename t/unit-tests/{t-oidmap.c => u-oidmap.c} (32%)\n\ndiff --git a/Makefile b/Makefile\nindex c9b0320fcb..e4e85e6007 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1357,6 +1357,7 @@ CLAR_TEST_SUITES += u-hash\n CLAR_TEST_SUITES += u-hashmap\n CLAR_TEST_SUITES += u-mem-pool\n CLAR_TEST_SUITES += u-oid-array\n+CLAR_TEST_SUITES += u-oidmap\n CLAR_TEST_SUITES += u-prio-queue\n CLAR_TEST_SUITES += u-reftable-tree\n CLAR_TEST_SUITES += u-strbuf\n@@ -1368,7 +1369,6 @@ CLAR_TEST_OBJS += $(UNIT_TEST_DIR)/clar/clar.o\n CLAR_TEST_OBJS += $(UNIT_TEST_DIR)/unit-test.o\n CLAR_TEST_OBJS += $(UNIT_TEST_DIR)/lib-oid.o\n \n-UNIT_TEST_PROGRAMS += t-oidmap\n UNIT_TEST_PROGRAMS += t-oidtree\n UNIT_TEST_PROGRAMS += t-reftable-basics\n UNIT_TEST_PROGRAMS += t-reftable-block\ndiff --git a/t/meson.build b/t/meson.build\nindex 1b17a53acc..d5b83cdb72 100644\n--- a/t/meson.build\n+++ b/t/meson.build\n@@ -5,6 +5,7 @@ clar_test_suites = [\n   'unit-tests/u-hashmap.c',\n   'unit-tests/u-mem-pool.c',\n   'unit-tests/u-oid-array.c',\n+  'unit-tests/u-oidmap.c',\n   'unit-tests/u-prio-queue.c',\n   'unit-tests/u-reftable-tree.c',\n   'unit-tests/u-strbuf.c',\n@@ -50,7 +51,6 @@ clar_unit_tests = executable('unit-tests',\n test('unit-tests', clar_unit_tests)\n \n unit_test_programs = [\n-  'unit-tests/t-oidmap.c',\n   'unit-tests/t-oidtree.c',\n   'unit-tests/t-reftable-basics.c',\n   'unit-tests/t-reftable-block.c',\ndiff --git a/t/unit-tests/t-oidmap.c b/t/unit-tests/u-oidmap.c\nsimilarity index 32%\nrename from t/unit-tests/t-oidmap.c\nrename to t/unit-tests/u-oidmap.c\nindex b22e52d08b..dc805b7e3c 100644\n--- a/t/unit-tests/t-oidmap.c\n+++ b/t/unit-tests/u-oidmap.c\n@@ -1,4 +1,4 @@\n-#include \"test-lib.h\"\n+#include \"unit-test.h\"\n #include \"lib-oid.h\"\n #include \"oidmap.h\"\n #include \"hash.h\"\n@@ -18,102 +18,85 @@ static const char *const key_val[][2] = { { \"11\", \"one\" },\n \t\t\t\t\t  { \"22\", \"two\" },\n \t\t\t\t\t  { \"33\", \"three\" } };\n \n-static void setup(void (*f)(struct oidmap *map))\n+static struct oidmap map;\n+\n+void test_oidmap__initialize(void)\n {\n-\tstruct oidmap map = OIDMAP_INIT;\n-\tint ret = 0;\n+\toidmap_init(&map, 0);\n \n \tfor (size_t i = 0; i < ARRAY_SIZE(key_val); i++){\n \t\tstruct test_entry *entry;\n \n \t\tFLEX_ALLOC_STR(entry, name, key_val[i][1]);\n-\t\tif ((ret = get_oid_arbitrary_hex(key_val[i][0], &entry->entry.oid))) {\n-\t\t\tfree(entry);\n-\t\t\tbreak;\n-\t\t}\n-\t\tentry = oidmap_put(&map, entry);\n-\t\tif (!check(entry == NULL))\n-\t\t\tfree(entry);\n+\t\tcl_parse_any_oid(key_val[i][0], &entry->entry.oid);\n+\t\tcl_assert(oidmap_put(&map, entry) == NULL);\n \t}\n+}\n \n-\tif (!ret)\n-\t\tf(&map);\n+void test_oidmap__cleanup(void)\n+{\n \toidmap_free(&map, 1);\n }\n \n-static void t_replace(struct oidmap *map)\n+void test_oidmap__replace(void)\n {\n \tstruct test_entry *entry, *prev;\n \n \tFLEX_ALLOC_STR(entry, name, \"un\");\n-\tif (get_oid_arbitrary_hex(\"11\", &entry->entry.oid))\n-\t\treturn;\n-\tprev = oidmap_put(map, entry);\n-\tif (!check(prev != NULL))\n-\t\treturn;\n-\tcheck_str(prev->name, \"one\");\n+\tcl_parse_any_oid(\"11\", &entry->entry.oid);\n+\tprev = oidmap_put(&map, entry);\n+\tcl_assert(prev != NULL);\n+\tcl_assert_equal_s(prev->name, \"one\");\n \tfree(prev);\n \n \tFLEX_ALLOC_STR(entry, name, \"deux\");\n-\tif (get_oid_arbitrary_hex(\"22\", &entry->entry.oid))\n-\t\treturn;\n-\tprev = oidmap_put(map, entry);\n-\tif (!check(prev != NULL))\n-\t\treturn;\n-\tcheck_str(prev->name, \"two\");\n+\tcl_parse_any_oid(\"22\", &entry->entry.oid);\n+\tprev = oidmap_put(&map, entry);\n+\tcl_assert(prev != NULL);\n+\tcl_assert_equal_s(prev->name, \"two\");\n \tfree(prev);\n }\n \n-static void t_get(struct oidmap *map)\n+void test_oidmap__get(void)\n {\n \tstruct test_entry *entry;\n \tstruct object_id oid;\n \n-\tif (get_oid_arbitrary_hex(\"22\", &oid))\n-\t\treturn;\n-\tentry = oidmap_get(map, &oid);\n-\tif (!check(entry != NULL))\n-\t\treturn;\n-\tcheck_str(entry->name, \"two\");\n-\n-\tif (get_oid_arbitrary_hex(\"44\", &oid))\n-\t\treturn;\n-\tcheck(oidmap_get(map, &oid) == NULL);\n-\n-\tif (get_oid_arbitrary_hex(\"11\", &oid))\n-\t\treturn;\n-\tentry = oidmap_get(map, &oid);\n-\tif (!check(entry != NULL))\n-\t\treturn;\n-\tcheck_str(entry->name, \"one\");\n+\tcl_parse_any_oid(\"22\", &oid);\n+\tentry = oidmap_get(&map, &oid);\n+\tcl_assert(entry != NULL);\n+\tcl_assert_equal_s(entry->name, \"two\");\n+\n+\tcl_parse_any_oid(\"44\", &oid);\n+\tcl_assert(oidmap_get(&map, &oid) == NULL);\n+\n+\tcl_parse_any_oid(\"11\", &oid);\n+\tentry = oidmap_get(&map, &oid);\n+\tcl_assert(entry != NULL);\n+\tcl_assert_equal_s(entry->name, \"one\");\n }\n \n-static void t_remove(struct oidmap *map)\n+void test_oidmap__remove(void)\n {\n \tstruct test_entry *entry;\n \tstruct object_id oid;\n \n-\tif (get_oid_arbitrary_hex(\"11\", &oid))\n-\t\treturn;\n-\tentry = oidmap_remove(map, &oid);\n-\tif (!check(entry != NULL))\n-\t\treturn;\n-\tcheck_str(entry->name, \"one\");\n-\tcheck(oidmap_get(map, &oid) == NULL);\n+\tcl_parse_any_oid(\"11\", &oid);\n+\tentry = oidmap_remove(&map, &oid);\n+\tcl_assert(entry != NULL);\n+\tcl_assert_equal_s(entry->name, \"one\");\n+\tcl_assert(oidmap_get(&map, &oid) == NULL);\n \tfree(entry);\n \n-\tif (get_oid_arbitrary_hex(\"22\", &oid))\n-\t\treturn;\n-\tentry = oidmap_remove(map, &oid);\n-\tif (!check(entry != NULL))\n-\t\treturn;\n-\tcheck_str(entry->name, \"two\");\n-\tcheck(oidmap_get(map, &oid) == NULL);\n+\tcl_parse_any_oid(\"22\", &oid);\n+\tentry = oidmap_remove(&map, &oid);\n+\tcl_assert(entry != NULL);\n+\tcl_assert_equal_s(entry->name, \"two\");\n+\tcl_assert(oidmap_get(&map, &oid) == NULL);\n \tfree(entry);\n \n-\tif (get_oid_arbitrary_hex(\"44\", &oid))\n-\t\treturn;\n-\tcheck(oidmap_remove(map, &oid) == NULL);\n+\tcl_parse_any_oid(\"44\", &oid);\n+\tcl_assert(oidmap_remove(&map, &oid) == NULL);\n }\n \n static int key_val_contains(struct test_entry *entry, char seen[])\n@@ -121,8 +104,7 @@ static int key_val_contains(struct test_entry *entry, char seen[])\n \tfor (size_t i = 0; i < ARRAY_SIZE(key_val); i++) {\n \t\tstruct object_id oid;\n \n-\t\tif (get_oid_arbitrary_hex(key_val[i][0], &oid))\n-\t\t\treturn -1;\n+\t\tcl_parse_any_oid(key_val[i][0], &oid);\n \n \t\tif (oideq(&entry->entry.oid, &oid)) {\n \t\t\tif (seen[i])\n@@ -134,48 +116,21 @@ static int key_val_contains(struct test_entry *entry, char seen[])\n \treturn 1;\n }\n \n-static void t_iterate(struct oidmap *map)\n+void test_oidmap__iterate(void)\n {\n \tstruct oidmap_iter iter;\n \tstruct test_entry *entry;\n \tchar seen[ARRAY_SIZE(key_val)] = { 0 };\n \tint count = 0;\n \n-\toidmap_iter_init(map, &iter);\n+\toidmap_iter_init(&map, &iter);\n \twhile ((entry = oidmap_iter_next(&iter))) {\n-\t\tint ret;\n-\t\tif (!check_int((ret = key_val_contains(entry, seen)), ==, 0)) {\n-\t\t\tswitch (ret) {\n-\t\t\tcase -1:\n-\t\t\t\tbreak; /* error message handled by get_oid_arbitrary_hex() */\n-\t\t\tcase 1:\n-\t\t\t\ttest_msg(\"obtained entry was not given in the input\\n\"\n-\t\t\t\t\t \"  name: %s\\n   oid: %s\\n\",\n-\t\t\t\t\t entry->name, oid_to_hex(&entry->entry.oid));\n-\t\t\t\tbreak;\n-\t\t\tcase 2:\n-\t\t\t\ttest_msg(\"duplicate entry detected\\n\"\n-\t\t\t\t\t \"  name: %s\\n   oid: %s\\n\",\n-\t\t\t\t\t entry->name, oid_to_hex(&entry->entry.oid));\n-\t\t\t\tbreak;\n-\t\t\tdefault:\n-\t\t\t\ttest_msg(\"BUG: invalid return value (%d) from key_val_contains()\",\n-\t\t\t\t\t ret);\n-\t\t\t\tbreak;\n-\t\t\t}\n-\t\t} else {\n-\t\t\tcount++;\n+\t\tif (key_val_contains(entry, seen) != 0) {\n+\t\t\tcl_failf(\"Unexpected entry: name = %s, oid = %s\",\n+\t\t\t\t entry->name, oid_to_hex(&entry->entry.oid));\n \t\t}\n+\t\tcount++;\n \t}\n-\tcheck_int(count, ==, ARRAY_SIZE(key_val));\n-\tcheck_int(hashmap_get_size(&map->map), ==, ARRAY_SIZE(key_val));\n-}\n-\n-int cmd_main(int argc UNUSED, const char **argv UNUSED)\n-{\n-\tTEST(setup(t_replace), \"replace works\");\n-\tTEST(setup(t_get), \"get works\");\n-\tTEST(setup(t_remove), \"remove works\");\n-\tTEST(setup(t_iterate), \"iterate works\");\n-\treturn test_done();\n+\tcl_assert_equal_i(count, ARRAY_SIZE(key_val));\n+\tcl_assert_equal_i(hashmap_get_size(&map.map), ARRAY_SIZE(key_val));\n }\n-- \n2.47.0.86.g15030f9556\n\n"},{"id":"512914","messageId":"20250224152704.70289-5-kuforiji98@gmail.com","threadId":"62979","inReplyTo":"20250224152704.70289-1-kuforiji98@gmail.com","subject":"[PATCH v2 4/4] t/unit-tests: convert oidtree test to use clar test framework","fromName":"Seyi Kuforiji","fromEmail":"kuforiji98@gmail.com","sentAt":"2025-02-24T15:27:04Z","receivedAt":"2025-02-24T15:28:10Z","isPatch":true,"sender":{"key":"kuforiji98@gmail.com","avatar":"https://avatars.githubusercontent.com/u/94905626?v=4"},"body":"Adapt oidtree test script to clar framework by using clar assertions\nwhere necessary. `cl_parse_any_oid()` ensures the hash algorithm is set\nbefore parsing. This prevents issues from an uninitialized or invalid\nhash algorithm.\n\nIntroduce 'test_oidtree__initialize` handles the to set up of the global\noidtree variable and `test_oidtree__cleanup` frees the oidtree when all\ntests are completed.\n\nWith this change, `check_each` stops at the first error encountered,\nmaking it easier to address it.\n\nMentored-by: Patrick Steinhardt <ps@pks.im>\nSigned-off-by: Seyi Kuforiji <kuforiji98@gmail.com>\n---\n Makefile                                  |  2 +-\n t/meson.build                             |  2 +-\n t/unit-tests/{t-oidtree.c => u-oidtree.c} | 79 +++++++++--------------\n 3 files changed, 34 insertions(+), 49 deletions(-)\n rename t/unit-tests/{t-oidtree.c => u-oidtree.c} (45%)\n\ndiff --git a/Makefile b/Makefile\nindex e4e85e6007..2b134efc70 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1358,6 +1358,7 @@ CLAR_TEST_SUITES += u-hashmap\n CLAR_TEST_SUITES += u-mem-pool\n CLAR_TEST_SUITES += u-oid-array\n CLAR_TEST_SUITES += u-oidmap\n+CLAR_TEST_SUITES += u-oidtree\n CLAR_TEST_SUITES += u-prio-queue\n CLAR_TEST_SUITES += u-reftable-tree\n CLAR_TEST_SUITES += u-strbuf\n@@ -1369,7 +1370,6 @@ CLAR_TEST_OBJS += $(UNIT_TEST_DIR)/clar/clar.o\n CLAR_TEST_OBJS += $(UNIT_TEST_DIR)/unit-test.o\n CLAR_TEST_OBJS += $(UNIT_TEST_DIR)/lib-oid.o\n \n-UNIT_TEST_PROGRAMS += t-oidtree\n UNIT_TEST_PROGRAMS += t-reftable-basics\n UNIT_TEST_PROGRAMS += t-reftable-block\n UNIT_TEST_PROGRAMS += t-reftable-merged\ndiff --git a/t/meson.build b/t/meson.build\nindex d5b83cdb72..91699917ff 100644\n--- a/t/meson.build\n+++ b/t/meson.build\n@@ -6,6 +6,7 @@ clar_test_suites = [\n   'unit-tests/u-mem-pool.c',\n   'unit-tests/u-oid-array.c',\n   'unit-tests/u-oidmap.c',\n+  'unit-tests/u-oidtree.c',\n   'unit-tests/u-prio-queue.c',\n   'unit-tests/u-reftable-tree.c',\n   'unit-tests/u-strbuf.c',\n@@ -51,7 +52,6 @@ clar_unit_tests = executable('unit-tests',\n test('unit-tests', clar_unit_tests)\n \n unit_test_programs = [\n-  'unit-tests/t-oidtree.c',\n   'unit-tests/t-reftable-basics.c',\n   'unit-tests/t-reftable-block.c',\n   'unit-tests/t-reftable-merged.c',\ndiff --git a/t/unit-tests/t-oidtree.c b/t/unit-tests/u-oidtree.c\nsimilarity index 45%\nrename from t/unit-tests/t-oidtree.c\nrename to t/unit-tests/u-oidtree.c\nindex a38754b066..e6eede2740 100644\n--- a/t/unit-tests/t-oidtree.c\n+++ b/t/unit-tests/u-oidtree.c\n@@ -1,10 +1,12 @@\n-#include \"test-lib.h\"\n+#include \"unit-test.h\"\n #include \"lib-oid.h\"\n #include \"oidtree.h\"\n #include \"hash.h\"\n #include \"hex.h\"\n #include \"strvec.h\"\n \n+static struct oidtree ot;\n+\n #define FILL_TREE(tree, ...)                                       \\\n \tdo {                                                       \\\n \t\tconst char *hexes[] = { __VA_ARGS__ };             \\\n@@ -16,8 +18,7 @@ static int fill_tree_loc(struct oidtree *ot, const char *hexes[], size_t n)\n {\n \tfor (size_t i = 0; i < n; i++) {\n \t\tstruct object_id oid;\n-\t\tif (!check_int(get_oid_arbitrary_hex(hexes[i], &oid), ==, 0))\n-\t\t\treturn -1;\n+\t\tcl_parse_any_oid(hexes[i], &oid);\n \t\toidtree_insert(ot, &oid);\n \t}\n \treturn 0;\n@@ -27,10 +28,8 @@ static void check_contains(struct oidtree *ot, const char *hex, int expected)\n {\n \tstruct object_id oid;\n \n-\tif (!check_int(get_oid_arbitrary_hex(hex, &oid), ==, 0))\n-\t\treturn;\n-\tif (!check_int(oidtree_contains(ot, &oid), ==, expected))\n-\t\ttest_msg(\"oid: %s\", oid_to_hex(&oid));\n+\tcl_parse_any_oid(hex, &oid);\n+\tcl_assert_equal_i(oidtree_contains(ot, &oid), expected);\n }\n \n struct expected_hex_iter {\n@@ -44,19 +43,11 @@ static enum cb_next check_each_cb(const struct object_id *oid, void *data)\n \tstruct expected_hex_iter *hex_iter = data;\n \tstruct object_id expected;\n \n-\tif (!check_int(hex_iter->i, <, hex_iter->expected_hexes.nr)) {\n-\t\ttest_msg(\"error: extraneous callback for query: ('%s'), object_id: ('%s')\",\n-\t\t\t hex_iter->query, oid_to_hex(oid));\n-\t\treturn CB_BREAK;\n-\t}\n-\n-\tif (!check_int(get_oid_arbitrary_hex(hex_iter->expected_hexes.v[hex_iter->i],\n-\t\t\t\t\t     &expected), ==, 0))\n-\t\t; /* the data is bogus and cannot be used */\n-\telse if (!check(oideq(oid, &expected)))\n-\t\ttest_msg(\"expected: %s\\n       got: %s\\n     query: %s\",\n-\t\t\t oid_to_hex(&expected), oid_to_hex(oid), hex_iter->query);\n+\tcl_assert(hex_iter->i < hex_iter->expected_hexes.nr);\n \n+\tcl_parse_any_oid(hex_iter->expected_hexes.v[hex_iter->i],\n+\t\t\t &expected);\n+\tcl_assert_equal_s(oid_to_hex(oid), oid_to_hex(&expected));\n \thex_iter->i += 1;\n \treturn CB_CONTINUE;\n }\n@@ -75,48 +66,42 @@ static void check_each(struct oidtree *ot, const char *query, ...)\n \t\tstrvec_push(&hex_iter.expected_hexes, arg);\n \tva_end(hex_args);\n \n-\tif (!check_int(get_oid_arbitrary_hex(query, &oid), ==, 0))\n-\t\treturn;\n+\tcl_parse_any_oid(query, &oid);\n \toidtree_each(ot, &oid, strlen(query), check_each_cb, &hex_iter);\n \n-\tif (!check_int(hex_iter.i, ==, hex_iter.expected_hexes.nr))\n-\t\ttest_msg(\"error: could not find some 'object_id's for query ('%s')\", query);\n+\tif (hex_iter.i != hex_iter.expected_hexes.nr)\n+\t\tcl_failf(\"error: could not find some 'object_id's for query ('%s')\", query);\n+\n \tstrvec_clear(&hex_iter.expected_hexes);\n }\n \n-static void setup(void (*f)(struct oidtree *ot))\n+void test_oidtree__initialize(void)\n {\n-\tstruct oidtree ot;\n-\n \toidtree_init(&ot);\n-\tf(&ot);\n-\toidtree_clear(&ot);\n }\n \n-static void t_contains(struct oidtree *ot)\n+void test_oidtree__cleanup(void)\n {\n-\tFILL_TREE(ot, \"444\", \"1\", \"2\", \"3\", \"4\", \"5\", \"a\", \"b\", \"c\", \"d\", \"e\");\n-\tcheck_contains(ot, \"44\", 0);\n-\tcheck_contains(ot, \"441\", 0);\n-\tcheck_contains(ot, \"440\", 0);\n-\tcheck_contains(ot, \"444\", 1);\n-\tcheck_contains(ot, \"4440\", 1);\n-\tcheck_contains(ot, \"4444\", 0);\n+\toidtree_clear(&ot);\n }\n \n-static void t_each(struct oidtree *ot)\n+void test_oidtree__contains(void)\n {\n-\tFILL_TREE(ot, \"f\", \"9\", \"8\", \"123\", \"321\", \"320\", \"a\", \"b\", \"c\", \"d\", \"e\");\n-\tcheck_each(ot, \"12300\", \"123\", NULL);\n-\tcheck_each(ot, \"3211\", NULL); /* should not reach callback */\n-\tcheck_each(ot, \"3210\", \"321\", NULL);\n-\tcheck_each(ot, \"32100\", \"321\", NULL);\n-\tcheck_each(ot, \"32\", \"320\", \"321\", NULL);\n+\tFILL_TREE(&ot, \"444\", \"1\", \"2\", \"3\", \"4\", \"5\", \"a\", \"b\", \"c\", \"d\", \"e\");\n+\tcheck_contains(&ot, \"44\", 0);\n+\tcheck_contains(&ot, \"441\", 0);\n+\tcheck_contains(&ot, \"440\", 0);\n+\tcheck_contains(&ot, \"444\", 1);\n+\tcheck_contains(&ot, \"4440\", 1);\n+\tcheck_contains(&ot, \"4444\", 0);\n }\n \n-int cmd_main(int argc UNUSED, const char **argv UNUSED)\n+void test_oidtree__each(void)\n {\n-\tTEST(setup(t_contains), \"oidtree insert and contains works\");\n-\tTEST(setup(t_each), \"oidtree each works\");\n-\treturn test_done();\n+\tFILL_TREE(&ot, \"f\", \"9\", \"8\", \"123\", \"321\", \"320\", \"a\", \"b\", \"c\", \"d\", \"e\");\n+\tcheck_each(&ot, \"12300\", \"123\", NULL);\n+\tcheck_each(&ot, \"3211\", NULL); /* should not reach callback */\n+\tcheck_each(&ot, \"3210\", \"321\", NULL);\n+\tcheck_each(&ot, \"32100\", \"321\", NULL);\n+\tcheck_each(&ot, \"32\", \"320\", \"321\", NULL);\n }\n-- \n2.47.0.86.g15030f9556\n\n"},{"id":"512925","messageId":"xmqqwmdf5is1.fsf@gitster.g","threadId":"62979","inReplyTo":"20250224152704.70289-2-kuforiji98@gmail.com","subject":"Re: [PATCH v2 1/4] t/unit-tests: implement clar specific oid helper functions","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-02-24T17:55:58Z","receivedAt":"2025-02-24T17:56:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Seyi Kuforiji <kuforiji98@gmail.com> writes:\n\n> -static int get_oid_arbitrary_hex_algop(const char *hex, struct object_id *oid,\n> +static void cl_parse_oid(const char *hex, struct object_id *oid,\n>  \t\t\t\t       const struct git_hash_algo *algop)\n>  {\n>  \tint ret;\n>  \tsize_t sz = strlen(hex);\n>  \tstruct strbuf buf = STRBUF_INIT;\n>  \n> -\tif (!check(sz <= algop->hexsz)) {\n> -\t\ttest_msg(\"BUG: hex string (%s) bigger than maximum allowed (%lu)\",\n> -\t\t\t hex, (unsigned long)algop->hexsz);\n> -\t\treturn -1;\n> -\t}\n> +\tcl_assert(sz <= algop->hexsz);\n>  \n>  \tstrbuf_add(&buf, hex, sz);\n>  \tstrbuf_addchars(&buf, '0', algop->hexsz - sz);\n>  \n> -\tret = get_oid_hex_algop(buf.buf, oid, algop);\n> -\tif (!check_int(ret, ==, 0))\n> -\t\ttest_msg(\"BUG: invalid hex input (%s) provided\", hex);\n> +\tcl_assert_equal_i(get_oid_hex_algop(buf.buf, oid, algop), 0);\n>  \n>  \tstrbuf_release(&buf);\n> -\treturn ret;\n>  }\n\nAs you are not returning \"ret\" and making the function void, you\nmade \"int ret\" an unused variable that needs removing.\n\nThanks.\n"},{"id":"512956","messageId":"CAGedMteOTewr9=m6=5NztLA_cDiuq=uEt-A26rFWRCmEw0=_UA@mail.gmail.com","threadId":"62979","inReplyTo":"xmqqwmdf5is1.fsf@gitster.g","subject":"Re: [PATCH v2 1/4] t/unit-tests: implement clar specific oid helper functions","fromName":"Seyi Chamber","fromEmail":"kuforiji98@gmail.com","sentAt":"2025-02-25T07:14:35Z","receivedAt":"2025-02-25T07:14:47Z","isPatch":true,"sender":{"key":"kuforiji98@gmail.com","avatar":"https://avatars.githubusercontent.com/u/94905626?v=4"},"body":"On Mon, 24 Feb 2025 at 18:56, Junio C Hamano <gitster@pobox.com> wrote:\n>\n> Seyi Kuforiji <kuforiji98@gmail.com> writes:\n>\n> > -static int get_oid_arbitrary_hex_algop(const char *hex, struct object_id *oid,\n> > +static void cl_parse_oid(const char *hex, struct object_id *oid,\n> >                                      const struct git_hash_algo *algop)\n> >  {\n> >       int ret;\n> >       size_t sz = strlen(hex);\n> >       struct strbuf buf = STRBUF_INIT;\n> >\n> > -     if (!check(sz <= algop->hexsz)) {\n> > -             test_msg(\"BUG: hex string (%s) bigger than maximum allowed (%lu)\",\n> > -                      hex, (unsigned long)algop->hexsz);\n> > -             return -1;\n> > -     }\n> > +     cl_assert(sz <= algop->hexsz);\n> >\n> >       strbuf_add(&buf, hex, sz);\n> >       strbuf_addchars(&buf, '0', algop->hexsz - sz);\n> >\n> > -     ret = get_oid_hex_algop(buf.buf, oid, algop);\n> > -     if (!check_int(ret, ==, 0))\n> > -             test_msg(\"BUG: invalid hex input (%s) provided\", hex);\n> > +     cl_assert_equal_i(get_oid_hex_algop(buf.buf, oid, algop), 0);\n> >\n> >       strbuf_release(&buf);\n> > -     return ret;\n> >  }\n>\n> As you are not returning \"ret\" and making the function void, you\n> made \"int ret\" an unused variable that needs removing.\n>\n> Thanks.\n\nHi Junio,\n\nDo I send in a new patch series addressing this?\n\nThanks,\nSeyi\n"},{"id":"512966","messageId":"Z713nJNLbEzAU4JM@pks.im","threadId":"62979","inReplyTo":"CAGedMteOTewr9=m6=5NztLA_cDiuq=uEt-A26rFWRCmEw0=_UA@mail.gmail.com","subject":"Re: [PATCH v2 1/4] t/unit-tests: implement clar specific oid helper functions","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-02-25T07:56:12Z","receivedAt":"2025-02-25T07:56:17Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Tue, Feb 25, 2025 at 08:14:35AM +0100, Seyi Chamber wrote:\n> On Mon, 24 Feb 2025 at 18:56, Junio C Hamano <gitster@pobox.com> wrote:\n> >\n> > Seyi Kuforiji <kuforiji98@gmail.com> writes:\n> >\n> > > -static int get_oid_arbitrary_hex_algop(const char *hex, struct object_id *oid,\n> > > +static void cl_parse_oid(const char *hex, struct object_id *oid,\n> > >                                      const struct git_hash_algo *algop)\n> > >  {\n> > >       int ret;\n> > >       size_t sz = strlen(hex);\n> > >       struct strbuf buf = STRBUF_INIT;\n> > >\n> > > -     if (!check(sz <= algop->hexsz)) {\n> > > -             test_msg(\"BUG: hex string (%s) bigger than maximum allowed (%lu)\",\n> > > -                      hex, (unsigned long)algop->hexsz);\n> > > -             return -1;\n> > > -     }\n> > > +     cl_assert(sz <= algop->hexsz);\n> > >\n> > >       strbuf_add(&buf, hex, sz);\n> > >       strbuf_addchars(&buf, '0', algop->hexsz - sz);\n> > >\n> > > -     ret = get_oid_hex_algop(buf.buf, oid, algop);\n> > > -     if (!check_int(ret, ==, 0))\n> > > -             test_msg(\"BUG: invalid hex input (%s) provided\", hex);\n> > > +     cl_assert_equal_i(get_oid_hex_algop(buf.buf, oid, algop), 0);\n> > >\n> > >       strbuf_release(&buf);\n> > > -     return ret;\n> > >  }\n> >\n> > As you are not returning \"ret\" and making the function void, you\n> > made \"int ret\" an unused variable that needs removing.\n> >\n> > Thanks.\n> \n> Hi Junio,\n> \n> Do I send in a new patch series addressing this?\n\nThe expectation would be that you reroll the series and send out a new\nversion thereof that gets rid of the return value in the same commit\nwhere you stop returning it.\n\nThis would typically be the case as long as your series has not yet been\nmerged to `next`. Once it has been merged to `next`, you would have to\nsend an entirely new patch series as a follow-up.\n\nPatrick\n"},{"id":"512997","messageId":"20250225101044.84210-1-kuforiji98@gmail.com","threadId":"62979","inReplyTo":"20250224152704.70289-1-kuforiji98@gmail.com","subject":"[PATCH v3 0/4] t/unit-tests: convert unit-tests to use clar","fromName":"Seyi Kuforiji","fromEmail":"kuforiji98@gmail.com","sentAt":"2025-02-25T10:10:40Z","receivedAt":"2025-02-25T10:12:36Z","isPatch":true,"sender":{"key":"kuforiji98@gmail.com","avatar":"https://avatars.githubusercontent.com/u/94905626?v=4"},"body":"Hello,\n\nThis small patch series transitions a couple more of our existing unit\ntest files to the Clar testing framework. This change is part of our\nongoing effort to standardize our testing framework to enhance\nmaintainability.\n\nChanges in v3:\n - minor code change based on review\n\nThanks\nSeyi\n\nMentored-by: Patrick Steinhardt <ps@pks.im>\nMentored-by: Philip Wood <phillip.wood@dunelm.org.uk>\nSigned-off-by: Seyi Kuforiji <kuforiji98@gmail.com>\n\n\nSeyi Kuforiji (4):\n  t/unit-tests: implement clar specific oid helper functions\n  t/unit-tests: convert oid-array test to use clar test framework\n  t/unit-tests: convert oidmap test to use clar test framework\n  t/unit-tests: convert oidtree test to use clar test framework\n\n Makefile                                      |   8 +-\n t/meson.build                                 |   8 +-\n t/unit-tests/lib-oid.c                        |  32 ++--\n t/unit-tests/lib-oid.h                        |   9 +-\n t/unit-tests/{t-oid-array.c => u-oid-array.c} | 125 +++++++-------\n t/unit-tests/{t-oidmap.c => u-oidmap.c}       | 153 +++++++-----------\n t/unit-tests/{t-oidtree.c => u-oidtree.c}     |  79 ++++-----\n t/unit-tests/unit-test.c                      |   2 +\n 8 files changed, 177 insertions(+), 239 deletions(-)\n rename t/unit-tests/{t-oid-array.c => u-oid-array.c} (34%)\n rename t/unit-tests/{t-oidmap.c => u-oidmap.c} (32%)\n rename t/unit-tests/{t-oidtree.c => u-oidtree.c} (45%)\n\nRange-diff against v2:\n1:  7f14d0d574 ! 1:  c5b6613617 t/unit-tests: implement clar specific oid helper functions\n    @@ t/unit-tests/lib-oid.c: int init_hash_algo(void)\n     +static void cl_parse_oid(const char *hex, struct object_id *oid,\n      \t\t\t\t       const struct git_hash_algo *algop)\n      {\n    - \tint ret;\n    +-\tint ret;\n      \tsize_t sz = strlen(hex);\n      \tstruct strbuf buf = STRBUF_INIT;\n      \n2:  430f5c5007 = 2:  d6cc4985a6 t/unit-tests: convert oid-array test to use clar test framework\n3:  319cea1265 = 3:  1087752df5 t/unit-tests: convert oidmap test to use clar test framework\n4:  ea63a5c9f1 = 4:  cda8dc194c t/unit-tests: convert oidtree test to use clar test framework\n-- \n2.47.0.86.g15030f9556\n\n"},{"id":"512998","messageId":"20250225101044.84210-2-kuforiji98@gmail.com","threadId":"62979","inReplyTo":"20250225101044.84210-1-kuforiji98@gmail.com","subject":"[PATCH v3 1/4] t/unit-tests: implement clar specific oid helper functions","fromName":"Seyi Kuforiji","fromEmail":"kuforiji98@gmail.com","sentAt":"2025-02-25T10:10:41Z","receivedAt":"2025-02-25T10:12:47Z","isPatch":true,"sender":{"key":"kuforiji98@gmail.com","avatar":"https://avatars.githubusercontent.com/u/94905626?v=4"},"body":"`get_oid_arbitrary_hex()` and `init_hash_algo()` are both required for\noid-related tests to run without errors. In the current implementation,\nboth functions are defined and declared in the\n`t/unit-tests/lib-oid.{c,h}` which is utilized by oid-related tests in\nthe homegrown unit tests structure.\n\nAdapt functions in lib-oid.{c,h} to use clar. Both these functions\nbecome available for oid-related test files implemented using the clar\ntesting framework, which requires them. This will be used by subsequent\ncommits.\n\nMentored-by: Patrick Steinhardt <ps@pks.im>\nSigned-off-by: Seyi Kuforiji <kuforiji98@gmail.com>\n---\n Makefile                 |  2 +-\n t/meson.build            |  2 +-\n t/unit-tests/lib-oid.c   | 32 +++++++++++---------------------\n t/unit-tests/lib-oid.h   |  9 ++++++---\n t/unit-tests/unit-test.c |  2 ++\n 5 files changed, 21 insertions(+), 26 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex bcf5ed3f85..81799488f0 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1365,6 +1365,7 @@ CLAR_TEST_PROG = $(UNIT_TEST_BIN)/unit-tests$(X)\n CLAR_TEST_OBJS = $(patsubst %,$(UNIT_TEST_DIR)/%.o,$(CLAR_TEST_SUITES))\n CLAR_TEST_OBJS += $(UNIT_TEST_DIR)/clar/clar.o\n CLAR_TEST_OBJS += $(UNIT_TEST_DIR)/unit-test.o\n+CLAR_TEST_OBJS += $(UNIT_TEST_DIR)/lib-oid.o\n \n UNIT_TEST_PROGRAMS += t-oid-array\n UNIT_TEST_PROGRAMS += t-oidmap\n@@ -1381,7 +1382,6 @@ UNIT_TEST_PROGRAMS += t-trailer\n UNIT_TEST_PROGRAMS += t-urlmatch-normalization\n UNIT_TEST_PROGS = $(patsubst %,$(UNIT_TEST_BIN)/%$X,$(UNIT_TEST_PROGRAMS))\n UNIT_TEST_OBJS += $(UNIT_TEST_DIR)/test-lib.o\n-UNIT_TEST_OBJS += $(UNIT_TEST_DIR)/lib-oid.o\n UNIT_TEST_OBJS += $(UNIT_TEST_DIR)/lib-reftable.o\n \n # xdiff and reftable libs may in turn depend on what is in libgit.a\ndiff --git a/t/meson.build b/t/meson.build\nindex 780939d49f..862cf1cfd4 100644\n--- a/t/meson.build\n+++ b/t/meson.build\n@@ -14,6 +14,7 @@ clar_test_suites = [\n clar_sources = [\n   'unit-tests/clar/clar.c',\n   'unit-tests/unit-test.c',\n+  'unit-tests/lib-oid.c'\n ]\n \n clar_decls_h = custom_target(\n@@ -68,7 +69,6 @@ foreach unit_test_program : unit_test_programs\n   unit_test = executable(unit_test_name,\n     sources: [\n       'unit-tests/test-lib.c',\n-      'unit-tests/lib-oid.c',\n       'unit-tests/lib-reftable.c',\n       unit_test_program,\n     ],\ndiff --git a/t/unit-tests/lib-oid.c b/t/unit-tests/lib-oid.c\nindex 8f0ccac532..e0b3180f23 100644\n--- a/t/unit-tests/lib-oid.c\n+++ b/t/unit-tests/lib-oid.c\n@@ -1,9 +1,9 @@\n-#include \"test-lib.h\"\n+#include \"unit-test.h\"\n #include \"lib-oid.h\"\n #include \"strbuf.h\"\n #include \"hex.h\"\n \n-int init_hash_algo(void)\n+int cl_setup_hash_algo(void)\n {\n \tstatic int algo = -1;\n \n@@ -11,42 +11,32 @@ int init_hash_algo(void)\n \t\tconst char *algo_name = getenv(\"GIT_TEST_DEFAULT_HASH\");\n \t\talgo = algo_name ? hash_algo_by_name(algo_name) : GIT_HASH_SHA1;\n \n-\t\tif (!check(algo != GIT_HASH_UNKNOWN))\n-\t\t\ttest_msg(\"BUG: invalid GIT_TEST_DEFAULT_HASH value ('%s')\",\n-\t\t\t\t algo_name);\n+\t\tcl_assert(algo != GIT_HASH_UNKNOWN);\n \t}\n \treturn algo;\n }\n \n-static int get_oid_arbitrary_hex_algop(const char *hex, struct object_id *oid,\n+static void cl_parse_oid(const char *hex, struct object_id *oid,\n \t\t\t\t       const struct git_hash_algo *algop)\n {\n-\tint ret;\n \tsize_t sz = strlen(hex);\n \tstruct strbuf buf = STRBUF_INIT;\n \n-\tif (!check(sz <= algop->hexsz)) {\n-\t\ttest_msg(\"BUG: hex string (%s) bigger than maximum allowed (%lu)\",\n-\t\t\t hex, (unsigned long)algop->hexsz);\n-\t\treturn -1;\n-\t}\n+\tcl_assert(sz <= algop->hexsz);\n \n \tstrbuf_add(&buf, hex, sz);\n \tstrbuf_addchars(&buf, '0', algop->hexsz - sz);\n \n-\tret = get_oid_hex_algop(buf.buf, oid, algop);\n-\tif (!check_int(ret, ==, 0))\n-\t\ttest_msg(\"BUG: invalid hex input (%s) provided\", hex);\n+\tcl_assert_equal_i(get_oid_hex_algop(buf.buf, oid, algop), 0);\n \n \tstrbuf_release(&buf);\n-\treturn ret;\n }\n \n-int get_oid_arbitrary_hex(const char *hex, struct object_id *oid)\n+\n+void cl_parse_any_oid(const char *hex, struct object_id *oid)\n {\n-\tint hash_algo = init_hash_algo();\n+\tint hash_algo = cl_setup_hash_algo();\n \n-\tif (!check_int(hash_algo, !=, GIT_HASH_UNKNOWN))\n-\t\treturn -1;\n-\treturn get_oid_arbitrary_hex_algop(hex, oid, &hash_algos[hash_algo]);\n+\tcl_assert(hash_algo != GIT_HASH_UNKNOWN);\n+\tcl_parse_oid(hex, oid, &hash_algos[hash_algo]);\n }\ndiff --git a/t/unit-tests/lib-oid.h b/t/unit-tests/lib-oid.h\nindex 4e77c04bd2..4031775104 100644\n--- a/t/unit-tests/lib-oid.h\n+++ b/t/unit-tests/lib-oid.h\n@@ -5,6 +5,7 @@\n \n /*\n  * Convert arbitrary hex string to object_id.\n+ *\n  * For example, passing \"abc12\" will generate\n  * \"abc1200000000000000000000000000000000000\" hex of length 40 for SHA-1 and\n  * create object_id with that.\n@@ -12,14 +13,16 @@\n  * algo is not allowed. The hash algo is decided based on GIT_TEST_DEFAULT_HASH\n  * environment variable.\n  */\n-int get_oid_arbitrary_hex(const char *s, struct object_id *oid);\n+\n+void cl_parse_any_oid (const char *s, struct object_id *oid);\n /*\n  * Returns one of GIT_HASH_{SHA1, SHA256, UNKNOWN} based on the value of\n  * GIT_TEST_DEFAULT_HASH environment variable. The fallback value in the\n  * absence of GIT_TEST_DEFAULT_HASH is GIT_HASH_SHA1. It also uses\n- * check(algo != GIT_HASH_UNKNOWN) before returning to verify if the\n+ * cl_assert(algo != GIT_HASH_UNKNOWN) before returning to verify if the\n  * GIT_TEST_DEFAULT_HASH's value is valid or not.\n  */\n-int init_hash_algo(void);\n+\n+int cl_setup_hash_algo(void);\n \n #endif /* LIB_OID_H */\ndiff --git a/t/unit-tests/unit-test.c b/t/unit-tests/unit-test.c\nindex fa8818842a..5af645048a 100644\n--- a/t/unit-tests/unit-test.c\n+++ b/t/unit-tests/unit-test.c\n@@ -1,5 +1,7 @@\n #include \"unit-test.h\"\n+#include \"hex.h\"\n #include \"parse-options.h\"\n+#include \"strbuf.h\"\n #include \"string-list.h\"\n #include \"strvec.h\"\n \n-- \n2.47.0.86.g15030f9556\n\n"},{"id":"512999","messageId":"20250225101044.84210-3-kuforiji98@gmail.com","threadId":"62979","inReplyTo":"20250225101044.84210-1-kuforiji98@gmail.com","subject":"[PATCH v3 2/4] t/unit-tests: convert oid-array test to use clar test framework","fromName":"Seyi Kuforiji","fromEmail":"kuforiji98@gmail.com","sentAt":"2025-02-25T10:10:42Z","receivedAt":"2025-02-25T10:12:51Z","isPatch":true,"sender":{"key":"kuforiji98@gmail.com","avatar":"https://avatars.githubusercontent.com/u/94905626?v=4"},"body":"Adapt oid-array test script to clar framework by using clar assertions\nwhere necessary. Remove descriptions from macros to reduce\nredundancy, and move test input arrays to global scope for reuse across\nmultiple test functions. Introduce `test_oid_array__initialize()` to\nexplicitly initialize the hash algorithm.\n\nThese changes streamline the test suite, making individual tests\nself-contained and reducing redundant code.\n\nMentored-by: Patrick Steinhardt <ps@pks.im>\nSigned-off-by: Seyi Kuforiji <kuforiji98@gmail.com>\n---\n Makefile                                      |   2 +-\n t/meson.build                                 |   2 +-\n t/unit-tests/{t-oid-array.c => u-oid-array.c} | 125 +++++++++---------\n 3 files changed, 66 insertions(+), 63 deletions(-)\n rename t/unit-tests/{t-oid-array.c => u-oid-array.c} (34%)\n\ndiff --git a/Makefile b/Makefile\nindex 81799488f0..c9b0320fcb 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1356,6 +1356,7 @@ CLAR_TEST_SUITES += u-example-decorate\n CLAR_TEST_SUITES += u-hash\n CLAR_TEST_SUITES += u-hashmap\n CLAR_TEST_SUITES += u-mem-pool\n+CLAR_TEST_SUITES += u-oid-array\n CLAR_TEST_SUITES += u-prio-queue\n CLAR_TEST_SUITES += u-reftable-tree\n CLAR_TEST_SUITES += u-strbuf\n@@ -1367,7 +1368,6 @@ CLAR_TEST_OBJS += $(UNIT_TEST_DIR)/clar/clar.o\n CLAR_TEST_OBJS += $(UNIT_TEST_DIR)/unit-test.o\n CLAR_TEST_OBJS += $(UNIT_TEST_DIR)/lib-oid.o\n \n-UNIT_TEST_PROGRAMS += t-oid-array\n UNIT_TEST_PROGRAMS += t-oidmap\n UNIT_TEST_PROGRAMS += t-oidtree\n UNIT_TEST_PROGRAMS += t-reftable-basics\ndiff --git a/t/meson.build b/t/meson.build\nindex 862cf1cfd4..1b17a53acc 100644\n--- a/t/meson.build\n+++ b/t/meson.build\n@@ -4,6 +4,7 @@ clar_test_suites = [\n   'unit-tests/u-hash.c',\n   'unit-tests/u-hashmap.c',\n   'unit-tests/u-mem-pool.c',\n+  'unit-tests/u-oid-array.c',\n   'unit-tests/u-prio-queue.c',\n   'unit-tests/u-reftable-tree.c',\n   'unit-tests/u-strbuf.c',\n@@ -49,7 +50,6 @@ clar_unit_tests = executable('unit-tests',\n test('unit-tests', clar_unit_tests)\n \n unit_test_programs = [\n-  'unit-tests/t-oid-array.c',\n   'unit-tests/t-oidmap.c',\n   'unit-tests/t-oidtree.c',\n   'unit-tests/t-reftable-basics.c',\ndiff --git a/t/unit-tests/t-oid-array.c b/t/unit-tests/u-oid-array.c\nsimilarity index 34%\nrename from t/unit-tests/t-oid-array.c\nrename to t/unit-tests/u-oid-array.c\nindex 45b59a2a51..e48a433f21 100644\n--- a/t/unit-tests/t-oid-array.c\n+++ b/t/unit-tests/u-oid-array.c\n@@ -1,22 +1,19 @@\n #define USE_THE_REPOSITORY_VARIABLE\n \n-#include \"test-lib.h\"\n+#include \"unit-test.h\"\n #include \"lib-oid.h\"\n #include \"oid-array.h\"\n #include \"hex.h\"\n \n-static int fill_array(struct oid_array *array, const char *hexes[], size_t n)\n+static void fill_array(struct oid_array *array, const char *hexes[], size_t n)\n {\n \tfor (size_t i = 0; i < n; i++) {\n \t\tstruct object_id oid;\n \n-\t\tif (!check_int(get_oid_arbitrary_hex(hexes[i], &oid), ==, 0))\n-\t\t\treturn -1;\n+\t\tcl_parse_any_oid(hexes[i], &oid);\n \t\toid_array_append(array, &oid);\n \t}\n-\tif (!check_uint(array->nr, ==, n))\n-\t\treturn -1;\n-\treturn 0;\n+\tcl_assert_equal_i(array->nr, n);\n }\n \n static int add_to_oid_array(const struct object_id *oid, void *data)\n@@ -34,30 +31,22 @@ static void t_enumeration(const char **input_args, size_t input_sz,\n \t\t\t actual = OID_ARRAY_INIT;\n \tsize_t i;\n \n-\tif (fill_array(&input, input_args, input_sz))\n-\t\treturn;\n-\tif (fill_array(&expect, expect_args, expect_sz))\n-\t\treturn;\n+\tfill_array(&input, input_args, input_sz);\n+\tfill_array(&expect, expect_args, expect_sz);\n \n \toid_array_for_each_unique(&input, add_to_oid_array, &actual);\n-\tif (!check_uint(actual.nr, ==, expect.nr))\n-\t\treturn;\n-\n-\tfor (i = 0; i < actual.nr; i++) {\n-\t\tif (!check(oideq(&actual.oid[i], &expect.oid[i])))\n-\t\t\ttest_msg(\"expected: %s\\n       got: %s\\n     index: %\" PRIuMAX,\n-\t\t\t\t oid_to_hex(&expect.oid[i]), oid_to_hex(&actual.oid[i]),\n-\t\t\t\t (uintmax_t)i);\n-\t}\n+\tcl_assert_equal_i(actual.nr, expect.nr);\n+\n+\tfor (i = 0; i < actual.nr; i++)\n+\t\tcl_assert(oideq(&actual.oid[i], &expect.oid[i]));\n \n \toid_array_clear(&actual);\n \toid_array_clear(&input);\n \toid_array_clear(&expect);\n }\n \n-#define TEST_ENUMERATION(input, expect, desc)                                     \\\n-\tTEST(t_enumeration(input, ARRAY_SIZE(input), expect, ARRAY_SIZE(expect)), \\\n-\t\t\t   desc \" works\")\n+#define TEST_ENUMERATION(input, expect)                                     \\\n+\tt_enumeration(input, ARRAY_SIZE(input), expect, ARRAY_SIZE(expect));\n \n static void t_lookup(const char **input_hexes, size_t n, const char *query_hex,\n \t\t     int lower_bound, int upper_bound)\n@@ -66,61 +55,75 @@ static void t_lookup(const char **input_hexes, size_t n, const char *query_hex,\n \tstruct object_id oid_query;\n \tint ret;\n \n-\tif (!check_int(get_oid_arbitrary_hex(query_hex, &oid_query), ==, 0))\n-\t\treturn;\n-\tif (fill_array(&array, input_hexes, n))\n-\t\treturn;\n+\tcl_parse_any_oid(query_hex, &oid_query);\n+\tfill_array(&array, input_hexes, n);\n \tret = oid_array_lookup(&array, &oid_query);\n \n-\tif (!check_int(ret, <=, upper_bound) ||\n-\t    !check_int(ret, >=, lower_bound))\n-\t\ttest_msg(\"oid query for lookup: %s\", oid_to_hex(&oid_query));\n+\tcl_assert(ret <= upper_bound);\n+\tcl_assert(ret >= lower_bound);\n \n \toid_array_clear(&array);\n }\n \n-#define TEST_LOOKUP(input_hexes, query, lower_bound, upper_bound, desc) \\\n-\tTEST(t_lookup(input_hexes, ARRAY_SIZE(input_hexes), query,      \\\n-\t\t      lower_bound, upper_bound),                        \\\n-\t     desc \" works\")\n+#define TEST_LOOKUP(input_hexes, query, lower_bound, upper_bound) \\\n+\tt_lookup(input_hexes, ARRAY_SIZE(input_hexes), query,      \\\n+\t\t      lower_bound, upper_bound);\n \n-static void setup(void)\n+void test_oid_array__initialize(void)\n {\n \t/* The hash algo is used by oid_array_lookup() internally */\n-\tint algo = init_hash_algo();\n-\tif (check_int(algo, !=, GIT_HASH_UNKNOWN))\n-\t\trepo_set_hash_algo(the_repository, algo);\n+\tint algo = cl_setup_hash_algo();\n+\trepo_set_hash_algo(the_repository, algo);\n }\n \n-int cmd_main(int argc UNUSED, const char **argv UNUSED)\n+static const char *arr_input[] = { \"88\", \"44\", \"aa\", \"55\" };\n+static const char *arr_input_dup[] = { \"88\", \"44\", \"aa\", \"55\",\n+\t\t\t\t       \"88\", \"44\", \"aa\", \"55\",\n+\t\t\t\t       \"88\", \"44\", \"aa\", \"55\" };\n+static const char *res_sorted[] = { \"44\", \"55\", \"88\", \"aa\" };\n+\n+void test_oid_array__enumerate_unique(void)\n {\n-\tconst char *arr_input[] = { \"88\", \"44\", \"aa\", \"55\" };\n-\tconst char *arr_input_dup[] = { \"88\", \"44\", \"aa\", \"55\",\n-\t\t\t\t\t\"88\", \"44\", \"aa\", \"55\",\n-\t\t\t\t\t\"88\", \"44\", \"aa\", \"55\" };\n-\tconst char *res_sorted[] = { \"44\", \"55\", \"88\", \"aa\" };\n-\tconst char *nearly_55;\n+\tTEST_ENUMERATION(arr_input, res_sorted);\n+}\n+\n+void test_oid_array__enumerate_duplicate(void)\n+{\n+\tTEST_ENUMERATION(arr_input_dup, res_sorted);\n+}\n+\n+void test_oid_array__lookup(void)\n+{\n+\tTEST_LOOKUP(arr_input, \"55\", 1, 1);\n+}\n \n-\tif (!TEST(setup(), \"setup\"))\n-\t\ttest_skip_all(\"hash algo initialization failed\");\n+void test_oid_array__lookup_non_existent(void)\n+{\n+\tTEST_LOOKUP(arr_input, \"33\", INT_MIN, -1);\n+}\n+\n+void test_oid_array__lookup_duplicates(void)\n+{\n+\tTEST_LOOKUP(arr_input_dup, \"55\", 3, 5);\n+}\n \n-\tTEST_ENUMERATION(arr_input, res_sorted, \"ordered enumeration\");\n-\tTEST_ENUMERATION(arr_input_dup, res_sorted,\n-\t\t\t \"ordered enumeration with duplicate suppression\");\n+void test_oid_array__lookup_non_existent_dup(void)\n+{\n+\tTEST_LOOKUP(arr_input_dup, \"66\", INT_MIN, -1);\n+}\n \n-\tTEST_LOOKUP(arr_input, \"55\", 1, 1, \"lookup\");\n-\tTEST_LOOKUP(arr_input, \"33\", INT_MIN, -1, \"lookup non-existent entry\");\n-\tTEST_LOOKUP(arr_input_dup, \"55\", 3, 5, \"lookup with duplicates\");\n-\tTEST_LOOKUP(arr_input_dup, \"66\", INT_MIN, -1,\n-\t\t    \"lookup non-existent entry with duplicates\");\n+void test_oid_array__lookup_almost_dup(void)\n+{\n+\tconst char *nearly_55;\n \n-\tnearly_55 = init_hash_algo() == GIT_HASH_SHA1 ?\n+\tnearly_55 = cl_setup_hash_algo() == GIT_HASH_SHA1 ?\n \t\t\t\"5500000000000000000000000000000000000001\" :\n \t\t\t\"5500000000000000000000000000000000000000000000000000000000000001\";\n-\tTEST_LOOKUP(((const char *[]){ \"55\", nearly_55 }), \"55\", 0, 0,\n-\t\t    \"lookup with almost duplicate values\");\n-\tTEST_LOOKUP(((const char *[]){ \"55\", \"55\" }), \"55\", 0, 1,\n-\t\t    \"lookup with single duplicate value\");\n \n-\treturn test_done();\n+\tTEST_LOOKUP(((const char *[]){ \"55\", nearly_55 }), \"55\", 0, 0);\n+}\n+\n+void test_oid_array__lookup_single_dup(void)\n+{\n+\tTEST_LOOKUP(((const char *[]){ \"55\", \"55\" }), \"55\", 0, 1);\n }\n-- \n2.47.0.86.g15030f9556\n\n"},{"id":"513000","messageId":"20250225101044.84210-4-kuforiji98@gmail.com","threadId":"62979","inReplyTo":"20250225101044.84210-1-kuforiji98@gmail.com","subject":"[PATCH v3 3/4] t/unit-tests: convert oidmap test to use clar test framework","fromName":"Seyi Kuforiji","fromEmail":"kuforiji98@gmail.com","sentAt":"2025-02-25T10:10:43Z","receivedAt":"2025-02-25T10:12:56Z","isPatch":true,"sender":{"key":"kuforiji98@gmail.com","avatar":"https://avatars.githubusercontent.com/u/94905626?v=4"},"body":"Adapt oidmap test script to clar framework by using clar assertions\nwhere necessary. `cl_parse_any_oid()` ensures the hash algorithm is set\nbefore parsing. This prevents issues from an uninitialized or invalid\nhash algorithm.\n\nIntroduce 'test_oidmap__initialize` handles the to set up of the global\noidmap map with predefined key-value pairs, and `test_oidmap__cleanup`\nfrees the oidmap and its entries when all tests are completed.\n\nThe test loops through all entries to detect multiple errors. With this\nchange, it stops at the first error encountered, making it easier to\naddress it.\n\nMentored-by: Patrick Steinhardt <ps@pks.im>\nSigned-off-by: Seyi Kuforiji <kuforiji98@gmail.com>\n---\n Makefile                                |   2 +-\n t/meson.build                           |   2 +-\n t/unit-tests/{t-oidmap.c => u-oidmap.c} | 153 +++++++++---------------\n 3 files changed, 56 insertions(+), 101 deletions(-)\n rename t/unit-tests/{t-oidmap.c => u-oidmap.c} (32%)\n\ndiff --git a/Makefile b/Makefile\nindex c9b0320fcb..e4e85e6007 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1357,6 +1357,7 @@ CLAR_TEST_SUITES += u-hash\n CLAR_TEST_SUITES += u-hashmap\n CLAR_TEST_SUITES += u-mem-pool\n CLAR_TEST_SUITES += u-oid-array\n+CLAR_TEST_SUITES += u-oidmap\n CLAR_TEST_SUITES += u-prio-queue\n CLAR_TEST_SUITES += u-reftable-tree\n CLAR_TEST_SUITES += u-strbuf\n@@ -1368,7 +1369,6 @@ CLAR_TEST_OBJS += $(UNIT_TEST_DIR)/clar/clar.o\n CLAR_TEST_OBJS += $(UNIT_TEST_DIR)/unit-test.o\n CLAR_TEST_OBJS += $(UNIT_TEST_DIR)/lib-oid.o\n \n-UNIT_TEST_PROGRAMS += t-oidmap\n UNIT_TEST_PROGRAMS += t-oidtree\n UNIT_TEST_PROGRAMS += t-reftable-basics\n UNIT_TEST_PROGRAMS += t-reftable-block\ndiff --git a/t/meson.build b/t/meson.build\nindex 1b17a53acc..d5b83cdb72 100644\n--- a/t/meson.build\n+++ b/t/meson.build\n@@ -5,6 +5,7 @@ clar_test_suites = [\n   'unit-tests/u-hashmap.c',\n   'unit-tests/u-mem-pool.c',\n   'unit-tests/u-oid-array.c',\n+  'unit-tests/u-oidmap.c',\n   'unit-tests/u-prio-queue.c',\n   'unit-tests/u-reftable-tree.c',\n   'unit-tests/u-strbuf.c',\n@@ -50,7 +51,6 @@ clar_unit_tests = executable('unit-tests',\n test('unit-tests', clar_unit_tests)\n \n unit_test_programs = [\n-  'unit-tests/t-oidmap.c',\n   'unit-tests/t-oidtree.c',\n   'unit-tests/t-reftable-basics.c',\n   'unit-tests/t-reftable-block.c',\ndiff --git a/t/unit-tests/t-oidmap.c b/t/unit-tests/u-oidmap.c\nsimilarity index 32%\nrename from t/unit-tests/t-oidmap.c\nrename to t/unit-tests/u-oidmap.c\nindex b22e52d08b..dc805b7e3c 100644\n--- a/t/unit-tests/t-oidmap.c\n+++ b/t/unit-tests/u-oidmap.c\n@@ -1,4 +1,4 @@\n-#include \"test-lib.h\"\n+#include \"unit-test.h\"\n #include \"lib-oid.h\"\n #include \"oidmap.h\"\n #include \"hash.h\"\n@@ -18,102 +18,85 @@ static const char *const key_val[][2] = { { \"11\", \"one\" },\n \t\t\t\t\t  { \"22\", \"two\" },\n \t\t\t\t\t  { \"33\", \"three\" } };\n \n-static void setup(void (*f)(struct oidmap *map))\n+static struct oidmap map;\n+\n+void test_oidmap__initialize(void)\n {\n-\tstruct oidmap map = OIDMAP_INIT;\n-\tint ret = 0;\n+\toidmap_init(&map, 0);\n \n \tfor (size_t i = 0; i < ARRAY_SIZE(key_val); i++){\n \t\tstruct test_entry *entry;\n \n \t\tFLEX_ALLOC_STR(entry, name, key_val[i][1]);\n-\t\tif ((ret = get_oid_arbitrary_hex(key_val[i][0], &entry->entry.oid))) {\n-\t\t\tfree(entry);\n-\t\t\tbreak;\n-\t\t}\n-\t\tentry = oidmap_put(&map, entry);\n-\t\tif (!check(entry == NULL))\n-\t\t\tfree(entry);\n+\t\tcl_parse_any_oid(key_val[i][0], &entry->entry.oid);\n+\t\tcl_assert(oidmap_put(&map, entry) == NULL);\n \t}\n+}\n \n-\tif (!ret)\n-\t\tf(&map);\n+void test_oidmap__cleanup(void)\n+{\n \toidmap_free(&map, 1);\n }\n \n-static void t_replace(struct oidmap *map)\n+void test_oidmap__replace(void)\n {\n \tstruct test_entry *entry, *prev;\n \n \tFLEX_ALLOC_STR(entry, name, \"un\");\n-\tif (get_oid_arbitrary_hex(\"11\", &entry->entry.oid))\n-\t\treturn;\n-\tprev = oidmap_put(map, entry);\n-\tif (!check(prev != NULL))\n-\t\treturn;\n-\tcheck_str(prev->name, \"one\");\n+\tcl_parse_any_oid(\"11\", &entry->entry.oid);\n+\tprev = oidmap_put(&map, entry);\n+\tcl_assert(prev != NULL);\n+\tcl_assert_equal_s(prev->name, \"one\");\n \tfree(prev);\n \n \tFLEX_ALLOC_STR(entry, name, \"deux\");\n-\tif (get_oid_arbitrary_hex(\"22\", &entry->entry.oid))\n-\t\treturn;\n-\tprev = oidmap_put(map, entry);\n-\tif (!check(prev != NULL))\n-\t\treturn;\n-\tcheck_str(prev->name, \"two\");\n+\tcl_parse_any_oid(\"22\", &entry->entry.oid);\n+\tprev = oidmap_put(&map, entry);\n+\tcl_assert(prev != NULL);\n+\tcl_assert_equal_s(prev->name, \"two\");\n \tfree(prev);\n }\n \n-static void t_get(struct oidmap *map)\n+void test_oidmap__get(void)\n {\n \tstruct test_entry *entry;\n \tstruct object_id oid;\n \n-\tif (get_oid_arbitrary_hex(\"22\", &oid))\n-\t\treturn;\n-\tentry = oidmap_get(map, &oid);\n-\tif (!check(entry != NULL))\n-\t\treturn;\n-\tcheck_str(entry->name, \"two\");\n-\n-\tif (get_oid_arbitrary_hex(\"44\", &oid))\n-\t\treturn;\n-\tcheck(oidmap_get(map, &oid) == NULL);\n-\n-\tif (get_oid_arbitrary_hex(\"11\", &oid))\n-\t\treturn;\n-\tentry = oidmap_get(map, &oid);\n-\tif (!check(entry != NULL))\n-\t\treturn;\n-\tcheck_str(entry->name, \"one\");\n+\tcl_parse_any_oid(\"22\", &oid);\n+\tentry = oidmap_get(&map, &oid);\n+\tcl_assert(entry != NULL);\n+\tcl_assert_equal_s(entry->name, \"two\");\n+\n+\tcl_parse_any_oid(\"44\", &oid);\n+\tcl_assert(oidmap_get(&map, &oid) == NULL);\n+\n+\tcl_parse_any_oid(\"11\", &oid);\n+\tentry = oidmap_get(&map, &oid);\n+\tcl_assert(entry != NULL);\n+\tcl_assert_equal_s(entry->name, \"one\");\n }\n \n-static void t_remove(struct oidmap *map)\n+void test_oidmap__remove(void)\n {\n \tstruct test_entry *entry;\n \tstruct object_id oid;\n \n-\tif (get_oid_arbitrary_hex(\"11\", &oid))\n-\t\treturn;\n-\tentry = oidmap_remove(map, &oid);\n-\tif (!check(entry != NULL))\n-\t\treturn;\n-\tcheck_str(entry->name, \"one\");\n-\tcheck(oidmap_get(map, &oid) == NULL);\n+\tcl_parse_any_oid(\"11\", &oid);\n+\tentry = oidmap_remove(&map, &oid);\n+\tcl_assert(entry != NULL);\n+\tcl_assert_equal_s(entry->name, \"one\");\n+\tcl_assert(oidmap_get(&map, &oid) == NULL);\n \tfree(entry);\n \n-\tif (get_oid_arbitrary_hex(\"22\", &oid))\n-\t\treturn;\n-\tentry = oidmap_remove(map, &oid);\n-\tif (!check(entry != NULL))\n-\t\treturn;\n-\tcheck_str(entry->name, \"two\");\n-\tcheck(oidmap_get(map, &oid) == NULL);\n+\tcl_parse_any_oid(\"22\", &oid);\n+\tentry = oidmap_remove(&map, &oid);\n+\tcl_assert(entry != NULL);\n+\tcl_assert_equal_s(entry->name, \"two\");\n+\tcl_assert(oidmap_get(&map, &oid) == NULL);\n \tfree(entry);\n \n-\tif (get_oid_arbitrary_hex(\"44\", &oid))\n-\t\treturn;\n-\tcheck(oidmap_remove(map, &oid) == NULL);\n+\tcl_parse_any_oid(\"44\", &oid);\n+\tcl_assert(oidmap_remove(&map, &oid) == NULL);\n }\n \n static int key_val_contains(struct test_entry *entry, char seen[])\n@@ -121,8 +104,7 @@ static int key_val_contains(struct test_entry *entry, char seen[])\n \tfor (size_t i = 0; i < ARRAY_SIZE(key_val); i++) {\n \t\tstruct object_id oid;\n \n-\t\tif (get_oid_arbitrary_hex(key_val[i][0], &oid))\n-\t\t\treturn -1;\n+\t\tcl_parse_any_oid(key_val[i][0], &oid);\n \n \t\tif (oideq(&entry->entry.oid, &oid)) {\n \t\t\tif (seen[i])\n@@ -134,48 +116,21 @@ static int key_val_contains(struct test_entry *entry, char seen[])\n \treturn 1;\n }\n \n-static void t_iterate(struct oidmap *map)\n+void test_oidmap__iterate(void)\n {\n \tstruct oidmap_iter iter;\n \tstruct test_entry *entry;\n \tchar seen[ARRAY_SIZE(key_val)] = { 0 };\n \tint count = 0;\n \n-\toidmap_iter_init(map, &iter);\n+\toidmap_iter_init(&map, &iter);\n \twhile ((entry = oidmap_iter_next(&iter))) {\n-\t\tint ret;\n-\t\tif (!check_int((ret = key_val_contains(entry, seen)), ==, 0)) {\n-\t\t\tswitch (ret) {\n-\t\t\tcase -1:\n-\t\t\t\tbreak; /* error message handled by get_oid_arbitrary_hex() */\n-\t\t\tcase 1:\n-\t\t\t\ttest_msg(\"obtained entry was not given in the input\\n\"\n-\t\t\t\t\t \"  name: %s\\n   oid: %s\\n\",\n-\t\t\t\t\t entry->name, oid_to_hex(&entry->entry.oid));\n-\t\t\t\tbreak;\n-\t\t\tcase 2:\n-\t\t\t\ttest_msg(\"duplicate entry detected\\n\"\n-\t\t\t\t\t \"  name: %s\\n   oid: %s\\n\",\n-\t\t\t\t\t entry->name, oid_to_hex(&entry->entry.oid));\n-\t\t\t\tbreak;\n-\t\t\tdefault:\n-\t\t\t\ttest_msg(\"BUG: invalid return value (%d) from key_val_contains()\",\n-\t\t\t\t\t ret);\n-\t\t\t\tbreak;\n-\t\t\t}\n-\t\t} else {\n-\t\t\tcount++;\n+\t\tif (key_val_contains(entry, seen) != 0) {\n+\t\t\tcl_failf(\"Unexpected entry: name = %s, oid = %s\",\n+\t\t\t\t entry->name, oid_to_hex(&entry->entry.oid));\n \t\t}\n+\t\tcount++;\n \t}\n-\tcheck_int(count, ==, ARRAY_SIZE(key_val));\n-\tcheck_int(hashmap_get_size(&map->map), ==, ARRAY_SIZE(key_val));\n-}\n-\n-int cmd_main(int argc UNUSED, const char **argv UNUSED)\n-{\n-\tTEST(setup(t_replace), \"replace works\");\n-\tTEST(setup(t_get), \"get works\");\n-\tTEST(setup(t_remove), \"remove works\");\n-\tTEST(setup(t_iterate), \"iterate works\");\n-\treturn test_done();\n+\tcl_assert_equal_i(count, ARRAY_SIZE(key_val));\n+\tcl_assert_equal_i(hashmap_get_size(&map.map), ARRAY_SIZE(key_val));\n }\n-- \n2.47.0.86.g15030f9556\n\n"},{"id":"513001","messageId":"20250225101044.84210-5-kuforiji98@gmail.com","threadId":"62979","inReplyTo":"20250225101044.84210-1-kuforiji98@gmail.com","subject":"[PATCH v3 4/4] t/unit-tests: convert oidtree test to use clar test framework","fromName":"Seyi Kuforiji","fromEmail":"kuforiji98@gmail.com","sentAt":"2025-02-25T10:10:44Z","receivedAt":"2025-02-25T10:13:01Z","isPatch":true,"sender":{"key":"kuforiji98@gmail.com","avatar":"https://avatars.githubusercontent.com/u/94905626?v=4"},"body":"Adapt oidtree test script to clar framework by using clar assertions\nwhere necessary. `cl_parse_any_oid()` ensures the hash algorithm is set\nbefore parsing. This prevents issues from an uninitialized or invalid\nhash algorithm.\n\nIntroduce 'test_oidtree__initialize` handles the to set up of the global\noidtree variable and `test_oidtree__cleanup` frees the oidtree when all\ntests are completed.\n\nWith this change, `check_each` stops at the first error encountered,\nmaking it easier to address it.\n\nMentored-by: Patrick Steinhardt <ps@pks.im>\nSigned-off-by: Seyi Kuforiji <kuforiji98@gmail.com>\n---\n Makefile                                  |  2 +-\n t/meson.build                             |  2 +-\n t/unit-tests/{t-oidtree.c => u-oidtree.c} | 79 +++++++++--------------\n 3 files changed, 34 insertions(+), 49 deletions(-)\n rename t/unit-tests/{t-oidtree.c => u-oidtree.c} (45%)\n\ndiff --git a/Makefile b/Makefile\nindex e4e85e6007..2b134efc70 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1358,6 +1358,7 @@ CLAR_TEST_SUITES += u-hashmap\n CLAR_TEST_SUITES += u-mem-pool\n CLAR_TEST_SUITES += u-oid-array\n CLAR_TEST_SUITES += u-oidmap\n+CLAR_TEST_SUITES += u-oidtree\n CLAR_TEST_SUITES += u-prio-queue\n CLAR_TEST_SUITES += u-reftable-tree\n CLAR_TEST_SUITES += u-strbuf\n@@ -1369,7 +1370,6 @@ CLAR_TEST_OBJS += $(UNIT_TEST_DIR)/clar/clar.o\n CLAR_TEST_OBJS += $(UNIT_TEST_DIR)/unit-test.o\n CLAR_TEST_OBJS += $(UNIT_TEST_DIR)/lib-oid.o\n \n-UNIT_TEST_PROGRAMS += t-oidtree\n UNIT_TEST_PROGRAMS += t-reftable-basics\n UNIT_TEST_PROGRAMS += t-reftable-block\n UNIT_TEST_PROGRAMS += t-reftable-merged\ndiff --git a/t/meson.build b/t/meson.build\nindex d5b83cdb72..91699917ff 100644\n--- a/t/meson.build\n+++ b/t/meson.build\n@@ -6,6 +6,7 @@ clar_test_suites = [\n   'unit-tests/u-mem-pool.c',\n   'unit-tests/u-oid-array.c',\n   'unit-tests/u-oidmap.c',\n+  'unit-tests/u-oidtree.c',\n   'unit-tests/u-prio-queue.c',\n   'unit-tests/u-reftable-tree.c',\n   'unit-tests/u-strbuf.c',\n@@ -51,7 +52,6 @@ clar_unit_tests = executable('unit-tests',\n test('unit-tests', clar_unit_tests)\n \n unit_test_programs = [\n-  'unit-tests/t-oidtree.c',\n   'unit-tests/t-reftable-basics.c',\n   'unit-tests/t-reftable-block.c',\n   'unit-tests/t-reftable-merged.c',\ndiff --git a/t/unit-tests/t-oidtree.c b/t/unit-tests/u-oidtree.c\nsimilarity index 45%\nrename from t/unit-tests/t-oidtree.c\nrename to t/unit-tests/u-oidtree.c\nindex a38754b066..e6eede2740 100644\n--- a/t/unit-tests/t-oidtree.c\n+++ b/t/unit-tests/u-oidtree.c\n@@ -1,10 +1,12 @@\n-#include \"test-lib.h\"\n+#include \"unit-test.h\"\n #include \"lib-oid.h\"\n #include \"oidtree.h\"\n #include \"hash.h\"\n #include \"hex.h\"\n #include \"strvec.h\"\n \n+static struct oidtree ot;\n+\n #define FILL_TREE(tree, ...)                                       \\\n \tdo {                                                       \\\n \t\tconst char *hexes[] = { __VA_ARGS__ };             \\\n@@ -16,8 +18,7 @@ static int fill_tree_loc(struct oidtree *ot, const char *hexes[], size_t n)\n {\n \tfor (size_t i = 0; i < n; i++) {\n \t\tstruct object_id oid;\n-\t\tif (!check_int(get_oid_arbitrary_hex(hexes[i], &oid), ==, 0))\n-\t\t\treturn -1;\n+\t\tcl_parse_any_oid(hexes[i], &oid);\n \t\toidtree_insert(ot, &oid);\n \t}\n \treturn 0;\n@@ -27,10 +28,8 @@ static void check_contains(struct oidtree *ot, const char *hex, int expected)\n {\n \tstruct object_id oid;\n \n-\tif (!check_int(get_oid_arbitrary_hex(hex, &oid), ==, 0))\n-\t\treturn;\n-\tif (!check_int(oidtree_contains(ot, &oid), ==, expected))\n-\t\ttest_msg(\"oid: %s\", oid_to_hex(&oid));\n+\tcl_parse_any_oid(hex, &oid);\n+\tcl_assert_equal_i(oidtree_contains(ot, &oid), expected);\n }\n \n struct expected_hex_iter {\n@@ -44,19 +43,11 @@ static enum cb_next check_each_cb(const struct object_id *oid, void *data)\n \tstruct expected_hex_iter *hex_iter = data;\n \tstruct object_id expected;\n \n-\tif (!check_int(hex_iter->i, <, hex_iter->expected_hexes.nr)) {\n-\t\ttest_msg(\"error: extraneous callback for query: ('%s'), object_id: ('%s')\",\n-\t\t\t hex_iter->query, oid_to_hex(oid));\n-\t\treturn CB_BREAK;\n-\t}\n-\n-\tif (!check_int(get_oid_arbitrary_hex(hex_iter->expected_hexes.v[hex_iter->i],\n-\t\t\t\t\t     &expected), ==, 0))\n-\t\t; /* the data is bogus and cannot be used */\n-\telse if (!check(oideq(oid, &expected)))\n-\t\ttest_msg(\"expected: %s\\n       got: %s\\n     query: %s\",\n-\t\t\t oid_to_hex(&expected), oid_to_hex(oid), hex_iter->query);\n+\tcl_assert(hex_iter->i < hex_iter->expected_hexes.nr);\n \n+\tcl_parse_any_oid(hex_iter->expected_hexes.v[hex_iter->i],\n+\t\t\t &expected);\n+\tcl_assert_equal_s(oid_to_hex(oid), oid_to_hex(&expected));\n \thex_iter->i += 1;\n \treturn CB_CONTINUE;\n }\n@@ -75,48 +66,42 @@ static void check_each(struct oidtree *ot, const char *query, ...)\n \t\tstrvec_push(&hex_iter.expected_hexes, arg);\n \tva_end(hex_args);\n \n-\tif (!check_int(get_oid_arbitrary_hex(query, &oid), ==, 0))\n-\t\treturn;\n+\tcl_parse_any_oid(query, &oid);\n \toidtree_each(ot, &oid, strlen(query), check_each_cb, &hex_iter);\n \n-\tif (!check_int(hex_iter.i, ==, hex_iter.expected_hexes.nr))\n-\t\ttest_msg(\"error: could not find some 'object_id's for query ('%s')\", query);\n+\tif (hex_iter.i != hex_iter.expected_hexes.nr)\n+\t\tcl_failf(\"error: could not find some 'object_id's for query ('%s')\", query);\n+\n \tstrvec_clear(&hex_iter.expected_hexes);\n }\n \n-static void setup(void (*f)(struct oidtree *ot))\n+void test_oidtree__initialize(void)\n {\n-\tstruct oidtree ot;\n-\n \toidtree_init(&ot);\n-\tf(&ot);\n-\toidtree_clear(&ot);\n }\n \n-static void t_contains(struct oidtree *ot)\n+void test_oidtree__cleanup(void)\n {\n-\tFILL_TREE(ot, \"444\", \"1\", \"2\", \"3\", \"4\", \"5\", \"a\", \"b\", \"c\", \"d\", \"e\");\n-\tcheck_contains(ot, \"44\", 0);\n-\tcheck_contains(ot, \"441\", 0);\n-\tcheck_contains(ot, \"440\", 0);\n-\tcheck_contains(ot, \"444\", 1);\n-\tcheck_contains(ot, \"4440\", 1);\n-\tcheck_contains(ot, \"4444\", 0);\n+\toidtree_clear(&ot);\n }\n \n-static void t_each(struct oidtree *ot)\n+void test_oidtree__contains(void)\n {\n-\tFILL_TREE(ot, \"f\", \"9\", \"8\", \"123\", \"321\", \"320\", \"a\", \"b\", \"c\", \"d\", \"e\");\n-\tcheck_each(ot, \"12300\", \"123\", NULL);\n-\tcheck_each(ot, \"3211\", NULL); /* should not reach callback */\n-\tcheck_each(ot, \"3210\", \"321\", NULL);\n-\tcheck_each(ot, \"32100\", \"321\", NULL);\n-\tcheck_each(ot, \"32\", \"320\", \"321\", NULL);\n+\tFILL_TREE(&ot, \"444\", \"1\", \"2\", \"3\", \"4\", \"5\", \"a\", \"b\", \"c\", \"d\", \"e\");\n+\tcheck_contains(&ot, \"44\", 0);\n+\tcheck_contains(&ot, \"441\", 0);\n+\tcheck_contains(&ot, \"440\", 0);\n+\tcheck_contains(&ot, \"444\", 1);\n+\tcheck_contains(&ot, \"4440\", 1);\n+\tcheck_contains(&ot, \"4444\", 0);\n }\n \n-int cmd_main(int argc UNUSED, const char **argv UNUSED)\n+void test_oidtree__each(void)\n {\n-\tTEST(setup(t_contains), \"oidtree insert and contains works\");\n-\tTEST(setup(t_each), \"oidtree each works\");\n-\treturn test_done();\n+\tFILL_TREE(&ot, \"f\", \"9\", \"8\", \"123\", \"321\", \"320\", \"a\", \"b\", \"c\", \"d\", \"e\");\n+\tcheck_each(&ot, \"12300\", \"123\", NULL);\n+\tcheck_each(&ot, \"3211\", NULL); /* should not reach callback */\n+\tcheck_each(&ot, \"3210\", \"321\", NULL);\n+\tcheck_each(&ot, \"32100\", \"321\", NULL);\n+\tcheck_each(&ot, \"32\", \"320\", \"321\", NULL);\n }\n-- \n2.47.0.86.g15030f9556\n\n"}]}