@@ -438,4 +438,45 @@ test_expect_success 'name-rev a rev shortly after epoch' '
test_cmp expect actual
'
+# A--------------master
+# \ /
+# \----------M2
+# \ /
+# \---M1-C
+# \ /
+# B
+test_expect_success 'name-rev covers all conditions while looking at parents' '
+ git init repo &&
+ (
+ cd repo &&
+
+ echo A >file &&
+ git add file &&
+ git commit -m A &&
+ A=$(git rev-parse HEAD) &&
+
+ 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} &&
+
+ echo "$B master^2^2~1^2" >expect &&
+ git name-rev $B >actual &&
+
+ test_cmp expect actual
+ )
+'
+
test_done
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) // branch #1 new_name = ... else // branch #2 new_name = ... name_rev(parent, new_name, ...); } else { // branch #3 name_rev(...); } 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. This can hide bugs: e.g. by replacing the 'new_name' parameter of the first recursive name_rev() call with 'tip_name' (effectively making both branch #1 and #2 a noop) 'git name-rev --all' shows thousands of bogus names in the Git repository, but the whole test suite still passes successfully. In an early version of a later patch in this series I managed to mess up all three branches (at once!), but the test suite still passed. So add a new test case that operates on the following history: A--------------master \ / \----------M2 \ / \---M1-C \ / B and names the commit 'B' to make sure that all three branches are crucial to determine 'B's name: - There is only a single ref, so all names are based on 'master', without any undesired interference from other refs. - Each time name_rev() follows the second parent of a merge commit, it appends "^2" to the name. Following 'master's second parent right at the start ensures that all commits on the ancestry path from 'master' to 'B' have a different base name from the original 'tip_name' of the very first name_rev() invocation. Currently, while name_rev() is recursive, it doesn't matter, but it will be necessary to properly cover all three branches after the recursion is eliminated later in this series. - Following 'M2's second parent makes sure that branch #2 (i.e. when 'generation = 0') affects 'B's name. - Following the only parent of the non-merge commit 'C' ensures that branch #3 affects 'B's name, and that it increments 'generation'. - Coming from 'C' 'generation' is 1, thus following 'M1's second parent makes sure that branch #1 affects 'B's name. Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> --- t/t6120-describe.sh | 41 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+)