On Sat, Aug 3, 2024 at 3:35 PM Ghanshyam Thakkar
<shyamthakkar001@xxxxxxxxx> wrote:
>
> helper/test-hashmap.c along with t0011-hashmap.sh test the hashmap.h
> library. Migrate them to the unit testing framework for better
> debugging, runtime performance and concise code.
>
> Along with the migration, make 'add' tests from the shell script order
> agnostic in unit tests, since they iterate over entries with the same
> keys and we do not guarantee the order. This was already done for the
> 'iterate' tests[1].
>
> The helper/test-hashmap.c is still not removed because it contains a
> performance test meant to be run by the user directly (not used in
> t/perf). And it makes sense for such a utility to be a helper.
>
> [1]: e1e7a77141 (t: sort output of hashmap iteration, 2019-07-30)
>
> Mentored-by: Christian Couder <chriscool@xxxxxxxxxxxxx>
> Mentored-by: Kaartic Sivaraam <kaartic.sivaraam@xxxxxxxxx>
> Helped-by: Josh Steadmon <steadmon@xxxxxxxxxx>
> Helped-by: Junio C Hamano <gitster@xxxxxxxxx>
> Helped-by: Phillip Wood <phillip.wood123@xxxxxxxxx>
> Signed-off-by: Ghanshyam Thakkar <shyamthakkar001@xxxxxxxxx>
> ---
> Changes in v5:
> - modify one of the inputs and queries inside t_get() to better check
> ignore-case scenario. This should address the comments from
> Christian in v4.
Yes, the only comment I had about the patch itself is addressed. There
were also comments about the email author, but that is fixed too.
> Range-diff against v4:
Nit (not worth a reroll): the range-diff usually goes before the
actual patch in the section starting with a line containing only 3
dashes
> 1: 04022c4cb9 ! 1: 49cb2db9c8 t: port helper/test-hashmap.c to unit-tests/t-hashmap.c
> @@ t/unit-tests/t-hashmap.c (new)
> + const char *key_val[][2] = { { "key1", "value1" },
> + { "key2", "value2" },
> + { "fooBarFrotz", "value3" },
> -+ { ignore_case ? "TeNor" : "tenor",
> -+ ignore_case ? "value4" : "value5" } };
> ++ { ignore_case ? "key4" : "foobarfrotz",
> ++ "value4" } };
> + const char *query[][2] = {
> + { ignore_case ? "Key1" : "key1", "value1" },
> + { ignore_case ? "keY2" : "key2", "value2" },
> + { ignore_case ? "FOObarFrotz" : "fooBarFrotz", "value3" },
> -+ { ignore_case ? "TENOR" : "tenor",
> -+ ignore_case ? "value4" : "value5" }
> ++ { ignore_case ? "FOObarFrotz" : "foobarfrotz",
> ++ ignore_case ? "value3" : "value4" }
> + };
Yeah, the above are the changes I suggested.
With this all the suggestions made by others and me have been taken
into account and this patch should be good to go.
Thanks.
