Gitversion calculated wrong version number on a recursive merge

[cuichenli]

Describe the bug
The version number is incorrect on a recursive merge commit. As shown in the following image, the version calculated in the normal commit (dummy) is 19.2.9, which is as expected. After we merge from develop branch, the version calculated would become 19.2.7.

Expected Behavior

The version calculated on merge commit should be 19.2.9

Actual Behavior

The version calculated on merge commit is 19.2.7

Possible Fix

GitVersion/src/GitVersionCore/Configuration/BranchConfigurationCalculator.cs

Lines 172 to 201 in 66dcbb5

	var parents = currentCommit.Parents.ToArray();
	var branches = repositoryMetadataProvider.GetBranchesForCommit(parents[1]);
	if (branches.Count == 1)
	{
	var branch = branches[0];
	excludedBranches = new[]
	{
	currentBranch,
	branch
	};
	currentBranch = branch;
	}
	else if (branches.Count > 1)
	{
	currentBranch = branches.FirstOrDefault(b => b.NameWithoutRemote() == "master") ?? branches.First();
	}
	else
	{
	var possibleTargetBranches = repositoryMetadataProvider.GetBranchesForCommit(parents[0]);
	if (possibleTargetBranches.Count > 1)
	{
	currentBranch = possibleTargetBranches.FirstOrDefault(b => b.NameWithoutRemote() == "master") ?? possibleTargetBranches.First();
	}
	else
	{
	currentBranch = possibleTargetBranches.FirstOrDefault() ?? currentBranch;
	}
	}
	log.Info("HEAD is merge commit, this is likely a pull request using " + currentBranch.FriendlyName + " as base");

Not sure if my understanding is correct, but L182 is suspicious to me. In my case, the currentBranch is changed to develop from my local feature branch, not sure what is the reason for this change.

Steps to Reproduce

Sorry I can not share my repo here, but the similar behaviour can be reproduced with the gitversion repo.

# Create a test branch based on previous commit 
git checkout -b test-branch d7abddbd4b51045cc33947bfe
gitversion # 5.3.5

# Make some dummy changes 
git commit -am "Dummy changes"
gitversion # 5.3.5


git merge master 
gitversion # 5.4.0

Your Environment

Windows

• Version Used:
  Windows 10, GitVersion v5.3.3

[asbjornu]

I can't say just by looking at the code why line 182 exists. If you submit a pull request where you change line 182, what happens? Which tests break, and why? It should give us an idea to why this line exists. If nothing breaks and your use-case is fixed, it's just a bug.

[cuichenli]

Ran the tests locally and got those two failures:

A total of 1 test files matched the specified pattern.
  X ShouldHaveAGreaterSemVerAfterDevelopIsMergedIntoFeature [16s 229ms]
  Error Message:
   Shouldly.ShouldAssertException : variables.FullSemVer
    should be
"16.24.0-feat-featX.4"
    but was
"16.23.1-feat-featX.4"
    difference
Difference     |                      |         |
               |                     \|/       \|/
Index          | 0    1    2    3    4    5    6    7    8    9    10   11   12   13   14   15   16   17   18   19
Expected Value | 1    6    .    2    4    .    0    -    f    e    a    t    -    f    e    a    t    X    .    4
Actual Value   | 1    6    .    2    3    .    1    -    f    e    a    t    -    f    e    a    t    X    .    4
Expected Code  | 49   54   46   50   52   46   48   45   102  101  97   116  45   102  101  97   116  88   46   52
Actual Code    | 49   54   46   50   51   46   49   45   102  101  97   116  45   102  101  97   116  88   46   52
  Stack Trace:
     at GitVersionCore.Tests.GitToolsTestingExtensions.AssertFullSemver(RepositoryFixtureBase fixture, String fullSemver, Config configuration, IRepository repository, String commitId, Boolean onlyTrackedBranches, String targetBranch) in C:\Users\cuichen.li\Desktop\dev\GitVersion\src\GitVersionCore.Tests\Extensions\GitToolsTestingExtensions.cs:line 82
   at GitVersionCore.Tests.IntegrationTests.FeatureBranchScenarios.ShouldHaveAGreaterSemVerAfterDevelopIsMergedIntoFeature() in C:\Users\cuichen.li\Desktop\dev\GitVersion\src\GitVersionCore.Tests\IntegrationTests\FeatureBranchScenarios.cs:line 561
  Standard Output Messages:
 Created git repository at 'C:\Users\cuichen.li\AppData\Local\Temp\TestRepositories\a756f437-9a6d-4c55-bc6f-21d0768671f6'
 ---------
 *   bd49bac in the future  (HEAD -> feature/featX)
 |\
 | * 2754625 in the future  (develop)
 * | 27a8b48 in the future
 |/
 * 26744c2 in the future
 * 0d406ac in the future  (tag: 16.23.0)
 * 649b4ba in the future  (master)


  X WhenMultipleDevelopBranchesExistAndCurrentBranchHasIncrementInheritPolicyAndCurrentCommitIsAMerge [9s 961ms]
  Error Message:
   Shouldly.ShouldAssertException : variables.FullSemVer
    should be
"1.1.0-x.1+3"
    but was
"1.0.1-x.1+3"
    difference
Difference     |            |         |
               |           \|/       \|/
Index          | 0    1    2    3    4    5    6    7    8    9    10
Expected Value | 1    .    1    .    0    -    x    .    1    +    3
Actual Value   | 1    .    0    .    1    -    x    .    1    +    3
Expected Code  | 49   46   49   46   48   45   120  46   49   43   51
Actual Code    | 49   46   48   46   49   45   120  46   49   43   51
  Stack Trace:
     at GitVersionCore.Tests.GitToolsTestingExtensions.AssertFullSemver(RepositoryFixtureBase fixture, String fullSemver, Config configuration, IRepository repository, String commitId, Boolean onlyTrackedBranches, String targetBranch) in C:\Users\cuichen.li\Desktop\dev\GitVersion\src\GitVersionCore.Tests\Extensions\GitToolsTestingExtensions.cs:line 82
   at GitVersionCore.Tests.IntegrationTests.DevelopScenarios.WhenMultipleDevelopBranchesExistAndCurrentBranchHasIncrementInheritPolicyAndCurrentCommitIsAMerge() in C:\Users\cuichen.li\Desktop\dev\GitVersion\src\GitVersionCore.Tests\IntegrationTests\DevelopScenarios.cs:line 186
  Standard Output Messages:
 Created git repository at 'C:\Users\cuichen.li\AppData\Local\Temp\TestRepositories\6d360801-f35c-4892-9a4f-4fc86c9f809b'
 ---------
 *   da899e4 in the future  (HEAD -> feature/x)
 |\
 | * eaa7a9e in the future  (develop)
 * | a3150bc in the future
 |/
 * 8fa4356 in the future  (tag: 1.0.0, master, bob_develop)



Test Run Failed.
Total tests: 607
     Passed: 605
     Failed: 2
 Total time: 1.4106 Minutes

[asbjornu]

Can you please submit this as a PR, so it's a bit easier to inspect both the code and failing tests?

[cuichenli]

I have raised a PR #2375. Thanks!

[HHobeck]

This was probably resolved in version 6.x. Please use 6.0.0-beta.1 version for testing and give my a feedback.

Cheers