From: "Ghanshyam Thakkar" <shyamthakkar001@gmail•com>
To: "Junio C Hamano" <gitster@pobox•com>
Cc: <git@vger•kernel.org>, "Jonathan Nieder" <jrnieder@gmail•com>,
"Josh Steadmon" <steadmon@google•com>,
<christian.couder@gmail•com>,
"Phillip Wood" <phillip.wood123@gmail•com>,
"Christian Couder" <chriscool@tuxfamily•org>,
"Kaartic Sivaraam" <kaartic.sivaraam@gmail•com>
Subject: Re: [GSoC][PATCH v2] t: migrate helper/test-oidmap.c to unit-tests/t-oidmap.c
Date: Tue, 02 Jul 2024 09:18:15 +0530 [thread overview]
Message-ID: <D2EQV4ZV3VEW.2CV0OLF3T4HVA@gmail.com> (raw)
In-Reply-To: <xmqqjzi4u52u.fsf@gitster.g>
Junio C Hamano <gitster@pobox•com> wrote:
> Junio C Hamano <gitster@pobox•com> 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() */
next prev parent reply other threads:[~2024-07-02 3:48 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-06-19 17:50 [GSoC][PATCH] t: migrate helper/test-oidmap.c to unit-tests/t-oidmap.c Ghanshyam Thakkar
2024-06-20 9:45 ` Jonathan Nieder
2024-06-25 1:35 ` Ghanshyam Thakkar
2024-06-25 10:14 ` Phillip Wood
2024-06-25 19:16 ` Ghanshyam Thakkar
2024-06-26 8:59 ` phillip.wood123
2024-06-28 12:20 ` [GSoC][PATCH v2] " Ghanshyam Thakkar
2024-07-01 20:32 ` Josh Steadmon
2024-07-01 21:19 ` Junio C Hamano
2024-07-01 21:14 ` Junio C Hamano
2024-07-01 22:20 ` Junio C Hamano
2024-07-02 3:48 ` Ghanshyam Thakkar [this message]
2024-07-02 15:17 ` Phillip Wood
2024-07-02 15:24 ` Phillip Wood
2024-07-02 16:25 ` Ghanshyam Thakkar
2024-07-02 18:55 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=D2EQV4ZV3VEW.2CV0OLF3T4HVA@gmail.com \
--to=shyamthakkar001@gmail$(echo .)com \
--cc=chriscool@tuxfamily$(echo .)org \
--cc=christian.couder@gmail$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=gitster@pobox$(echo .)com \
--cc=jrnieder@gmail$(echo .)com \
--cc=kaartic.sivaraam@gmail$(echo .)com \
--cc=phillip.wood123@gmail$(echo .)com \
--cc=steadmon@google$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox