Re: [PATCH 09/12] t4211-line-log: add tests for parent oids
- From
Taylor Blau <me@ttaylorr.com>
- Date
- May 4, 2020, 17:31 UTC
- Message-ID
- <20200504173106.GA35579@syl.local>
- In-Reply-To
- <4e3d7233095064de197c447cb31cf4652d42b775.1588347029.git.gitgitgadget@gmail.com>
On Fri, May 01, 2020 at 03:30:26PM +0000, SZEDER Gábor via GitGitGadget wrote:
Show 33 quoted lines
> From: =?UTF-8?q?SZEDER=20G=C3=A1bor?= <szeder.dev@gmail.com>
>
> 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 <szeder.dev@gmail.com>
> Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
> ---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 <me@ttaylorr.com>
Thanks, Taylor