Re: [GSoC][PATCH] t/: migrate helper/test-oidtree.c to unit-tests/t-oidtree.c
- From
Christian Couder <christian.couder@gmail.com>
- Date
- Jun 7, 2024, 08:36 UTC
- Message-ID
- <CAP8UFD2eUxrr2Z0etZKbakXEB5VAfXH63jLhfAYJyOxBwvJPEQ@mail.gmail.com>
- In-Reply-To
- <CAP8UFD0r+YYxAvN2Ej1mGa2Kt5M2dQgQEGLraB3iQ30cPWuA6Q@mail.gmail.com>
On Fri, Jun 7, 2024 at 10:31 AM Christian Couder <christian.couder@gmail.com> wrote:
Show 24 quoted lines
>
> On Fri, Jun 7, 2024 at 1:35 AM Ghanshyam Thakkar
> <shyamthakkar001@gmail.com> wrote:
>
> > > Come to think of it, how is your check_each_cb() ensuring that it is
> > > only called once with "123" when queried with "12300"? If the
> > > callback is made with "123" 100 times with the single query with
> > > "12300", would it even notice? I would imagine that the original
> > > would (simply because it dumps each and every callback to a file to
> > > be compared with the golden copy).
> >
> > That's true! I did not think of that. What do you think about something
> > like this then? I will clean it up to send in v2.
> >
> > ---
> >
> > struct cb_data {
> > int *i;
> > struct strvec *expected_hexes;
> > };
>
> It might be better to use a more meaningful name for the struct, like
> perhaps 'expected_hex_iter'. Also I think 'i' could be just 'size_t i'
> instead of 'int *i', and 'expected_hexes' could be just 'hexes'.Maybe even:
struct expected_hex_iter {
size_t i;
struct strvec hexes;
};(so without any pointer)
...
Show 34 quoted lines
> > static enum cb_next check_each_cb(const struct object_id *oid, void *data)
> > {
> > struct cb_data *cb_data = data;
>
> Maybe: `struct expected_hex_iter *hex_iter = data;`
>
> > struct object_id expected;
> >
> > if(!check_int(*cb_data->i, <, cb_data->hexes->nr)) {
>
> A space character is missing between 'if' and '('. And by the way you
> use 'hexes' instead of 'expected_hexes' here.
>
> > test_msg("error: extraneous callback. found oid: %s", oid_to_hex(oid));
> > return CB_BREAK;
> > }
> >
> > if (!check_int(get_oid_arbitrary_hex(cb_data->expected_hexes->v[*cb_data->i], &expected), ==, 0))
> > return CB_BREAK;
> > if (!check(oideq(oid, &expected)))
> > test_msg("expected: %s\n got: %s",
> > hash_to_hex(expected.hash), hash_to_hex(oid->hash));
> >
> > *cb_data->i += 1;
> > return CB_CONTINUE;
> > }
> >
> > static void check_each(struct oidtree *ot, char *query, ...)
> > {
> > struct object_id oid;
> > struct strvec hexes = STRVEC_INIT;
> > struct cb_data cb_data;
> > const char *arg;
> > int i = 0;... and above only:
struct object_id oid;
const char *arg;
struct expected_hex_iter hex_iter = { 0 };