On 9/19/2019 5:47 PM, SZEDER Gábor wrote:
> In 'builtin/name-rev.c' in the name_rev() function there is a loop
> iterating over all parents of the given commit, and the loop body
> looks like this:
>
> if (parent_number > 1) {
> if (generation > 0)
> // do stuff #1
> else
> // do stuff #2
> } else {
> // do stuff #3
> }
>
> These conditions are not covered properly in the test suite. As far
> as purely test coverage goes, they are all executed several times over
> in 't6120-describe.sh'. However, they don't directly influence the
> command's output, because the repository used in that test script
> contains several branches and tags pointing somewhere into the middle
> of the commit DAG, and thus result in a better name for the
> to-be-named commit. In an early version of this patch series I
> managed to mess up those conditions (every single one of them at
> once!), but the whole test suite still passed successfully.
>
> So add a new test case that operates on the following history:
>
> -----------master
> / /
> A----------M2
> \ /
> \---M1-C
> \ /
> B
>
> and names the commit 'B', where:
>
> - The merge commit at master makes sure that the 'do stuff #3'
> affects the final name.
>
> - The merge commit M2 make sure that the 'do stuff #1' part
> affects the final name.
>
> - And M1 makes sure that the 'do stuff #2' part affects the final
> name.
>
> Signed-off-by: SZEDER Gábor <szeder.dev@xxxxxxxxx>
> ---
> t/t6120-describe.sh | 43 +++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 43 insertions(+)
>
> diff --git a/t/t6120-describe.sh b/t/t6120-describe.sh
> index 07e6793e84..2a0f2204c4 100755
> --- a/t/t6120-describe.sh
> +++ b/t/t6120-describe.sh
> @@ -421,4 +421,47 @@ test_expect_success 'describe complains about missing object' '
> test_must_fail git describe $ZERO_OID
> '
>
> +# -----------master
> +# / /
> +# A----------M2
> +# \ /
> +# \---M1-C
> +# \ /
> +# B
> +test_expect_success 'test' '
> + git init repo &&
> + (
> + cd repo &&
> +
> + echo A >file &&
> + git add file &&
> + git commit -m A &&
> + A=$(git rev-parse HEAD) &&
Is it not enough to do something like test_commit here?
> +
> + git checkout --detach &&
> + echo B >file &&
> + git commit -m B file &&
> + B=$(git rev-parse HEAD) &&
> +
> + git checkout $A &&
> + git merge --no-ff $B && # M1
> +
> + echo C >file &&
> + git commit -m C file &&
> +
> + git checkout $A &&
> + git merge --no-ff HEAD@{1} && # M2
> +
> + git checkout master &&
> + git merge --no-ff HEAD@{1} &&
> +
> + git log --graph --oneline &&
> +
> + echo "$B master^2^2~1^2" >expect &&
> + git name-rev $B >actual &&
This matches your description.
Thanks,
-Stolee
