Re: [PATCH 2/6] t1414: document some reflog-walk oddities

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 5, 2017, 17:56 UTC
Message-ID: <xmqqh8yqix7p.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20170705080036.qxzy7havcy72mvff@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 11 quoted lines

> Since its inception, the general strategy of the reflog-walk
> code has been to start with the tip commit for the ref, and
> as we traverse replace each commit's parent pointers with
> fake parents pointing to the previous reflog entry.
>
> This lets us traverse the reflog as if it were a real
> history, but it has some user-visible oddities. Namely:
>
>   1. The fake parents are used for commit selection and
>      display. So for example, "--merges" or "--no-merges"
>      are useful, because the history appears as a linear

s/useful/useless/ perhaps?

Show 9 quoted lines

>      string. Likewise, pathspec limiting is based on the
>      diff between adjacent entries, not the changes actually
>      introduced by a commit.
> ...
> This commit adds several expect_failure tests, to show how
> the tool ought to behave.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---

Show 21 quoted lines

> diff --git a/t/t1414-reflog-walk.sh b/t/t1414-reflog-walk.sh
> new file mode 100755
> index 0000000000..bb847f797d
> --- /dev/null
> +++ b/t/t1414-reflog-walk.sh
> @@ -0,0 +1,83 @@
> +#!/bin/sh
> +
> +test_description='various tests of reflog walk (log -g) behavior'
> +. ./test-lib.sh
> +
> +test_expect_success 'set up some reflog entries' '
> +	test_commit one &&
> +	test_commit two &&
> +	git checkout -b side HEAD^ &&
> +	test_commit three &&
> +	git merge --no-commit master &&
> +	echo evil-merge-content >>one.t &&
> +	test_tick &&
> +	git commit --no-edit -a
> +'

Mental note: logically, what we want to see in the log are:

    master:  one-->two
    side:                one-->three-->(evil)
    HEAD:    one-->two-->one-->three-->(evil)

where the middle one in HEAD is "switching from master to side".

Show 5 quoted lines

> +test_expect_failure 'pathspec limiting handles merges' '
> +	sed -n "1p;3p;5p" expect.all >expect &&
> +	do_walk -- one.t >actual &&
> +	test_cmp expect actual
> +'

OK (it was a bit tricky to see why the topmost one should, but the evilness of the merge makes it eligible).

> +test_expect_failure '--parents shows true parents' '
> +	# convert newlines to spaces
> +	echo $(git rev-parse HEAD HEAD^1 HEAD^2) >expect &&

I saw something related to this nearby this morning. Good thinking to add a comment on this 'echo' ;-).

Show 12 quoted lines

> +	git rev-list -g --parents -1 HEAD >actual &&
> +	test_cmp expect actual
> +'
> +
> +test_expect_failure 'walking multiple reflogs shows both' '
> +	{
> +		do_walk HEAD &&
> +		do_walk side
> +	} >expect &&
> +	do_walk HEAD side >actual &&
> +	test_cmp expect actual
> +'

I somehow find this one a bit iffy.

The order that commits appear in the "walk from HEAD and side at the same time" may want to be different from what this test expects. "rev-list --since=3.days -g master next", for example, would want to refrain from reading the reflog of 'master' for all 90 days before switching to the reflog of 'next', no?

All others I did not comment on and omitted from quoting make sense to me.

Thanks.