{"thread":{"id":"62873","subject":"[PATCH 0/4] t/unit-tests: convert unit-tests to use clar","startedAt":"2025-01-30T09:13:46Z","lastAt":"2025-02-03T14:57:02Z","messageCount":21,"participants":["Seyi Kuforiji","Patrick Steinhardt","Junio C Hamano","phillip.wood123@gmail.com","Phillip Wood"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"511476","messageId":"20250130091334.39922-1-kuforiji98@gmail.com","threadId":"62873","inReplyTo":null,"subject":"[PATCH 0/4] t/unit-tests: convert unit-tests to use clar","fromName":"Seyi Kuforiji","fromEmail":"kuforiji98@gmail.com","sentAt":"2025-01-30T09:13:30Z","receivedAt":"2025-01-30T09:13:46Z","isPatch":true,"sender":{"key":"kuforiji98@gmail.com","avatar":"https://avatars.githubusercontent.com/u/94905626?v=4"},"body":"Hello,\n\nThis small patch series transitions the existing unit test files to the\nClar testing framework. This change is part of our ongoing effort to\nstandardize our testing framework to enhance maintainability.\n\nThanks\nSeyi\n\nMentored-by: Patrick Steinhardt ps@pks.im\nSigned-off-by: Seyi Kuforiji kuforiji98@gmail.com\nSeyi Kuforiji (4):\n  t/unit-tests: convert hashmap test to use clar test framework\n  t/unit-tests: adapt example decorate test to use clar test framework\n  t/unit-tests: convert strbuf test to use clar test framework\n  t/unit-tests: convert strcmp-offset test to use clar test framework\n\n Makefile                                  |   8 +-\n t/meson.build                             |   8 +-\n t/unit-tests/t-example-decorate.c         |  74 -------\n t/unit-tests/t-strbuf.c                   | 122 ------------\n t/unit-tests/t-strcmp-offset.c            |  35 ----\n t/unit-tests/u-example-decorate.c         |  76 ++++++++\n t/unit-tests/{t-hashmap.c => u-hashmap.c} | 226 +++++++++++-----------\n t/unit-tests/u-strbuf.c                   | 121 ++++++++++++\n t/unit-tests/u-strcmp-offset.c            |  45 +++++\n 9 files changed, 362 insertions(+), 353 deletions(-)\n delete mode 100644 t/unit-tests/t-example-decorate.c\n delete mode 100644 t/unit-tests/t-strbuf.c\n delete mode 100644 t/unit-tests/t-strcmp-offset.c\n create mode 100644 t/unit-tests/u-example-decorate.c\n rename t/unit-tests/{t-hashmap.c => u-hashmap.c} (60%)\n create mode 100644 t/unit-tests/u-strbuf.c\n create mode 100644 t/unit-tests/u-strcmp-offset.c\n\n-- \n2.47.0.86.g15030f9556\n\n"},{"id":"511477","messageId":"20250130091334.39922-2-kuforiji98@gmail.com","threadId":"62873","inReplyTo":"20250130091334.39922-1-kuforiji98@gmail.com","subject":"[PATCH 1/4] t/unit-tests: convert hashmap test to clar framework","fromName":"Seyi Kuforiji","fromEmail":"kuforiji98@gmail.com","sentAt":"2025-01-30T09:13:31Z","receivedAt":"2025-01-30T09:13:52Z","isPatch":true,"sender":{"key":"kuforiji98@gmail.com","avatar":"https://avatars.githubusercontent.com/u/94905626?v=4"},"body":"Adapts hashmap test script to clar framework by using clar assertions\nwhere necessary. Test functions are created as both standalone and\ninline to test different test cases.\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-hashmap.c => u-hashmap.c} | 226 +++++++++++-----------\n 3 files changed, 114 insertions(+), 116 deletions(-)\n rename t/unit-tests/{t-hashmap.c => u-hashmap.c} (60%)\n\ndiff --git a/Makefile b/Makefile\nindex b0cdc1fd4a..2d9dad119a 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1339,6 +1339,7 @@ THIRD_PARTY_SOURCES += $(UNIT_TEST_DIR)/clar/clar/%\n \n CLAR_TEST_SUITES += u-ctype\n CLAR_TEST_SUITES += u-hash\n+CLAR_TEST_SUITES += u-hashmap\n CLAR_TEST_SUITES += u-mem-pool\n CLAR_TEST_SUITES += u-prio-queue\n CLAR_TEST_SUITES += u-reftable-tree\n@@ -1349,7 +1350,6 @@ 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-example-decorate\n-UNIT_TEST_PROGRAMS += t-hashmap\n UNIT_TEST_PROGRAMS += t-oid-array\n UNIT_TEST_PROGRAMS += t-oidmap\n UNIT_TEST_PROGRAMS += t-oidtree\ndiff --git a/t/meson.build b/t/meson.build\nindex 14fea8dddf..af597f9804 100644\n--- a/t/meson.build\n+++ b/t/meson.build\n@@ -1,6 +1,7 @@\n clar_test_suites = [\n   'unit-tests/u-ctype.c',\n   'unit-tests/u-hash.c',\n+  'unit-tests/u-hashmap.c',\n   'unit-tests/u-mem-pool.c',\n   'unit-tests/u-prio-queue.c',\n   'unit-tests/u-reftable-tree.c',\n@@ -45,7 +46,6 @@ test('unit-tests', clar_unit_tests)\n \n unit_test_programs = [\n   'unit-tests/t-example-decorate.c',\n-  'unit-tests/t-hashmap.c',\n   'unit-tests/t-oid-array.c',\n   'unit-tests/t-oidmap.c',\n   'unit-tests/t-oidtree.c',\ndiff --git a/t/unit-tests/t-hashmap.c b/t/unit-tests/u-hashmap.c\nsimilarity index 60%\nrename from t/unit-tests/t-hashmap.c\nrename to t/unit-tests/u-hashmap.c\nindex 83b79dff39..6b6d22005a 100644\n--- a/t/unit-tests/t-hashmap.c\n+++ b/t/unit-tests/u-hashmap.c\n@@ -1,4 +1,4 @@\n-#include \"test-lib.h\"\n+#include \"unit-test.h\"\n #include \"hashmap.h\"\n #include \"strbuf.h\"\n \n@@ -83,23 +83,23 @@ static void t_replace(struct hashmap *map, unsigned int ignore_case)\n \tstruct test_entry *entry;\n \n \tentry = alloc_test_entry(\"key1\", \"value1\", ignore_case);\n-\tcheck_pointer_eq(hashmap_put_entry(map, entry, ent), NULL);\n+\tcl_assert_equal_p(hashmap_put_entry(map, entry, ent), NULL);\n \n \tentry = alloc_test_entry(ignore_case ? \"Key1\" : \"key1\", \"value2\",\n \t\t\t\t ignore_case);\n \tentry = hashmap_put_entry(map, entry, ent);\n-\tif (check(entry != NULL))\n-\t\tcheck_str(get_value(entry), \"value1\");\n+\tcl_assert(entry != NULL);\n+\tcl_assert_equal_s(get_value(entry), \"value1\");\n \tfree(entry);\n \n \tentry = alloc_test_entry(\"fooBarFrotz\", \"value3\", ignore_case);\n-\tcheck_pointer_eq(hashmap_put_entry(map, entry, ent), NULL);\n+\tcl_assert_equal_p(hashmap_put_entry(map, entry, ent), NULL);\n \n \tentry = alloc_test_entry(ignore_case ? \"FOObarFrotz\" : \"fooBarFrotz\",\n \t\t\t\t \"value4\", ignore_case);\n \tentry = hashmap_put_entry(map, entry, ent);\n-\tif (check(entry != NULL))\n-\t\tcheck_str(get_value(entry), \"value3\");\n+\tcl_assert(entry != NULL);\n+\tcl_assert_equal_s(get_value(entry), \"value3\");\n \tfree(entry);\n }\n \n@@ -122,20 +122,18 @@ static void t_get(struct hashmap *map, unsigned int ignore_case)\n \tfor (size_t i = 0; i < ARRAY_SIZE(key_val); i++) {\n \t\tentry = alloc_test_entry(key_val[i][0], key_val[i][1],\n \t\t\t\t\t ignore_case);\n-\t\tcheck_pointer_eq(hashmap_put_entry(map, entry, ent), NULL);\n+\t\tcl_assert_equal_p(hashmap_put_entry(map, entry, ent), NULL);\n \t}\n \n \tfor (size_t i = 0; i < ARRAY_SIZE(query); i++) {\n \t\tentry = get_test_entry(map, query[i][0], ignore_case);\n-\t\tif (check(entry != NULL))\n-\t\t\tcheck_str(get_value(entry), query[i][1]);\n-\t\telse\n-\t\t\ttest_msg(\"query key: %s\", query[i][0]);\n+\t\tcl_assert(entry != NULL);\n+\t\tcl_assert_equal_s(get_value(entry), query[i][1]);\n \t}\n \n-\tcheck_pointer_eq(get_test_entry(map, \"notInMap\", ignore_case), NULL);\n-\tcheck_int(map->tablesize, ==, 64);\n-\tcheck_int(hashmap_get_size(map), ==, ARRAY_SIZE(key_val));\n+\tcl_assert_equal_p(get_test_entry(map, \"notInMap\", ignore_case), NULL);\n+\tcl_assert_equal_i(map->tablesize, 64);\n+\tcl_assert_equal_i(hashmap_get_size(map), ARRAY_SIZE(key_val));\n }\n \n static void t_add(struct hashmap *map, unsigned int ignore_case)\n@@ -165,39 +163,19 @@ static void t_add(struct hashmap *map, unsigned int ignore_case)\n \n \t\thashmap_for_each_entry_from(map, entry, ent)\n \t\t{\n-\t\t\tint ret;\n-\t\t\tif (!check_int((ret = key_val_contains(\n-\t\t\t\t\t\tkey_val, seen,\n-\t\t\t\t\t\tARRAY_SIZE(key_val), entry)),\n-\t\t\t\t       ==, 0)) {\n-\t\t\t\tswitch (ret) {\n-\t\t\t\tcase 1:\n-\t\t\t\t\ttest_msg(\"found entry was not given in the input\\n\"\n-\t\t\t\t\t\t \"    key: %s\\n  value: %s\",\n-\t\t\t\t\t\t entry->key, get_value(entry));\n-\t\t\t\t\tbreak;\n-\t\t\t\tcase 2:\n-\t\t\t\t\ttest_msg(\"duplicate entry detected\\n\"\n-\t\t\t\t\t\t \"    key: %s\\n  value: %s\",\n-\t\t\t\t\t\t entry->key, get_value(entry));\n-\t\t\t\t\tbreak;\n-\t\t\t\t}\n-\t\t\t} else {\n-\t\t\t\tcount++;\n-\t\t\t}\n+\t\t\tint ret = key_val_contains(key_val, seen,\n+\t\t\t\t\t\t   ARRAY_SIZE(key_val), entry);\n+\t\t\tcl_assert(ret == 0);\n+\t\t\tcount++;\n \t\t}\n-\t\tcheck_int(count, ==, 2);\n+\t\tcl_assert_equal_i(count, 2);\n \t}\n \n-\tfor (size_t i = 0; i < ARRAY_SIZE(seen); i++) {\n-\t\tif (!check_int(seen[i], ==, 1))\n-\t\t\ttest_msg(\"following key-val pair was not iterated over:\\n\"\n-\t\t\t\t \"    key: %s\\n  value: %s\",\n-\t\t\t\t key_val[i][0], key_val[i][1]);\n-\t}\n+\tfor (size_t i = 0; i < ARRAY_SIZE(seen); i++)\n+\t\tcl_assert_equal_i(seen[i], 1);\n \n-\tcheck_int(hashmap_get_size(map), ==, ARRAY_SIZE(key_val));\n-\tcheck_pointer_eq(get_test_entry(map, \"notInMap\", ignore_case), NULL);\n+\tcl_assert_equal_i(hashmap_get_size(map), ARRAY_SIZE(key_val));\n+\tcl_assert_equal_p(get_test_entry(map, \"notInMap\", ignore_case), NULL);\n }\n \n static void t_remove(struct hashmap *map, unsigned int ignore_case)\n@@ -211,24 +189,25 @@ static void t_remove(struct hashmap *map, unsigned int ignore_case)\n \n \tfor (size_t i = 0; i < ARRAY_SIZE(key_val); i++) {\n \t\tentry = alloc_test_entry(key_val[i][0], key_val[i][1], ignore_case);\n-\t\tcheck_pointer_eq(hashmap_put_entry(map, entry, ent), NULL);\n+\t\tcl_assert_equal_p(hashmap_put_entry(map, entry, ent), NULL);\n \t}\n \n \tfor (size_t i = 0; i < ARRAY_SIZE(remove); i++) {\n \t\tentry = alloc_test_entry(remove[i][0], \"\", ignore_case);\n \t\tremoved = hashmap_remove_entry(map, entry, ent, remove[i][0]);\n-\t\tif (check(removed != NULL))\n-\t\t\tcheck_str(get_value(removed), remove[i][1]);\n+\t\tcl_assert(removed != NULL);\n+\t\tcl_assert_equal_s(get_value(removed), remove[i][1]);\n \t\tfree(entry);\n \t\tfree(removed);\n \t}\n \n \tentry = alloc_test_entry(\"notInMap\", \"\", ignore_case);\n-\tcheck_pointer_eq(hashmap_remove_entry(map, entry, ent, \"notInMap\"), NULL);\n+\tcl_assert_equal_p(hashmap_remove_entry(map, entry, ent, \"notInMap\"), NULL);\n \tfree(entry);\n \n-\tcheck_int(map->tablesize, ==, 64);\n-\tcheck_int(hashmap_get_size(map), ==, ARRAY_SIZE(key_val) - ARRAY_SIZE(remove));\n+\tcl_assert_equal_i(map->tablesize, 64);\n+\tcl_assert_equal_i(hashmap_get_size(map),\n+\t\t\t  ARRAY_SIZE(key_val) - ARRAY_SIZE(remove));\n }\n \n static void t_iterate(struct hashmap *map, unsigned int ignore_case)\n@@ -242,38 +221,21 @@ static void t_iterate(struct hashmap *map, unsigned int ignore_case)\n \n \tfor (size_t i = 0; i < ARRAY_SIZE(key_val); i++) {\n \t\tentry = alloc_test_entry(key_val[i][0], key_val[i][1], ignore_case);\n-\t\tcheck_pointer_eq(hashmap_put_entry(map, entry, ent), NULL);\n+\t\tcl_assert_equal_p(hashmap_put_entry(map, entry, ent), NULL);\n \t}\n \n \thashmap_for_each_entry(map, &iter, entry, ent /* member name */)\n \t{\n-\t\tint ret;\n-\t\tif (!check_int((ret = key_val_contains(key_val, seen,\n-\t\t\t\t\t\t       ARRAY_SIZE(key_val),\n-\t\t\t\t\t\t       entry)), ==, 0)) {\n-\t\t\tswitch (ret) {\n-\t\t\tcase 1:\n-\t\t\t\ttest_msg(\"found entry was not given in the input\\n\"\n-\t\t\t\t\t \"    key: %s\\n  value: %s\",\n-\t\t\t\t\t entry->key, get_value(entry));\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 \"    key: %s\\n  value: %s\",\n-\t\t\t\t\t entry->key, get_value(entry));\n-\t\t\t\tbreak;\n-\t\t\t}\n-\t\t}\n+\t\tint ret = key_val_contains(key_val, seen,\n+\t\t\t\t\t\tARRAY_SIZE(key_val),\n+\t\t\t\t\t\tentry);\n+\t\tcl_assert(ret == 0);\n \t}\n \n-\tfor (size_t i = 0; i < ARRAY_SIZE(seen); i++) {\n-\t\tif (!check_int(seen[i], ==, 1))\n-\t\t\ttest_msg(\"following key-val pair was not iterated over:\\n\"\n-\t\t\t\t \"    key: %s\\n  value: %s\",\n-\t\t\t\t key_val[i][0], key_val[i][1]);\n-\t}\n+\tfor (size_t i = 0; i < ARRAY_SIZE(seen); i++)\n+\t\tcl_assert_equal_i(seen[i], 1);\n \n-\tcheck_int(hashmap_get_size(map), ==, ARRAY_SIZE(key_val));\n+\tcl_assert_equal_i(hashmap_get_size(map), ARRAY_SIZE(key_val));\n }\n \n static void t_alloc(struct hashmap *map, unsigned int ignore_case)\n@@ -284,17 +246,17 @@ static void t_alloc(struct hashmap *map, unsigned int ignore_case)\n \t\tchar *key = xstrfmt(\"key%d\", i);\n \t\tchar *value = xstrfmt(\"value%d\", i);\n \t\tentry = alloc_test_entry(key, value, ignore_case);\n-\t\tcheck_pointer_eq(hashmap_put_entry(map, entry, ent), NULL);\n+\t\tcl_assert_equal_p(hashmap_put_entry(map, entry, ent), NULL);\n \t\tfree(key);\n \t\tfree(value);\n \t}\n-\tcheck_int(map->tablesize, ==, 64);\n-\tcheck_int(hashmap_get_size(map), ==, 51);\n+\tcl_assert_equal_i(map->tablesize, 64);\n+\tcl_assert_equal_i(hashmap_get_size(map), 51);\n \n \tentry = alloc_test_entry(\"key52\", \"value52\", ignore_case);\n-\tcheck_pointer_eq(hashmap_put_entry(map, entry, ent), NULL);\n-\tcheck_int(map->tablesize, ==, 256);\n-\tcheck_int(hashmap_get_size(map), ==, 52);\n+\tcl_assert_equal_p(hashmap_put_entry(map, entry, ent), NULL);\n+\tcl_assert_equal_i(map->tablesize, 256);\n+\tcl_assert_equal_i(hashmap_get_size(map), 52);\n \n \tfor (int i = 1; i <= 12; i++) {\n \t\tchar *key = xstrfmt(\"key%d\", i);\n@@ -302,27 +264,27 @@ static void t_alloc(struct hashmap *map, unsigned int ignore_case)\n \n \t\tentry = alloc_test_entry(key, \"\", ignore_case);\n \t\tremoved = hashmap_remove_entry(map, entry, ent, key);\n-\t\tif (check(removed != NULL))\n-\t\t\tcheck_str(value, get_value(removed));\n+\t\tcl_assert(removed != NULL);\n+\t\tcl_assert_equal_s(value, get_value(removed));\n \t\tfree(key);\n \t\tfree(value);\n \t\tfree(entry);\n \t\tfree(removed);\n \t}\n-\tcheck_int(map->tablesize, ==, 256);\n-\tcheck_int(hashmap_get_size(map), ==, 40);\n+\tcl_assert_equal_i(map->tablesize, 256);\n+\tcl_assert_equal_i(hashmap_get_size(map), 40);\n \n \tentry = alloc_test_entry(\"key40\", \"\", ignore_case);\n \tremoved = hashmap_remove_entry(map, entry, ent, \"key40\");\n-\tif (check(removed != NULL))\n-\t\tcheck_str(\"value40\", get_value(removed));\n-\tcheck_int(map->tablesize, ==, 64);\n-\tcheck_int(hashmap_get_size(map), ==, 39);\n+\tcl_assert(removed != NULL);\n+\tcl_assert_equal_s(\"value40\", get_value(removed));\n+\tcl_assert_equal_i(map->tablesize, 64);\n+\tcl_assert_equal_i(hashmap_get_size(map), 39);\n \tfree(entry);\n \tfree(removed);\n }\n \n-static void t_intern(void)\n+void test_hashmap__intern(void)\n {\n \tconst char *values[] = { \"value1\", \"Value1\", \"value2\", \"value2\" };\n \n@@ -330,32 +292,68 @@ static void t_intern(void)\n \t\tconst char *i1 = strintern(values[i]);\n \t\tconst char *i2 = strintern(values[i]);\n \n-\t\tif (!check(!strcmp(i1, values[i])))\n-\t\t\ttest_msg(\"strintern(%s) returns %s\\n\", values[i], i1);\n-\t\telse if (!check(i1 != values[i]))\n-\t\t\ttest_msg(\"strintern(%s) returns input pointer\\n\",\n-\t\t\t\t values[i]);\n-\t\telse if (!check_pointer_eq(i1, i2))\n-\t\t\ttest_msg(\"address('%s') != address('%s'), so strintern('%s') != strintern('%s')\",\n-\t\t\t\t i1, i2, values[i], values[i]);\n-\t\telse\n-\t\t\tcheck_str(i1, values[i]);\n+\t\tcl_assert_equal_s(i1, values[i]);\n+\t\tcl_assert(i1 != values[i]);\n+\t\tcl_assert_equal_p(i1, i2);\n \t}\n }\n \n-int cmd_main(int argc UNUSED, const char **argv UNUSED)\n+void test_hashmap__replace_case_sensitive(void)\n+{\n+\tsetup(t_replace, 0);\n+}\n+\n+void test_hashmap__replace_case_insensitive(void)\n+{\n+\tsetup(t_replace, 1);\n+}\n+\n+void test_hashmap__get_case_sensitive(void)\n+{\n+\tsetup(t_get, 0);\n+}\n+\n+void test_hashmap__get_case_insensitive(void)\n+{\n+\tsetup(t_get, 1);\n+}\n+\n+void test_hashmap__add_case_sensitive(void)\n+{\n+\tsetup(t_add, 0);\n+}\n+\n+void test_hashmap__add_case_insensitive(void)\n+{\n+\tsetup(t_add, 1);\n+}\n+\n+void test_hashmap__remove_case_sensitive(void)\n+{\n+\tsetup(t_remove, 0);\n+}\n+\n+void test_hashmap__remove_case_insensitive(void)\n+{\n+\tsetup(t_remove, 1);\n+}\n+\n+void test_hashmap__iterate_case_sensitive(void)\n+{\n+\tsetup(t_iterate, 0);\n+}\n+\n+void test_hashmap__iterate_case_insensitive(void)\n+{\n+\tsetup(t_iterate, 1);\n+}\n+\n+void test_hashmap__alloc_case_sensitive(void)\n+{\n+\tsetup(t_alloc, 0);\n+}\n+\n+void test_hashmap__alloc_case_insensitive(void)\n {\n-\tTEST(setup(t_replace, 0), \"replace works\");\n-\tTEST(setup(t_replace, 1), \"replace (case insensitive) works\");\n-\tTEST(setup(t_get, 0), \"get works\");\n-\tTEST(setup(t_get, 1), \"get (case insensitive) works\");\n-\tTEST(setup(t_add, 0), \"add works\");\n-\tTEST(setup(t_add, 1), \"add (case insensitive) works\");\n-\tTEST(setup(t_remove, 0), \"remove works\");\n-\tTEST(setup(t_remove, 1), \"remove (case insensitive) works\");\n-\tTEST(setup(t_iterate, 0), \"iterate works\");\n-\tTEST(setup(t_iterate, 1), \"iterate (case insensitive) works\");\n-\tTEST(setup(t_alloc, 0), \"grow / shrink works\");\n-\tTEST(t_intern(), \"string interning works\");\n-\treturn test_done();\n+\tsetup(t_alloc, 1);\n }\n-- \n2.47.0.86.g15030f9556\n\n"},{"id":"511478","messageId":"20250130091334.39922-3-kuforiji98@gmail.com","threadId":"62873","inReplyTo":"20250130091334.39922-1-kuforiji98@gmail.com","subject":"[PATCH 2/4] t/unit-tests: adapt example decorate test to clar framework","fromName":"Seyi Kuforiji","fromEmail":"kuforiji98@gmail.com","sentAt":"2025-01-30T09:13:32Z","receivedAt":"2025-01-30T09:13:56Z","isPatch":true,"sender":{"key":"kuforiji98@gmail.com","avatar":"https://avatars.githubusercontent.com/u/94905626?v=4"},"body":"Adapts example decorate test script to clar framework by using clar\nassertions where necessary. Test functions are created as standalone to\ntest different test cases.\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-example-decorate.c | 74 ------------------------------\n t/unit-tests/u-example-decorate.c | 76 +++++++++++++++++++++++++++++++\n 4 files changed, 78 insertions(+), 76 deletions(-)\n delete mode 100644 t/unit-tests/t-example-decorate.c\n create mode 100644 t/unit-tests/u-example-decorate.c\n\ndiff --git a/Makefile b/Makefile\nindex 2d9dad119a..732d765f1c 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1338,6 +1338,7 @@ THIRD_PARTY_SOURCES += $(UNIT_TEST_DIR)/clar/%\n THIRD_PARTY_SOURCES += $(UNIT_TEST_DIR)/clar/clar/%\n \n CLAR_TEST_SUITES += u-ctype\n+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@@ -1349,7 +1350,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-example-decorate\n UNIT_TEST_PROGRAMS += t-oid-array\n UNIT_TEST_PROGRAMS += t-oidmap\n UNIT_TEST_PROGRAMS += t-oidtree\ndiff --git a/t/meson.build b/t/meson.build\nindex af597f9804..c7e08eca6f 100644\n--- a/t/meson.build\n+++ b/t/meson.build\n@@ -1,5 +1,6 @@\n clar_test_suites = [\n   'unit-tests/u-ctype.c',\n+  'unit-tests/u-example-decorate.c',\n   'unit-tests/u-hash.c',\n   'unit-tests/u-hashmap.c',\n   'unit-tests/u-mem-pool.c',\n@@ -45,7 +46,6 @@ clar_unit_tests = executable('unit-tests',\n test('unit-tests', clar_unit_tests)\n \n unit_test_programs = [\n-  'unit-tests/t-example-decorate.c',\n   'unit-tests/t-oid-array.c',\n   'unit-tests/t-oidmap.c',\n   'unit-tests/t-oidtree.c',\ndiff --git a/t/unit-tests/t-example-decorate.c b/t/unit-tests/t-example-decorate.c\ndeleted file mode 100644\nindex bfc776e223..0000000000\n--- a/t/unit-tests/t-example-decorate.c\n+++ /dev/null\n@@ -1,74 +0,0 @@\n-#define USE_THE_REPOSITORY_VARIABLE\n-\n-#include \"test-lib.h\"\n-#include \"object.h\"\n-#include \"decorate.h\"\n-#include \"repository.h\"\n-\n-struct test_vars {\n-\tstruct object *one, *two, *three;\n-\tstruct decoration n;\n-\tint decoration_a, decoration_b;\n-};\n-\n-static void t_add(struct test_vars *vars)\n-{\n-\tvoid *ret = add_decoration(&vars->n, vars->one, &vars->decoration_a);\n-\n-\tcheck(ret == NULL);\n-\tret = add_decoration(&vars->n, vars->two, NULL);\n-\tcheck(ret == NULL);\n-}\n-\n-static void t_readd(struct test_vars *vars)\n-{\n-\tvoid *ret = add_decoration(&vars->n, vars->one, NULL);\n-\n-\tcheck(ret == &vars->decoration_a);\n-\tret = add_decoration(&vars->n, vars->two, &vars->decoration_b);\n-\tcheck(ret == NULL);\n-}\n-\n-static void t_lookup(struct test_vars *vars)\n-{\n-\tvoid *ret = lookup_decoration(&vars->n, vars->one);\n-\n-\tcheck(ret == NULL);\n-\tret = lookup_decoration(&vars->n, vars->two);\n-\tcheck(ret == &vars->decoration_b);\n-\tret = lookup_decoration(&vars->n, vars->three);\n-\tcheck(ret == NULL);\n-}\n-\n-static void t_loop(struct test_vars *vars)\n-{\n-\tint objects_noticed = 0;\n-\n-\tfor (size_t i = 0; i < vars->n.size; i++) {\n-\t\tif (vars->n.entries[i].base)\n-\t\t\tobjects_noticed++;\n-\t}\n-\tcheck_int(objects_noticed, ==, 2);\n-}\n-\n-int cmd_main(int argc UNUSED, const char **argv UNUSED)\n-{\n-\tstruct object_id one_oid = { { 1 } }, two_oid = { { 2 } }, three_oid = { { 3 } };\n-\tstruct test_vars vars = { 0 };\n-\n-\tvars.one = lookup_unknown_object(the_repository, &one_oid);\n-\tvars.two = lookup_unknown_object(the_repository, &two_oid);\n-\tvars.three = lookup_unknown_object(the_repository, &three_oid);\n-\n-\tTEST(t_add(&vars),\n-\t     \"Add 2 objects, one with a non-NULL decoration and one with a NULL decoration.\");\n-\tTEST(t_readd(&vars),\n-\t     \"When re-adding an already existing object, the old decoration is returned.\");\n-\tTEST(t_lookup(&vars),\n-\t     \"Lookup returns the added declarations, or NULL if the object was never added.\");\n-\tTEST(t_loop(&vars), \"The user can also loop through all entries.\");\n-\n-\tclear_decoration(&vars.n, NULL);\n-\n-\treturn test_done();\n-}\ndiff --git a/t/unit-tests/u-example-decorate.c b/t/unit-tests/u-example-decorate.c\nnew file mode 100644\nindex 0000000000..3a457d41fc\n--- /dev/null\n+++ b/t/unit-tests/u-example-decorate.c\n@@ -0,0 +1,76 @@\n+#define USE_THE_REPOSITORY_VARIABLE\n+\n+#include \"unit-test.h\"\n+#include \"object.h\"\n+#include \"decorate.h\"\n+#include \"repository.h\"\n+\n+struct test_vars {\n+\tstruct object *one, *two, *three;\n+\tstruct decoration n;\n+\tint decoration_a, decoration_b;\n+};\n+\n+static struct test_vars vars;\n+\n+void test_example_decorate__add(void)\n+{\n+\tvoid *ret = add_decoration(&vars.n, vars.one, &vars.decoration_a);\n+\tcl_assert(ret == NULL);\n+\tret = add_decoration(&vars.n, vars.two, NULL);\n+\tcl_assert(ret == NULL);\n+}\n+\n+void test_example_decorate__readd(void)\n+{\n+\tvoid *ret;\n+\n+\tcl_assert(add_decoration(&vars.n, vars.one, &vars.decoration_a) == NULL);\n+\tcl_assert(add_decoration(&vars.n, vars.two, NULL) == NULL);\n+\n+\tret = add_decoration(&vars.n, vars.one, NULL);\n+\tcl_assert(ret == &vars.decoration_a);\n+\tret = add_decoration(&vars.n, vars.two, &vars.decoration_b);\n+\tcl_assert(ret == NULL);\n+}\n+\n+void test_example_decorate__lookup(void)\n+{\n+\tvoid *ret;\n+\n+\tadd_decoration(&vars.n, vars.two, &vars.decoration_b);\n+\tadd_decoration(&vars.n, vars.one, NULL);\n+\n+\tret = lookup_decoration(&vars.n, vars.two);\n+\tcl_assert(ret == &vars.decoration_b);\n+\tret = lookup_decoration(&vars.n, vars.one);\n+\tcl_assert(ret == NULL);\n+}\n+\n+void test_example_decorate__loop(void)\n+{\n+\tint objects_noticed = 0;\n+\n+\tadd_decoration(&vars.n, vars.one, &vars.decoration_a);\n+\tadd_decoration(&vars.n, vars.two, &vars.decoration_b);\n+\n+\tfor (size_t i = 0; i < vars.n.size; i++) {\n+\t\tif (vars.n.entries[i].base)\n+\t\t\tobjects_noticed++;\n+\t}\n+\tcl_assert_equal_i(objects_noticed, 2);\n+}\n+\n+void test_example_decorate__initialize(void)\n+{\n+\tstruct object_id one_oid = { { 1 } }, two_oid = { { 2 } }, three_oid = { { 3 } };\n+\n+\tvars.one = lookup_unknown_object(the_repository, &one_oid);\n+\tvars.two = lookup_unknown_object(the_repository, &two_oid);\n+\tvars.three = lookup_unknown_object(the_repository, &three_oid);\n+}\n+\n+void test_example_decorate__cleanup(void)\n+{\n+\tclear_decoration(&vars.n, NULL);\n+}\n-- \n2.47.0.86.g15030f9556\n\n"},{"id":"511479","messageId":"20250130091334.39922-4-kuforiji98@gmail.com","threadId":"62873","inReplyTo":"20250130091334.39922-1-kuforiji98@gmail.com","subject":"[PATCH 3/4] t/unit-tests: convert strbuf test to clar framework","fromName":"Seyi Kuforiji","fromEmail":"kuforiji98@gmail.com","sentAt":"2025-01-30T09:13:33Z","receivedAt":"2025-01-30T09:13:59Z","isPatch":true,"sender":{"key":"kuforiji98@gmail.com","avatar":"https://avatars.githubusercontent.com/u/94905626?v=4"},"body":"Adapt strbuf test script to clar framework by using clar assertions\nwhere necessary. Test functions are created as standalone to test\ndifferent test cases.\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-strbuf.c | 122 ----------------------------------------\n t/unit-tests/u-strbuf.c | 121 +++++++++++++++++++++++++++++++++++++++\n 4 files changed, 123 insertions(+), 124 deletions(-)\n delete mode 100644 t/unit-tests/t-strbuf.c\n create mode 100644 t/unit-tests/u-strbuf.c\n\ndiff --git a/Makefile b/Makefile\nindex 732d765f1c..358193597f 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1344,6 +1344,7 @@ CLAR_TEST_SUITES += u-hashmap\n CLAR_TEST_SUITES += u-mem-pool\n CLAR_TEST_SUITES += u-prio-queue\n CLAR_TEST_SUITES += u-reftable-tree\n+CLAR_TEST_SUITES += u-strbuf\n CLAR_TEST_SUITES += u-strvec\n CLAR_TEST_PROG = $(UNIT_TEST_BIN)/unit-tests$(X)\n CLAR_TEST_OBJS = $(patsubst %,$(UNIT_TEST_DIR)/%.o,$(CLAR_TEST_SUITES))\n@@ -1361,7 +1362,6 @@ UNIT_TEST_PROGRAMS += t-reftable-reader\n UNIT_TEST_PROGRAMS += t-reftable-readwrite\n UNIT_TEST_PROGRAMS += t-reftable-record\n UNIT_TEST_PROGRAMS += t-reftable-stack\n-UNIT_TEST_PROGRAMS += t-strbuf\n UNIT_TEST_PROGRAMS += t-strcmp-offset\n UNIT_TEST_PROGRAMS += t-trailer\n UNIT_TEST_PROGRAMS += t-urlmatch-normalization\ndiff --git a/t/meson.build b/t/meson.build\nindex c7e08eca6f..6cb72842b1 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-prio-queue.c',\n   'unit-tests/u-reftable-tree.c',\n+  'unit-tests/u-strbuf.c',\n   'unit-tests/u-strvec.c',\n ]\n \n@@ -57,7 +58,6 @@ unit_test_programs = [\n   'unit-tests/t-reftable-readwrite.c',\n   'unit-tests/t-reftable-record.c',\n   'unit-tests/t-reftable-stack.c',\n-  'unit-tests/t-strbuf.c',\n   'unit-tests/t-strcmp-offset.c',\n   'unit-tests/t-trailer.c',\n   'unit-tests/t-urlmatch-normalization.c',\ndiff --git a/t/unit-tests/t-strbuf.c b/t/unit-tests/t-strbuf.c\ndeleted file mode 100644\nindex 3f4044d697..0000000000\n--- a/t/unit-tests/t-strbuf.c\n+++ /dev/null\n@@ -1,122 +0,0 @@\n-#include \"test-lib.h\"\n-#include \"strbuf.h\"\n-\n-/* wrapper that supplies tests with an empty, initialized strbuf */\n-static void setup(void (*f)(struct strbuf*, const void*),\n-\t\t  const void *data)\n-{\n-\tstruct strbuf buf = STRBUF_INIT;\n-\n-\tf(&buf, data);\n-\tstrbuf_release(&buf);\n-\tcheck_uint(buf.len, ==, 0);\n-\tcheck_uint(buf.alloc, ==, 0);\n-}\n-\n-/* wrapper that supplies tests with a populated, initialized strbuf */\n-static void setup_populated(void (*f)(struct strbuf*, const void*),\n-\t\t\t    const char *init_str, const void *data)\n-{\n-\tstruct strbuf buf = STRBUF_INIT;\n-\n-\tstrbuf_addstr(&buf, init_str);\n-\tcheck_uint(buf.len, ==, strlen(init_str));\n-\tf(&buf, data);\n-\tstrbuf_release(&buf);\n-\tcheck_uint(buf.len, ==, 0);\n-\tcheck_uint(buf.alloc, ==, 0);\n-}\n-\n-static int assert_sane_strbuf(struct strbuf *buf)\n-{\n-\t/* Initialized strbufs should always have a non-NULL buffer */\n-\tif (!check(!!buf->buf))\n-\t\treturn 0;\n-\t/* Buffers should always be NUL-terminated */\n-\tif (!check_char(buf->buf[buf->len], ==, '\\0'))\n-\t\treturn 0;\n-\t/*\n-\t * Freshly-initialized strbufs may not have a dynamically allocated\n-\t * buffer\n-\t */\n-\tif (buf->len == 0 && buf->alloc == 0)\n-\t\treturn 1;\n-\t/* alloc must be at least one byte larger than len */\n-\treturn check_uint(buf->len, <, buf->alloc);\n-}\n-\n-static void t_static_init(void)\n-{\n-\tstruct strbuf buf = STRBUF_INIT;\n-\n-\tcheck_uint(buf.len, ==, 0);\n-\tcheck_uint(buf.alloc, ==, 0);\n-\tcheck_char(buf.buf[0], ==, '\\0');\n-}\n-\n-static void t_dynamic_init(void)\n-{\n-\tstruct strbuf buf;\n-\n-\tstrbuf_init(&buf, 1024);\n-\tcheck(assert_sane_strbuf(&buf));\n-\tcheck_uint(buf.len, ==, 0);\n-\tcheck_uint(buf.alloc, >=, 1024);\n-\tcheck_char(buf.buf[0], ==, '\\0');\n-\tstrbuf_release(&buf);\n-}\n-\n-static void t_addch(struct strbuf *buf, const void *data)\n-{\n-\tconst char *p_ch = data;\n-\tconst char ch = *p_ch;\n-\tsize_t orig_alloc = buf->alloc;\n-\tsize_t orig_len = buf->len;\n-\n-\tif (!check(assert_sane_strbuf(buf)))\n-\t\treturn;\n-\tstrbuf_addch(buf, ch);\n-\tif (!check(assert_sane_strbuf(buf)))\n-\t\treturn;\n-\tif (!(check_uint(buf->len, ==, orig_len + 1) &&\n-\t      check_uint(buf->alloc, >=, orig_alloc)))\n-\t\treturn; /* avoid de-referencing buf->buf */\n-\tcheck_char(buf->buf[buf->len - 1], ==, ch);\n-\tcheck_char(buf->buf[buf->len], ==, '\\0');\n-}\n-\n-static void t_addstr(struct strbuf *buf, const void *data)\n-{\n-\tconst char *text = data;\n-\tsize_t len = strlen(text);\n-\tsize_t orig_alloc = buf->alloc;\n-\tsize_t orig_len = buf->len;\n-\n-\tif (!check(assert_sane_strbuf(buf)))\n-\t\treturn;\n-\tstrbuf_addstr(buf, text);\n-\tif (!check(assert_sane_strbuf(buf)))\n-\t\treturn;\n-\tif (!(check_uint(buf->len, ==, orig_len + len) &&\n-\t      check_uint(buf->alloc, >=, orig_alloc) &&\n-\t      check_uint(buf->alloc, >, orig_len + len) &&\n-\t      check_char(buf->buf[orig_len + len], ==, '\\0')))\n-\t    return;\n-\tcheck_str(buf->buf + orig_len, text);\n-}\n-\n-int cmd_main(int argc UNUSED, const char **argv UNUSED)\n-{\n-\tif (!TEST(t_static_init(), \"static initialization works\"))\n-\t\ttest_skip_all(\"STRBUF_INIT is broken\");\n-\tTEST(t_dynamic_init(), \"dynamic initialization works\");\n-\tTEST(setup(t_addch, \"a\"), \"strbuf_addch adds char\");\n-\tTEST(setup(t_addch, \"\"), \"strbuf_addch adds NUL char\");\n-\tTEST(setup_populated(t_addch, \"initial value\", \"a\"),\n-\t     \"strbuf_addch appends to initial value\");\n-\tTEST(setup(t_addstr, \"hello there\"), \"strbuf_addstr adds string\");\n-\tTEST(setup_populated(t_addstr, \"initial value\", \"hello there\"),\n-\t     \"strbuf_addstr appends string to initial value\");\n-\n-\treturn test_done();\n-}\ndiff --git a/t/unit-tests/u-strbuf.c b/t/unit-tests/u-strbuf.c\nnew file mode 100644\nindex 0000000000..fec3c768d2\n--- /dev/null\n+++ b/t/unit-tests/u-strbuf.c\n@@ -0,0 +1,121 @@\n+#include \"unit-test.h\"\n+#include \"strbuf.h\"\n+\n+/* wrapper that supplies tests with an empty, initialized strbuf */\n+static void setup(void (*f)(struct strbuf*, const void*),\n+\t\t  const void *data)\n+{\n+\tstruct strbuf buf = STRBUF_INIT;\n+\n+\tf(&buf, data);\n+\tstrbuf_release(&buf);\n+\tcl_assert_equal_i(buf.len, 0);\n+\tcl_assert_equal_i(buf.alloc, 0);\n+}\n+\n+/* wrapper that supplies tests with a populated, initialized strbuf */\n+static void setup_populated(void (*f)(struct strbuf*, const void*),\n+\t\t\t    const char *init_str, const void *data)\n+{\n+\tstruct strbuf buf = STRBUF_INIT;\n+\n+\tstrbuf_addstr(&buf, init_str);\n+\tcl_assert_equal_i(buf.len, strlen(init_str));\n+\tf(&buf, data);\n+\tstrbuf_release(&buf);\n+\tcl_assert_equal_i(buf.len, 0);\n+\tcl_assert_equal_i(buf.alloc, 0);\n+}\n+\n+static void assert_sane_strbuf(struct strbuf *buf)\n+{\n+\t/* Initialized strbufs should always have a non-NULL buffer */\n+\tcl_assert(buf->buf != NULL);\n+\t/* Buffers should always be NUL-terminated */\n+\tcl_assert(buf->buf[buf->len] == '\\0');\n+\t/*\n+\t * Freshly-initialized strbufs may not have a dynamically allocated\n+\t * buffer\n+\t */\n+\tif (buf->len == 0 && buf->alloc == 0)\n+\t\treturn;\n+\t/* alloc must be at least one byte larger than len */\n+\tcl_assert(buf->len < buf->alloc);\n+}\n+\n+void test_strbuf__static_init(void)\n+{\n+\tstruct strbuf buf = STRBUF_INIT;\n+\n+\tcl_assert_equal_i(buf.len, 0);\n+\tcl_assert_equal_i(buf.alloc, 0);\n+\tcl_assert(buf.buf[0] == '\\0');\n+}\n+\n+void test_strbuf__dynamic_init(void)\n+{\n+\tstruct strbuf buf;\n+\n+\tstrbuf_init(&buf, 1024);\n+\tassert_sane_strbuf(&buf);\n+\tcl_assert_equal_i(buf.len, 0);\n+\tcl_assert(buf.alloc >= 1024);\n+\tcl_assert(buf.buf[0] == '\\0');\n+\tstrbuf_release(&buf);\n+}\n+\n+static void t_addch(struct strbuf *buf, const void *data)\n+{\n+\tconst char *p_ch = data;\n+\tconst char ch = *p_ch;\n+\tsize_t orig_alloc = buf->alloc;\n+\tsize_t orig_len = buf->len;\n+\n+\tassert_sane_strbuf(buf);\n+\tstrbuf_addch(buf, ch);\n+\tassert_sane_strbuf(buf);\n+\tcl_assert_equal_i(buf->len, orig_len + 1);\n+\tcl_assert(buf->alloc >= orig_alloc);\n+\tcl_assert(buf->buf[buf->len] == '\\0');\n+}\n+\n+static void t_addstr(struct strbuf *buf, const void *data)\n+{\n+\tconst char *text = data;\n+\tsize_t len = strlen(text);\n+\tsize_t orig_alloc = buf->alloc;\n+\tsize_t orig_len = buf->len;\n+\n+\tassert_sane_strbuf(buf);\n+\tstrbuf_addstr(buf, text);\n+\tassert_sane_strbuf(buf);\n+\tcl_assert_equal_i(buf->len, orig_len + len);\n+\tcl_assert(buf->alloc >= orig_alloc);\n+\tcl_assert(buf->buf[buf->len] == '\\0');\n+\tcl_assert_equal_s(buf->buf + orig_len, text);\n+}\n+\n+void test_strbuf__add_single_char(void)\n+{\n+\tsetup(t_addch, \"a\");\n+}\n+\n+void test_strbuf__add_empty_char(void)\n+{\n+\tsetup(t_addch, \"\");\n+}\n+\n+void test_strbuf__add_multi_char(void)\n+{\n+\tsetup_populated(t_addch, \"initial value\", \"a\");\n+}\n+\n+void test_strbuf__add_single_str(void)\n+{\n+\tsetup(t_addstr, \"hello there\");\n+}\n+\n+void test_strbuf__add_multi_str(void)\n+{\n+\tsetup_populated(t_addstr, \"initial value\", \"hello there\");\n+}\n-- \n2.47.0.86.g15030f9556\n\n"},{"id":"511480","messageId":"20250130091334.39922-5-kuforiji98@gmail.com","threadId":"62873","inReplyTo":"20250130091334.39922-1-kuforiji98@gmail.com","subject":"[PATCH 4/4] t/unit-tests: convert strcmp-offset test to clar framework","fromName":"Seyi Kuforiji","fromEmail":"kuforiji98@gmail.com","sentAt":"2025-01-30T09:13:34Z","receivedAt":"2025-01-30T09:14:04Z","isPatch":true,"sender":{"key":"kuforiji98@gmail.com","avatar":"https://avatars.githubusercontent.com/u/94905626?v=4"},"body":"Adapt strcmp-offset test script to clar framework by using clar\nassertions where necessary. Test functions are created as standalone to\ntest different test cases.\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-strcmp-offset.c | 35 --------------------------\n t/unit-tests/u-strcmp-offset.c | 45 ++++++++++++++++++++++++++++++++++\n 4 files changed, 47 insertions(+), 37 deletions(-)\n delete mode 100644 t/unit-tests/t-strcmp-offset.c\n create mode 100644 t/unit-tests/u-strcmp-offset.c\n\ndiff --git a/Makefile b/Makefile\nindex 358193597f..76b5de4fdd 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1345,6 +1345,7 @@ CLAR_TEST_SUITES += u-mem-pool\n CLAR_TEST_SUITES += u-prio-queue\n CLAR_TEST_SUITES += u-reftable-tree\n CLAR_TEST_SUITES += u-strbuf\n+CLAR_TEST_SUITES += u-strcmp-offset\n CLAR_TEST_SUITES += u-strvec\n CLAR_TEST_PROG = $(UNIT_TEST_BIN)/unit-tests$(X)\n CLAR_TEST_OBJS = $(patsubst %,$(UNIT_TEST_DIR)/%.o,$(CLAR_TEST_SUITES))\n@@ -1362,7 +1363,6 @@ UNIT_TEST_PROGRAMS += t-reftable-reader\n UNIT_TEST_PROGRAMS += t-reftable-readwrite\n UNIT_TEST_PROGRAMS += t-reftable-record\n UNIT_TEST_PROGRAMS += t-reftable-stack\n-UNIT_TEST_PROGRAMS += t-strcmp-offset\n UNIT_TEST_PROGRAMS += t-trailer\n UNIT_TEST_PROGRAMS += t-urlmatch-normalization\n UNIT_TEST_PROGS = $(patsubst %,$(UNIT_TEST_BIN)/%$X,$(UNIT_TEST_PROGRAMS))\ndiff --git a/t/meson.build b/t/meson.build\nindex 6cb72842b1..3935782bbb 100644\n--- a/t/meson.build\n+++ b/t/meson.build\n@@ -7,6 +7,7 @@ clar_test_suites = [\n   'unit-tests/u-prio-queue.c',\n   'unit-tests/u-reftable-tree.c',\n   'unit-tests/u-strbuf.c',\n+  'unit-tests/u-strcmp-offset.c',\n   'unit-tests/u-strvec.c',\n ]\n \n@@ -58,7 +59,6 @@ unit_test_programs = [\n   'unit-tests/t-reftable-readwrite.c',\n   'unit-tests/t-reftable-record.c',\n   'unit-tests/t-reftable-stack.c',\n-  'unit-tests/t-strcmp-offset.c',\n   'unit-tests/t-trailer.c',\n   'unit-tests/t-urlmatch-normalization.c',\n ]\ndiff --git a/t/unit-tests/t-strcmp-offset.c b/t/unit-tests/t-strcmp-offset.c\ndeleted file mode 100644\nindex 6880f21161..0000000000\n--- a/t/unit-tests/t-strcmp-offset.c\n+++ /dev/null\n@@ -1,35 +0,0 @@\n-#include \"test-lib.h\"\n-#include \"read-cache-ll.h\"\n-\n-static void check_strcmp_offset(const char *string1, const char *string2,\n-\t\t\t\tint expect_result, uintmax_t expect_offset)\n-{\n-\tsize_t offset;\n-\tint result = strcmp_offset(string1, string2, &offset);\n-\n-\t/*\n-\t * Because different CRTs behave differently, only rely on signs of the\n-\t * result values.\n-\t */\n-\tresult = (result < 0 ? -1 :\n-\t\t\tresult > 0 ? 1 :\n-\t\t\t0);\n-\n-\tcheck_int(result, ==, expect_result);\n-\tcheck_uint((uintmax_t)offset, ==, expect_offset);\n-}\n-\n-#define TEST_STRCMP_OFFSET(string1, string2, expect_result, expect_offset) \\\n-\tTEST(check_strcmp_offset(string1, string2, expect_result,          \\\n-\t\t\t\t expect_offset),                           \\\n-\t     \"strcmp_offset(%s, %s) works\", #string1, #string2)\n-\n-int cmd_main(int argc UNUSED, const char **argv UNUSED)\n-{\n-\tTEST_STRCMP_OFFSET(\"abc\", \"abc\", 0, 3);\n-\tTEST_STRCMP_OFFSET(\"abc\", \"def\", -1, 0);\n-\tTEST_STRCMP_OFFSET(\"abc\", \"abz\", -1, 2);\n-\tTEST_STRCMP_OFFSET(\"abc\", \"abcdef\", -1, 3);\n-\n-\treturn test_done();\n-}\ndiff --git a/t/unit-tests/u-strcmp-offset.c b/t/unit-tests/u-strcmp-offset.c\nnew file mode 100644\nindex 0000000000..7e8e9acf3c\n--- /dev/null\n+++ b/t/unit-tests/u-strcmp-offset.c\n@@ -0,0 +1,45 @@\n+#include \"unit-test.h\"\n+#include \"read-cache-ll.h\"\n+\n+static void check_strcmp_offset(const char *string1, const char *string2,\n+\t\t\t\tint expect_result, uintmax_t expect_offset)\n+{\n+\tsize_t offset;\n+\tint result = strcmp_offset(string1, string2, &offset);\n+\n+\t/*\n+\t * Because different CRTs behave differently, only rely on signs of the\n+\t * result values.\n+\t */\n+\tresult = (result < 0 ? -1 :\n+\t\t\tresult > 0 ? 1 :\n+\t\t\t0);\n+\n+\tcl_assert_equal_i(result, expect_result);\n+\tcl_assert_equal_i((uintmax_t)offset, expect_offset);\n+}\n+\n+void test_strcmp_offset__empty(void)\n+{\n+\tcheck_strcmp_offset(\"\", \"\", 0, 0);\n+}\n+\n+void test_strcmp_offset__equal(void)\n+{\n+\tcheck_strcmp_offset(\"abc\", \"abc\", 0, 3);\n+}\n+\n+void test_strcmp_offset__different(void)\n+{\n+\tcheck_strcmp_offset(\"abc\", \"def\", -1, 0);\n+}\n+\n+void test_strcmp_offset__mismatch(void)\n+{\n+\tcheck_strcmp_offset(\"abc\", \"abz\", -1, 2);\n+}\n+\n+void test_strcmp_offset__different_length(void)\n+{\n+\tcheck_strcmp_offset(\"abc\", \"abcdef\", -1, 3);\n+}\n-- \n2.47.0.86.g15030f9556\n\n"},{"id":"511546","messageId":"Z5y3cQ70M10yNqoJ@pks.im","threadId":"62873","inReplyTo":"20250130091334.39922-1-kuforiji98@gmail.com","subject":"Re: [PATCH 0/4] t/unit-tests: convert unit-tests to use clar","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-01-31T11:43:45Z","receivedAt":"2025-01-31T11:43:54Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Thu, Jan 30, 2025 at 10:13:30AM +0100, Seyi Kuforiji wrote:\n> Hello,\n> \n> This small patch series transitions the existing unit test files to the\n> Clar testing framework. This change is part of our ongoing effort to\n> standardize our testing framework to enhance maintainability.\n\nThanks for your patches! I've got a couple of comments, but overall this\nlooks mostly good to me.\n\nPatrick\n"},{"id":"511547","messageId":"Z5y3ebBpzYM0wUVG@pks.im","threadId":"62873","inReplyTo":"20250130091334.39922-2-kuforiji98@gmail.com","subject":"Re: [PATCH 1/4] t/unit-tests: convert hashmap test to clar framework","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-01-31T11:43:53Z","receivedAt":"2025-01-31T11:43:55Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Thu, Jan 30, 2025 at 10:13:31AM +0100, Seyi Kuforiji wrote:\n> Adapts hashmap test script to clar framework by using clar assertions\n\ns/Adapts/Adapt as we generally want to use imperative wording in commit\nmessages, as if we're instructing the code to change.\n\nAlso, please note that the code is not a script. Scripts get executed by\nan interpreter, but C files are compiled by a compiler before they can\nget executed. So you should be talking about \"code\", not \"scripts\".\n\n> where necessary. Test functions are created as both standalone and\n> inline to test different test cases.\n\nI honestly don't quite know what this second sentence is supposed to say\n:)\n\n> diff --git a/t/unit-tests/t-hashmap.c b/t/unit-tests/u-hashmap.c\n> similarity index 60%\n> rename from t/unit-tests/t-hashmap.c\n> rename to t/unit-tests/u-hashmap.c\n> index 83b79dff39..6b6d22005a 100644\n> --- a/t/unit-tests/t-hashmap.c\n> +++ b/t/unit-tests/u-hashmap.c\n> @@ -1,4 +1,4 @@\n> -#include \"test-lib.h\"\n> +#include \"unit-test.h\"\n>  #include \"hashmap.h\"\n>  #include \"strbuf.h\"\n>  \n> @@ -83,23 +83,23 @@ static void t_replace(struct hashmap *map, unsigned int ignore_case)\n>  \tstruct test_entry *entry;\n>  \n>  \tentry = alloc_test_entry(\"key1\", \"value1\", ignore_case);\n> -\tcheck_pointer_eq(hashmap_put_entry(map, entry, ent), NULL);\n> +\tcl_assert_equal_p(hashmap_put_entry(map, entry, ent), NULL);\n>  \n>  \tentry = alloc_test_entry(ignore_case ? \"Key1\" : \"key1\", \"value2\",\n>  \t\t\t\t ignore_case);\n>  \tentry = hashmap_put_entry(map, entry, ent);\n> -\tif (check(entry != NULL))\n> -\t\tcheck_str(get_value(entry), \"value1\");\n> +\tcl_assert(entry != NULL);\n\nWe usually avoid explicit `!= NULL`, but I guess in this case it's fine\nto keep this as-is as you simply keep the preexisting style.\n\n> @@ -165,39 +163,19 @@ static void t_add(struct hashmap *map, unsigned int ignore_case)\n>  \n>  \t\thashmap_for_each_entry_from(map, entry, ent)\n>  \t\t{\n> -\t\t\tint ret;\n> -\t\t\tif (!check_int((ret = key_val_contains(\n> -\t\t\t\t\t\tkey_val, seen,\n> -\t\t\t\t\t\tARRAY_SIZE(key_val), entry)),\n> -\t\t\t\t       ==, 0)) {\n> -\t\t\t\tswitch (ret) {\n> -\t\t\t\tcase 1:\n> -\t\t\t\t\ttest_msg(\"found entry was not given in the input\\n\"\n> -\t\t\t\t\t\t \"    key: %s\\n  value: %s\",\n> -\t\t\t\t\t\t entry->key, get_value(entry));\n> -\t\t\t\t\tbreak;\n> -\t\t\t\tcase 2:\n> -\t\t\t\t\ttest_msg(\"duplicate entry detected\\n\"\n> -\t\t\t\t\t\t \"    key: %s\\n  value: %s\",\n> -\t\t\t\t\t\t entry->key, get_value(entry));\n> -\t\t\t\t\tbreak;\n> -\t\t\t\t}\n> -\t\t\t} else {\n> -\t\t\t\tcount++;\n> -\t\t\t}\n> +\t\t\tint ret = key_val_contains(key_val, seen,\n> +\t\t\t\t\t\t   ARRAY_SIZE(key_val), entry);\n> +\t\t\tcl_assert(ret == 0);\n\nThis could instead use `cl_assert_equal_i(ret, 0)` so that the error\nmessage mentions what the observed error code is.\n\n> @@ -242,38 +221,21 @@ static void t_iterate(struct hashmap *map, unsigned int ignore_case)\n>  \n>  \tfor (size_t i = 0; i < ARRAY_SIZE(key_val); i++) {\n>  \t\tentry = alloc_test_entry(key_val[i][0], key_val[i][1], ignore_case);\n> -\t\tcheck_pointer_eq(hashmap_put_entry(map, entry, ent), NULL);\n> +\t\tcl_assert_equal_p(hashmap_put_entry(map, entry, ent), NULL);\n>  \t}\n>  \n>  \thashmap_for_each_entry(map, &iter, entry, ent /* member name */)\n>  \t{\n> -\t\tint ret;\n> -\t\tif (!check_int((ret = key_val_contains(key_val, seen,\n> -\t\t\t\t\t\t       ARRAY_SIZE(key_val),\n> -\t\t\t\t\t\t       entry)), ==, 0)) {\n> -\t\t\tswitch (ret) {\n> -\t\t\tcase 1:\n> -\t\t\t\ttest_msg(\"found entry was not given in the input\\n\"\n> -\t\t\t\t\t \"    key: %s\\n  value: %s\",\n> -\t\t\t\t\t entry->key, get_value(entry));\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 \"    key: %s\\n  value: %s\",\n> -\t\t\t\t\t entry->key, get_value(entry));\n> -\t\t\t\tbreak;\n> -\t\t\t}\n> -\t\t}\n> +\t\tint ret = key_val_contains(key_val, seen,\n> +\t\t\t\t\t\tARRAY_SIZE(key_val),\n> +\t\t\t\t\t\tentry);\n\nIndentation is off here.\n\n> @@ -330,32 +292,68 @@ static void t_intern(void)\n> -int cmd_main(int argc UNUSED, const char **argv UNUSED)\n> +void test_hashmap__replace_case_sensitive(void)\n> +{\n> +\tsetup(t_replace, 0);\n> +}\n\nI was briefly wondering whether we should use `__initialize()` and\n`__teardown()` functions instead of this setup function so that we can\nthen get rid of `t_replace()` and other test functions. But then I\nrealized that we want ot have these separate functions anyway so that we\ncan easily test with case-sensitivity enabled and disabled, so this is\nprobably okay.\n\nPatrick\n"},{"id":"511548","messageId":"Z5y3fI7o6ZXaSoas@pks.im","threadId":"62873","inReplyTo":"20250130091334.39922-3-kuforiji98@gmail.com","subject":"Re: [PATCH 2/4] t/unit-tests: adapt example decorate test to clar framework","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-01-31T11:43:56Z","receivedAt":"2025-01-31T11:44:00Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Thu, Jan 30, 2025 at 10:13:32AM +0100, Seyi Kuforiji wrote:\n> Adapts example decorate test script to clar framework by using clar\n> assertions where necessary. Test functions are created as standalone to\n> test different test cases.\n\nAgain, I don't think that the second sentence doesn't add much\ninformation to the commit message and can probably just be dropped. It\nwould be more interesting to talk about any design decisions that\nweren't immediately obvious, as these may also make the reviewer wonder.\n\nOne thing that would be worth to explain here are the `initialize()` and\n`cleanup()` functions. Readers not familiar with the clar framework may\nnot know that those get picked up by the framework automatically and\nthat they are executed before and after every single test.\n\n> Mentored-by: Patrick Steinhardt <ps@pks.im>\n> Signed-off-by: Seyi Kuforiji <kuforiji98@gmail.com>\n> ---\n>  Makefile                          |  2 +-\n>  t/meson.build                     |  2 +-\n>  t/unit-tests/t-example-decorate.c | 74 ------------------------------\n>  t/unit-tests/u-example-decorate.c | 76 +++++++++++++++++++++++++++++++\n>  4 files changed, 78 insertions(+), 76 deletions(-)\n>  delete mode 100644 t/unit-tests/t-example-decorate.c\n>  create mode 100644 t/unit-tests/u-example-decorate.c\n> \n> diff --git a/Makefile b/Makefile\n> index 2d9dad119a..732d765f1c 100644\n> --- a/Makefile\n> +++ b/Makefile\n> @@ -1338,6 +1338,7 @@ THIRD_PARTY_SOURCES += $(UNIT_TEST_DIR)/clar/%\n>  THIRD_PARTY_SOURCES += $(UNIT_TEST_DIR)/clar/clar/%\n>  \n>  CLAR_TEST_SUITES += u-ctype\n> +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> @@ -1349,7 +1350,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-example-decorate\n>  UNIT_TEST_PROGRAMS += t-oid-array\n>  UNIT_TEST_PROGRAMS += t-oidmap\n>  UNIT_TEST_PROGRAMS += t-oidtree\n> diff --git a/t/meson.build b/t/meson.build\n> index af597f9804..c7e08eca6f 100644\n> --- a/t/meson.build\n> +++ b/t/meson.build\n> @@ -1,5 +1,6 @@\n>  clar_test_suites = [\n>    'unit-tests/u-ctype.c',\n> +  'unit-tests/u-example-decorate.c',\n>    'unit-tests/u-hash.c',\n>    'unit-tests/u-hashmap.c',\n>    'unit-tests/u-mem-pool.c',\n> @@ -45,7 +46,6 @@ clar_unit_tests = executable('unit-tests',\n>  test('unit-tests', clar_unit_tests)\n>  \n>  unit_test_programs = [\n> -  'unit-tests/t-example-decorate.c',\n>    'unit-tests/t-oid-array.c',\n>    'unit-tests/t-oidmap.c',\n>    'unit-tests/t-oidtree.c',\n> diff --git a/t/unit-tests/t-example-decorate.c b/t/unit-tests/t-example-decorate.c\n> deleted file mode 100644\n> index bfc776e223..0000000000\n> --- a/t/unit-tests/t-example-decorate.c\n> +++ /dev/null\n> @@ -1,74 +0,0 @@\n> -#define USE_THE_REPOSITORY_VARIABLE\n> -\n> -#include \"test-lib.h\"\n> -#include \"object.h\"\n> -#include \"decorate.h\"\n> -#include \"repository.h\"\n> -\n> -struct test_vars {\n> -\tstruct object *one, *two, *three;\n> -\tstruct decoration n;\n> -\tint decoration_a, decoration_b;\n> -};\n> -\n> -static void t_add(struct test_vars *vars)\n> -{\n> -\tvoid *ret = add_decoration(&vars->n, vars->one, &vars->decoration_a);\n> -\n> -\tcheck(ret == NULL);\n> -\tret = add_decoration(&vars->n, vars->two, NULL);\n> -\tcheck(ret == NULL);\n> -}\n> -\n> -static void t_readd(struct test_vars *vars)\n> -{\n> -\tvoid *ret = add_decoration(&vars->n, vars->one, NULL);\n> -\n> -\tcheck(ret == &vars->decoration_a);\n> -\tret = add_decoration(&vars->n, vars->two, &vars->decoration_b);\n> -\tcheck(ret == NULL);\n> -}\n> -\n> -static void t_lookup(struct test_vars *vars)\n> -{\n> -\tvoid *ret = lookup_decoration(&vars->n, vars->one);\n> -\n> -\tcheck(ret == NULL);\n> -\tret = lookup_decoration(&vars->n, vars->two);\n> -\tcheck(ret == &vars->decoration_b);\n> -\tret = lookup_decoration(&vars->n, vars->three);\n> -\tcheck(ret == NULL);\n> -}\n> -\n> -static void t_loop(struct test_vars *vars)\n> -{\n> -\tint objects_noticed = 0;\n> -\n> -\tfor (size_t i = 0; i < vars->n.size; i++) {\n> -\t\tif (vars->n.entries[i].base)\n> -\t\t\tobjects_noticed++;\n> -\t}\n> -\tcheck_int(objects_noticed, ==, 2);\n> -}\n> -\n> -int cmd_main(int argc UNUSED, const char **argv UNUSED)\n> -{\n> -\tstruct object_id one_oid = { { 1 } }, two_oid = { { 2 } }, three_oid = { { 3 } };\n> -\tstruct test_vars vars = { 0 };\n> -\n> -\tvars.one = lookup_unknown_object(the_repository, &one_oid);\n> -\tvars.two = lookup_unknown_object(the_repository, &two_oid);\n> -\tvars.three = lookup_unknown_object(the_repository, &three_oid);\n> -\n> -\tTEST(t_add(&vars),\n> -\t     \"Add 2 objects, one with a non-NULL decoration and one with a NULL decoration.\");\n> -\tTEST(t_readd(&vars),\n> -\t     \"When re-adding an already existing object, the old decoration is returned.\");\n> -\tTEST(t_lookup(&vars),\n> -\t     \"Lookup returns the added declarations, or NULL if the object was never added.\");\n> -\tTEST(t_loop(&vars), \"The user can also loop through all entries.\");\n> -\n> -\tclear_decoration(&vars.n, NULL);\n> -\n> -\treturn test_done();\n> -}\n> diff --git a/t/unit-tests/u-example-decorate.c b/t/unit-tests/u-example-decorate.c\n> new file mode 100644\n> index 0000000000..3a457d41fc\n> --- /dev/null\n> +++ b/t/unit-tests/u-example-decorate.c\n> @@ -0,0 +1,76 @@\n> +#define USE_THE_REPOSITORY_VARIABLE\n> +\n> +#include \"unit-test.h\"\n> +#include \"object.h\"\n> +#include \"decorate.h\"\n> +#include \"repository.h\"\n> +\n> +struct test_vars {\n> +\tstruct object *one, *two, *three;\n> +\tstruct decoration n;\n> +\tint decoration_a, decoration_b;\n> +};\n> +\n> +static struct test_vars vars;\n> +\n> +void test_example_decorate__add(void)\n> +{\n> +\tvoid *ret = add_decoration(&vars.n, vars.one, &vars.decoration_a);\n> +\tcl_assert(ret == NULL);\n> +\tret = add_decoration(&vars.n, vars.two, NULL);\n> +\tcl_assert(ret == NULL);\n> +}\n\nWe could avoid the separate variable here, e.g. like this:\n\nvoid test_example_decorate__add(void)\n{\n\tcl_assert_equal_p(add_decoration(&vars.n, vars.one, &vars.decoration_a), NULL);\n\tcl_assert_equal_p(add_decoration(&vars.n, vars.two, NULL), NULL);\n}\n\nThe same is true for some of the other test, too.\n\nThe test itself could be a bit more thorough in verifying what has been\ndone, but that is not a new issue that your conversion introduces as the\nold testcase was somewhat light on that, too. So this is nothing that I\nthink you need to change.\n\n> +void test_example_decorate__readd(void)\n> +{\n> +\tvoid *ret;\n> +\n> +\tcl_assert(add_decoration(&vars.n, vars.one, &vars.decoration_a) == NULL);\n> +\tcl_assert(add_decoration(&vars.n, vars.two, NULL) == NULL);\n\nOkay. This wasn't present before because tests were actually dependent\non the data that previous tests wrote. Now that we decouple the tests\nfrom one another we have to make sure to recreate the expected setup.\nWhich I think is a good thing, as it makes tests fully self-contained\nand the setup easier to understand.\n\nI guess this is another thing that you should be explaining in the\ncommit message, as it is not obvious at all.\n\n> +\tret = add_decoration(&vars.n, vars.one, NULL);\n> +\tcl_assert(ret == &vars.decoration_a);\n> +\tret = add_decoration(&vars.n, vars.two, &vars.decoration_b);\n> +\tcl_assert(ret == NULL);\n> +}\n> +\n> +void test_example_decorate__lookup(void)\n> +{\n> +\tvoid *ret;\n> +\n> +\tadd_decoration(&vars.n, vars.two, &vars.decoration_b);\n> +\tadd_decoration(&vars.n, vars.one, NULL);\n> +\n> +\tret = lookup_decoration(&vars.n, vars.two);\n> +\tcl_assert(ret == &vars.decoration_b);\n> +\tret = lookup_decoration(&vars.n, vars.one);\n> +\tcl_assert(ret == NULL);\n> +}\n> +\n> +void test_example_decorate__loop(void)\n> +{\n> +\tint objects_noticed = 0;\n> +\n> +\tadd_decoration(&vars.n, vars.one, &vars.decoration_a);\n> +\tadd_decoration(&vars.n, vars.two, &vars.decoration_b);\n> +\n> +\tfor (size_t i = 0; i < vars.n.size; i++) {\n> +\t\tif (vars.n.entries[i].base)\n> +\t\t\tobjects_noticed++;\n> +\t}\n\nNit: we can get rid of the curly braces here.\n\n> +\tcl_assert_equal_i(objects_noticed, 2);\n> +}\n> +\n> +void test_example_decorate__initialize(void)\n> +{\n> +\tstruct object_id one_oid = { { 1 } }, two_oid = { { 2 } }, three_oid = { { 3 } };\n> +\n> +\tvars.one = lookup_unknown_object(the_repository, &one_oid);\n> +\tvars.two = lookup_unknown_object(the_repository, &two_oid);\n> +\tvars.three = lookup_unknown_object(the_repository, &three_oid);\n> +}\n> +\n> +void test_example_decorate__cleanup(void)\n> +{\n> +\tclear_decoration(&vars.n, NULL);\n> +}\n\nLet's move both `initialize()` and `cleanup()` to the top so that it\nbecomes easy to see that all tests use this commen setup.\n\nPatrick\n"},{"id":"511549","messageId":"Z5y3f1B450LZOp3v@pks.im","threadId":"62873","inReplyTo":"20250130091334.39922-4-kuforiji98@gmail.com","subject":"Re: [PATCH 3/4] t/unit-tests: convert strbuf test to clar framework","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-01-31T11:43:59Z","receivedAt":"2025-01-31T11:44:03Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Thu, Jan 30, 2025 at 10:13:33AM +0100, Seyi Kuforiji wrote:\n> Adapt strbuf test script to clar framework by using clar assertions\n> where necessary. Test functions are created as standalone to test\n> different test cases.\n\nSame comment here regarding the second sentence.\n\n> diff --git a/t/unit-tests/u-strbuf.c b/t/unit-tests/u-strbuf.c\n> new file mode 100644\n> index 0000000000..fec3c768d2\n> --- /dev/null\n> +++ b/t/unit-tests/u-strbuf.c\n> @@ -0,0 +1,121 @@\n> +#include \"unit-test.h\"\n> +#include \"strbuf.h\"\n\nIt's somewhat sad that Git doesn't not think of this as a rename, as it\nwould've made the diff easier to read. You might want to try using\n`git format-patch --find-renames=20' to ask Git to still consider this a\nrename.\n\n[snip]\n> +static void assert_sane_strbuf(struct strbuf *buf)\n> +{\n> +\t/* Initialized strbufs should always have a non-NULL buffer */\n> +\tcl_assert(buf->buf != NULL);\n> +\t/* Buffers should always be NUL-terminated */\n> +\tcl_assert(buf->buf[buf->len] == '\\0');\n> +\t/*\n> +\t * Freshly-initialized strbufs may not have a dynamically allocated\n> +\t * buffer\n> +\t */\n> +\tif (buf->len == 0 && buf->alloc == 0)\n> +\t\treturn;\n> +\t/* alloc must be at least one byte larger than len */\n> +\tcl_assert(buf->len < buf->alloc);\n\nI think this condition can be simplified a bit:\n\n        /*\n         * In case the buffer contains anything, `alloc` must alloc must\n         * be at least one byte larger than `len`.\n         */\n        if (buf->len)\n            cl_assert(buf->len < buf->alloc);\n\n> +void test_strbuf__add_empty_char(void)\n> +{\n> +\tsetup(t_addch, \"\");\n> +}\n> +\n> +void test_strbuf__add_multi_char(void)\n\nNit: I think \"add_multi_char\" is slightly misleading. What this test\nexercises is that we can append to a non-empty buffer. So maybe\n\"append_char\" would be a better fit?\n\n> +{\n> +\tsetup_populated(t_addch, \"initial value\", \"a\");\n> +}\n> +\n> +void test_strbuf__add_single_str(void)\n> +{\n> +\tsetup(t_addstr, \"hello there\");\n> +}\n> +\n> +void test_strbuf__add_multi_str(void)\n\nSimilarly, we could name this \"append_str\".\n\n> +{\n> +\tsetup_populated(t_addstr, \"initial value\", \"hello there\");\n> +}\n\nPatrick\n"},{"id":"511550","messageId":"Z5y3gwWwLFlHiLkj@pks.im","threadId":"62873","inReplyTo":"20250130091334.39922-5-kuforiji98@gmail.com","subject":"Re: [PATCH 4/4] t/unit-tests: convert strcmp-offset test to clar framework","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-01-31T11:44:03Z","receivedAt":"2025-01-31T11:44:06Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Thu, Jan 30, 2025 at 10:13:34AM +0100, Seyi Kuforiji wrote:\n> Adapt strcmp-offset test script to clar framework by using clar\n> assertions where necessary. Test functions are created as standalone to\n> test different test cases.\n\nSame comment here regarding the message.\n\n> diff --git a/t/unit-tests/u-strcmp-offset.c b/t/unit-tests/u-strcmp-offset.c\n> new file mode 100644\n> index 0000000000..7e8e9acf3c\n> --- /dev/null\n> +++ b/t/unit-tests/u-strcmp-offset.c\n> @@ -0,0 +1,45 @@\n> +#include \"unit-test.h\"\n> +#include \"read-cache-ll.h\"\n> +\n> +static void check_strcmp_offset(const char *string1, const char *string2,\n> +\t\t\t\tint expect_result, uintmax_t expect_offset)\n> +{\n> +\tsize_t offset;\n> +\tint result = strcmp_offset(string1, string2, &offset);\n> +\n> +\t/*\n> +\t * Because different CRTs behave differently, only rely on signs of the\n> +\t * result values.\n> +\t */\n> +\tresult = (result < 0 ? -1 :\n> +\t\t\tresult > 0 ? 1 :\n> +\t\t\t0);\n> +\n> +\tcl_assert_equal_i(result, expect_result);\n> +\tcl_assert_equal_i((uintmax_t)offset, expect_offset);\n> +}\n> +\n> +void test_strcmp_offset__empty(void)\n> +{\n> +\tcheck_strcmp_offset(\"\", \"\", 0, 0);\n\nThis test didn't exist in the preimage, right? I don't mind adding it,\nbut it is somewhat surprising as the commit message does not mention\nthis at all.\n\nPatrick\n"},{"id":"511597","messageId":"20250131221420.38161-1-kuforiji98@gmail.com","threadId":"62873","inReplyTo":"20250130091334.39922-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-01-31T22:14:16Z","receivedAt":"2025-01-31T22:14:28Z","isPatch":true,"sender":{"key":"kuforiji98@gmail.com","avatar":"https://avatars.githubusercontent.com/u/94905626?v=4"},"body":"Hello,\n\nThis small patch series transitions the existing unit test files to the\nClar testing framework. This change is part of our ongoing effort to\nstandardize our testing approach and enhance maintainability.\n\nChanges in v2:\n- small fixes to the commit messages and how they read\n- some small code fix up and refactoring\n\nThanks\nSeyi\n\nMentored-by: Patrick Steinhardt ps@pks.im\nSigned-off-by: Seyi Kuforiji kuforiji98@gmail.com\n\nSeyi Kuforiji (4):\n  t/unit-tests: convert hashmap test to use clar test framework\n  t/unit-tests: adapt example decorate test to use clar test framework\n  t/unit-tests: convert strbuf test to use clar test framework\n  t/unit-tests: convert strcmp-offset test to use clar test framework\n\n Makefile                                      |   8 +-\n t/meson.build                                 |   8 +-\n ...xample-decorate.c => u-example-decorate.c} |  76 +++---\n t/unit-tests/{t-hashmap.c => u-hashmap.c}     | 226 +++++++++---------\n t/unit-tests/{t-strbuf.c => u-strbuf.c}       | 115 +++++----\n .../{t-strcmp-offset.c => u-strcmp-offset.c}  |  36 ++-\n 6 files changed, 232 insertions(+), 237 deletions(-)\n rename t/unit-tests/{t-example-decorate.c => u-example-decorate.c} (30%)\n rename t/unit-tests/{t-hashmap.c => u-hashmap.c} (60%)\n rename t/unit-tests/{t-strbuf.c => u-strbuf.c} (35%)\n rename t/unit-tests/{t-strcmp-offset.c => u-strcmp-offset.c} (39%)\n\nRange-diff against v1:\n1:  90accb2f75 ! 1:  19697be26b t/unit-tests: convert hashmap test to use clar test framework\n    @@ Commit message\n         t/unit-tests: convert hashmap test to use clar test framework\n     \n         Adapts hashmap test script to clar framework by using clar assertions\n    -    where necessary. Test functions are created as both standalone and\n    -    inline to test different test cases.\n    +    where necessary.\n     \n         Mentored-by: Patrick Steinhardt <ps@pks.im>\n         Signed-off-by: Seyi Kuforiji <kuforiji98@gmail.com>\n    @@ t/unit-tests/u-hashmap.c: static void t_add(struct hashmap *map, unsigned int ig\n     -\t\t\t}\n     +\t\t\tint ret = key_val_contains(key_val, seen,\n     +\t\t\t\t\t\t   ARRAY_SIZE(key_val), entry);\n    -+\t\t\tcl_assert(ret == 0);\n    ++\t\t\tcl_assert_equal_i(ret, 0);\n     +\t\t\tcount++;\n      \t\t}\n     -\t\tcheck_int(count, ==, 2);\n    @@ t/unit-tests/u-hashmap.c: static void t_iterate(struct hashmap *map, unsigned in\n     -\t\t\t}\n     -\t\t}\n     +\t\tint ret = key_val_contains(key_val, seen,\n    -+\t\t\t\t\t\tARRAY_SIZE(key_val),\n    -+\t\t\t\t\t\tentry);\n    ++\t\t\t\t\t   ARRAY_SIZE(key_val),\n    ++\t\t\t\t\t   entry);\n     +\t\tcl_assert(ret == 0);\n      \t}\n      \n2:  13a407d504 ! 2:  1d8f8974a5 t/unit-tests: adapt example decorate test to use clar test framework\n    @@ Metadata\n      ## Commit message ##\n         t/unit-tests: adapt example decorate test to use clar test framework\n     \n    -    Adapts example decorate test script to clar framework by using clar\n    -    assertions where necessary. Test functions are created as standalone to\n    -    test different test cases.\n    +    Introduce `test_example_decorate__initialize()` to explicitly set up\n    +    object IDs and retrieve corresponding objects before tests run. This\n    +    ensures a consistent and predictable test state without relying on data\n    +    from previous tests.\n    +\n    +    Add `test_example_decorate__cleanup()` to clear decorations after each\n    +    test, preventing interference between tests and ensuring each runs in\n    +    isolation.\n    +\n    +    Adapt example decorate test script to clar framework by using clar\n    +    assertions where necessary. Previously, tests relied on data written by\n    +    earlier tests, leading to unintended dependencies between them. This\n    +    explicitly initializes the necessary state within\n    +    `test_example_decorate__readd`, ensuring it does not depend on prior\n    +    test executions.\n     \n         Mentored-by: Patrick Steinhardt <ps@pks.im>\n         Signed-off-by: Seyi Kuforiji <kuforiji98@gmail.com>\n    @@ t/unit-tests/u-example-decorate.c (new)\n     +\n     +static struct test_vars vars;\n     +\n    -+void test_example_decorate__add(void)\n    ++void test_example_decorate__initialize(void)\n     +{\n    -+\tvoid *ret = add_decoration(&vars.n, vars.one, &vars.decoration_a);\n    -+\tcl_assert(ret == NULL);\n    -+\tret = add_decoration(&vars.n, vars.two, NULL);\n    -+\tcl_assert(ret == NULL);\n    ++\tstruct object_id one_oid = { { 1 } }, two_oid = { { 2 } }, three_oid = { { 3 } };\n    ++\n    ++\tvars.one = lookup_unknown_object(the_repository, &one_oid);\n    ++\tvars.two = lookup_unknown_object(the_repository, &two_oid);\n    ++\tvars.three = lookup_unknown_object(the_repository, &three_oid);\n     +}\n     +\n    -+void test_example_decorate__readd(void)\n    ++void test_example_decorate__cleanup(void)\n     +{\n    -+\tvoid *ret;\n    ++\tclear_decoration(&vars.n, NULL);\n    ++}\n     +\n    -+\tcl_assert(add_decoration(&vars.n, vars.one, &vars.decoration_a) == NULL);\n    -+\tcl_assert(add_decoration(&vars.n, vars.two, NULL) == NULL);\n    ++void test_example_decorate__add(void)\n    ++{\n    ++\tcl_assert_equal_p(add_decoration(&vars.n, vars.one, &vars.decoration_a), NULL);\n    ++\tcl_assert_equal_p(add_decoration(&vars.n, vars.two, NULL), NULL);\n    ++}\n     +\n    -+\tret = add_decoration(&vars.n, vars.one, NULL);\n    -+\tcl_assert(ret == &vars.decoration_a);\n    -+\tret = add_decoration(&vars.n, vars.two, &vars.decoration_b);\n    -+\tcl_assert(ret == NULL);\n    ++void test_example_decorate__readd(void)\n    ++{\n    ++\tcl_assert_equal_p(add_decoration(&vars.n, vars.one, &vars.decoration_a), NULL);\n    ++\tcl_assert_equal_p(add_decoration(&vars.n, vars.two, NULL), NULL);\n    ++\tcl_assert_equal_p(add_decoration(&vars.n, vars.one, NULL), &vars.decoration_a);\n    ++\tcl_assert_equal_p(add_decoration(&vars.n, vars.two, &vars.decoration_b), NULL);\n     +}\n     +\n     +void test_example_decorate__lookup(void)\n     +{\n    -+\tvoid *ret;\n    -+\n    -+\tadd_decoration(&vars.n, vars.two, &vars.decoration_b);\n    -+\tadd_decoration(&vars.n, vars.one, NULL);\n    -+\n    -+\tret = lookup_decoration(&vars.n, vars.two);\n    -+\tcl_assert(ret == &vars.decoration_b);\n    -+\tret = lookup_decoration(&vars.n, vars.one);\n    -+\tcl_assert(ret == NULL);\n    ++\tcl_assert_equal_p(add_decoration(&vars.n, vars.two, &vars.decoration_b), NULL);\n    ++\tcl_assert_equal_p(add_decoration(&vars.n, vars.one, NULL), NULL);\n    ++\tcl_assert_equal_p(lookup_decoration(&vars.n, vars.two), &vars.decoration_b);\n    ++\tcl_assert_equal_p(lookup_decoration(&vars.n, vars.one), NULL);\n     +}\n     +\n     +void test_example_decorate__loop(void)\n     +{\n     +\tint objects_noticed = 0;\n     +\n    -+\tadd_decoration(&vars.n, vars.one, &vars.decoration_a);\n    -+\tadd_decoration(&vars.n, vars.two, &vars.decoration_b);\n    ++\tcl_assert_equal_p(add_decoration(&vars.n, vars.one, &vars.decoration_a), NULL);\n    ++\tcl_assert_equal_p(add_decoration(&vars.n, vars.two, &vars.decoration_b), NULL);\n     +\n    -+\tfor (size_t i = 0; i < vars.n.size; i++) {\n    ++\tfor (size_t i = 0; i < vars.n.size; i++)\n     +\t\tif (vars.n.entries[i].base)\n     +\t\t\tobjects_noticed++;\n    -+\t}\n    -+\tcl_assert_equal_i(objects_noticed, 2);\n    -+}\n     +\n    -+void test_example_decorate__initialize(void)\n    -+{\n    -+\tstruct object_id one_oid = { { 1 } }, two_oid = { { 2 } }, three_oid = { { 3 } };\n    -+\n    -+\tvars.one = lookup_unknown_object(the_repository, &one_oid);\n    -+\tvars.two = lookup_unknown_object(the_repository, &two_oid);\n    -+\tvars.three = lookup_unknown_object(the_repository, &three_oid);\n    -+}\n    -+\n    -+void test_example_decorate__cleanup(void)\n    -+{\n    -+\tclear_decoration(&vars.n, NULL);\n    ++\tcl_assert_equal_i(objects_noticed, 2);\n     +}\n3:  08ade6b5cf ! 3:  e88ab7ab5f t/unit-tests: convert strbuf test to use clar test framework\n    @@ Commit message\n         t/unit-tests: convert strbuf test to use clar test framework\n     \n         Adapt strbuf test script to clar framework by using clar assertions\n    -    where necessary. Test functions are created as standalone to test\n    -    different test cases.\n    +    where necessary.\n     \n         Mentored-by: Patrick Steinhardt <ps@pks.im>\n         Signed-off-by: Seyi Kuforiji <kuforiji98@gmail.com>\n    @@ t/unit-tests/u-strbuf.c (new)\n     +\t/* Buffers should always be NUL-terminated */\n     +\tcl_assert(buf->buf[buf->len] == '\\0');\n     +\t/*\n    -+\t * Freshly-initialized strbufs may not have a dynamically allocated\n    -+\t * buffer\n    -+\t */\n    -+\tif (buf->len == 0 && buf->alloc == 0)\n    -+\t\treturn;\n    -+\t/* alloc must be at least one byte larger than len */\n    -+\tcl_assert(buf->len < buf->alloc);\n    ++         * In case the buffer contains anything, `alloc` must alloc must\n    ++         * be at least one byte larger than `len`.\n    ++         */\n    ++\tif (buf->len)\n    ++            cl_assert(buf->len < buf->alloc);\n     +}\n     +\n     +void test_strbuf__static_init(void)\n    @@ t/unit-tests/u-strbuf.c (new)\n     +\tsetup(t_addch, \"\");\n     +}\n     +\n    -+void test_strbuf__add_multi_char(void)\n    ++void test_strbuf__add_append_char(void)\n     +{\n     +\tsetup_populated(t_addch, \"initial value\", \"a\");\n     +}\n    @@ t/unit-tests/u-strbuf.c (new)\n     +\tsetup(t_addstr, \"hello there\");\n     +}\n     +\n    -+void test_strbuf__add_multi_str(void)\n    ++void test_strbuf__add_append_str(void)\n     +{\n     +\tsetup_populated(t_addstr, \"initial value\", \"hello there\");\n     +}\n4:  f648cf4a4d ! 4:  2dde9110c2 t/unit-tests: convert strcmp-offset test to use clar test framework\n    @@ Commit message\n         t/unit-tests: convert strcmp-offset test to use clar test framework\n     \n         Adapt strcmp-offset test script to clar framework by using clar\n    -    assertions where necessary. Test functions are created as standalone to\n    -    test different test cases.\n    +    assertions where necessary. Introduce `test_strcmp_offset__empty()` to\n    +    verify `check_strcmp_offset()` behavior when both input strings are\n    +    empty. This ensures the function correctly handles edge cases and\n    +    returns expected values.\n     \n         Mentored-by: Patrick Steinhardt <ps@pks.im>\n         Signed-off-by: Seyi Kuforiji <kuforiji98@gmail.com>\n-- \n2.47.0.86.g15030f9556\n\n"},{"id":"511598","messageId":"20250131221420.38161-2-kuforiji98@gmail.com","threadId":"62873","inReplyTo":"20250131221420.38161-1-kuforiji98@gmail.com","subject":"[PATCH v2 1/4] t/unit-tests: convert hashmap test to use clar test framework","fromName":"Seyi Kuforiji","fromEmail":"kuforiji98@gmail.com","sentAt":"2025-01-31T22:14:17Z","receivedAt":"2025-01-31T22:14:32Z","isPatch":true,"sender":{"key":"kuforiji98@gmail.com","avatar":"https://avatars.githubusercontent.com/u/94905626?v=4"},"body":"Adapts hashmap test script to clar framework by using clar assertions\nwhere necessary.\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-hashmap.c => u-hashmap.c} | 226 +++++++++++-----------\n 3 files changed, 114 insertions(+), 116 deletions(-)\n rename t/unit-tests/{t-hashmap.c => u-hashmap.c} (60%)\n\ndiff --git a/Makefile b/Makefile\nindex b0cdc1fd4a..2d9dad119a 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1339,6 +1339,7 @@ THIRD_PARTY_SOURCES += $(UNIT_TEST_DIR)/clar/clar/%\n \n CLAR_TEST_SUITES += u-ctype\n CLAR_TEST_SUITES += u-hash\n+CLAR_TEST_SUITES += u-hashmap\n CLAR_TEST_SUITES += u-mem-pool\n CLAR_TEST_SUITES += u-prio-queue\n CLAR_TEST_SUITES += u-reftable-tree\n@@ -1349,7 +1350,6 @@ 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-example-decorate\n-UNIT_TEST_PROGRAMS += t-hashmap\n UNIT_TEST_PROGRAMS += t-oid-array\n UNIT_TEST_PROGRAMS += t-oidmap\n UNIT_TEST_PROGRAMS += t-oidtree\ndiff --git a/t/meson.build b/t/meson.build\nindex 14fea8dddf..af597f9804 100644\n--- a/t/meson.build\n+++ b/t/meson.build\n@@ -1,6 +1,7 @@\n clar_test_suites = [\n   'unit-tests/u-ctype.c',\n   'unit-tests/u-hash.c',\n+  'unit-tests/u-hashmap.c',\n   'unit-tests/u-mem-pool.c',\n   'unit-tests/u-prio-queue.c',\n   'unit-tests/u-reftable-tree.c',\n@@ -45,7 +46,6 @@ test('unit-tests', clar_unit_tests)\n \n unit_test_programs = [\n   'unit-tests/t-example-decorate.c',\n-  'unit-tests/t-hashmap.c',\n   'unit-tests/t-oid-array.c',\n   'unit-tests/t-oidmap.c',\n   'unit-tests/t-oidtree.c',\ndiff --git a/t/unit-tests/t-hashmap.c b/t/unit-tests/u-hashmap.c\nsimilarity index 60%\nrename from t/unit-tests/t-hashmap.c\nrename to t/unit-tests/u-hashmap.c\nindex 83b79dff39..eb80aa1348 100644\n--- a/t/unit-tests/t-hashmap.c\n+++ b/t/unit-tests/u-hashmap.c\n@@ -1,4 +1,4 @@\n-#include \"test-lib.h\"\n+#include \"unit-test.h\"\n #include \"hashmap.h\"\n #include \"strbuf.h\"\n \n@@ -83,23 +83,23 @@ static void t_replace(struct hashmap *map, unsigned int ignore_case)\n \tstruct test_entry *entry;\n \n \tentry = alloc_test_entry(\"key1\", \"value1\", ignore_case);\n-\tcheck_pointer_eq(hashmap_put_entry(map, entry, ent), NULL);\n+\tcl_assert_equal_p(hashmap_put_entry(map, entry, ent), NULL);\n \n \tentry = alloc_test_entry(ignore_case ? \"Key1\" : \"key1\", \"value2\",\n \t\t\t\t ignore_case);\n \tentry = hashmap_put_entry(map, entry, ent);\n-\tif (check(entry != NULL))\n-\t\tcheck_str(get_value(entry), \"value1\");\n+\tcl_assert(entry != NULL);\n+\tcl_assert_equal_s(get_value(entry), \"value1\");\n \tfree(entry);\n \n \tentry = alloc_test_entry(\"fooBarFrotz\", \"value3\", ignore_case);\n-\tcheck_pointer_eq(hashmap_put_entry(map, entry, ent), NULL);\n+\tcl_assert_equal_p(hashmap_put_entry(map, entry, ent), NULL);\n \n \tentry = alloc_test_entry(ignore_case ? \"FOObarFrotz\" : \"fooBarFrotz\",\n \t\t\t\t \"value4\", ignore_case);\n \tentry = hashmap_put_entry(map, entry, ent);\n-\tif (check(entry != NULL))\n-\t\tcheck_str(get_value(entry), \"value3\");\n+\tcl_assert(entry != NULL);\n+\tcl_assert_equal_s(get_value(entry), \"value3\");\n \tfree(entry);\n }\n \n@@ -122,20 +122,18 @@ static void t_get(struct hashmap *map, unsigned int ignore_case)\n \tfor (size_t i = 0; i < ARRAY_SIZE(key_val); i++) {\n \t\tentry = alloc_test_entry(key_val[i][0], key_val[i][1],\n \t\t\t\t\t ignore_case);\n-\t\tcheck_pointer_eq(hashmap_put_entry(map, entry, ent), NULL);\n+\t\tcl_assert_equal_p(hashmap_put_entry(map, entry, ent), NULL);\n \t}\n \n \tfor (size_t i = 0; i < ARRAY_SIZE(query); i++) {\n \t\tentry = get_test_entry(map, query[i][0], ignore_case);\n-\t\tif (check(entry != NULL))\n-\t\t\tcheck_str(get_value(entry), query[i][1]);\n-\t\telse\n-\t\t\ttest_msg(\"query key: %s\", query[i][0]);\n+\t\tcl_assert(entry != NULL);\n+\t\tcl_assert_equal_s(get_value(entry), query[i][1]);\n \t}\n \n-\tcheck_pointer_eq(get_test_entry(map, \"notInMap\", ignore_case), NULL);\n-\tcheck_int(map->tablesize, ==, 64);\n-\tcheck_int(hashmap_get_size(map), ==, ARRAY_SIZE(key_val));\n+\tcl_assert_equal_p(get_test_entry(map, \"notInMap\", ignore_case), NULL);\n+\tcl_assert_equal_i(map->tablesize, 64);\n+\tcl_assert_equal_i(hashmap_get_size(map), ARRAY_SIZE(key_val));\n }\n \n static void t_add(struct hashmap *map, unsigned int ignore_case)\n@@ -165,39 +163,19 @@ static void t_add(struct hashmap *map, unsigned int ignore_case)\n \n \t\thashmap_for_each_entry_from(map, entry, ent)\n \t\t{\n-\t\t\tint ret;\n-\t\t\tif (!check_int((ret = key_val_contains(\n-\t\t\t\t\t\tkey_val, seen,\n-\t\t\t\t\t\tARRAY_SIZE(key_val), entry)),\n-\t\t\t\t       ==, 0)) {\n-\t\t\t\tswitch (ret) {\n-\t\t\t\tcase 1:\n-\t\t\t\t\ttest_msg(\"found entry was not given in the input\\n\"\n-\t\t\t\t\t\t \"    key: %s\\n  value: %s\",\n-\t\t\t\t\t\t entry->key, get_value(entry));\n-\t\t\t\t\tbreak;\n-\t\t\t\tcase 2:\n-\t\t\t\t\ttest_msg(\"duplicate entry detected\\n\"\n-\t\t\t\t\t\t \"    key: %s\\n  value: %s\",\n-\t\t\t\t\t\t entry->key, get_value(entry));\n-\t\t\t\t\tbreak;\n-\t\t\t\t}\n-\t\t\t} else {\n-\t\t\t\tcount++;\n-\t\t\t}\n+\t\t\tint ret = key_val_contains(key_val, seen,\n+\t\t\t\t\t\t   ARRAY_SIZE(key_val), entry);\n+\t\t\tcl_assert_equal_i(ret, 0);\n+\t\t\tcount++;\n \t\t}\n-\t\tcheck_int(count, ==, 2);\n+\t\tcl_assert_equal_i(count, 2);\n \t}\n \n-\tfor (size_t i = 0; i < ARRAY_SIZE(seen); i++) {\n-\t\tif (!check_int(seen[i], ==, 1))\n-\t\t\ttest_msg(\"following key-val pair was not iterated over:\\n\"\n-\t\t\t\t \"    key: %s\\n  value: %s\",\n-\t\t\t\t key_val[i][0], key_val[i][1]);\n-\t}\n+\tfor (size_t i = 0; i < ARRAY_SIZE(seen); i++)\n+\t\tcl_assert_equal_i(seen[i], 1);\n \n-\tcheck_int(hashmap_get_size(map), ==, ARRAY_SIZE(key_val));\n-\tcheck_pointer_eq(get_test_entry(map, \"notInMap\", ignore_case), NULL);\n+\tcl_assert_equal_i(hashmap_get_size(map), ARRAY_SIZE(key_val));\n+\tcl_assert_equal_p(get_test_entry(map, \"notInMap\", ignore_case), NULL);\n }\n \n static void t_remove(struct hashmap *map, unsigned int ignore_case)\n@@ -211,24 +189,25 @@ static void t_remove(struct hashmap *map, unsigned int ignore_case)\n \n \tfor (size_t i = 0; i < ARRAY_SIZE(key_val); i++) {\n \t\tentry = alloc_test_entry(key_val[i][0], key_val[i][1], ignore_case);\n-\t\tcheck_pointer_eq(hashmap_put_entry(map, entry, ent), NULL);\n+\t\tcl_assert_equal_p(hashmap_put_entry(map, entry, ent), NULL);\n \t}\n \n \tfor (size_t i = 0; i < ARRAY_SIZE(remove); i++) {\n \t\tentry = alloc_test_entry(remove[i][0], \"\", ignore_case);\n \t\tremoved = hashmap_remove_entry(map, entry, ent, remove[i][0]);\n-\t\tif (check(removed != NULL))\n-\t\t\tcheck_str(get_value(removed), remove[i][1]);\n+\t\tcl_assert(removed != NULL);\n+\t\tcl_assert_equal_s(get_value(removed), remove[i][1]);\n \t\tfree(entry);\n \t\tfree(removed);\n \t}\n \n \tentry = alloc_test_entry(\"notInMap\", \"\", ignore_case);\n-\tcheck_pointer_eq(hashmap_remove_entry(map, entry, ent, \"notInMap\"), NULL);\n+\tcl_assert_equal_p(hashmap_remove_entry(map, entry, ent, \"notInMap\"), NULL);\n \tfree(entry);\n \n-\tcheck_int(map->tablesize, ==, 64);\n-\tcheck_int(hashmap_get_size(map), ==, ARRAY_SIZE(key_val) - ARRAY_SIZE(remove));\n+\tcl_assert_equal_i(map->tablesize, 64);\n+\tcl_assert_equal_i(hashmap_get_size(map),\n+\t\t\t  ARRAY_SIZE(key_val) - ARRAY_SIZE(remove));\n }\n \n static void t_iterate(struct hashmap *map, unsigned int ignore_case)\n@@ -242,38 +221,21 @@ static void t_iterate(struct hashmap *map, unsigned int ignore_case)\n \n \tfor (size_t i = 0; i < ARRAY_SIZE(key_val); i++) {\n \t\tentry = alloc_test_entry(key_val[i][0], key_val[i][1], ignore_case);\n-\t\tcheck_pointer_eq(hashmap_put_entry(map, entry, ent), NULL);\n+\t\tcl_assert_equal_p(hashmap_put_entry(map, entry, ent), NULL);\n \t}\n \n \thashmap_for_each_entry(map, &iter, entry, ent /* member name */)\n \t{\n-\t\tint ret;\n-\t\tif (!check_int((ret = key_val_contains(key_val, seen,\n-\t\t\t\t\t\t       ARRAY_SIZE(key_val),\n-\t\t\t\t\t\t       entry)), ==, 0)) {\n-\t\t\tswitch (ret) {\n-\t\t\tcase 1:\n-\t\t\t\ttest_msg(\"found entry was not given in the input\\n\"\n-\t\t\t\t\t \"    key: %s\\n  value: %s\",\n-\t\t\t\t\t entry->key, get_value(entry));\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 \"    key: %s\\n  value: %s\",\n-\t\t\t\t\t entry->key, get_value(entry));\n-\t\t\t\tbreak;\n-\t\t\t}\n-\t\t}\n+\t\tint ret = key_val_contains(key_val, seen,\n+\t\t\t\t\t   ARRAY_SIZE(key_val),\n+\t\t\t\t\t   entry);\n+\t\tcl_assert(ret == 0);\n \t}\n \n-\tfor (size_t i = 0; i < ARRAY_SIZE(seen); i++) {\n-\t\tif (!check_int(seen[i], ==, 1))\n-\t\t\ttest_msg(\"following key-val pair was not iterated over:\\n\"\n-\t\t\t\t \"    key: %s\\n  value: %s\",\n-\t\t\t\t key_val[i][0], key_val[i][1]);\n-\t}\n+\tfor (size_t i = 0; i < ARRAY_SIZE(seen); i++)\n+\t\tcl_assert_equal_i(seen[i], 1);\n \n-\tcheck_int(hashmap_get_size(map), ==, ARRAY_SIZE(key_val));\n+\tcl_assert_equal_i(hashmap_get_size(map), ARRAY_SIZE(key_val));\n }\n \n static void t_alloc(struct hashmap *map, unsigned int ignore_case)\n@@ -284,17 +246,17 @@ static void t_alloc(struct hashmap *map, unsigned int ignore_case)\n \t\tchar *key = xstrfmt(\"key%d\", i);\n \t\tchar *value = xstrfmt(\"value%d\", i);\n \t\tentry = alloc_test_entry(key, value, ignore_case);\n-\t\tcheck_pointer_eq(hashmap_put_entry(map, entry, ent), NULL);\n+\t\tcl_assert_equal_p(hashmap_put_entry(map, entry, ent), NULL);\n \t\tfree(key);\n \t\tfree(value);\n \t}\n-\tcheck_int(map->tablesize, ==, 64);\n-\tcheck_int(hashmap_get_size(map), ==, 51);\n+\tcl_assert_equal_i(map->tablesize, 64);\n+\tcl_assert_equal_i(hashmap_get_size(map), 51);\n \n \tentry = alloc_test_entry(\"key52\", \"value52\", ignore_case);\n-\tcheck_pointer_eq(hashmap_put_entry(map, entry, ent), NULL);\n-\tcheck_int(map->tablesize, ==, 256);\n-\tcheck_int(hashmap_get_size(map), ==, 52);\n+\tcl_assert_equal_p(hashmap_put_entry(map, entry, ent), NULL);\n+\tcl_assert_equal_i(map->tablesize, 256);\n+\tcl_assert_equal_i(hashmap_get_size(map), 52);\n \n \tfor (int i = 1; i <= 12; i++) {\n \t\tchar *key = xstrfmt(\"key%d\", i);\n@@ -302,27 +264,27 @@ static void t_alloc(struct hashmap *map, unsigned int ignore_case)\n \n \t\tentry = alloc_test_entry(key, \"\", ignore_case);\n \t\tremoved = hashmap_remove_entry(map, entry, ent, key);\n-\t\tif (check(removed != NULL))\n-\t\t\tcheck_str(value, get_value(removed));\n+\t\tcl_assert(removed != NULL);\n+\t\tcl_assert_equal_s(value, get_value(removed));\n \t\tfree(key);\n \t\tfree(value);\n \t\tfree(entry);\n \t\tfree(removed);\n \t}\n-\tcheck_int(map->tablesize, ==, 256);\n-\tcheck_int(hashmap_get_size(map), ==, 40);\n+\tcl_assert_equal_i(map->tablesize, 256);\n+\tcl_assert_equal_i(hashmap_get_size(map), 40);\n \n \tentry = alloc_test_entry(\"key40\", \"\", ignore_case);\n \tremoved = hashmap_remove_entry(map, entry, ent, \"key40\");\n-\tif (check(removed != NULL))\n-\t\tcheck_str(\"value40\", get_value(removed));\n-\tcheck_int(map->tablesize, ==, 64);\n-\tcheck_int(hashmap_get_size(map), ==, 39);\n+\tcl_assert(removed != NULL);\n+\tcl_assert_equal_s(\"value40\", get_value(removed));\n+\tcl_assert_equal_i(map->tablesize, 64);\n+\tcl_assert_equal_i(hashmap_get_size(map), 39);\n \tfree(entry);\n \tfree(removed);\n }\n \n-static void t_intern(void)\n+void test_hashmap__intern(void)\n {\n \tconst char *values[] = { \"value1\", \"Value1\", \"value2\", \"value2\" };\n \n@@ -330,32 +292,68 @@ static void t_intern(void)\n \t\tconst char *i1 = strintern(values[i]);\n \t\tconst char *i2 = strintern(values[i]);\n \n-\t\tif (!check(!strcmp(i1, values[i])))\n-\t\t\ttest_msg(\"strintern(%s) returns %s\\n\", values[i], i1);\n-\t\telse if (!check(i1 != values[i]))\n-\t\t\ttest_msg(\"strintern(%s) returns input pointer\\n\",\n-\t\t\t\t values[i]);\n-\t\telse if (!check_pointer_eq(i1, i2))\n-\t\t\ttest_msg(\"address('%s') != address('%s'), so strintern('%s') != strintern('%s')\",\n-\t\t\t\t i1, i2, values[i], values[i]);\n-\t\telse\n-\t\t\tcheck_str(i1, values[i]);\n+\t\tcl_assert_equal_s(i1, values[i]);\n+\t\tcl_assert(i1 != values[i]);\n+\t\tcl_assert_equal_p(i1, i2);\n \t}\n }\n \n-int cmd_main(int argc UNUSED, const char **argv UNUSED)\n+void test_hashmap__replace_case_sensitive(void)\n+{\n+\tsetup(t_replace, 0);\n+}\n+\n+void test_hashmap__replace_case_insensitive(void)\n+{\n+\tsetup(t_replace, 1);\n+}\n+\n+void test_hashmap__get_case_sensitive(void)\n+{\n+\tsetup(t_get, 0);\n+}\n+\n+void test_hashmap__get_case_insensitive(void)\n+{\n+\tsetup(t_get, 1);\n+}\n+\n+void test_hashmap__add_case_sensitive(void)\n+{\n+\tsetup(t_add, 0);\n+}\n+\n+void test_hashmap__add_case_insensitive(void)\n+{\n+\tsetup(t_add, 1);\n+}\n+\n+void test_hashmap__remove_case_sensitive(void)\n+{\n+\tsetup(t_remove, 0);\n+}\n+\n+void test_hashmap__remove_case_insensitive(void)\n+{\n+\tsetup(t_remove, 1);\n+}\n+\n+void test_hashmap__iterate_case_sensitive(void)\n+{\n+\tsetup(t_iterate, 0);\n+}\n+\n+void test_hashmap__iterate_case_insensitive(void)\n+{\n+\tsetup(t_iterate, 1);\n+}\n+\n+void test_hashmap__alloc_case_sensitive(void)\n+{\n+\tsetup(t_alloc, 0);\n+}\n+\n+void test_hashmap__alloc_case_insensitive(void)\n {\n-\tTEST(setup(t_replace, 0), \"replace works\");\n-\tTEST(setup(t_replace, 1), \"replace (case insensitive) works\");\n-\tTEST(setup(t_get, 0), \"get works\");\n-\tTEST(setup(t_get, 1), \"get (case insensitive) works\");\n-\tTEST(setup(t_add, 0), \"add works\");\n-\tTEST(setup(t_add, 1), \"add (case insensitive) works\");\n-\tTEST(setup(t_remove, 0), \"remove works\");\n-\tTEST(setup(t_remove, 1), \"remove (case insensitive) works\");\n-\tTEST(setup(t_iterate, 0), \"iterate works\");\n-\tTEST(setup(t_iterate, 1), \"iterate (case insensitive) works\");\n-\tTEST(setup(t_alloc, 0), \"grow / shrink works\");\n-\tTEST(t_intern(), \"string interning works\");\n-\treturn test_done();\n+\tsetup(t_alloc, 1);\n }\n-- \n2.47.0.86.g15030f9556\n\n"},{"id":"511599","messageId":"20250131221420.38161-3-kuforiji98@gmail.com","threadId":"62873","inReplyTo":"20250131221420.38161-1-kuforiji98@gmail.com","subject":"[PATCH v2 2/4] t/unit-tests: adapt example decorate test to use clar test framework","fromName":"Seyi Kuforiji","fromEmail":"kuforiji98@gmail.com","sentAt":"2025-01-31T22:14:18Z","receivedAt":"2025-01-31T22:14:35Z","isPatch":true,"sender":{"key":"kuforiji98@gmail.com","avatar":"https://avatars.githubusercontent.com/u/94905626?v=4"},"body":"Introduce `test_example_decorate__initialize()` to explicitly set up\nobject IDs and retrieve corresponding objects before tests run. This\nensures a consistent and predictable test state without relying on data\nfrom previous tests.\n\nAdd `test_example_decorate__cleanup()` to clear decorations after each\ntest, preventing interference between tests and ensuring each runs in\nisolation.\n\nAdapt example decorate test script to clar framework by using clar\nassertions where necessary. Previously, tests relied on data written by\nearlier tests, leading to unintended dependencies between them. This\nexplicitly initializes the necessary state within\n`test_example_decorate__readd`, ensuring it does not depend on prior\ntest executions.\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 ...xample-decorate.c => u-example-decorate.c} | 76 ++++++++-----------\n 3 files changed, 35 insertions(+), 45 deletions(-)\n rename t/unit-tests/{t-example-decorate.c => u-example-decorate.c} (30%)\n\ndiff --git a/Makefile b/Makefile\nindex 2d9dad119a..732d765f1c 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1338,6 +1338,7 @@ THIRD_PARTY_SOURCES += $(UNIT_TEST_DIR)/clar/%\n THIRD_PARTY_SOURCES += $(UNIT_TEST_DIR)/clar/clar/%\n \n CLAR_TEST_SUITES += u-ctype\n+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@@ -1349,7 +1350,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-example-decorate\n UNIT_TEST_PROGRAMS += t-oid-array\n UNIT_TEST_PROGRAMS += t-oidmap\n UNIT_TEST_PROGRAMS += t-oidtree\ndiff --git a/t/meson.build b/t/meson.build\nindex af597f9804..c7e08eca6f 100644\n--- a/t/meson.build\n+++ b/t/meson.build\n@@ -1,5 +1,6 @@\n clar_test_suites = [\n   'unit-tests/u-ctype.c',\n+  'unit-tests/u-example-decorate.c',\n   'unit-tests/u-hash.c',\n   'unit-tests/u-hashmap.c',\n   'unit-tests/u-mem-pool.c',\n@@ -45,7 +46,6 @@ clar_unit_tests = executable('unit-tests',\n test('unit-tests', clar_unit_tests)\n \n unit_test_programs = [\n-  'unit-tests/t-example-decorate.c',\n   'unit-tests/t-oid-array.c',\n   'unit-tests/t-oidmap.c',\n   'unit-tests/t-oidtree.c',\ndiff --git a/t/unit-tests/t-example-decorate.c b/t/unit-tests/u-example-decorate.c\nsimilarity index 30%\nrename from t/unit-tests/t-example-decorate.c\nrename to t/unit-tests/u-example-decorate.c\nindex bfc776e223..9b1d1ce753 100644\n--- a/t/unit-tests/t-example-decorate.c\n+++ b/t/unit-tests/u-example-decorate.c\n@@ -1,6 +1,6 @@\n #define USE_THE_REPOSITORY_VARIABLE\n \n-#include \"test-lib.h\"\n+#include \"unit-test.h\"\n #include \"object.h\"\n #include \"decorate.h\"\n #include \"repository.h\"\n@@ -11,64 +11,54 @@ struct test_vars {\n \tint decoration_a, decoration_b;\n };\n \n-static void t_add(struct test_vars *vars)\n+static struct test_vars vars;\n+\n+void test_example_decorate__initialize(void)\n {\n-\tvoid *ret = add_decoration(&vars->n, vars->one, &vars->decoration_a);\n+\tstruct object_id one_oid = { { 1 } }, two_oid = { { 2 } }, three_oid = { { 3 } };\n \n-\tcheck(ret == NULL);\n-\tret = add_decoration(&vars->n, vars->two, NULL);\n-\tcheck(ret == NULL);\n+\tvars.one = lookup_unknown_object(the_repository, &one_oid);\n+\tvars.two = lookup_unknown_object(the_repository, &two_oid);\n+\tvars.three = lookup_unknown_object(the_repository, &three_oid);\n }\n \n-static void t_readd(struct test_vars *vars)\n+void test_example_decorate__cleanup(void)\n {\n-\tvoid *ret = add_decoration(&vars->n, vars->one, NULL);\n-\n-\tcheck(ret == &vars->decoration_a);\n-\tret = add_decoration(&vars->n, vars->two, &vars->decoration_b);\n-\tcheck(ret == NULL);\n+\tclear_decoration(&vars.n, NULL);\n }\n \n-static void t_lookup(struct test_vars *vars)\n+void test_example_decorate__add(void)\n {\n-\tvoid *ret = lookup_decoration(&vars->n, vars->one);\n-\n-\tcheck(ret == NULL);\n-\tret = lookup_decoration(&vars->n, vars->two);\n-\tcheck(ret == &vars->decoration_b);\n-\tret = lookup_decoration(&vars->n, vars->three);\n-\tcheck(ret == NULL);\n+\tcl_assert_equal_p(add_decoration(&vars.n, vars.one, &vars.decoration_a), NULL);\n+\tcl_assert_equal_p(add_decoration(&vars.n, vars.two, NULL), NULL);\n }\n \n-static void t_loop(struct test_vars *vars)\n+void test_example_decorate__readd(void)\n {\n-\tint objects_noticed = 0;\n-\n-\tfor (size_t i = 0; i < vars->n.size; i++) {\n-\t\tif (vars->n.entries[i].base)\n-\t\t\tobjects_noticed++;\n-\t}\n-\tcheck_int(objects_noticed, ==, 2);\n+\tcl_assert_equal_p(add_decoration(&vars.n, vars.one, &vars.decoration_a), NULL);\n+\tcl_assert_equal_p(add_decoration(&vars.n, vars.two, NULL), NULL);\n+\tcl_assert_equal_p(add_decoration(&vars.n, vars.one, NULL), &vars.decoration_a);\n+\tcl_assert_equal_p(add_decoration(&vars.n, vars.two, &vars.decoration_b), NULL);\n }\n \n-int cmd_main(int argc UNUSED, const char **argv UNUSED)\n+void test_example_decorate__lookup(void)\n {\n-\tstruct object_id one_oid = { { 1 } }, two_oid = { { 2 } }, three_oid = { { 3 } };\n-\tstruct test_vars vars = { 0 };\n+\tcl_assert_equal_p(add_decoration(&vars.n, vars.two, &vars.decoration_b), NULL);\n+\tcl_assert_equal_p(add_decoration(&vars.n, vars.one, NULL), NULL);\n+\tcl_assert_equal_p(lookup_decoration(&vars.n, vars.two), &vars.decoration_b);\n+\tcl_assert_equal_p(lookup_decoration(&vars.n, vars.one), NULL);\n+}\n \n-\tvars.one = lookup_unknown_object(the_repository, &one_oid);\n-\tvars.two = lookup_unknown_object(the_repository, &two_oid);\n-\tvars.three = lookup_unknown_object(the_repository, &three_oid);\n+void test_example_decorate__loop(void)\n+{\n+\tint objects_noticed = 0;\n \n-\tTEST(t_add(&vars),\n-\t     \"Add 2 objects, one with a non-NULL decoration and one with a NULL decoration.\");\n-\tTEST(t_readd(&vars),\n-\t     \"When re-adding an already existing object, the old decoration is returned.\");\n-\tTEST(t_lookup(&vars),\n-\t     \"Lookup returns the added declarations, or NULL if the object was never added.\");\n-\tTEST(t_loop(&vars), \"The user can also loop through all entries.\");\n+\tcl_assert_equal_p(add_decoration(&vars.n, vars.one, &vars.decoration_a), NULL);\n+\tcl_assert_equal_p(add_decoration(&vars.n, vars.two, &vars.decoration_b), NULL);\n \n-\tclear_decoration(&vars.n, NULL);\n+\tfor (size_t i = 0; i < vars.n.size; i++)\n+\t\tif (vars.n.entries[i].base)\n+\t\t\tobjects_noticed++;\n \n-\treturn test_done();\n+\tcl_assert_equal_i(objects_noticed, 2);\n }\n-- \n2.47.0.86.g15030f9556\n\n"},{"id":"511600","messageId":"20250131221420.38161-4-kuforiji98@gmail.com","threadId":"62873","inReplyTo":"20250131221420.38161-1-kuforiji98@gmail.com","subject":"[PATCH v2 3/4] t/unit-tests: convert strbuf test to use clar test framework","fromName":"Seyi Kuforiji","fromEmail":"kuforiji98@gmail.com","sentAt":"2025-01-31T22:14:19Z","receivedAt":"2025-01-31T22:14:38Z","isPatch":true,"sender":{"key":"kuforiji98@gmail.com","avatar":"https://avatars.githubusercontent.com/u/94905626?v=4"},"body":"Adapt strbuf test script to clar framework by using clar assertions\nwhere necessary.\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-strbuf.c => u-strbuf.c} | 115 ++++++++++++------------\n 3 files changed, 58 insertions(+), 61 deletions(-)\n rename t/unit-tests/{t-strbuf.c => u-strbuf.c} (35%)\n\ndiff --git a/Makefile b/Makefile\nindex 732d765f1c..358193597f 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1344,6 +1344,7 @@ CLAR_TEST_SUITES += u-hashmap\n CLAR_TEST_SUITES += u-mem-pool\n CLAR_TEST_SUITES += u-prio-queue\n CLAR_TEST_SUITES += u-reftable-tree\n+CLAR_TEST_SUITES += u-strbuf\n CLAR_TEST_SUITES += u-strvec\n CLAR_TEST_PROG = $(UNIT_TEST_BIN)/unit-tests$(X)\n CLAR_TEST_OBJS = $(patsubst %,$(UNIT_TEST_DIR)/%.o,$(CLAR_TEST_SUITES))\n@@ -1361,7 +1362,6 @@ UNIT_TEST_PROGRAMS += t-reftable-reader\n UNIT_TEST_PROGRAMS += t-reftable-readwrite\n UNIT_TEST_PROGRAMS += t-reftable-record\n UNIT_TEST_PROGRAMS += t-reftable-stack\n-UNIT_TEST_PROGRAMS += t-strbuf\n UNIT_TEST_PROGRAMS += t-strcmp-offset\n UNIT_TEST_PROGRAMS += t-trailer\n UNIT_TEST_PROGRAMS += t-urlmatch-normalization\ndiff --git a/t/meson.build b/t/meson.build\nindex c7e08eca6f..6cb72842b1 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-prio-queue.c',\n   'unit-tests/u-reftable-tree.c',\n+  'unit-tests/u-strbuf.c',\n   'unit-tests/u-strvec.c',\n ]\n \n@@ -57,7 +58,6 @@ unit_test_programs = [\n   'unit-tests/t-reftable-readwrite.c',\n   'unit-tests/t-reftable-record.c',\n   'unit-tests/t-reftable-stack.c',\n-  'unit-tests/t-strbuf.c',\n   'unit-tests/t-strcmp-offset.c',\n   'unit-tests/t-trailer.c',\n   'unit-tests/t-urlmatch-normalization.c',\ndiff --git a/t/unit-tests/t-strbuf.c b/t/unit-tests/u-strbuf.c\nsimilarity index 35%\nrename from t/unit-tests/t-strbuf.c\nrename to t/unit-tests/u-strbuf.c\nindex 3f4044d697..caa5d78aa3 100644\n--- a/t/unit-tests/t-strbuf.c\n+++ b/t/unit-tests/u-strbuf.c\n@@ -1,4 +1,4 @@\n-#include \"test-lib.h\"\n+#include \"unit-test.h\"\n #include \"strbuf.h\"\n \n /* wrapper that supplies tests with an empty, initialized strbuf */\n@@ -9,8 +9,8 @@ static void setup(void (*f)(struct strbuf*, const void*),\n \n \tf(&buf, data);\n \tstrbuf_release(&buf);\n-\tcheck_uint(buf.len, ==, 0);\n-\tcheck_uint(buf.alloc, ==, 0);\n+\tcl_assert_equal_i(buf.len, 0);\n+\tcl_assert_equal_i(buf.alloc, 0);\n }\n \n /* wrapper that supplies tests with a populated, initialized strbuf */\n@@ -20,49 +20,45 @@ static void setup_populated(void (*f)(struct strbuf*, const void*),\n \tstruct strbuf buf = STRBUF_INIT;\n \n \tstrbuf_addstr(&buf, init_str);\n-\tcheck_uint(buf.len, ==, strlen(init_str));\n+\tcl_assert_equal_i(buf.len, strlen(init_str));\n \tf(&buf, data);\n \tstrbuf_release(&buf);\n-\tcheck_uint(buf.len, ==, 0);\n-\tcheck_uint(buf.alloc, ==, 0);\n+\tcl_assert_equal_i(buf.len, 0);\n+\tcl_assert_equal_i(buf.alloc, 0);\n }\n \n-static int assert_sane_strbuf(struct strbuf *buf)\n+static void assert_sane_strbuf(struct strbuf *buf)\n {\n \t/* Initialized strbufs should always have a non-NULL buffer */\n-\tif (!check(!!buf->buf))\n-\t\treturn 0;\n+\tcl_assert(buf->buf != NULL);\n \t/* Buffers should always be NUL-terminated */\n-\tif (!check_char(buf->buf[buf->len], ==, '\\0'))\n-\t\treturn 0;\n+\tcl_assert(buf->buf[buf->len] == '\\0');\n \t/*\n-\t * Freshly-initialized strbufs may not have a dynamically allocated\n-\t * buffer\n-\t */\n-\tif (buf->len == 0 && buf->alloc == 0)\n-\t\treturn 1;\n-\t/* alloc must be at least one byte larger than len */\n-\treturn check_uint(buf->len, <, buf->alloc);\n+         * In case the buffer contains anything, `alloc` must alloc must\n+         * be at least one byte larger than `len`.\n+         */\n+\tif (buf->len)\n+            cl_assert(buf->len < buf->alloc);\n }\n \n-static void t_static_init(void)\n+void test_strbuf__static_init(void)\n {\n \tstruct strbuf buf = STRBUF_INIT;\n \n-\tcheck_uint(buf.len, ==, 0);\n-\tcheck_uint(buf.alloc, ==, 0);\n-\tcheck_char(buf.buf[0], ==, '\\0');\n+\tcl_assert_equal_i(buf.len, 0);\n+\tcl_assert_equal_i(buf.alloc, 0);\n+\tcl_assert(buf.buf[0] == '\\0');\n }\n \n-static void t_dynamic_init(void)\n+void test_strbuf__dynamic_init(void)\n {\n \tstruct strbuf buf;\n \n \tstrbuf_init(&buf, 1024);\n-\tcheck(assert_sane_strbuf(&buf));\n-\tcheck_uint(buf.len, ==, 0);\n-\tcheck_uint(buf.alloc, >=, 1024);\n-\tcheck_char(buf.buf[0], ==, '\\0');\n+\tassert_sane_strbuf(&buf);\n+\tcl_assert_equal_i(buf.len, 0);\n+\tcl_assert(buf.alloc >= 1024);\n+\tcl_assert(buf.buf[0] == '\\0');\n \tstrbuf_release(&buf);\n }\n \n@@ -73,16 +69,12 @@ static void t_addch(struct strbuf *buf, const void *data)\n \tsize_t orig_alloc = buf->alloc;\n \tsize_t orig_len = buf->len;\n \n-\tif (!check(assert_sane_strbuf(buf)))\n-\t\treturn;\n+\tassert_sane_strbuf(buf);\n \tstrbuf_addch(buf, ch);\n-\tif (!check(assert_sane_strbuf(buf)))\n-\t\treturn;\n-\tif (!(check_uint(buf->len, ==, orig_len + 1) &&\n-\t      check_uint(buf->alloc, >=, orig_alloc)))\n-\t\treturn; /* avoid de-referencing buf->buf */\n-\tcheck_char(buf->buf[buf->len - 1], ==, ch);\n-\tcheck_char(buf->buf[buf->len], ==, '\\0');\n+\tassert_sane_strbuf(buf);\n+\tcl_assert_equal_i(buf->len, orig_len + 1);\n+\tcl_assert(buf->alloc >= orig_alloc);\n+\tcl_assert(buf->buf[buf->len] == '\\0');\n }\n \n static void t_addstr(struct strbuf *buf, const void *data)\n@@ -92,31 +84,36 @@ static void t_addstr(struct strbuf *buf, const void *data)\n \tsize_t orig_alloc = buf->alloc;\n \tsize_t orig_len = buf->len;\n \n-\tif (!check(assert_sane_strbuf(buf)))\n-\t\treturn;\n+\tassert_sane_strbuf(buf);\n \tstrbuf_addstr(buf, text);\n-\tif (!check(assert_sane_strbuf(buf)))\n-\t\treturn;\n-\tif (!(check_uint(buf->len, ==, orig_len + len) &&\n-\t      check_uint(buf->alloc, >=, orig_alloc) &&\n-\t      check_uint(buf->alloc, >, orig_len + len) &&\n-\t      check_char(buf->buf[orig_len + len], ==, '\\0')))\n-\t    return;\n-\tcheck_str(buf->buf + orig_len, text);\n+\tassert_sane_strbuf(buf);\n+\tcl_assert_equal_i(buf->len, orig_len + len);\n+\tcl_assert(buf->alloc >= orig_alloc);\n+\tcl_assert(buf->buf[buf->len] == '\\0');\n+\tcl_assert_equal_s(buf->buf + orig_len, text);\n }\n \n-int cmd_main(int argc UNUSED, const char **argv UNUSED)\n+void test_strbuf__add_single_char(void)\n {\n-\tif (!TEST(t_static_init(), \"static initialization works\"))\n-\t\ttest_skip_all(\"STRBUF_INIT is broken\");\n-\tTEST(t_dynamic_init(), \"dynamic initialization works\");\n-\tTEST(setup(t_addch, \"a\"), \"strbuf_addch adds char\");\n-\tTEST(setup(t_addch, \"\"), \"strbuf_addch adds NUL char\");\n-\tTEST(setup_populated(t_addch, \"initial value\", \"a\"),\n-\t     \"strbuf_addch appends to initial value\");\n-\tTEST(setup(t_addstr, \"hello there\"), \"strbuf_addstr adds string\");\n-\tTEST(setup_populated(t_addstr, \"initial value\", \"hello there\"),\n-\t     \"strbuf_addstr appends string to initial value\");\n-\n-\treturn test_done();\n+\tsetup(t_addch, \"a\");\n+}\n+\n+void test_strbuf__add_empty_char(void)\n+{\n+\tsetup(t_addch, \"\");\n+}\n+\n+void test_strbuf__add_append_char(void)\n+{\n+\tsetup_populated(t_addch, \"initial value\", \"a\");\n+}\n+\n+void test_strbuf__add_single_str(void)\n+{\n+\tsetup(t_addstr, \"hello there\");\n+}\n+\n+void test_strbuf__add_append_str(void)\n+{\n+\tsetup_populated(t_addstr, \"initial value\", \"hello there\");\n }\n-- \n2.47.0.86.g15030f9556\n\n"},{"id":"511601","messageId":"20250131221420.38161-5-kuforiji98@gmail.com","threadId":"62873","inReplyTo":"20250131221420.38161-1-kuforiji98@gmail.com","subject":"[PATCH v2 4/4] t/unit-tests: convert strcmp-offset test to use clar test framework","fromName":"Seyi Kuforiji","fromEmail":"kuforiji98@gmail.com","sentAt":"2025-01-31T22:14:20Z","receivedAt":"2025-01-31T22:14:42Z","isPatch":true,"sender":{"key":"kuforiji98@gmail.com","avatar":"https://avatars.githubusercontent.com/u/94905626?v=4"},"body":"Adapt strcmp-offset test script to clar framework by using clar\nassertions where necessary. Introduce `test_strcmp_offset__empty()` to\nverify `check_strcmp_offset()` behavior when both input strings are\nempty. This ensures the function correctly handles edge cases and\nreturns expected values.\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-strcmp-offset.c => u-strcmp-offset.c}  | 36 ++++++++++++-------\n 3 files changed, 25 insertions(+), 15 deletions(-)\n rename t/unit-tests/{t-strcmp-offset.c => u-strcmp-offset.c} (39%)\n\ndiff --git a/Makefile b/Makefile\nindex 358193597f..76b5de4fdd 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1345,6 +1345,7 @@ CLAR_TEST_SUITES += u-mem-pool\n CLAR_TEST_SUITES += u-prio-queue\n CLAR_TEST_SUITES += u-reftable-tree\n CLAR_TEST_SUITES += u-strbuf\n+CLAR_TEST_SUITES += u-strcmp-offset\n CLAR_TEST_SUITES += u-strvec\n CLAR_TEST_PROG = $(UNIT_TEST_BIN)/unit-tests$(X)\n CLAR_TEST_OBJS = $(patsubst %,$(UNIT_TEST_DIR)/%.o,$(CLAR_TEST_SUITES))\n@@ -1362,7 +1363,6 @@ UNIT_TEST_PROGRAMS += t-reftable-reader\n UNIT_TEST_PROGRAMS += t-reftable-readwrite\n UNIT_TEST_PROGRAMS += t-reftable-record\n UNIT_TEST_PROGRAMS += t-reftable-stack\n-UNIT_TEST_PROGRAMS += t-strcmp-offset\n UNIT_TEST_PROGRAMS += t-trailer\n UNIT_TEST_PROGRAMS += t-urlmatch-normalization\n UNIT_TEST_PROGS = $(patsubst %,$(UNIT_TEST_BIN)/%$X,$(UNIT_TEST_PROGRAMS))\ndiff --git a/t/meson.build b/t/meson.build\nindex 6cb72842b1..3935782bbb 100644\n--- a/t/meson.build\n+++ b/t/meson.build\n@@ -7,6 +7,7 @@ clar_test_suites = [\n   'unit-tests/u-prio-queue.c',\n   'unit-tests/u-reftable-tree.c',\n   'unit-tests/u-strbuf.c',\n+  'unit-tests/u-strcmp-offset.c',\n   'unit-tests/u-strvec.c',\n ]\n \n@@ -58,7 +59,6 @@ unit_test_programs = [\n   'unit-tests/t-reftable-readwrite.c',\n   'unit-tests/t-reftable-record.c',\n   'unit-tests/t-reftable-stack.c',\n-  'unit-tests/t-strcmp-offset.c',\n   'unit-tests/t-trailer.c',\n   'unit-tests/t-urlmatch-normalization.c',\n ]\ndiff --git a/t/unit-tests/t-strcmp-offset.c b/t/unit-tests/u-strcmp-offset.c\nsimilarity index 39%\nrename from t/unit-tests/t-strcmp-offset.c\nrename to t/unit-tests/u-strcmp-offset.c\nindex 6880f21161..7e8e9acf3c 100644\n--- a/t/unit-tests/t-strcmp-offset.c\n+++ b/t/unit-tests/u-strcmp-offset.c\n@@ -1,4 +1,4 @@\n-#include \"test-lib.h\"\n+#include \"unit-test.h\"\n #include \"read-cache-ll.h\"\n \n static void check_strcmp_offset(const char *string1, const char *string2,\n@@ -15,21 +15,31 @@ static void check_strcmp_offset(const char *string1, const char *string2,\n \t\t\tresult > 0 ? 1 :\n \t\t\t0);\n \n-\tcheck_int(result, ==, expect_result);\n-\tcheck_uint((uintmax_t)offset, ==, expect_offset);\n+\tcl_assert_equal_i(result, expect_result);\n+\tcl_assert_equal_i((uintmax_t)offset, expect_offset);\n }\n \n-#define TEST_STRCMP_OFFSET(string1, string2, expect_result, expect_offset) \\\n-\tTEST(check_strcmp_offset(string1, string2, expect_result,          \\\n-\t\t\t\t expect_offset),                           \\\n-\t     \"strcmp_offset(%s, %s) works\", #string1, #string2)\n+void test_strcmp_offset__empty(void)\n+{\n+\tcheck_strcmp_offset(\"\", \"\", 0, 0);\n+}\n+\n+void test_strcmp_offset__equal(void)\n+{\n+\tcheck_strcmp_offset(\"abc\", \"abc\", 0, 3);\n+}\n \n-int cmd_main(int argc UNUSED, const char **argv UNUSED)\n+void test_strcmp_offset__different(void)\n {\n-\tTEST_STRCMP_OFFSET(\"abc\", \"abc\", 0, 3);\n-\tTEST_STRCMP_OFFSET(\"abc\", \"def\", -1, 0);\n-\tTEST_STRCMP_OFFSET(\"abc\", \"abz\", -1, 2);\n-\tTEST_STRCMP_OFFSET(\"abc\", \"abcdef\", -1, 3);\n+\tcheck_strcmp_offset(\"abc\", \"def\", -1, 0);\n+}\n \n-\treturn test_done();\n+void test_strcmp_offset__mismatch(void)\n+{\n+\tcheck_strcmp_offset(\"abc\", \"abz\", -1, 2);\n+}\n+\n+void test_strcmp_offset__different_length(void)\n+{\n+\tcheck_strcmp_offset(\"abc\", \"abcdef\", -1, 3);\n }\n-- \n2.47.0.86.g15030f9556\n\n"},{"id":"511604","messageId":"xmqqikpu60yg.fsf@gitster.g","threadId":"62873","inReplyTo":"20250131221420.38161-2-kuforiji98@gmail.com","subject":"Re: [PATCH v2 1/4] t/unit-tests: convert hashmap test to use clar test framework","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-01-31T23:03:03Z","receivedAt":"2025-01-31T23:03:06Z","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> Adapts hashmap test script to clar framework by using clar assertions\n> where necessary.\n>\n> Mentored-by: Patrick Steinhardt <ps@pks.im>\n> Signed-off-by: Seyi Kuforiji <kuforiji98@gmail.com>\n> ---\n>  Makefile                                  |   2 +-\n>  t/meson.build                             |   2 +-\n>  t/unit-tests/{t-hashmap.c => u-hashmap.c} | 226 +++++++++++-----------\n>  3 files changed, 114 insertions(+), 116 deletions(-)\n>  rename t/unit-tests/{t-hashmap.c => u-hashmap.c} (60%)\n\nThe conversion looks fairly straight-forward.\nNice work.\n\n"},{"id":"511605","messageId":"xmqqbjvm60tb.fsf@gitster.g","threadId":"62873","inReplyTo":"20250131221420.38161-1-kuforiji98@gmail.com","subject":"Re: [PATCH v2 0/4] t/unit-tests: convert unit-tests to use clar","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-01-31T23:06:08Z","receivedAt":"2025-01-31T23:06:11Z","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> Hello,\n>\n> This small patch series transitions the existing unit test files to the\n> Clar testing framework. This change is part of our ongoing effort to\n> standardize our testing approach and enhance maintainability.\n>\n> Changes in v2:\n> - small fixes to the commit messages and how they read\n> - some small code fix up and refactoring\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 (4):\n>   t/unit-tests: convert hashmap test to use clar test framework\n>   t/unit-tests: adapt example decorate test to use clar test framework\n>   t/unit-tests: convert strbuf test to use clar test framework\n>   t/unit-tests: convert strcmp-offset test to use clar test framework\n\nOverall they looked quite straight-forward rewrite.  Nicely done.\n\nQueued with automated fix-ups, so there is no need to resend only to\nfix below.\n\nThanks.\n\nApplying: t/unit-tests: convert hashmap test to use clar test framework\nApplying: t/unit-tests: adapt example decorate test to use clar test framework\n.git/rebase-apply/patch:105: indent with spaces.\n         * In case the buffer contains anything, `alloc` must alloc must\n.git/rebase-apply/patch:106: indent with spaces.\n         * be at least one byte larger than `len`.\n.git/rebase-apply/patch:107: indent with spaces.\n         */\n.git/rebase-apply/patch:109: indent with spaces.\n            cl_assert(buf->len < buf->alloc);\nwarning: 4 lines add whitespace errors.\nApplying: t/unit-tests: convert strbuf test to use clar test framework\nApplying: t/unit-tests: convert strcmp-offset test to use clar test framework\n\n"},{"id":"511655","messageId":"6be977a0-4bf9-4568-9b28-cdc988a49b89@gmail.com","threadId":"62873","inReplyTo":"20250131221420.38161-2-kuforiji98@gmail.com","subject":"Re: [PATCH v2 1/4] t/unit-tests: convert hashmap test to use clar test framework","fromName":"","fromEmail":"phillip.wood123@gmail.com","sentAt":"2025-02-02T11:09:25Z","receivedAt":"2025-02-02T11:09:30Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Seyi\n\nOn 31/01/2025 22:14, Seyi Kuforiji wrote:\n> Adapts hashmap test script to clar framework by using clar assertions\n> where necessary.\n\nAs Patrick mentioned we use the imperative mood for our commit messages\nso this should start \"Adapt\". I'm a bit confused by \"where necessary\" -\nisn't that everywhere because we're using a different framework. Maybe\nit would be clearer to say that we're replacing the existing assertions\nwith the equivalent clar assertions.\n\nThe conversion here looks correct but I'm concerned about the loss of\ndiagnostic messages.\n\n> -\tif (check(entry != NULL))\n> -\t\tcheck_str(get_value(entry), \"value3\");\n> +\tcl_assert(entry != NULL);\n> +\tcl_assert_equal_s(get_value(entry), \"value3\");\n\nUnfortunately cl_assert_equal_s() is not equivalent to check_str()\nbecause it does not handle NULL correctly. I think it would be very\nhelpful to fix that upstream. The diff below shows a possible solution\nwhich avoids any ambiguity as to which pointer is NULL by only quoting\nnon-NULL values. In the long run it would be good to fix\ncl_assert_equal_s() to properly quote control characters as check_str()\ndoes.\n\ndiff --git a/t/unit-tests/clar/clar.c b/t/unit-tests/clar/clar.c\nindex d54e4553674..16f86c952f7 100644\n--- a/t/unit-tests/clar/clar.c\n+++ b/t/unit-tests/clar/clar.c\n@@ -754,7 +754,12 @@ void clar__assert_equal(\n                                  p_snprintf(buf, sizeof(buf), \"'%s' != '%s' (at byte %d)\",\n                                          s1, s2, pos);\n                          } else {\n-                                p_snprintf(buf, sizeof(buf), \"'%s' != '%s'\", s1, s2);\n+                                const char *q1 = s1 ? \"'\" : \"\";\n+                                const char *q2 = s2 ? \"'\" : \"\";\n+                                s1 = s1 ? s1 : \"NULL\";\n+                                s2 = s2 ? s2 : \"NULL\";\n+                                p_snprintf(buf, sizeof(buf), \"%s%s%s != %s%s%s\",\n+                                           q1, s1, q1, q2, s2, q2);\n                          }\n                  }\n          }\n  \n>   \tfor (size_t i = 0; i < ARRAY_SIZE(query); i++) {\n>   \t\tentry = get_test_entry(map, query[i][0], ignore_case);\n> -\t\tif (check(entry != NULL))\n> -\t\t\tcheck_str(get_value(entry), query[i][1]);\n> -\t\telse\n> -\t\t\ttest_msg(\"query key: %s\", query[i][0]);\n\nIt is a shame that we're removing all of the helpful debugging messages\nfrom this test. It would be much nicer if we could keep them by using an\nif statement and cl_failf() as we do in u-ctype.c\n\nBest Wishes\n\nPhillip\n\n> +\t\tcl_assert(entry != NULL);\n> +\t\tcl_assert_equal_s(get_value(entry), query[i][1]);\n>   \t}\n>   \n> -\tcheck_pointer_eq(get_test_entry(map, \"notInMap\", ignore_case), NULL);\n> -\tcheck_int(map->tablesize, ==, 64);\n> -\tcheck_int(hashmap_get_size(map), ==, ARRAY_SIZE(key_val));\n> +\tcl_assert_equal_p(get_test_entry(map, \"notInMap\", ignore_case), NULL);\n> +\tcl_assert_equal_i(map->tablesize, 64);\n> +\tcl_assert_equal_i(hashmap_get_size(map), ARRAY_SIZE(key_val));\n>   }\n>   \n>   static void t_add(struct hashmap *map, unsigned int ignore_case)\n> @@ -165,39 +163,19 @@ static void t_add(struct hashmap *map, unsigned int ignore_case)\n>   \n>   \t\thashmap_for_each_entry_from(map, entry, ent)\n>   \t\t{\n> -\t\t\tint ret;\n> -\t\t\tif (!check_int((ret = key_val_contains(\n> -\t\t\t\t\t\tkey_val, seen,\n> -\t\t\t\t\t\tARRAY_SIZE(key_val), entry)),\n> -\t\t\t\t       ==, 0)) {\n> -\t\t\t\tswitch (ret) {\n> -\t\t\t\tcase 1:\n> -\t\t\t\t\ttest_msg(\"found entry was not given in the input\\n\"\n> -\t\t\t\t\t\t \"    key: %s\\n  value: %s\",\n> -\t\t\t\t\t\t entry->key, get_value(entry));\n> -\t\t\t\t\tbreak;\n> -\t\t\t\tcase 2:\n> -\t\t\t\t\ttest_msg(\"duplicate entry detected\\n\"\n> -\t\t\t\t\t\t \"    key: %s\\n  value: %s\",\n> -\t\t\t\t\t\t entry->key, get_value(entry));\n> -\t\t\t\t\tbreak;\n> -\t\t\t\t}\n> -\t\t\t} else {\n> -\t\t\t\tcount++;\n> -\t\t\t}\n> +\t\t\tint ret = key_val_contains(key_val, seen,\n> +\t\t\t\t\t\t   ARRAY_SIZE(key_val), entry);\n> +\t\t\tcl_assert_equal_i(ret, 0);\n> +\t\t\tcount++;\n>   \t\t}\n> -\t\tcheck_int(count, ==, 2);\n> +\t\tcl_assert_equal_i(count, 2);\n>   \t}\n>   \n> -\tfor (size_t i = 0; i < ARRAY_SIZE(seen); i++) {\n> -\t\tif (!check_int(seen[i], ==, 1))\n> -\t\t\ttest_msg(\"following key-val pair was not iterated over:\\n\"\n> -\t\t\t\t \"    key: %s\\n  value: %s\",\n> -\t\t\t\t key_val[i][0], key_val[i][1]);\n> -\t}\n> +\tfor (size_t i = 0; i < ARRAY_SIZE(seen); i++)\n> +\t\tcl_assert_equal_i(seen[i], 1);\n>   \n> -\tcheck_int(hashmap_get_size(map), ==, ARRAY_SIZE(key_val));\n> -\tcheck_pointer_eq(get_test_entry(map, \"notInMap\", ignore_case), NULL);\n> +\tcl_assert_equal_i(hashmap_get_size(map), ARRAY_SIZE(key_val));\n> +\tcl_assert_equal_p(get_test_entry(map, \"notInMap\", ignore_case), NULL);\n>   }\n>   \n>   static void t_remove(struct hashmap *map, unsigned int ignore_case)\n> @@ -211,24 +189,25 @@ static void t_remove(struct hashmap *map, unsigned int ignore_case)\n>   \n>   \tfor (size_t i = 0; i < ARRAY_SIZE(key_val); i++) {\n>   \t\tentry = alloc_test_entry(key_val[i][0], key_val[i][1], ignore_case);\n> -\t\tcheck_pointer_eq(hashmap_put_entry(map, entry, ent), NULL);\n> +\t\tcl_assert_equal_p(hashmap_put_entry(map, entry, ent), NULL);\n>   \t}\n>   \n>   \tfor (size_t i = 0; i < ARRAY_SIZE(remove); i++) {\n>   \t\tentry = alloc_test_entry(remove[i][0], \"\", ignore_case);\n>   \t\tremoved = hashmap_remove_entry(map, entry, ent, remove[i][0]);\n> -\t\tif (check(removed != NULL))\n> -\t\t\tcheck_str(get_value(removed), remove[i][1]);\n> +\t\tcl_assert(removed != NULL);\n> +\t\tcl_assert_equal_s(get_value(removed), remove[i][1]);\n>   \t\tfree(entry);\n>   \t\tfree(removed);\n>   \t}\n>   \n>   \tentry = alloc_test_entry(\"notInMap\", \"\", ignore_case);\n> -\tcheck_pointer_eq(hashmap_remove_entry(map, entry, ent, \"notInMap\"), NULL);\n> +\tcl_assert_equal_p(hashmap_remove_entry(map, entry, ent, \"notInMap\"), NULL);\n>   \tfree(entry);\n>   \n> -\tcheck_int(map->tablesize, ==, 64);\n> -\tcheck_int(hashmap_get_size(map), ==, ARRAY_SIZE(key_val) - ARRAY_SIZE(remove));\n> +\tcl_assert_equal_i(map->tablesize, 64);\n> +\tcl_assert_equal_i(hashmap_get_size(map),\n> +\t\t\t  ARRAY_SIZE(key_val) - ARRAY_SIZE(remove));\n>   }\n>   \n>   static void t_iterate(struct hashmap *map, unsigned int ignore_case)\n> @@ -242,38 +221,21 @@ static void t_iterate(struct hashmap *map, unsigned int ignore_case)\n>   \n>   \tfor (size_t i = 0; i < ARRAY_SIZE(key_val); i++) {\n>   \t\tentry = alloc_test_entry(key_val[i][0], key_val[i][1], ignore_case);\n> -\t\tcheck_pointer_eq(hashmap_put_entry(map, entry, ent), NULL);\n> +\t\tcl_assert_equal_p(hashmap_put_entry(map, entry, ent), NULL);\n>   \t}\n>   \n>   \thashmap_for_each_entry(map, &iter, entry, ent /* member name */)\n>   \t{\n> -\t\tint ret;\n> -\t\tif (!check_int((ret = key_val_contains(key_val, seen,\n> -\t\t\t\t\t\t       ARRAY_SIZE(key_val),\n> -\t\t\t\t\t\t       entry)), ==, 0)) {\n> -\t\t\tswitch (ret) {\n> -\t\t\tcase 1:\n> -\t\t\t\ttest_msg(\"found entry was not given in the input\\n\"\n> -\t\t\t\t\t \"    key: %s\\n  value: %s\",\n> -\t\t\t\t\t entry->key, get_value(entry));\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 \"    key: %s\\n  value: %s\",\n> -\t\t\t\t\t entry->key, get_value(entry));\n> -\t\t\t\tbreak;\n> -\t\t\t}\n> -\t\t}\n> +\t\tint ret = key_val_contains(key_val, seen,\n> +\t\t\t\t\t   ARRAY_SIZE(key_val),\n> +\t\t\t\t\t   entry);\n> +\t\tcl_assert(ret == 0);\n>   \t}\n>   \n> -\tfor (size_t i = 0; i < ARRAY_SIZE(seen); i++) {\n> -\t\tif (!check_int(seen[i], ==, 1))\n> -\t\t\ttest_msg(\"following key-val pair was not iterated over:\\n\"\n> -\t\t\t\t \"    key: %s\\n  value: %s\",\n> -\t\t\t\t key_val[i][0], key_val[i][1]);\n> -\t}\n> +\tfor (size_t i = 0; i < ARRAY_SIZE(seen); i++)\n> +\t\tcl_assert_equal_i(seen[i], 1);\n>   \n> -\tcheck_int(hashmap_get_size(map), ==, ARRAY_SIZE(key_val));\n> +\tcl_assert_equal_i(hashmap_get_size(map), ARRAY_SIZE(key_val));\n>   }\n>   \n>   static void t_alloc(struct hashmap *map, unsigned int ignore_case)\n> @@ -284,17 +246,17 @@ static void t_alloc(struct hashmap *map, unsigned int ignore_case)\n>   \t\tchar *key = xstrfmt(\"key%d\", i);\n>   \t\tchar *value = xstrfmt(\"value%d\", i);\n>   \t\tentry = alloc_test_entry(key, value, ignore_case);\n> -\t\tcheck_pointer_eq(hashmap_put_entry(map, entry, ent), NULL);\n> +\t\tcl_assert_equal_p(hashmap_put_entry(map, entry, ent), NULL);\n>   \t\tfree(key);\n>   \t\tfree(value);\n>   \t}\n> -\tcheck_int(map->tablesize, ==, 64);\n> -\tcheck_int(hashmap_get_size(map), ==, 51);\n> +\tcl_assert_equal_i(map->tablesize, 64);\n> +\tcl_assert_equal_i(hashmap_get_size(map), 51);\n>   \n>   \tentry = alloc_test_entry(\"key52\", \"value52\", ignore_case);\n> -\tcheck_pointer_eq(hashmap_put_entry(map, entry, ent), NULL);\n> -\tcheck_int(map->tablesize, ==, 256);\n> -\tcheck_int(hashmap_get_size(map), ==, 52);\n> +\tcl_assert_equal_p(hashmap_put_entry(map, entry, ent), NULL);\n> +\tcl_assert_equal_i(map->tablesize, 256);\n> +\tcl_assert_equal_i(hashmap_get_size(map), 52);\n>   \n>   \tfor (int i = 1; i <= 12; i++) {\n>   \t\tchar *key = xstrfmt(\"key%d\", i);\n> @@ -302,27 +264,27 @@ static void t_alloc(struct hashmap *map, unsigned int ignore_case)\n>   \n>   \t\tentry = alloc_test_entry(key, \"\", ignore_case);\n>   \t\tremoved = hashmap_remove_entry(map, entry, ent, key);\n> -\t\tif (check(removed != NULL))\n> -\t\t\tcheck_str(value, get_value(removed));\n> +\t\tcl_assert(removed != NULL);\n> +\t\tcl_assert_equal_s(value, get_value(removed));\n>   \t\tfree(key);\n>   \t\tfree(value);\n>   \t\tfree(entry);\n>   \t\tfree(removed);\n>   \t}\n> -\tcheck_int(map->tablesize, ==, 256);\n> -\tcheck_int(hashmap_get_size(map), ==, 40);\n> +\tcl_assert_equal_i(map->tablesize, 256);\n> +\tcl_assert_equal_i(hashmap_get_size(map), 40);\n>   \n>   \tentry = alloc_test_entry(\"key40\", \"\", ignore_case);\n>   \tremoved = hashmap_remove_entry(map, entry, ent, \"key40\");\n> -\tif (check(removed != NULL))\n> -\t\tcheck_str(\"value40\", get_value(removed));\n> -\tcheck_int(map->tablesize, ==, 64);\n> -\tcheck_int(hashmap_get_size(map), ==, 39);\n> +\tcl_assert(removed != NULL);\n> +\tcl_assert_equal_s(\"value40\", get_value(removed));\n> +\tcl_assert_equal_i(map->tablesize, 64);\n> +\tcl_assert_equal_i(hashmap_get_size(map), 39);\n>   \tfree(entry);\n>   \tfree(removed);\n>   }\n>   \n> -static void t_intern(void)\n> +void test_hashmap__intern(void)\n>   {\n>   \tconst char *values[] = { \"value1\", \"Value1\", \"value2\", \"value2\" };\n>   \n> @@ -330,32 +292,68 @@ static void t_intern(void)\n>   \t\tconst char *i1 = strintern(values[i]);\n>   \t\tconst char *i2 = strintern(values[i]);\n>   \n> -\t\tif (!check(!strcmp(i1, values[i])))\n> -\t\t\ttest_msg(\"strintern(%s) returns %s\\n\", values[i], i1);\n> -\t\telse if (!check(i1 != values[i]))\n> -\t\t\ttest_msg(\"strintern(%s) returns input pointer\\n\",\n> -\t\t\t\t values[i]);\n> -\t\telse if (!check_pointer_eq(i1, i2))\n> -\t\t\ttest_msg(\"address('%s') != address('%s'), so strintern('%s') != strintern('%s')\",\n> -\t\t\t\t i1, i2, values[i], values[i]);\n> -\t\telse\n> -\t\t\tcheck_str(i1, values[i]);\n> +\t\tcl_assert_equal_s(i1, values[i]);\n> +\t\tcl_assert(i1 != values[i]);\n> +\t\tcl_assert_equal_p(i1, i2);\n>   \t}\n>   }\n>   \n> -int cmd_main(int argc UNUSED, const char **argv UNUSED)\n> +void test_hashmap__replace_case_sensitive(void)\n> +{\n> +\tsetup(t_replace, 0);\n> +}\n> +\n> +void test_hashmap__replace_case_insensitive(void)\n> +{\n> +\tsetup(t_replace, 1);\n> +}\n> +\n> +void test_hashmap__get_case_sensitive(void)\n> +{\n> +\tsetup(t_get, 0);\n> +}\n> +\n> +void test_hashmap__get_case_insensitive(void)\n> +{\n> +\tsetup(t_get, 1);\n> +}\n> +\n> +void test_hashmap__add_case_sensitive(void)\n> +{\n> +\tsetup(t_add, 0);\n> +}\n> +\n> +void test_hashmap__add_case_insensitive(void)\n> +{\n> +\tsetup(t_add, 1);\n> +}\n> +\n> +void test_hashmap__remove_case_sensitive(void)\n> +{\n> +\tsetup(t_remove, 0);\n> +}\n> +\n> +void test_hashmap__remove_case_insensitive(void)\n> +{\n> +\tsetup(t_remove, 1);\n> +}\n> +\n> +void test_hashmap__iterate_case_sensitive(void)\n> +{\n> +\tsetup(t_iterate, 0);\n> +}\n> +\n> +void test_hashmap__iterate_case_insensitive(void)\n> +{\n> +\tsetup(t_iterate, 1);\n> +}\n> +\n> +void test_hashmap__alloc_case_sensitive(void)\n> +{\n> +\tsetup(t_alloc, 0);\n> +}\n> +\n> +void test_hashmap__alloc_case_insensitive(void)\n>   {\n> -\tTEST(setup(t_replace, 0), \"replace works\");\n> -\tTEST(setup(t_replace, 1), \"replace (case insensitive) works\");\n> -\tTEST(setup(t_get, 0), \"get works\");\n> -\tTEST(setup(t_get, 1), \"get (case insensitive) works\");\n> -\tTEST(setup(t_add, 0), \"add works\");\n> -\tTEST(setup(t_add, 1), \"add (case insensitive) works\");\n> -\tTEST(setup(t_remove, 0), \"remove works\");\n> -\tTEST(setup(t_remove, 1), \"remove (case insensitive) works\");\n> -\tTEST(setup(t_iterate, 0), \"iterate works\");\n> -\tTEST(setup(t_iterate, 1), \"iterate (case insensitive) works\");\n> -\tTEST(setup(t_alloc, 0), \"grow / shrink works\");\n> -\tTEST(t_intern(), \"string interning works\");\n> -\treturn test_done();\n> +\tsetup(t_alloc, 1);\n>   }\n\n"},{"id":"511662","messageId":"ab05e979-2a5f-4532-bb0c-e8c59665647c@gmail.com","threadId":"62873","inReplyTo":"20250131221420.38161-4-kuforiji98@gmail.com","subject":"Re: [PATCH v2 3/4] t/unit-tests: convert strbuf test to use clar test framework","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2025-02-02T14:38:55Z","receivedAt":"2025-02-02T14:39:01Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Seyi\n\nOn 31/01/2025 22:14, Seyi Kuforiji wrote:\n> Adapt strbuf test script to clar framework by using clar assertions\n> where necessary.\n\nThis patch looks correct but it looses the nice messages we get from \ncheck_char() and check_uint(a, </<=/>/>=, b). I think it would be worth \nadding equivalent assertions to clar to avoid that.\n\n>   /* wrapper that supplies tests with an empty, initialized strbuf */\n> @@ -9,8 +9,8 @@ static void setup(void (*f)(struct strbuf*, const void*),\n>   \n>   \tf(&buf, data);\n>   \tstrbuf_release(&buf);\n> -\tcheck_uint(buf.len, ==, 0);\n> -\tcheck_uint(buf.alloc, ==, 0);\n> +\tcl_assert_equal_i(buf.len, 0);\n> +\tcl_assert_equal_i(buf.alloc, 0);\n\nIt is a shame that we do not have an assertion for checking two unsigned \nintegers are equal. In this case it probably does not matter but large \nunsigned values will be printed as negative numbers which could be \nrather confusing for someone trying to debug a test failure.\n\n> -static int assert_sane_strbuf(struct strbuf *buf)\n> +static void assert_sane_strbuf(struct strbuf *buf)\n>   {\n>   \t/* Initialized strbufs should always have a non-NULL buffer */\n> -\tif (!check(!!buf->buf))\n> -\t\treturn 0;\n> +\tcl_assert(buf->buf != NULL);\n>   \t/* Buffers should always be NUL-terminated */\n> -\tif (!check_char(buf->buf[buf->len], ==, '\\0'))\n> -\t\treturn 0;\n> +\tcl_assert(buf->buf[buf->len] == '\\0');\n\nIt is unfortunate that this looses the helpful diagnostic message that \nshows the values of the two chars being compared that is printed by \ncheck_char() when the check fails. I think it would be worth porting \ncheck_char() arcoss to clar. When we decided to move to the new test \nframework we planned to add new assertions to clar to match our existing \nframework.\n\n>   \t/*\n> -\t * Freshly-initialized strbufs may not have a dynamically allocated\n> -\t * buffer\n> -\t */\n> -\tif (buf->len == 0 && buf->alloc == 0)\n> -\t\treturn 1;\n> -\t/* alloc must be at least one byte larger than len */\n> -\treturn check_uint(buf->len, <, buf->alloc);\n> +         * In case the buffer contains anything, `alloc` must alloc must\n> +         * be at least one byte larger than `len`.\n> +         */\n> +\tif (buf->len)\n> +            cl_assert(buf->len < buf->alloc);\n\nThis is another case where we loose a helpful diagnostic message because \nclar lacks cl_assert_lt_u(a, b) to check that unsigned integer a is less \nthan unsigned integer b. I think it would be worth adding the assertions \nthat are missing show that we have the equivalent of\n\n\tcheck_int(a, <, b)\n\tcheck_int(a, <=, b)\n\tcheck_int(a, >, b)\n\tcheck_int(a, >=, b)\n\nand their unsigned companions so we do not regress the diagnostic output \nwhen a test fails.\n\nLooking at the comment that is deleted above I wonder if we should add\n\n\telse\n\t\tcl_assert_equal_i(buf->alloc, 0);\n\nTo check that alloc is zero when len is zero.\n\nBest Wishes\n\nPhillip\n\n>   }\n>   \n> -static void t_static_init(void)\n> +void test_strbuf__static_init(void)\n>   {\n>   \tstruct strbuf buf = STRBUF_INIT;\n>   \n> -\tcheck_uint(buf.len, ==, 0);\n> -\tcheck_uint(buf.alloc, ==, 0);\n> -\tcheck_char(buf.buf[0], ==, '\\0');\n> +\tcl_assert_equal_i(buf.len, 0);\n> +\tcl_assert_equal_i(buf.alloc, 0);\n> +\tcl_assert(buf.buf[0] == '\\0');\n>   }\n>   \n> -static void t_dynamic_init(void)\n> +void test_strbuf__dynamic_init(void)\n>   {\n>   \tstruct strbuf buf;\n>   \n>   \tstrbuf_init(&buf, 1024);\n> -\tcheck(assert_sane_strbuf(&buf));\n> -\tcheck_uint(buf.len, ==, 0);\n> -\tcheck_uint(buf.alloc, >=, 1024);\n> -\tcheck_char(buf.buf[0], ==, '\\0');\n> +\tassert_sane_strbuf(&buf);\n> +\tcl_assert_equal_i(buf.len, 0);\n> +\tcl_assert(buf.alloc >= 1024);\n> +\tcl_assert(buf.buf[0] == '\\0');\n>   \tstrbuf_release(&buf);\n>   }\n>   \n> @@ -73,16 +69,12 @@ static void t_addch(struct strbuf *buf, const void *data)\n>   \tsize_t orig_alloc = buf->alloc;\n>   \tsize_t orig_len = buf->len;\n>   \n> -\tif (!check(assert_sane_strbuf(buf)))\n> -\t\treturn;\n> +\tassert_sane_strbuf(buf);\n>   \tstrbuf_addch(buf, ch);\n> -\tif (!check(assert_sane_strbuf(buf)))\n> -\t\treturn;\n> -\tif (!(check_uint(buf->len, ==, orig_len + 1) &&\n> -\t      check_uint(buf->alloc, >=, orig_alloc)))\n> -\t\treturn; /* avoid de-referencing buf->buf */\n> -\tcheck_char(buf->buf[buf->len - 1], ==, ch);\n> -\tcheck_char(buf->buf[buf->len], ==, '\\0');\n> +\tassert_sane_strbuf(buf);\n> +\tcl_assert_equal_i(buf->len, orig_len + 1);\n> +\tcl_assert(buf->alloc >= orig_alloc);\n> +\tcl_assert(buf->buf[buf->len] == '\\0');\n>   }\n>   \n>   static void t_addstr(struct strbuf *buf, const void *data)\n> @@ -92,31 +84,36 @@ static void t_addstr(struct strbuf *buf, const void *data)\n>   \tsize_t orig_alloc = buf->alloc;\n>   \tsize_t orig_len = buf->len;\n>   \n> -\tif (!check(assert_sane_strbuf(buf)))\n> -\t\treturn;\n> +\tassert_sane_strbuf(buf);\n>   \tstrbuf_addstr(buf, text);\n> -\tif (!check(assert_sane_strbuf(buf)))\n> -\t\treturn;\n> -\tif (!(check_uint(buf->len, ==, orig_len + len) &&\n> -\t      check_uint(buf->alloc, >=, orig_alloc) &&\n> -\t      check_uint(buf->alloc, >, orig_len + len) &&\n> -\t      check_char(buf->buf[orig_len + len], ==, '\\0')))\n> -\t    return;\n> -\tcheck_str(buf->buf + orig_len, text);\n> +\tassert_sane_strbuf(buf);\n> +\tcl_assert_equal_i(buf->len, orig_len + len);\n> +\tcl_assert(buf->alloc >= orig_alloc);\n> +\tcl_assert(buf->buf[buf->len] == '\\0');\n> +\tcl_assert_equal_s(buf->buf + orig_len, text);\n>   }\n>   \n> -int cmd_main(int argc UNUSED, const char **argv UNUSED)\n> +void test_strbuf__add_single_char(void)\n>   {\n> -\tif (!TEST(t_static_init(), \"static initialization works\"))\n> -\t\ttest_skip_all(\"STRBUF_INIT is broken\");\n> -\tTEST(t_dynamic_init(), \"dynamic initialization works\");\n> -\tTEST(setup(t_addch, \"a\"), \"strbuf_addch adds char\");\n> -\tTEST(setup(t_addch, \"\"), \"strbuf_addch adds NUL char\");\n> -\tTEST(setup_populated(t_addch, \"initial value\", \"a\"),\n> -\t     \"strbuf_addch appends to initial value\");\n> -\tTEST(setup(t_addstr, \"hello there\"), \"strbuf_addstr adds string\");\n> -\tTEST(setup_populated(t_addstr, \"initial value\", \"hello there\"),\n> -\t     \"strbuf_addstr appends string to initial value\");\n> -\n> -\treturn test_done();\n> +\tsetup(t_addch, \"a\");\n> +}\n> +\n> +void test_strbuf__add_empty_char(void)\n> +{\n> +\tsetup(t_addch, \"\");\n> +}\n> +\n> +void test_strbuf__add_append_char(void)\n> +{\n> +\tsetup_populated(t_addch, \"initial value\", \"a\");\n> +}\n> +\n> +void test_strbuf__add_single_str(void)\n> +{\n> +\tsetup(t_addstr, \"hello there\");\n> +}\n> +\n> +void test_strbuf__add_append_str(void)\n> +{\n> +\tsetup_populated(t_addstr, \"initial value\", \"hello there\");\n>   }\n\n"},{"id":"511690","messageId":"Z6Bwr6hM54nu8nSS@pks.im","threadId":"62873","inReplyTo":"6be977a0-4bf9-4568-9b28-cdc988a49b89@gmail.com","subject":"Re: [PATCH v2 1/4] t/unit-tests: convert hashmap test to use clar test framework","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-02-03T07:30:55Z","receivedAt":"2025-02-03T07:31:05Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Sun, Feb 02, 2025 at 11:09:25AM +0000, phillip.wood123@gmail.com wrote:\n> On 31/01/2025 22:14, Seyi Kuforiji wrote:\n> > -\tif (check(entry != NULL))\n> > -\t\tcheck_str(get_value(entry), \"value3\");\n> > +\tcl_assert(entry != NULL);\n> > +\tcl_assert_equal_s(get_value(entry), \"value3\");\n> \n> Unfortunately cl_assert_equal_s() is not equivalent to check_str()\n> because it does not handle NULL correctly. I think it would be very\n> helpful to fix that upstream. The diff below shows a possible solution\n> which avoids any ambiguity as to which pointer is NULL by only quoting\n> non-NULL values. In the long run it would be good to fix\n> cl_assert_equal_s() to properly quote control characters as check_str()\n> does.\n\nThat's indeed something we should fix in clar.\n\n> diff --git a/t/unit-tests/clar/clar.c b/t/unit-tests/clar/clar.c\n> index d54e4553674..16f86c952f7 100644\n> --- a/t/unit-tests/clar/clar.c\n> +++ b/t/unit-tests/clar/clar.c\n> @@ -754,7 +754,12 @@ void clar__assert_equal(\n>                                  p_snprintf(buf, sizeof(buf), \"'%s' != '%s' (at byte %d)\",\n>                                          s1, s2, pos);\n>                          } else {\n> -                                p_snprintf(buf, sizeof(buf), \"'%s' != '%s'\", s1, s2);\n> +                                const char *q1 = s1 ? \"'\" : \"\";\n> +                                const char *q2 = s2 ? \"'\" : \"\";\n> +                                s1 = s1 ? s1 : \"NULL\";\n> +                                s2 = s2 ? s2 : \"NULL\";\n> +                                p_snprintf(buf, sizeof(buf), \"%s%s%s != %s%s%s\",\n> +                                           q1, s1, q1, q2, s2, q2);\n>                          }\n>                  }\n>          }\n\nWould you mind creating an upstream pull request with these changes? I'm\nhappy to review, and then we can update our embedded version of clar.\n\n> >   \tfor (size_t i = 0; i < ARRAY_SIZE(query); i++) {\n> >   \t\tentry = get_test_entry(map, query[i][0], ignore_case);\n> > -\t\tif (check(entry != NULL))\n> > -\t\t\tcheck_str(get_value(entry), query[i][1]);\n> > -\t\telse\n> > -\t\t\ttest_msg(\"query key: %s\", query[i][0]);\n> \n> It is a shame that we're removing all of the helpful debugging messages\n> from this test. It would be much nicer if we could keep them by using an\n> if statement and cl_failf() as we do in u-ctype.c\n\nI honestly think that the debug messages don't add much and only add to\nthe noise. You shouldn't ever see them, and if you do something is\nbroken and you'll likely end up pulling out the debugger anyway. So I'm\nmore in the camp of writing unit tests in a concise way rather than the\nneedlessly-verbose style we previously had.\n\nPatrick\n"},{"id":"511738","messageId":"288fbcf3-f6c1-46e4-8124-f4e566abe8af@gmail.com","threadId":"62873","inReplyTo":"Z6Bwr6hM54nu8nSS@pks.im","subject":"Re: [PATCH v2 1/4] t/unit-tests: convert hashmap test to use clar test framework","fromName":"","fromEmail":"phillip.wood123@gmail.com","sentAt":"2025-02-03T14:56:57Z","receivedAt":"2025-02-03T14:57:02Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Patrick\n\nOn 03/02/2025 07:30, Patrick Steinhardt wrote:\n> On Sun, Feb 02, 2025 at 11:09:25AM +0000, phillip.wood123@gmail.com wrote:\n>> On 31/01/2025 22:14, Seyi Kuforiji wrote:\n> \n>> diff --git a/t/unit-tests/clar/clar.c b/t/unit-tests/clar/clar.c\n>> index d54e4553674..16f86c952f7 100644\n>> --- a/t/unit-tests/clar/clar.c\n>> +++ b/t/unit-tests/clar/clar.c\n>> @@ -754,7 +754,12 @@ void clar__assert_equal(\n>>                                   p_snprintf(buf, sizeof(buf), \"'%s' != '%s' (at byte %d)\",\n>>                                           s1, s2, pos);\n>>                           } else {\n>> -                                p_snprintf(buf, sizeof(buf), \"'%s' != '%s'\", s1, s2);\n>> +                                const char *q1 = s1 ? \"'\" : \"\";\n>> +                                const char *q2 = s2 ? \"'\" : \"\";\n>> +                                s1 = s1 ? s1 : \"NULL\";\n>> +                                s2 = s2 ? s2 : \"NULL\";\n>> +                                p_snprintf(buf, sizeof(buf), \"%s%s%s != %s%s%s\",\n>> +                                           q1, s1, q1, q2, s2, q2);\n>>                           }\n>>                   }\n>>           }\n> \n> Would you mind creating an upstream pull request with these changes? I'm\n> happy to review, and then we can update our embedded version of clar.\n\nI've opened a PR at https://github.com/clar-test/clar/pull/114\n\n>>>    \tfor (size_t i = 0; i < ARRAY_SIZE(query); i++) {\n>>>    \t\tentry = get_test_entry(map, query[i][0], ignore_case);\n>>> -\t\tif (check(entry != NULL))\n>>> -\t\t\tcheck_str(get_value(entry), query[i][1]);\n>>> -\t\telse\n>>> -\t\t\ttest_msg(\"query key: %s\", query[i][0]);\n>>\n>> It is a shame that we're removing all of the helpful debugging messages\n>> from this test. It would be much nicer if we could keep them by using an\n>> if statement and cl_failf() as we do in u-ctype.c\n> \n> I honestly think that the debug messages don't add much and only add to\n> the noise. You shouldn't ever see them, and if you do something is\n> broken and you'll likely end up pulling out the debugger anyway. So I'm\n> more in the camp of writing unit tests in a concise way rather than the\n> needlessly-verbose style we previously had.\n\nIf I'm firing up the debugger I'd rather as much detail as I can about \nwhat went wrong so I can see where to set my breakpoints. Otherwise I \nneed to waste time repeating the test to find out exactly what went \nwrong before I can make any progress. My experience with debugging our \nintegration tests is that those tests that take care to print helpful \ndiagnostic messages when they fail are a lot easier to debug than those \nthat don't.\n\nBest Wishes\n\nPhillip\n\n"}]}