| Message ID | 20250131221420.38161-2-kuforiji98@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 38b066ee7685d0074d3430284f975addda934c17 |
| Headers | show |
| Series | [v2,1/4] t/unit-tests: convert hashmap test to use clar test framework | expand |
Seyi Kuforiji <kuforiji98@gmail.com> writes: > Adapts hashmap test script to clar framework by using clar assertions > where necessary. > > Mentored-by: Patrick Steinhardt <ps@pks.im> > Signed-off-by: Seyi Kuforiji <kuforiji98@gmail.com> > --- > Makefile | 2 +- > t/meson.build | 2 +- > t/unit-tests/{t-hashmap.c => u-hashmap.c} | 226 +++++++++++----------- > 3 files changed, 114 insertions(+), 116 deletions(-) > rename t/unit-tests/{t-hashmap.c => u-hashmap.c} (60%) The conversion looks fairly straight-forward. Nice work.
Hi Seyi On 31/01/2025 22:14, Seyi Kuforiji wrote: > Adapts hashmap test script to clar framework by using clar assertions > where necessary. As Patrick mentioned we use the imperative mood for our commit messages so this should start "Adapt". I'm a bit confused by "where necessary" - isn't that everywhere because we're using a different framework. Maybe it would be clearer to say that we're replacing the existing assertions with the equivalent clar assertions. The conversion here looks correct but I'm concerned about the loss of diagnostic messages. > - if (check(entry != NULL)) > - check_str(get_value(entry), "value3"); > + cl_assert(entry != NULL); > + cl_assert_equal_s(get_value(entry), "value3"); Unfortunately cl_assert_equal_s() is not equivalent to check_str() because it does not handle NULL correctly. I think it would be very helpful to fix that upstream. The diff below shows a possible solution which avoids any ambiguity as to which pointer is NULL by only quoting non-NULL values. In the long run it would be good to fix cl_assert_equal_s() to properly quote control characters as check_str() does. diff --git a/t/unit-tests/clar/clar.c b/t/unit-tests/clar/clar.c index d54e4553674..16f86c952f7 100644 --- a/t/unit-tests/clar/clar.c +++ b/t/unit-tests/clar/clar.c @@ -754,7 +754,12 @@ void clar__assert_equal( p_snprintf(buf, sizeof(buf), "'%s' != '%s' (at byte %d)", s1, s2, pos); } else { - p_snprintf(buf, sizeof(buf), "'%s' != '%s'", s1, s2); + const char *q1 = s1 ? "'" : ""; + const char *q2 = s2 ? "'" : ""; + s1 = s1 ? s1 : "NULL"; + s2 = s2 ? s2 : "NULL"; + p_snprintf(buf, sizeof(buf), "%s%s%s != %s%s%s", + q1, s1, q1, q2, s2, q2); } } } > for (size_t i = 0; i < ARRAY_SIZE(query); i++) { > entry = get_test_entry(map, query[i][0], ignore_case); > - if (check(entry != NULL)) > - check_str(get_value(entry), query[i][1]); > - else > - test_msg("query key: %s", query[i][0]); It is a shame that we're removing all of the helpful debugging messages from this test. It would be much nicer if we could keep them by using an if statement and cl_failf() as we do in u-ctype.c Best Wishes Phillip > + cl_assert(entry != NULL); > + cl_assert_equal_s(get_value(entry), query[i][1]); > } > > - check_pointer_eq(get_test_entry(map, "notInMap", ignore_case), NULL); > - check_int(map->tablesize, ==, 64); > - check_int(hashmap_get_size(map), ==, ARRAY_SIZE(key_val)); > + cl_assert_equal_p(get_test_entry(map, "notInMap", ignore_case), NULL); > + cl_assert_equal_i(map->tablesize, 64); > + cl_assert_equal_i(hashmap_get_size(map), ARRAY_SIZE(key_val)); > } > > static void t_add(struct hashmap *map, unsigned int ignore_case) > @@ -165,39 +163,19 @@ static void t_add(struct hashmap *map, unsigned int ignore_case) > > hashmap_for_each_entry_from(map, entry, ent) > { > - int ret; > - if (!check_int((ret = key_val_contains( > - key_val, seen, > - ARRAY_SIZE(key_val), entry)), > - ==, 0)) { > - switch (ret) { > - case 1: > - test_msg("found entry was not given in the input\n" > - " key: %s\n value: %s", > - entry->key, get_value(entry)); > - break; > - case 2: > - test_msg("duplicate entry detected\n" > - " key: %s\n value: %s", > - entry->key, get_value(entry)); > - break; > - } > - } else { > - count++; > - } > + int ret = key_val_contains(key_val, seen, > + ARRAY_SIZE(key_val), entry); > + cl_assert_equal_i(ret, 0); > + count++; > } > - check_int(count, ==, 2); > + cl_assert_equal_i(count, 2); > } > > - for (size_t i = 0; i < ARRAY_SIZE(seen); i++) { > - if (!check_int(seen[i], ==, 1)) > - test_msg("following key-val pair was not iterated over:\n" > - " key: %s\n value: %s", > - key_val[i][0], key_val[i][1]); > - } > + for (size_t i = 0; i < ARRAY_SIZE(seen); i++) > + cl_assert_equal_i(seen[i], 1); > > - check_int(hashmap_get_size(map), ==, ARRAY_SIZE(key_val)); > - check_pointer_eq(get_test_entry(map, "notInMap", ignore_case), NULL); > + cl_assert_equal_i(hashmap_get_size(map), ARRAY_SIZE(key_val)); > + cl_assert_equal_p(get_test_entry(map, "notInMap", ignore_case), NULL); > } > > static void t_remove(struct hashmap *map, unsigned int ignore_case) > @@ -211,24 +189,25 @@ static void t_remove(struct hashmap *map, unsigned int ignore_case) > > for (size_t i = 0; i < ARRAY_SIZE(key_val); i++) { > entry = alloc_test_entry(key_val[i][0], key_val[i][1], ignore_case); > - check_pointer_eq(hashmap_put_entry(map, entry, ent), NULL); > + cl_assert_equal_p(hashmap_put_entry(map, entry, ent), NULL); > } > > for (size_t i = 0; i < ARRAY_SIZE(remove); i++) { > entry = alloc_test_entry(remove[i][0], "", ignore_case); > removed = hashmap_remove_entry(map, entry, ent, remove[i][0]); > - if (check(removed != NULL)) > - check_str(get_value(removed), remove[i][1]); > + cl_assert(removed != NULL); > + cl_assert_equal_s(get_value(removed), remove[i][1]); > free(entry); > free(removed); > } > > entry = alloc_test_entry("notInMap", "", ignore_case); > - check_pointer_eq(hashmap_remove_entry(map, entry, ent, "notInMap"), NULL); > + cl_assert_equal_p(hashmap_remove_entry(map, entry, ent, "notInMap"), NULL); > free(entry); > > - check_int(map->tablesize, ==, 64); > - check_int(hashmap_get_size(map), ==, ARRAY_SIZE(key_val) - ARRAY_SIZE(remove)); > + cl_assert_equal_i(map->tablesize, 64); > + cl_assert_equal_i(hashmap_get_size(map), > + ARRAY_SIZE(key_val) - ARRAY_SIZE(remove)); > } > > static void t_iterate(struct hashmap *map, unsigned int ignore_case) > @@ -242,38 +221,21 @@ static void t_iterate(struct hashmap *map, unsigned int ignore_case) > > for (size_t i = 0; i < ARRAY_SIZE(key_val); i++) { > entry = alloc_test_entry(key_val[i][0], key_val[i][1], ignore_case); > - check_pointer_eq(hashmap_put_entry(map, entry, ent), NULL); > + cl_assert_equal_p(hashmap_put_entry(map, entry, ent), NULL); > } > > hashmap_for_each_entry(map, &iter, entry, ent /* member name */) > { > - int ret; > - if (!check_int((ret = key_val_contains(key_val, seen, > - ARRAY_SIZE(key_val), > - entry)), ==, 0)) { > - switch (ret) { > - case 1: > - test_msg("found entry was not given in the input\n" > - " key: %s\n value: %s", > - entry->key, get_value(entry)); > - break; > - case 2: > - test_msg("duplicate entry detected\n" > - " key: %s\n value: %s", > - entry->key, get_value(entry)); > - break; > - } > - } > + int ret = key_val_contains(key_val, seen, > + ARRAY_SIZE(key_val), > + entry); > + cl_assert(ret == 0); > } > > - for (size_t i = 0; i < ARRAY_SIZE(seen); i++) { > - if (!check_int(seen[i], ==, 1)) > - test_msg("following key-val pair was not iterated over:\n" > - " key: %s\n value: %s", > - key_val[i][0], key_val[i][1]); > - } > + for (size_t i = 0; i < ARRAY_SIZE(seen); i++) > + cl_assert_equal_i(seen[i], 1); > > - check_int(hashmap_get_size(map), ==, ARRAY_SIZE(key_val)); > + cl_assert_equal_i(hashmap_get_size(map), ARRAY_SIZE(key_val)); > } > > static void t_alloc(struct hashmap *map, unsigned int ignore_case) > @@ -284,17 +246,17 @@ static void t_alloc(struct hashmap *map, unsigned int ignore_case) > char *key = xstrfmt("key%d", i); > char *value = xstrfmt("value%d", i); > entry = alloc_test_entry(key, value, ignore_case); > - check_pointer_eq(hashmap_put_entry(map, entry, ent), NULL); > + cl_assert_equal_p(hashmap_put_entry(map, entry, ent), NULL); > free(key); > free(value); > } > - check_int(map->tablesize, ==, 64); > - check_int(hashmap_get_size(map), ==, 51); > + cl_assert_equal_i(map->tablesize, 64); > + cl_assert_equal_i(hashmap_get_size(map), 51); > > entry = alloc_test_entry("key52", "value52", ignore_case); > - check_pointer_eq(hashmap_put_entry(map, entry, ent), NULL); > - check_int(map->tablesize, ==, 256); > - check_int(hashmap_get_size(map), ==, 52); > + cl_assert_equal_p(hashmap_put_entry(map, entry, ent), NULL); > + cl_assert_equal_i(map->tablesize, 256); > + cl_assert_equal_i(hashmap_get_size(map), 52); > > for (int i = 1; i <= 12; i++) { > char *key = xstrfmt("key%d", i); > @@ -302,27 +264,27 @@ static void t_alloc(struct hashmap *map, unsigned int ignore_case) > > entry = alloc_test_entry(key, "", ignore_case); > removed = hashmap_remove_entry(map, entry, ent, key); > - if (check(removed != NULL)) > - check_str(value, get_value(removed)); > + cl_assert(removed != NULL); > + cl_assert_equal_s(value, get_value(removed)); > free(key); > free(value); > free(entry); > free(removed); > } > - check_int(map->tablesize, ==, 256); > - check_int(hashmap_get_size(map), ==, 40); > + cl_assert_equal_i(map->tablesize, 256); > + cl_assert_equal_i(hashmap_get_size(map), 40); > > entry = alloc_test_entry("key40", "", ignore_case); > removed = hashmap_remove_entry(map, entry, ent, "key40"); > - if (check(removed != NULL)) > - check_str("value40", get_value(removed)); > - check_int(map->tablesize, ==, 64); > - check_int(hashmap_get_size(map), ==, 39); > + cl_assert(removed != NULL); > + cl_assert_equal_s("value40", get_value(removed)); > + cl_assert_equal_i(map->tablesize, 64); > + cl_assert_equal_i(hashmap_get_size(map), 39); > free(entry); > free(removed); > } > > -static void t_intern(void) > +void test_hashmap__intern(void) > { > const char *values[] = { "value1", "Value1", "value2", "value2" }; > > @@ -330,32 +292,68 @@ static void t_intern(void) > const char *i1 = strintern(values[i]); > const char *i2 = strintern(values[i]); > > - if (!check(!strcmp(i1, values[i]))) > - test_msg("strintern(%s) returns %s\n", values[i], i1); > - else if (!check(i1 != values[i])) > - test_msg("strintern(%s) returns input pointer\n", > - values[i]); > - else if (!check_pointer_eq(i1, i2)) > - test_msg("address('%s') != address('%s'), so strintern('%s') != strintern('%s')", > - i1, i2, values[i], values[i]); > - else > - check_str(i1, values[i]); > + cl_assert_equal_s(i1, values[i]); > + cl_assert(i1 != values[i]); > + cl_assert_equal_p(i1, i2); > } > } > > -int cmd_main(int argc UNUSED, const char **argv UNUSED) > +void test_hashmap__replace_case_sensitive(void) > +{ > + setup(t_replace, 0); > +} > + > +void test_hashmap__replace_case_insensitive(void) > +{ > + setup(t_replace, 1); > +} > + > +void test_hashmap__get_case_sensitive(void) > +{ > + setup(t_get, 0); > +} > + > +void test_hashmap__get_case_insensitive(void) > +{ > + setup(t_get, 1); > +} > + > +void test_hashmap__add_case_sensitive(void) > +{ > + setup(t_add, 0); > +} > + > +void test_hashmap__add_case_insensitive(void) > +{ > + setup(t_add, 1); > +} > + > +void test_hashmap__remove_case_sensitive(void) > +{ > + setup(t_remove, 0); > +} > + > +void test_hashmap__remove_case_insensitive(void) > +{ > + setup(t_remove, 1); > +} > + > +void test_hashmap__iterate_case_sensitive(void) > +{ > + setup(t_iterate, 0); > +} > + > +void test_hashmap__iterate_case_insensitive(void) > +{ > + setup(t_iterate, 1); > +} > + > +void test_hashmap__alloc_case_sensitive(void) > +{ > + setup(t_alloc, 0); > +} > + > +void test_hashmap__alloc_case_insensitive(void) > { > - TEST(setup(t_replace, 0), "replace works"); > - TEST(setup(t_replace, 1), "replace (case insensitive) works"); > - TEST(setup(t_get, 0), "get works"); > - TEST(setup(t_get, 1), "get (case insensitive) works"); > - TEST(setup(t_add, 0), "add works"); > - TEST(setup(t_add, 1), "add (case insensitive) works"); > - TEST(setup(t_remove, 0), "remove works"); > - TEST(setup(t_remove, 1), "remove (case insensitive) works"); > - TEST(setup(t_iterate, 0), "iterate works"); > - TEST(setup(t_iterate, 1), "iterate (case insensitive) works"); > - TEST(setup(t_alloc, 0), "grow / shrink works"); > - TEST(t_intern(), "string interning works"); > - return test_done(); > + setup(t_alloc, 1); > }
On Sun, Feb 02, 2025 at 11:09:25AM +0000, phillip.wood123@gmail.com wrote: > On 31/01/2025 22:14, Seyi Kuforiji wrote: > > - if (check(entry != NULL)) > > - check_str(get_value(entry), "value3"); > > + cl_assert(entry != NULL); > > + cl_assert_equal_s(get_value(entry), "value3"); > > Unfortunately cl_assert_equal_s() is not equivalent to check_str() > because it does not handle NULL correctly. I think it would be very > helpful to fix that upstream. The diff below shows a possible solution > which avoids any ambiguity as to which pointer is NULL by only quoting > non-NULL values. In the long run it would be good to fix > cl_assert_equal_s() to properly quote control characters as check_str() > does. That's indeed something we should fix in clar. > diff --git a/t/unit-tests/clar/clar.c b/t/unit-tests/clar/clar.c > index d54e4553674..16f86c952f7 100644 > --- a/t/unit-tests/clar/clar.c > +++ b/t/unit-tests/clar/clar.c > @@ -754,7 +754,12 @@ void clar__assert_equal( > p_snprintf(buf, sizeof(buf), "'%s' != '%s' (at byte %d)", > s1, s2, pos); > } else { > - p_snprintf(buf, sizeof(buf), "'%s' != '%s'", s1, s2); > + const char *q1 = s1 ? "'" : ""; > + const char *q2 = s2 ? "'" : ""; > + s1 = s1 ? s1 : "NULL"; > + s2 = s2 ? s2 : "NULL"; > + p_snprintf(buf, sizeof(buf), "%s%s%s != %s%s%s", > + q1, s1, q1, q2, s2, q2); > } > } > } Would you mind creating an upstream pull request with these changes? I'm happy to review, and then we can update our embedded version of clar. > > for (size_t i = 0; i < ARRAY_SIZE(query); i++) { > > entry = get_test_entry(map, query[i][0], ignore_case); > > - if (check(entry != NULL)) > > - check_str(get_value(entry), query[i][1]); > > - else > > - test_msg("query key: %s", query[i][0]); > > It is a shame that we're removing all of the helpful debugging messages > from this test. It would be much nicer if we could keep them by using an > if statement and cl_failf() as we do in u-ctype.c I honestly think that the debug messages don't add much and only add to the noise. You shouldn't ever see them, and if you do something is broken and you'll likely end up pulling out the debugger anyway. So I'm more in the camp of writing unit tests in a concise way rather than the needlessly-verbose style we previously had. Patrick
Hi Patrick On 03/02/2025 07:30, Patrick Steinhardt wrote: > On Sun, Feb 02, 2025 at 11:09:25AM +0000, phillip.wood123@gmail.com wrote: >> On 31/01/2025 22:14, Seyi Kuforiji wrote: > >> diff --git a/t/unit-tests/clar/clar.c b/t/unit-tests/clar/clar.c >> index d54e4553674..16f86c952f7 100644 >> --- a/t/unit-tests/clar/clar.c >> +++ b/t/unit-tests/clar/clar.c >> @@ -754,7 +754,12 @@ void clar__assert_equal( >> p_snprintf(buf, sizeof(buf), "'%s' != '%s' (at byte %d)", >> s1, s2, pos); >> } else { >> - p_snprintf(buf, sizeof(buf), "'%s' != '%s'", s1, s2); >> + const char *q1 = s1 ? "'" : ""; >> + const char *q2 = s2 ? "'" : ""; >> + s1 = s1 ? s1 : "NULL"; >> + s2 = s2 ? s2 : "NULL"; >> + p_snprintf(buf, sizeof(buf), "%s%s%s != %s%s%s", >> + q1, s1, q1, q2, s2, q2); >> } >> } >> } > > Would you mind creating an upstream pull request with these changes? I'm > happy to review, and then we can update our embedded version of clar. I've opened a PR at https://github.com/clar-test/clar/pull/114 >>> for (size_t i = 0; i < ARRAY_SIZE(query); i++) { >>> entry = get_test_entry(map, query[i][0], ignore_case); >>> - if (check(entry != NULL)) >>> - check_str(get_value(entry), query[i][1]); >>> - else >>> - test_msg("query key: %s", query[i][0]); >> >> It is a shame that we're removing all of the helpful debugging messages >> from this test. It would be much nicer if we could keep them by using an >> if statement and cl_failf() as we do in u-ctype.c > > I honestly think that the debug messages don't add much and only add to > the noise. You shouldn't ever see them, and if you do something is > broken and you'll likely end up pulling out the debugger anyway. So I'm > more in the camp of writing unit tests in a concise way rather than the > needlessly-verbose style we previously had. If I'm firing up the debugger I'd rather as much detail as I can about what went wrong so I can see where to set my breakpoints. Otherwise I need to waste time repeating the test to find out exactly what went wrong before I can make any progress. My experience with debugging our integration tests is that those tests that take care to print helpful diagnostic messages when they fail are a lot easier to debug than those that don't. Best Wishes Phillip
diff --git a/Makefile b/Makefile index b0cdc1fd4a..2d9dad119a 100644 --- a/Makefile +++ b/Makefile @@ -1339,6 +1339,7 @@ THIRD_PARTY_SOURCES += $(UNIT_TEST_DIR)/clar/clar/% CLAR_TEST_SUITES += u-ctype CLAR_TEST_SUITES += u-hash +CLAR_TEST_SUITES += u-hashmap CLAR_TEST_SUITES += u-mem-pool CLAR_TEST_SUITES += u-prio-queue CLAR_TEST_SUITES += u-reftable-tree @@ -1349,7 +1350,6 @@ CLAR_TEST_OBJS += $(UNIT_TEST_DIR)/clar/clar.o CLAR_TEST_OBJS += $(UNIT_TEST_DIR)/unit-test.o UNIT_TEST_PROGRAMS += t-example-decorate -UNIT_TEST_PROGRAMS += t-hashmap UNIT_TEST_PROGRAMS += t-oid-array UNIT_TEST_PROGRAMS += t-oidmap UNIT_TEST_PROGRAMS += t-oidtree diff --git a/t/meson.build b/t/meson.build index 14fea8dddf..af597f9804 100644 --- a/t/meson.build +++ b/t/meson.build @@ -1,6 +1,7 @@ clar_test_suites = [ 'unit-tests/u-ctype.c', 'unit-tests/u-hash.c', + 'unit-tests/u-hashmap.c', 'unit-tests/u-mem-pool.c', 'unit-tests/u-prio-queue.c', 'unit-tests/u-reftable-tree.c', @@ -45,7 +46,6 @@ test('unit-tests', clar_unit_tests) unit_test_programs = [ 'unit-tests/t-example-decorate.c', - 'unit-tests/t-hashmap.c', 'unit-tests/t-oid-array.c', 'unit-tests/t-oidmap.c', 'unit-tests/t-oidtree.c', diff --git a/t/unit-tests/t-hashmap.c b/t/unit-tests/u-hashmap.c similarity index 60% rename from t/unit-tests/t-hashmap.c rename to t/unit-tests/u-hashmap.c index 83b79dff39..eb80aa1348 100644 --- a/t/unit-tests/t-hashmap.c +++ b/t/unit-tests/u-hashmap.c @@ -1,4 +1,4 @@ -#include "test-lib.h" +#include "unit-test.h" #include "hashmap.h" #include "strbuf.h" @@ -83,23 +83,23 @@ static void t_replace(struct hashmap *map, unsigned int ignore_case) struct test_entry *entry; entry = alloc_test_entry("key1", "value1", ignore_case); - check_pointer_eq(hashmap_put_entry(map, entry, ent), NULL); + cl_assert_equal_p(hashmap_put_entry(map, entry, ent), NULL); entry = alloc_test_entry(ignore_case ? "Key1" : "key1", "value2", ignore_case); entry = hashmap_put_entry(map, entry, ent); - if (check(entry != NULL)) - check_str(get_value(entry), "value1"); + cl_assert(entry != NULL); + cl_assert_equal_s(get_value(entry), "value1"); free(entry); entry = alloc_test_entry("fooBarFrotz", "value3", ignore_case); - check_pointer_eq(hashmap_put_entry(map, entry, ent), NULL); + cl_assert_equal_p(hashmap_put_entry(map, entry, ent), NULL); entry = alloc_test_entry(ignore_case ? "FOObarFrotz" : "fooBarFrotz", "value4", ignore_case); entry = hashmap_put_entry(map, entry, ent); - if (check(entry != NULL)) - check_str(get_value(entry), "value3"); + cl_assert(entry != NULL); + cl_assert_equal_s(get_value(entry), "value3"); free(entry); } @@ -122,20 +122,18 @@ static void t_get(struct hashmap *map, unsigned int ignore_case) for (size_t i = 0; i < ARRAY_SIZE(key_val); i++) { entry = alloc_test_entry(key_val[i][0], key_val[i][1], ignore_case); - check_pointer_eq(hashmap_put_entry(map, entry, ent), NULL); + cl_assert_equal_p(hashmap_put_entry(map, entry, ent), NULL); } for (size_t i = 0; i < ARRAY_SIZE(query); i++) { entry = get_test_entry(map, query[i][0], ignore_case); - if (check(entry != NULL)) - check_str(get_value(entry), query[i][1]); - else - test_msg("query key: %s", query[i][0]); + cl_assert(entry != NULL); + cl_assert_equal_s(get_value(entry), query[i][1]); } - check_pointer_eq(get_test_entry(map, "notInMap", ignore_case), NULL); - check_int(map->tablesize, ==, 64); - check_int(hashmap_get_size(map), ==, ARRAY_SIZE(key_val)); + cl_assert_equal_p(get_test_entry(map, "notInMap", ignore_case), NULL); + cl_assert_equal_i(map->tablesize, 64); + cl_assert_equal_i(hashmap_get_size(map), ARRAY_SIZE(key_val)); } static void t_add(struct hashmap *map, unsigned int ignore_case) @@ -165,39 +163,19 @@ static void t_add(struct hashmap *map, unsigned int ignore_case) hashmap_for_each_entry_from(map, entry, ent) { - int ret; - if (!check_int((ret = key_val_contains( - key_val, seen, - ARRAY_SIZE(key_val), entry)), - ==, 0)) { - switch (ret) { - case 1: - test_msg("found entry was not given in the input\n" - " key: %s\n value: %s", - entry->key, get_value(entry)); - break; - case 2: - test_msg("duplicate entry detected\n" - " key: %s\n value: %s", - entry->key, get_value(entry)); - break; - } - } else { - count++; - } + int ret = key_val_contains(key_val, seen, + ARRAY_SIZE(key_val), entry); + cl_assert_equal_i(ret, 0); + count++; } - check_int(count, ==, 2); + cl_assert_equal_i(count, 2); } - for (size_t i = 0; i < ARRAY_SIZE(seen); i++) { - if (!check_int(seen[i], ==, 1)) - test_msg("following key-val pair was not iterated over:\n" - " key: %s\n value: %s", - key_val[i][0], key_val[i][1]); - } + for (size_t i = 0; i < ARRAY_SIZE(seen); i++) + cl_assert_equal_i(seen[i], 1); - check_int(hashmap_get_size(map), ==, ARRAY_SIZE(key_val)); - check_pointer_eq(get_test_entry(map, "notInMap", ignore_case), NULL); + cl_assert_equal_i(hashmap_get_size(map), ARRAY_SIZE(key_val)); + cl_assert_equal_p(get_test_entry(map, "notInMap", ignore_case), NULL); } static void t_remove(struct hashmap *map, unsigned int ignore_case) @@ -211,24 +189,25 @@ static void t_remove(struct hashmap *map, unsigned int ignore_case) for (size_t i = 0; i < ARRAY_SIZE(key_val); i++) { entry = alloc_test_entry(key_val[i][0], key_val[i][1], ignore_case); - check_pointer_eq(hashmap_put_entry(map, entry, ent), NULL); + cl_assert_equal_p(hashmap_put_entry(map, entry, ent), NULL); } for (size_t i = 0; i < ARRAY_SIZE(remove); i++) { entry = alloc_test_entry(remove[i][0], "", ignore_case); removed = hashmap_remove_entry(map, entry, ent, remove[i][0]); - if (check(removed != NULL)) - check_str(get_value(removed), remove[i][1]); + cl_assert(removed != NULL); + cl_assert_equal_s(get_value(removed), remove[i][1]); free(entry); free(removed); } entry = alloc_test_entry("notInMap", "", ignore_case); - check_pointer_eq(hashmap_remove_entry(map, entry, ent, "notInMap"), NULL); + cl_assert_equal_p(hashmap_remove_entry(map, entry, ent, "notInMap"), NULL); free(entry); - check_int(map->tablesize, ==, 64); - check_int(hashmap_get_size(map), ==, ARRAY_SIZE(key_val) - ARRAY_SIZE(remove)); + cl_assert_equal_i(map->tablesize, 64); + cl_assert_equal_i(hashmap_get_size(map), + ARRAY_SIZE(key_val) - ARRAY_SIZE(remove)); } static void t_iterate(struct hashmap *map, unsigned int ignore_case) @@ -242,38 +221,21 @@ static void t_iterate(struct hashmap *map, unsigned int ignore_case) for (size_t i = 0; i < ARRAY_SIZE(key_val); i++) { entry = alloc_test_entry(key_val[i][0], key_val[i][1], ignore_case); - check_pointer_eq(hashmap_put_entry(map, entry, ent), NULL); + cl_assert_equal_p(hashmap_put_entry(map, entry, ent), NULL); } hashmap_for_each_entry(map, &iter, entry, ent /* member name */) { - int ret; - if (!check_int((ret = key_val_contains(key_val, seen, - ARRAY_SIZE(key_val), - entry)), ==, 0)) { - switch (ret) { - case 1: - test_msg("found entry was not given in the input\n" - " key: %s\n value: %s", - entry->key, get_value(entry)); - break; - case 2: - test_msg("duplicate entry detected\n" - " key: %s\n value: %s", - entry->key, get_value(entry)); - break; - } - } + int ret = key_val_contains(key_val, seen, + ARRAY_SIZE(key_val), + entry); + cl_assert(ret == 0); } - for (size_t i = 0; i < ARRAY_SIZE(seen); i++) { - if (!check_int(seen[i], ==, 1)) - test_msg("following key-val pair was not iterated over:\n" - " key: %s\n value: %s", - key_val[i][0], key_val[i][1]); - } + for (size_t i = 0; i < ARRAY_SIZE(seen); i++) + cl_assert_equal_i(seen[i], 1); - check_int(hashmap_get_size(map), ==, ARRAY_SIZE(key_val)); + cl_assert_equal_i(hashmap_get_size(map), ARRAY_SIZE(key_val)); } static void t_alloc(struct hashmap *map, unsigned int ignore_case) @@ -284,17 +246,17 @@ static void t_alloc(struct hashmap *map, unsigned int ignore_case) char *key = xstrfmt("key%d", i); char *value = xstrfmt("value%d", i); entry = alloc_test_entry(key, value, ignore_case); - check_pointer_eq(hashmap_put_entry(map, entry, ent), NULL); + cl_assert_equal_p(hashmap_put_entry(map, entry, ent), NULL); free(key); free(value); } - check_int(map->tablesize, ==, 64); - check_int(hashmap_get_size(map), ==, 51); + cl_assert_equal_i(map->tablesize, 64); + cl_assert_equal_i(hashmap_get_size(map), 51); entry = alloc_test_entry("key52", "value52", ignore_case); - check_pointer_eq(hashmap_put_entry(map, entry, ent), NULL); - check_int(map->tablesize, ==, 256); - check_int(hashmap_get_size(map), ==, 52); + cl_assert_equal_p(hashmap_put_entry(map, entry, ent), NULL); + cl_assert_equal_i(map->tablesize, 256); + cl_assert_equal_i(hashmap_get_size(map), 52); for (int i = 1; i <= 12; i++) { char *key = xstrfmt("key%d", i); @@ -302,27 +264,27 @@ static void t_alloc(struct hashmap *map, unsigned int ignore_case) entry = alloc_test_entry(key, "", ignore_case); removed = hashmap_remove_entry(map, entry, ent, key); - if (check(removed != NULL)) - check_str(value, get_value(removed)); + cl_assert(removed != NULL); + cl_assert_equal_s(value, get_value(removed)); free(key); free(value); free(entry); free(removed); } - check_int(map->tablesize, ==, 256); - check_int(hashmap_get_size(map), ==, 40); + cl_assert_equal_i(map->tablesize, 256); + cl_assert_equal_i(hashmap_get_size(map), 40); entry = alloc_test_entry("key40", "", ignore_case); removed = hashmap_remove_entry(map, entry, ent, "key40"); - if (check(removed != NULL)) - check_str("value40", get_value(removed)); - check_int(map->tablesize, ==, 64); - check_int(hashmap_get_size(map), ==, 39); + cl_assert(removed != NULL); + cl_assert_equal_s("value40", get_value(removed)); + cl_assert_equal_i(map->tablesize, 64); + cl_assert_equal_i(hashmap_get_size(map), 39); free(entry); free(removed); } -static void t_intern(void) +void test_hashmap__intern(void) { const char *values[] = { "value1", "Value1", "value2", "value2" }; @@ -330,32 +292,68 @@ static void t_intern(void) const char *i1 = strintern(values[i]); const char *i2 = strintern(values[i]); - if (!check(!strcmp(i1, values[i]))) - test_msg("strintern(%s) returns %s\n", values[i], i1); - else if (!check(i1 != values[i])) - test_msg("strintern(%s) returns input pointer\n", - values[i]); - else if (!check_pointer_eq(i1, i2)) - test_msg("address('%s') != address('%s'), so strintern('%s') != strintern('%s')", - i1, i2, values[i], values[i]); - else - check_str(i1, values[i]); + cl_assert_equal_s(i1, values[i]); + cl_assert(i1 != values[i]); + cl_assert_equal_p(i1, i2); } } -int cmd_main(int argc UNUSED, const char **argv UNUSED) +void test_hashmap__replace_case_sensitive(void) +{ + setup(t_replace, 0); +} + +void test_hashmap__replace_case_insensitive(void) +{ + setup(t_replace, 1); +} + +void test_hashmap__get_case_sensitive(void) +{ + setup(t_get, 0); +} + +void test_hashmap__get_case_insensitive(void) +{ + setup(t_get, 1); +} + +void test_hashmap__add_case_sensitive(void) +{ + setup(t_add, 0); +} + +void test_hashmap__add_case_insensitive(void) +{ + setup(t_add, 1); +} + +void test_hashmap__remove_case_sensitive(void) +{ + setup(t_remove, 0); +} + +void test_hashmap__remove_case_insensitive(void) +{ + setup(t_remove, 1); +} + +void test_hashmap__iterate_case_sensitive(void) +{ + setup(t_iterate, 0); +} + +void test_hashmap__iterate_case_insensitive(void) +{ + setup(t_iterate, 1); +} + +void test_hashmap__alloc_case_sensitive(void) +{ + setup(t_alloc, 0); +} + +void test_hashmap__alloc_case_insensitive(void) { - TEST(setup(t_replace, 0), "replace works"); - TEST(setup(t_replace, 1), "replace (case insensitive) works"); - TEST(setup(t_get, 0), "get works"); - TEST(setup(t_get, 1), "get (case insensitive) works"); - TEST(setup(t_add, 0), "add works"); - TEST(setup(t_add, 1), "add (case insensitive) works"); - TEST(setup(t_remove, 0), "remove works"); - TEST(setup(t_remove, 1), "remove (case insensitive) works"); - TEST(setup(t_iterate, 0), "iterate works"); - TEST(setup(t_iterate, 1), "iterate (case insensitive) works"); - TEST(setup(t_alloc, 0), "grow / shrink works"); - TEST(t_intern(), "string interning works"); - return test_done(); + setup(t_alloc, 1); }
Adapts hashmap test script to clar framework by using clar assertions where necessary. Mentored-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Seyi Kuforiji <kuforiji98@gmail.com> --- Makefile | 2 +- t/meson.build | 2 +- t/unit-tests/{t-hashmap.c => u-hashmap.c} | 226 +++++++++++----------- 3 files changed, 114 insertions(+), 116 deletions(-) rename t/unit-tests/{t-hashmap.c => u-hashmap.c} (60%)