Re: [PATCH 3/6] t6601: add helper for testing path-walk API

From: Jonathan Tan <jonathantanmy@google.com>
Date: Nov 1, 2024, 22:23 UTC
Message-ID: <20241101222334.1724050-1-jonathantanmy@google.com>
In-Reply-To: <6f93dff88e7cf81800c8f2c8d39175d962a0576f.1730356023.git.gitgitgadget@gmail.com>

I haven't looked thoroughly at the rest of the patches yet, but had a comment about this test. Rearranging:

"Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:

Show 14 quoted lines

> +test_expect_success 'all' '
> +	test-tool path-walk -- --all >out &&
> +
> +	cat >expect <<-EOF &&
> +	TREE::$(git rev-parse topic^{tree})
> +	TREE::$(git rev-parse base^{tree})
> +	TREE::$(git rev-parse base~1^{tree})
> +	TREE::$(git rev-parse base~2^{tree})
> +	TREE:left/:$(git rev-parse base:left)
> +	TREE:left/:$(git rev-parse base~2:left)
> +	TREE:right/:$(git rev-parse topic:right)
> +	TREE:right/:$(git rev-parse base~1:right)
> +	TREE:right/:$(git rev-parse base~2:right)
> +	trees:9

[snip rest of "expect"]

The way you're testing this, wouldn't the tests pass even if the OIDs aren't emitted in path order? (E.g. if topic:right and base~1:right were somehow grouped into two different groups, even though they have the same path.)

I would have expected the test output to be something like:

  TREE:right/ $(rp :right topic base~1 base~2)

where rp is a function that takes in a suffix and one or more prefixes - I haven't figured out its contents yet, but

  echo $(git rev-parse HEAD^^ HEAD^ HEAD | sort)

gives us a space-separated list, so it doesn't seem too difficult to define such a function.

Show 23 quoted lines

> +static int emit_block(const char *path, struct oid_array *oids,
> +		      enum object_type type, void *data)
> +{
> +	struct path_walk_test_data *tdata = data;
> +	const char *typestr;
> +
> +	switch (type) {
> +	case OBJ_TREE:
> +		typestr = "TREE";
> +		tdata->tree_nr += oids->nr;
> +		break;
> +
> +	case OBJ_BLOB:
> +		typestr = "BLOB";
> +		tdata->blob_nr += oids->nr;
> +		break;
> +
> +	default:
> +		BUG("we do not understand this type");
> +	}
> +
> +	for (size_t i = 0; i < oids->nr; i++)
> +		printf("%s:%s:%s\n", typestr, path, oid_to_hex(&oids->oid[i]));

Then here, you would print typestr and path before the "for" loop. In the "for" loop you would add oid_to_hex() results to a sorted string list, have another "for" loop that prints each element preceded by a space, then print a "\n" after both "for" loops.