Skip to content

Fix no minor version detected on pull request that contains organization name #3424

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ public void GetReleaseBranchConfigReturnsAllReleaseBranches()
[TestCase("refs/remotes/origin/release/2.0.0",
"refs/remotes/origin/release/2.0.0", "origin/release/2.0.0", "release/2.0.0",
false, false, true, false, true)]
public void EnsureIsReleaseBranchWithReferenceNameWorksAsExpected(string branchName, string expectedCanonical, string expectedFriendly, string expectedWithoutRemote,
public void EnsureIsReleaseBranchWithReferenceNameWorksAsExpected(string branchName, string expectedCanonical, string expectedFriendly, string expectedWithoutOrigin,
bool expectedIsLocalBranch, bool expectedIsPullRequest, bool expectedIsRemoteBranch, bool expectedIsTag, bool expectedIsReleaseBranch)
{
var configuration = GitFlowConfigurationBuilder.New.Build();
Expand All @@ -50,7 +50,7 @@ public void EnsureIsReleaseBranchWithReferenceNameWorksAsExpected(string branchN

actual.Canonical.ShouldBe(expectedCanonical);
actual.Friendly.ShouldBe(expectedFriendly);
actual.WithoutRemote.ShouldBe(expectedWithoutRemote);
actual.WithoutOrigin.ShouldBe(expectedWithoutOrigin);
actual.IsLocalBranch.ShouldBe(expectedIsLocalBranch);
actual.IsPullRequest.ShouldBe(expectedIsPullRequest);
actual.IsRemoteBranch.ShouldBe(expectedIsRemoteBranch);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ public static IBranch CreateMockBranch(string name, params ICommit[] commits)
}

public static IBranch FindBranch(this IGitRepository repository, string branchName)
=> repository.Branches.FirstOrDefault(branch => branch.Name.WithoutRemote == branchName)
=> repository.Branches.FirstOrDefault(branch => branch.Name.WithoutOrigin == branchName)
?? throw new GitVersionException($"Branch {branchName} not found");

public static void DumpGraph(this IGitRepository repository, Action<string>? writer = null, int? maxCommits = null) => GitExtensions.DumpGraph(repository.Path, writer, maxCommits);
Expand Down
67 changes: 35 additions & 32 deletions src/GitVersion.Core.Tests/MergeMessageTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ public class MergeMessageTests : TestBase
[Test]
public void NullMessageStringThrows() =>
// Act / Assert
Should.Throw<NullReferenceException>(() => new MergeMessage(null, this.configuration));
Should.Throw<ArgumentNullException>(() => new MergeMessage(null!, this.configuration));

[TestCase("")]
[TestCase("\t\t ")]
Expand All @@ -22,7 +22,7 @@ public void EmptyMessageString(string message)

// Assert
sut.TargetBranch.ShouldBeNull();
sut.MergedBranch.ShouldBeEmpty();
sut.MergedBranch.ShouldBeNull();
sut.IsMergedPullRequest.ShouldBeFalse();
sut.PullRequestNumber.ShouldBe(null);
sut.Version.ShouldBeNull();
Expand All @@ -42,7 +42,7 @@ public void EmptyTagPrefix(string prefix)

// Assert
sut.TargetBranch.ShouldBeNull();
sut.MergedBranch.ShouldBeEmpty();
sut.MergedBranch.ShouldBeNull();
sut.IsMergedPullRequest.ShouldBeFalse();
sut.PullRequestNumber.ShouldBe(null);
sut.Version.ShouldBeNull();
Expand Down Expand Up @@ -72,21 +72,20 @@ public void ParsesMergeMessage(
// Assert
sut.FormatName.ShouldBe("Default");
sut.TargetBranch.ShouldBe(expectedTargetBranch);
sut.MergedBranch.ShouldBe(expectedMergedBranch);
sut.MergedBranch.ShouldNotBeNull();
sut.MergedBranch.Friendly.ShouldBe(expectedMergedBranch);
sut.IsMergedPullRequest.ShouldBeFalse();
sut.PullRequestNumber.ShouldBe(null);
sut.Version.ShouldBe(expectedVersion);
}

private static readonly object?[] GitHubPullPullMergeMessages =
{
new object?[] { "Merge pull request #1234 from feature/one", "feature/one", null, null, 1234 },
new object?[] { "Merge pull request #1234 in feature/one", "feature/one", null, null, 1234 },
new object?[] { "Merge pull request #1234 from organization/feature/one", "feature/one", null, null, 1234 },
new object?[] { "Merge pull request #1234 in organization/feature/one", "feature/one", null, null, 1234 },
new object?[] { "Merge pull request #1234 in v4.0.0", "v4.0.0", null, new SemanticVersion(4), 1234 },
new object?[] { "Merge pull request #1234 from origin/feature/one", "origin/feature/one", null, null, 1234 },
new object?[] { "Merge pull request #1234 in feature/4.1.0/one", "feature/4.1.0/one", null, new SemanticVersion(4,1), 1234 },
new object?[] { "Merge pull request #1234 in V://10.10.10.10", "V://10.10.10.10", null, null, 1234 },
new object?[] { "Merge pull request #1234 from feature/one into dev", "feature/one", "dev", null, 1234 }
new object?[] { "Merge pull request #1234 in organization/feature/4.1.0/one", "feature/4.1.0/one", null, new SemanticVersion(4,1), 1234 },
new object?[] { "Merge pull request #1234 from organization/feature/one into dev", "feature/one", "dev", null, 1234 }
};

[TestCaseSource(nameof(GitHubPullPullMergeMessages))]
Expand All @@ -103,7 +102,8 @@ public void ParsesGitHubPullMergeMessage(
// Assert
sut.FormatName.ShouldBe("GitHubPull");
sut.TargetBranch.ShouldBe(expectedTargetBranch);
sut.MergedBranch.ShouldBe(expectedMergedBranch);
sut.MergedBranch.ShouldNotBeNull();
sut.MergedBranch.Friendly.ShouldBe(expectedMergedBranch);
sut.IsMergedPullRequest.ShouldBeTrue();
sut.PullRequestNumber.ShouldBe(expectedPullRequestNumber);
sut.Version.ShouldBe(expectedVersion);
Expand Down Expand Up @@ -137,7 +137,8 @@ public void ParsesBitBucketPullMergeMessage(
// Assert
sut.FormatName.ShouldBe("BitBucketPull");
sut.TargetBranch.ShouldBe(expectedTargetBranch);
sut.MergedBranch.ShouldBe(expectedMergedBranch);
sut.MergedBranch.ShouldNotBeNull();
sut.MergedBranch.Friendly.ShouldBe(expectedMergedBranch);
sut.IsMergedPullRequest.ShouldBeTrue();
sut.PullRequestNumber.ShouldBe(expectedPullRequestNumber);
sut.Version.ShouldBe(expectedVersion);
Expand Down Expand Up @@ -166,7 +167,8 @@ public void ParsesBitBucketPullMergeMessage_v7(
// Assert
sut.FormatName.ShouldBe("BitBucketPullv7");
sut.TargetBranch.ShouldBe(expectedTargetBranch);
sut.MergedBranch.ShouldBe(expectedMergedBranch);
sut.MergedBranch.ShouldNotBeNull();
sut.MergedBranch.Friendly.ShouldBe(expectedMergedBranch);
sut.IsMergedPullRequest.ShouldBeTrue();
sut.PullRequestNumber.ShouldBe(expectedPullRequestNumber);
sut.Version.ShouldBe(expectedVersion);
Expand Down Expand Up @@ -196,7 +198,8 @@ public void ParsesSmartGitMergeMessage(
// Assert
sut.FormatName.ShouldBe("SmartGit");
sut.TargetBranch.ShouldBe(expectedTargetBranch);
sut.MergedBranch.ShouldBe(expectedMergedBranch);
sut.MergedBranch.ShouldNotBeNull();
sut.MergedBranch.Friendly.ShouldBe(expectedMergedBranch);
sut.IsMergedPullRequest.ShouldBeFalse();
sut.PullRequestNumber.ShouldBe(null);
sut.Version.ShouldBe(expectedVersion);
Expand Down Expand Up @@ -226,37 +229,33 @@ public void ParsesRemoteTrackingMergeMessage(
// Assert
sut.FormatName.ShouldBe("RemoteTracking");
sut.TargetBranch.ShouldBe(expectedTargetBranch);
sut.MergedBranch.ShouldBe(expectedMergedBranch);
sut.MergedBranch.ShouldNotBeNull();
sut.MergedBranch.Friendly.ShouldBe(expectedMergedBranch);
sut.IsMergedPullRequest.ShouldBeFalse();
sut.PullRequestNumber.ShouldBe(null);
sut.Version.ShouldBe(expectedVersion);
}

private static readonly object?[] InvalidMergeMessages =
{
new object?[] { "Merge pull request # from feature/one", "", null, null, null },
new object?[] { $"Merge pull request # in feature/one from feature/two to {MainBranch}" , "", null, null, null },
new object?[] { $"Zusammengeführter PR : feature/one mit {MainBranch} mergen", "", null, null, null }
new object?[] { "Merge pull request # from feature/one" },
new object?[] { $"Merge pull request # in feature/one from feature/two to {MainBranch}" },
new object?[] { $"Zusammengeführter PR : feature/one mit {MainBranch} mergen" }
};

[TestCaseSource(nameof(InvalidMergeMessages))]
public void ParsesInvalidMergeMessage(
string message,
string expectedMergedBranch,
string expectedTargetBranch,
SemanticVersion expectedVersion,
int? expectedPullRequestNumber)
public void ParsesInvalidMergeMessage(string message)
{
// Act
var sut = new MergeMessage(message, this.configuration);

// Assert
sut.FormatName.ShouldBeNull();
sut.TargetBranch.ShouldBe(expectedTargetBranch);
sut.MergedBranch.ShouldBe(expectedMergedBranch);
sut.TargetBranch.ShouldBeNull();
sut.MergedBranch.ShouldBeNull();
sut.IsMergedPullRequest.ShouldBeFalse();
sut.PullRequestNumber.ShouldBe(expectedPullRequestNumber);
sut.Version.ShouldBe(expectedVersion);
sut.PullRequestNumber.ShouldBeNull();
sut.Version.ShouldBeNull();
}

[Test]
Expand All @@ -276,7 +275,8 @@ public void MatchesSingleCustomMessage()
// Assert
sut.FormatName.ShouldBe(definition);
sut.TargetBranch.ShouldBeNull();
sut.MergedBranch.ShouldBeEmpty();
sut.MergedBranch.ShouldNotBeNull();
sut.MergedBranch.Friendly.ShouldBeEmpty();
sut.IsMergedPullRequest.ShouldBeFalse();
sut.PullRequestNumber.ShouldBe(null);
sut.Version.ShouldBeNull();
Expand All @@ -301,7 +301,8 @@ public void MatchesMultipleCustomMessages()
// Assert
sut.FormatName.ShouldBe(definition);
sut.TargetBranch.ShouldBeNull();
sut.MergedBranch.ShouldBeEmpty();
sut.MergedBranch.ShouldNotBeNull();
sut.MergedBranch.Friendly.ShouldBeEmpty();
sut.IsMergedPullRequest.ShouldBeFalse();
sut.PullRequestNumber.ShouldBe(null);
sut.Version.ShouldBeNull();
Expand All @@ -327,7 +328,8 @@ public void MatchesCaptureGroupsFromCustomMessages()
// Assert
sut.FormatName.ShouldBe(definition);
sut.TargetBranch.ShouldBe(target);
sut.MergedBranch.ShouldBe(source);
sut.MergedBranch.ShouldNotBeNull();
sut.MergedBranch.Friendly.ShouldBe(source);
sut.IsMergedPullRequest.ShouldBeTrue();
sut.PullRequestNumber.ShouldBe(pr);
sut.Version.ShouldBe(new SemanticVersion(2));
Expand All @@ -352,7 +354,8 @@ public void ReturnsAfterFirstMatchingPattern()
// Assert
sut.FormatName.ShouldBe(definition);
sut.TargetBranch.ShouldBeNull();
sut.MergedBranch.ShouldBe("this");
sut.MergedBranch.ShouldNotBeNull();
sut.MergedBranch.Friendly.ShouldBe("this");
sut.IsMergedPullRequest.ShouldBeFalse();
sut.PullRequestNumber.ShouldBe(null);
sut.Version.ShouldBeNull();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,9 +104,9 @@ public void ShouldNotTakeVersionFromMergeOfNonReleaseBranch(string message, bool
AssertMergeMessage(message + "\n ", null, parents);
}

[TestCase("Merge pull request #165 from Particular/release-1.0.0", true)]
[TestCase("Merge pull request #165 in Particular/release-1.0.0", true)]
[TestCase("Merge pull request #500 in FOO/bar from Particular/release-1.0.0 to develop)", true)]
[TestCase("Merge pull request #165 from organization/Particular/release-1.0.0", true)]
[TestCase("Merge pull request #165 in organization/Particular/release-1.0.0", true)]
[TestCase("Merge pull request #500 in FOO/bar from organization/Particular/release-1.0.0 to develop)", true)]
public void ShouldNotTakeVersionFromMergeOfReleaseBranchWithRemoteOtherThanOrigin(string message, bool isMergeCommit)
{
var parents = GetParents(isMergeCommit);
Expand Down
10 changes: 8 additions & 2 deletions src/GitVersion.Core/Configuration/ConfigurationExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,14 @@ public static BranchConfiguration GetBranchConfiguration(this GitVersionConfigur

public static BranchConfiguration GetBranchConfiguration(this GitVersionConfiguration configuration, ReferenceName branchName)
{
var branchConfiguration = GetBranchConfigurations(configuration, branchName.WithoutRemote).FirstOrDefault();
branchConfiguration ??= new() { Name = branchName.WithoutRemote, Regex = string.Empty, Label = ConfigurationConstants.BranchNamePlaceholder, Increment = IncrementStrategy.Inherit };
var branchConfiguration = GetBranchConfigurations(configuration, branchName.WithoutOrigin).FirstOrDefault();
branchConfiguration ??= new()
{
Name = branchName.WithoutOrigin,
Regex = string.Empty,
Label = ConfigurationConstants.BranchNamePlaceholder,
Increment = IncrementStrategy.Inherit
};
return branchConfiguration;
}

Expand Down
2 changes: 1 addition & 1 deletion src/GitVersion.Core/Core/MainlineBranchFinder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ public bool IsMainline(BranchConfiguration value)
return false;

var mainlineRegex = value.Regex;
var branchName = this.branch.Name.WithoutRemote;
var branchName = this.branch.Name.WithoutOrigin;
var match = Regex.IsMatch(branchName, mainlineRegex);
this.log.Info($"'{mainlineRegex}' {(match ? "matches" : "does not match")} '{branchName}'.");
return match;
Expand Down
2 changes: 1 addition & 1 deletion src/GitVersion.Core/Core/MergeCommitFinder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ public IEnumerable<BranchCommit> FindMergeCommitsFor(IBranch branch)

this.mergeBaseCommitsCache.Add(branch, branchMergeBases);

return branchMergeBases.Where(b => !branch.Name.EquivalentTo(b.Branch.Name.WithoutRemote));
return branchMergeBases.Where(b => !branch.Name.EquivalentTo(b.Branch.Name.WithoutOrigin));
}

private IEnumerable<BranchCommit> FindMergeBases(IBranch branch)
Expand Down
4 changes: 2 additions & 2 deletions src/GitVersion.Core/Core/RepositoryStore.cs
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ public IBranch GetTargetBranch(string? targetBranchName)
}

return this.repository.Branches.FirstOrDefault(b =>
Regex.IsMatch(b.Name.WithoutRemote, mainBranchRegex, RegexOptions.IgnoreCase));
Regex.IsMatch(b.Name.WithoutOrigin, mainBranchRegex, RegexOptions.IgnoreCase));
}

public IEnumerable<IBranch> FindMainlineBranches(GitVersionConfiguration configuration)
Expand All @@ -134,7 +134,7 @@ public IEnumerable<IBranch> GetReleaseBranches(IEnumerable<KeyValuePair<string,
=> this.repository.Branches.Where(b => IsReleaseBranch(b, releaseBranchConfig));

private static bool IsReleaseBranch(INamedReference branch, IEnumerable<KeyValuePair<string, BranchConfiguration>> releaseBranchConfig)
=> releaseBranchConfig.Any(c => c.Value.Regex != null && Regex.IsMatch(branch.Name.WithoutRemote, c.Value.Regex));
=> releaseBranchConfig.Any(c => c.Value.Regex != null && Regex.IsMatch(branch.Name.WithoutOrigin, c.Value.Regex));

public IEnumerable<IBranch> ExcludingBranches(IEnumerable<IBranch> branchesToExclude) => this.repository.Branches.ExcludeBranches(branchesToExclude);

Expand Down
2 changes: 1 addition & 1 deletion src/GitVersion.Core/Core/SourceBranchFinder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ public bool IsSourceBranch(INamedReference sourceBranchCandidate)
if (Equals(sourceBranchCandidate, this.branch))
return false;

var branchName = sourceBranchCandidate.Name.WithoutRemote;
var branchName = sourceBranchCandidate.Name.WithoutOrigin;

return this.sourceBranchRegexes.Any(regex => Regex.IsMatch(branchName, regex));
}
Expand Down
14 changes: 7 additions & 7 deletions src/GitVersion.Core/Git/ReferenceName.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ public class ReferenceName : IEquatable<ReferenceName?>, IComparable<ReferenceNa
public const string LocalBranchPrefix = "refs/heads/";
public const string RemoteTrackingBranchPrefix = "refs/remotes/";
public const string TagPrefix = "refs/tags/";
public const string RemotePrefix = "origin/";
public const string OriginPrefix = "origin/";

private static readonly string[] PullRequestPrefixes =
{
Expand All @@ -32,7 +32,7 @@ public ReferenceName(string canonical)
IsPullRequest = IsPrefixedBy(Canonical, PullRequestPrefixes);

Friendly = Shorten();
WithoutRemote = RemoveRemote();
WithoutOrigin = RemoveOrigin();
}

public static ReferenceName Parse(string canonicalName)
Expand Down Expand Up @@ -64,7 +64,7 @@ public static ReferenceName FromBranchName(string branchName)

public string Canonical { get; }
public string Friendly { get; }
public string WithoutRemote { get; }
public string WithoutOrigin { get; }
public bool IsLocalBranch { get; }
public bool IsRemoteBranch { get; }
public bool IsTag { get; }
Expand All @@ -79,7 +79,7 @@ public static ReferenceName FromBranchName(string branchName)
public bool EquivalentTo(string? name) =>
Canonical.Equals(name, StringComparison.OrdinalIgnoreCase)
|| Friendly.Equals(name, StringComparison.OrdinalIgnoreCase)
|| WithoutRemote.Equals(name, StringComparison.OrdinalIgnoreCase);
|| WithoutOrigin.Equals(name, StringComparison.OrdinalIgnoreCase);

private string Shorten()
{
Expand All @@ -95,11 +95,11 @@ private string Shorten()
return Canonical;
}

private string RemoveRemote()
private string RemoveOrigin()
{
if (IsRemoteBranch && !IsPullRequest && Friendly.StartsWith(RemotePrefix, StringComparison.Ordinal))
if (IsRemoteBranch && !IsPullRequest && Friendly.StartsWith(OriginPrefix, StringComparison.Ordinal))
{
return Friendly[RemotePrefix.Length..];
return Friendly[OriginPrefix.Length..];
}
return Friendly;
}
Expand Down
Loading