From 2caab3e3f19740d626965ab60267d32957f17609 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?U=C4=9Fra=C5=9F=20Erg=C3=BCn?= <96827714+ugras-ergun-sonarsource@users.noreply.github.com> Date: Tue, 2 Apr 2024 09:20:48 +0200 Subject: [PATCH] Prioritise "Main" when matching branches (#5332) Fixes #5092 --- .../BranchMatcherTests.cs | 76 +++++++++++++------ src/ConnectedMode/BranchMatcher.cs | 14 ++-- 2 files changed, 58 insertions(+), 32 deletions(-) diff --git a/src/ConnectedMode.UnitTests/BranchMatcherTests.cs b/src/ConnectedMode.UnitTests/BranchMatcherTests.cs index 3ee74224bf..d6a5b4deef 100644 --- a/src/ConnectedMode.UnitTests/BranchMatcherTests.cs +++ b/src/ConnectedMode.UnitTests/BranchMatcherTests.cs @@ -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; @@ -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; @@ -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); @@ -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); @@ -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; @@ -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; @@ -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; @@ -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; @@ -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); @@ -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"); @@ -325,15 +351,15 @@ private static Mock CreateSonarQubeServiceWithTypes(string ma var service = new Mock(); service.Setup(x => x.IsConnected).Returns(true); - IList remoteBranches = new List - { - new SonarQubeProjectBranch(mainBranch, true, DateTime.Now, "BRANCH") - }; + IList remoteBranches = new List(); 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(), It.IsAny())).Returns(Task.FromResult(remoteBranches)); return service; diff --git a/src/ConnectedMode/BranchMatcher.cs b/src/ConnectedMode/BranchMatcher.cs index fccccd0251..209ef4b207 100644 --- a/src/ConnectedMode/BranchMatcher.cs +++ b/src/ConnectedMode/BranchMatcher.cs @@ -66,7 +66,7 @@ public BranchMatcher(ISonarQubeService sonarQubeService, ILogger logger) public async Task 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); @@ -89,7 +89,7 @@ private async Task DoGetMatchingBranch(string projectKey, IRepository gi { var head = gitRepo.Head; - if (head == null) + if (head is null) { logger.LogVerbose(Resources.BranchMapper_NoHead); return null; @@ -113,11 +113,11 @@ private async Task 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; @@ -125,7 +125,7 @@ private async Task DoGetMatchingBranch(string projectKey, IRepository gi } } - if (closestBranch == null) + if (closestBranch is null) { logger.LogVerbose(Resources.BranchMapper_NoMatchingBranchFound); closestBranch = remoteBranches.First(rb => rb.IsMain).Name; @@ -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); @@ -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; }