Hi Ghanshyam
On 28/06/2024 13:20, Ghanshyam Thakkar wrote:
helper/test-oidmap.c along with t0016-oidmap.sh test the oidmap.h
library which is built on top of hashmap.h.
Migrate them to the unit testing framework for better performance,
concise code and better debugging. Along with the migration also plug
memory leaks and make the test logic independent for all the tests.
The migration removes 'put' tests from t0016, because it is used as
setup to all the other tests, so testing it separately does not yield
any benefit.
Helped-by: Phillip Wood <phillip.wood123@xxxxxxxxx>
Mentored-by: Christian Couder <chriscool@xxxxxxxxxxxxx>
Mentored-by: Kaartic Sivaraam <kaartic.sivaraam@xxxxxxxxx>
Signed-off-by: Ghanshyam Thakkar <shyamthakkar001@xxxxxxxxx>
---
This version addresses Phillip's review about detecting duplicates in
oidmap when iterating over it and removing put_and_check_null() to move
the relevant code to setup() instead. And contains some grammer fixes
in the comment.
This version with Junio's fixup addresses my previous comments. One more
thing occurred to me as I was reading it again
+static void t_iterate(struct oidmap *map)
+{
+ struct oidmap_iter iter;
+ struct test_entry *entry;
I wonder if we want to add a bit of paranoia with
int count = 0;
+ oidmap_iter_init(map, &iter);
+ while ((entry = oidmap_iter_next(&iter))) {
+ int ret;
+ if (!check_int((ret = key_val_contains(entry)), ==, 0)) {
+ switch (ret) {
+ case -1:
+ break; /* error message handled by get_oid_arbitrary_hex() */
+ case 1:
+ test_msg("obtained entry was not given in the input\n"
+ " name: %s\n oid: %s\n",
+ entry->name, oid_to_hex(&entry->entry.oid));
+ break;
+ case 2:
+ test_msg("duplicate entry detected\n"
+ " name: %s\n oid: %s\n",
+ entry->name, oid_to_hex(&entry->entry.oid));
+ break;
+ default:
+ test_msg("BUG: invalid return value (%d) from key_val_contains()",
+ ret);
+ break;
+ }
+ }
} else {
count++;
}
+ }
check_int(count, ARRAY_SIZE(key_val));
to check that we iterate over all the entries as well as checking the
size of the hashmap here.
> + check_int(hashmap_get_size(&map->map), ==, ARRAY_SIZE(key_val));
Best Wishes
Phillip
+}
+
+int cmd_main(int argc UNUSED, const char **argv UNUSED)
+{
+ TEST(setup(t_replace), "replace works");
+ TEST(setup(t_get), "get works");
+ TEST(setup(t_remove), "remove works");
+ TEST(setup(t_iterate), "iterate works");
+ return test_done();
+}
