On 09/07/2024 20:34, Josh Steadmon wrote:
On 2024.07.08 21:45, Ghanshyam Thakkar wrote:
+static void t_put(struct hashmap *map, int ignore_case)
+{
+ struct test_entry *entry;
+ const char *key_val[][2] = { { "key1", "value1" },
+ { "key2", "value2" },
+ { "fooBarFrotz", "value3" } };
+
+ for (size_t i = 0; i < ARRAY_SIZE(key_val); i++) {
+ entry = alloc_test_entry(ignore_case, key_val[i][0], key_val[i][1]);
+ check(hashmap_put_entry(map, entry, ent) == NULL);
+ }
+
+ entry = alloc_test_entry(ignore_case, "foobarfrotz", "value4");
+ entry = hashmap_put_entry(map, entry, ent);
+ check(ignore_case ? entry != NULL : entry == NULL);
+ free(entry);
+
+ check_int(map->tablesize, ==, 64);
+ check_int(hashmap_get_size(map), ==,
+ ignore_case ? ARRAY_SIZE(key_val) : ARRAY_SIZE(key_val) + 1);
+}
Ahhh, so you're using the same function for both case-sensitive and
-insensitive tests. So I guess TEST_RUN isn't useful here after all.
Personally I'd still rather get rid of setup(), but I don't feel super
strongly about it.
I'm not sure - we have to pass ignore_case to HASHMAP_INIT and the test
function so using setup() means we cannot pass different values to the
two different functions.
For parameterized tests where we calling the same function with
different inputs using a setup function allows us to
- write more concise test code
- easily change the setup of all the tests if the api changes in the
future.
- consistently free resources at the end of a test making it easier to
write leak-free tests
- assert pre- and post- conditions on all tests
Using TEST_RUN is useful to declare the different inputs for each test.
For example in the oid-array tests it allows us to write
TEST_RUN("ordered enumeration") {
const char *input[] = { "88", "44", "aa", "55" };
const char *expected[] = { "44", "55", "88", "aa" };
TEST_ENUMERATION(input, expected)
}
rather than declaring a bunch of variables up front a long way from
where they are used.
+static void t_add(struct hashmap *map, int ignore_case)
+{
+ struct test_entry *entry;
+ const char *key_val[][3] = {
+ { "key1", "value1", "UNUSED" },
+ { ignore_case ? "Key1" : "key1", "value2", "UNUSED" },
+ { "fooBarFrotz", "value3", "UNUSED" },
+ { ignore_case ? "Foobarfrotz" : "fooBarFrotz", "value4", "UNUSED" }
+ };
+ const char *queries[] = { "key1",
+ ignore_case ? "Foobarfrotz" : "fooBarFrotz" };
+
+ for (size_t i = 0; i < ARRAY_SIZE(key_val); i++) {
+ entry = alloc_test_entry(ignore_case, key_val[i][0], key_val[i][1]);
+ hashmap_add(map, &entry->ent);
+ }
+
+ for (size_t i = 0; i < ARRAY_SIZE(queries); i++) {
Since we only have one query, can we remove the loop and simplify the
following block of code?
Also (here and elsewhere), it might be less confusing to say "UNSEEN" /
"SEEN" instead of "UNUSED" / "USED". The latter makes it sound to me
like there's some API requirement to have a 3-item array that we don't
actually need, but in this case those fields are actually used in
key_val_contains() to track duplicates.
The third element just needs to be a boolean flag so it might be better
to use a struct
const struct {
char *key;
char *val;
char seen;
} key_val[] = {
{ .key = "key1", .val = "value1" },
{ .key = ignore_case ? "Key1" : "key1" .val = "value2" }, { .key =
"fooBarFrotz" .val = "value3" },
{ .key = ignore_case ? "Foobarfrotz" : "fooBarFrotz", .value = "value4" }
};
Best Wishes
Phillip
