Subject: Re: [PATCH 09/12] t4211-line-log: add tests for parent oids

From: Taylor Blau <[email-masked]>
Date: Mon, 4 May 2020 11:31:06 -0600

On Fri, May 01, 2020 at 03:30:26PM +0000, SZEDER Gábor via GitGitGadget wrote:
> From: =?UTF-8?q?SZEDER=20G=C3=A1bor?= <[email-masked]>
>
> None of the tests in 't4211-line-log.sh' really check which parent
> object IDs are shown in the output, either implicitly as part of
> "Merge: ..." lines [1] or explicitly via the '%p' or '%P' format
> specifiers in a custom pretty format.
>
> Add two tests to 't4211-line-log.sh' to check which parent object IDs
> are shown, one without and one with explicitly requested parent
> rewriting, IOW without and with the '--parents' option.
>
> The test without '--parents' is marked as failing, because without
> that option parent rewriting should not be performed, and thus the
> parent object ID should be that of the immediate parent, just like in
> case of a pathspec-limited history traversal without parent rewriting.
> The current line-level log implementation, however, performs parent
> rewriting unconditionally and without a possibility to turn it off,
> and, consequently, it shows the object ID of the most recent ancestor
> that modified the given line range.
>
> In both of these new tests we only really care about the object IDs of
> the listed commits and their parents, but not the diffs of the line
> ranges; the diffs have already been thoroughly checked in the previous
> tests.
>
> [1] While one of the tests ('-M -L ':f:b.c' parallel-change') does
>     list a merge commit, both of its parents happen to modify the
>     given line range and are listed as well, so the implications of
>     parent rewriting remained hidden and untested.
>
> Signed-off-by: SZEDER Gábor <[email-masked]>
> Signed-off-by: Derrick Stolee <[email-masked]>
> ---

These tests make sense, and demonstrate the problem nicely. I don't
think that I have a ton to add to this patch, so:

  Reviewed-by: Taylor Blau <[email-masked]>

Thanks,
Taylor