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

[Junio C Hamano <gitster@pobox.com>]

Ghanshyam Thakkar <shyamthakkar001@gmail.com> writes:

>> 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.

I do not see a strong reason to have a pointer to int in cb_data, as
the caller has access to cb_data after the callback finishes using
it so check_each() can check cb_data.i instead of *cb_data.i (or i)
at the end.

But other than that, yes, it is the direction you would want to go,
I would think.

>
> ---
>
> struct cb_data {
> 	int *i;
> 	struct strvec *expected_hexes;
> };
>
> static enum cb_next check_each_cb(const struct object_id *oid, void *data)
> {
> 	struct cb_data *cb_data = data;
> 	struct object_id expected;
>
> 	if(!check_int(*cb_data->i, <, cb_data->hexes->nr)) {
> 		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;
>
> 	va_list expected;
> 	va_start(expected, query);
>
> 	while ((arg = va_arg(expected, const char *)))
> 		strvec_push(&hexes, arg);
>
> 	cb_data.i = &i;
> 	cb_data.expected_hexes = &hexes;
>
> 	if (!check_int(get_oid_arbitrary_hex(query, &oid), ==, 0))
> 		return;
> 	oidtree_each(ot, &oid, strlen(query), check_each_cb, &cb_data);
>
> 	if (!check_int(*cb_data.i, ==, cb_data.expected_hexes->nr))
> 		test_msg("error: could not find some oids");
> }
> ---
>
> Thanks for the review.