Skip to content

Commit

Permalink
Prioritise "Main" when matching branches (#5332)
Browse files Browse the repository at this point in the history 
Fixes #5092
  • Loading branch information
@ugras-ergun-sonarsource @georgii-borovinskikh-sonarsource
ugras-ergun-sonarsource authored and georgii-borovinskikh-sonarsource committed Apr 2, 2024
1 parent 3a60413 commit 2caab3e
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 32 deletions.
76 changes: 51 additions & 25 deletions src/ConnectedMode.UnitTests/BranchMatcherTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ public void MefCtor_CheckIsExported()
[DataRow("branch")]
[DataRow("Branch")]
[TestMethod]
public async Task GetMatchedBranch_ChooseBranchWithSameName(string serverBranchName)
public async Task GetMatchingBranch_ChooseBranchWithSameName(string serverBranchName)
{
var service = CreateSonarQubeService("master", serverBranchName).Object;

Expand All @@ -64,7 +64,7 @@ public async Task GetMatchedBranch_ChooseBranchWithSameName(string serverBranchN
}

[TestMethod]
public async Task GetMatchedBranch_NoBranchWithSameName_ChooseClosestMatch()
public async Task GetMatchingBranch_NoBranchWithSameName_ChooseClosestMatch()
{
var service = CreateSonarQubeService("master", "dev").Object;

Expand All @@ -86,10 +86,11 @@ public async Task GetMatchedBranch_NoBranchWithSameName_ChooseClosestMatch()
}

[TestMethod]
public async Task GetMatchedBranch_ShorterPathFoundBefore_EarlyOut()
public async Task GetMatchingBranch_ShorterPathFoundBefore_EarlyOut()
{
var service = CreateSonarQubeService("serverBranch", "master").Object;
var service = CreateSonarQubeService("master", "serverbranch").Object;

var masterCommit0 = new CommitWrapper(10);
var masterCommit1 = new CommitWrapper(11);
var masterCommit2 = new CommitWrapper(12);
var masterCommit3 = new CommitWrapper(13);
Expand All @@ -99,9 +100,9 @@ public async Task GetMatchedBranch_ShorterPathFoundBefore_EarlyOut()
var localBranchCommit1 = new CommitWrapper(31);
var localBranchCommit2 = new CommitWrapper(32);

var masterBranch = CreateBranchWithEnumerationCount("master", masterCommit3, masterCommit2, masterCommit1);
var serverBranch = CreateBranchWithEnumerationCount("serverbranch", serverBranchCommit3, serverBranchCommit2, serverBranchCommit1, masterCommit3, masterCommit2, masterCommit1);
var localBranch = CreateBranch("localbranch", localBranchCommit2, localBranchCommit1, serverBranchCommit3, serverBranchCommit2, serverBranchCommit1, masterCommit3, masterCommit2, masterCommit1);
var masterBranch = CreateBranchWithEnumerationCount("master", masterCommit3, masterCommit2, masterCommit1, masterCommit0);
var serverBranch = CreateBranchWithEnumerationCount("serverbranch", serverBranchCommit3, serverBranchCommit2, serverBranchCommit1, masterCommit3, masterCommit2, masterCommit1, masterCommit0);
var localBranch = CreateBranch("localbranch", localBranchCommit2, localBranchCommit1, serverBranchCommit3, serverBranchCommit2, serverBranchCommit1, masterCommit3, masterCommit2, masterCommit1, masterCommit0);

var repo = CreateRepo(localBranch, serverBranch, masterBranch);

Expand All @@ -113,21 +114,21 @@ public async Task GetMatchedBranch_ShorterPathFoundBefore_EarlyOut()

// Compares local commits to server branch and can't match so loops server branch twice
// ServerBranchCommit3 matches the first one on serverbranch
// * 6 = localbranchCommit2 -> serverBranchCommit3, serverBranchCommit2, serverBranchCommit1, masterCommit3, masterCommit2, masterCommit1
// * 6 = localbranchCommit1 -> serverBranchCommit3, serverBranchCommit2, serverBranchCommit1, masterCommit3, masterCommit2, masterCommit1
// * 7 = localbranchCommit2 -> serverBranchCommit3, serverBranchCommit2, serverBranchCommit1, masterCommit3, masterCommit2, masterCommit1, masterCommit0
// * 7 = localbranchCommit1 -> serverBranchCommit3, serverBranchCommit2, serverBranchCommit1, masterCommit3, masterCommit2, masterCommit1, masterCommit0
// * 1 = serverBranchCommit3 -> serverBranchCommit3
// Total = 6 + 6 + 1 = 13, and closestMatch = 2
((CommitLogWrapperWithEnumerationCount)serverBranch.Commits).EnumerateCount.Should().Be(13);
// Total = 7 + 7 + 1 = 13, and closestMatch = 2
((CommitLogWrapperWithEnumerationCount)serverBranch.Commits).EnumerateCount.Should().Be(15);

// ClosestDistance is now 2 so any tries passes those should fail
// in first try on head branch we try 2 times on master
// * 2 = localBranchCommit2 -> masterCommit3, masterCommit2, no match -> early out after 2
// * 1 + 1 = localBranchCommit1 -> masterCommit3, no match -> early out after [head commit index (1) + master commit index (1) = 2]
((CommitLogWrapperWithEnumerationCount)masterBranch.Commits).EnumerateCount.Should().Be(3);
// * 3 = localBranchCommit2 -> masterCommit3, masterCommit2, masterCommit1 -> early out after 3
// * 1 + 2 = localBranchCommit1 -> masterCommit3, masterCommit2 -> early out after [head commit index (1) + master commit index (2) = 3]
((CommitLogWrapperWithEnumerationCount)masterBranch.Commits).EnumerateCount.Should().Be(6);
}

[TestMethod]
public async Task GetMatchedBranch_ClosestBranchNotOnTheServer_IgnoreClosest()
public async Task GetMatchingBranch_ClosestBranchNotOnTheServer_IgnoreClosest()
{
var service = CreateSonarQubeService("master", "dev").Object;

Expand Down Expand Up @@ -184,7 +185,7 @@ public async Task GetMatchedBranch_ClosestBranchNotOnTheServer_IgnoreClosest()
*/

[TestMethod]
public async Task GetMatchedBranch_MultipleCandidate_ChooseClosestMatch()
public async Task GetMatchingBranch_MultipleCandidate_ChooseClosestMatch()
{
var service = CreateSonarQubeService("master", "Branch1", "Branch2").Object;

Expand Down Expand Up @@ -214,7 +215,7 @@ public async Task GetMatchedBranch_MultipleCandidate_ChooseClosestMatch()
}

[TestMethod]
public async Task GetMatchedBranch_NoMatchingBranch_ChooseMain()
public async Task GetMatchingBranch_NoMatchingBranch_ChooseMain()
{
var service = CreateSonarQubeService("premier", "branch1", "branch2").Object;

Expand All @@ -238,7 +239,32 @@ public async Task GetMatchedBranch_NoMatchingBranch_ChooseMain()
}

[TestMethod]
public async Task GetMatchedBranch_HasShortLivedBranches_IgnoreShortLivedBranch()
public async Task GetMatchingBranch_MultipleMatchingBranchesWithMain_ChooseMain()
{
var service = CreateSonarQubeService("master", "branch1", "branch2").Object;

var commit1 = new CommitWrapper(1);
var commit2 = new CommitWrapper(2);
var commit3 = new CommitWrapper(3);
var commit4 = new CommitWrapper(4);
var commit5 = new CommitWrapper(5);

var masterBranch = CreateBranch("master", commit1);
var branch1 = CreateBranch("branch1", commit1);
var branch2 = CreateBranch("branch2", commit4, commit3, commit2, commit1);
var branch3 = CreateBranch("branch3", commit5, commit1);

var repo = CreateRepo(branch3, branch1, branch2, masterBranch);

var testSubject = CreateTestSubject(service);

var result = await testSubject.GetMatchingBranch("projectKey", repo, CancellationToken.None);

result.Should().Be("master");
}

[TestMethod]
public async Task GetMatchingBranch_HasShortLivedBranches_IgnoreShortLivedBranch()
{
var service = CreateSonarQubeServiceWithTypes("master", ("branch1", "LONG"), ("branch2", "SHORT")).Object;

Expand All @@ -262,7 +288,7 @@ public async Task GetMatchedBranch_HasShortLivedBranches_IgnoreShortLivedBranch(
}

[TestMethod]
public async Task GetMatchedBranch_RepoHasNoHead_ReturnsNull()
public async Task GetMatchingBranch_RepoHasNoHead_ReturnsNull()
{
var testSubject = CreateTestSubject(CreateSonarQubeService("any").Object);

Expand All @@ -272,7 +298,7 @@ public async Task GetMatchedBranch_RepoHasNoHead_ReturnsNull()
}

[TestMethod]
public async Task GetMatchedBranch_CancellationToken_IsPropagatedToWebClient()
public async Task GetMatchingBranch_CancellationToken_IsPropagatedToWebClient()
{
var service = CreateSonarQubeService("any");

Expand Down Expand Up @@ -325,15 +351,15 @@ private static Mock<ISonarQubeService> CreateSonarQubeServiceWithTypes(string ma
var service = new Mock<ISonarQubeService>();
service.Setup(x => x.IsConnected).Returns(true);

IList<SonarQubeProjectBranch> remoteBranches = new List<SonarQubeProjectBranch>
{
new SonarQubeProjectBranch(mainBranch, true, DateTime.Now, "BRANCH")
};
IList<SonarQubeProjectBranch> remoteBranches = new List<SonarQubeProjectBranch>();

foreach (var branch in branches)
{
remoteBranches.Add(new SonarQubeProjectBranch(branch.branchName, true, DateTime.Now, branch.type));
remoteBranches.Add(new SonarQubeProjectBranch(branch.branchName, false, DateTime.Now, branch.type));
}

remoteBranches.Add(new SonarQubeProjectBranch(mainBranch, true, DateTime.Now, "BRANCH"));

service.Setup(s => s.GetProjectBranchesAsync(It.IsAny<string>(), It.IsAny<CancellationToken>())).Returns(Task.FromResult(remoteBranches));

return service;
Expand Down
14 changes: 7 additions & 7 deletions src/ConnectedMode/BranchMatcher.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ public BranchMatcher(ISonarQubeService sonarQubeService, ILogger logger)
public async Task<string> GetMatchingBranch(string projectKey, IRepository gitRepo, CancellationToken token)
{
Debug.Assert(sonarQubeService.IsConnected,
"Not expecting GetMatchedBranch to be called unless we are in Connected Mode");
"Not expecting GetMatchingBranch to be called unless we are in Connected Mode");

logger.LogVerbose(Resources.BranchMapper_CalculatingServerBranch_Started);

Expand All @@ -89,7 +89,7 @@ private async Task<string> DoGetMatchingBranch(string projectKey, IRepository gi
{
var head = gitRepo.Head;

if (head == null)
if (head is null)
{
logger.LogVerbose(Resources.BranchMapper_NoHead);
return null;
Expand All @@ -113,19 +113,19 @@ private async Task<string> DoGetMatchingBranch(string projectKey, IRepository gi
{
var localBranch = gitRepo.Branches.FirstOrDefault(r => string.Equals(r.FriendlyName, remoteBranch.Name, StringComparison.InvariantCultureIgnoreCase));

if (localBranch == null) { continue; }
if (localBranch is null) { continue; }

var distance = GetDistance(headCommits.Value, localBranch, closestDistance);

if (distance < closestDistance)
if (distance < closestDistance || (distance < int.MaxValue && distance == closestDistance && remoteBranch.IsMain))
{
closestBranch = localBranch.FriendlyName;
closestDistance = distance;
logger.LogVerbose(Resources.BranchMapper_UpdatingClosestMatch, closestBranch, distance);
}
}

if (closestBranch == null)
if (closestBranch is null)
{
logger.LogVerbose(Resources.BranchMapper_NoMatchingBranchFound);
closestBranch = remoteBranches.First(rb => rb.IsMain).Name;
Expand All @@ -142,7 +142,7 @@ private int GetDistance(Commit[] headCommits, Branch branch, int closestDistance
CodeMarkers.Instance.GetDistanceStart(branch.FriendlyName);
for (int i = 0; i < headCommits.Length; i++)
{
if (i >= closestDistance) { break; }
if (i > closestDistance) { break; }

var commitID = headCommits[i].Id;
var branchCommitIndex = GetIndexOfCommit(branch.Commits, commitID, closestDistance - i);
Expand All @@ -167,7 +167,7 @@ private int GetIndexOfCommit(ICommitLog commits, ObjectId commitId, int remainin
{
if (commit.Id == commitId) { return i; }
i++;
if (i >= remainingStepsToClosestDistance) { break; }
if (i > remainingStepsToClosestDistance) { break; }
}
return -1;
}
Expand Down

0 comments on commit 2caab3e

Please sign in to comment.