Re: [PATCH 08/16] test-reach: create new test tool for ref_newer

[Jonathan Tan <jonathantanmy@google.com>]

> To use the new test-tool, use 'test-tool reach <method>' and provide
> input to stdin that describes the inputs to the method. Currently, we
> only implement the ref_newer method, which requires two commits. Use
> lines "A:<committish>" and "B:<committish>" for the two inputs. We will
> expand this input later to accommodate methods that take lists of
> commits.

It would be nice if "A" and "B" were "ancestor" and "descendant" (or
something like that) instead, so that I don't have to check which
direction the reach is calculated in.

> +int cmd__reach(int ac, const char **av)
> +{
> +	struct object_id oid_A, oid_B;
> +	struct strbuf buf = STRBUF_INIT;
> +	struct repository *r = the_repository;
> +
> +	setup_git_directory();
> +
> +	if (ac < 2)
> +		exit(1);
> +
> +
> +	while (strbuf_getline(&buf, stdin) != EOF) {
> +		struct object_id oid;
> +		struct object *o;
> +		struct commit *c;
> +		if (buf.len < 3)
> +			continue;
> +
> +		if (get_oid_committish(buf.buf + 2, &oid))
> +			die("failed to resolve %s", buf.buf + 2);

You can also use skip_prefix() instead of using arithmetic to determine
the start of the OID.

> +# Construct a grid-like commit graph with points (x,y)
> +# with 1 <= x <= 10, 1 <= y <= 10, where (x,y) has
> +# parents (x-1, y) and (x, y-1), keeping in mind that
> +# we drop a parent if a coordinate is nonpositive.
> +#
> +#             (10,10)
> +#            /       \
> +#         (10,9)    (9,10)
> +#        /     \   /      \
> +#    (10,8)    (9,9)      (8,10)
> +#   /     \    /   \      /    \
> +#         ( continued...)
> +#   \     /    \   /      \    /
> +#    (3,1)     (2,2)      (1,3)
> +#        \     /    \     /
> +#         (2,1)      (2,1)
> +#              \    /
> +#              (1,1)

This is quite a good design, thanks.

> +# We use branch 'comit-x-y' to refer to (x,y).

s/comit/commit/

> +	git show-ref -s commit-7-7 | git commit-graph write --stdin-commits &&
> +	mv .git/objects/info/commit-graph commit-graph-half &&

My understanding is that this writes for 7-7 and all its ancestors,
but...

> +test_expect_success 'ref_newer:hit' '
> +	cat >input <<-\EOF &&
> +		A:commit-5-7
> +		B:commit-2-3
> +	EOF
> +	printf "ref_newer:1\n" >expect &&
> +	test_three_modes ref_newer
> +'
> +
> +test_done

...both 5-7 and 2-3 are ancestors of 7-7, right? Which means that you
don't test the "half" commit graph here. (It's probably sufficient to
just adjust the numbers.)