Skip to content

Commit

Permalink
t6120: add a test to cover inner conditions in 'git name-rev's name_r…
Browse files Browse the repository at this point in the history
…ev()

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>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
  • Loading branch information
szeder authored and gitster committed Dec 6, 2019
1 parent bf43abc commit d59fc83
Showing 1 changed file with 41 additions and 0 deletions.
41 changes: 41 additions & 0 deletions t/t6120-describe.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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

0 comments on commit d59fc83

Please sign in to comment.