Subject: Re: [GSoC][PATCH v2] t: migrate helper/test-oidmap.c to unit-tests/t-oidmap.c

From: "Ghanshyam Thakkar" <[email-masked]>
Date: Tue, 02 Jul 2024 09:18:15 +0530

Junio C Hamano <[email-masked]> wrote:
> Junio C Hamano <[email-masked]> writes:
>
> > Hmph.  You seem to overwrite key_val[i][1] ...
> > ...
> > ... in this test, rendering the key_val[] array unusuable for
> > further tests.  Is that intended and desirable?
> > ...
> > The TEST(setup(t_foo)) pattern is done so nicely to make sure that
> > everybody is independent from everybody else, preparing the oidmap
> > used for each specific test from scratch.  It is a bit disappointing
> > that we are now invalidating this nice property.
>
> It may be just the matter of doing something silly like this to
> restore the "different tests are independent and the source of truth
> array is intact" property.
>
> The first hunk should be reindented properly, if you are going to
> take this and squash into your patch, by the way.
>
> Thanks.

I think this is very reasonable. I'll squash this into my patch and wait
for any other comments from reviewers before sending another version.

Thanks.

> t/unit-tests/t-oidmap.c | 11 ++++++-----
> 1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git c/t/unit-tests/t-oidmap.c w/t/unit-tests/t-oidmap.c
> index 13532aa98b..be2741c6c7 100644
> --- c/t/unit-tests/t-oidmap.c
> +++ w/t/unit-tests/t-oidmap.c
> @@ -14,7 +14,7 @@ struct test_entry {
> char name[FLEX_ARRAY];
> };
>  
> -static const char *key_val[][2] = { { "11", "one" },
> +static const char * const key_val[][2] = { { "11", "one" },
> { "22", "two" },
> { "33", "three" } };
>  
> @@ -116,7 +116,7 @@ static void t_remove(struct oidmap *map)
> check(oidmap_remove(map, &oid) == NULL);
> }
>  
> -static int key_val_contains(struct test_entry *entry)
> +static int key_val_contains(struct test_entry *entry, char seen[])
> {
> for (size_t i = 0; i < ARRAY_SIZE(key_val); i++) {
> struct object_id oid;
> @@ -125,9 +125,9 @@ static int key_val_contains(struct test_entry
> *entry)
> return -1;
>  
> if (oideq(&entry->entry.oid, &oid)) {
> - if (!strcmp(key_val[i][1], "USED"))
> + if (seen[i])
> return 2;
> - key_val[i][1] = "USED";
> + seen[i] = 1;
> return 0;
> }
> }
> @@ -138,11 +138,12 @@ static void t_iterate(struct oidmap *map)
> {
> struct oidmap_iter iter;
> struct test_entry *entry;
> + char seen[ARRAY_SIZE(key_val)] = { 0 };
>  
> oidmap_iter_init(map, &iter);
> while ((entry = oidmap_iter_next(&iter))) {
> int ret;
> - if (!check_int((ret = key_val_contains(entry)), ==, 0)) {
> + if (!check_int((ret = key_val_contains(entry, seen)), ==, 0)) {
> switch (ret) {
> case -1:
> break; /* error message handled by get_oid_arbitrary_hex() */