Skip to content

Commit 65edb4f

Browse files
Kieran MarronKieranties
Kieran Marron
authored and
Kieranties
committed
Refactoring of merge message
1 parent 5c1db80 commit 65edb4f

File tree

2 files changed

+102
-131
lines changed

2 files changed

+102
-131
lines changed

‎src/GitVersionCore.Tests/MergeMessageTests.cs

Lines changed: 49 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,7 @@ public void ParsesMergeMessage(
7575
var sut = new MergeMessage(message, _config);
7676

7777
// Assert
78+
sut.MatchDefinition.ShouldBe("Default");
7879
sut.TargetBranch.ShouldBe(expectedTargetBranch);
7980
sut.MergedBranch.ShouldBe(expectedMergedBranch);
8081
sut.IsMergedPullRequest.ShouldBeFalse();
@@ -91,13 +92,7 @@ public void ParsesMergeMessage(
9192
new object[] { "Merge pull request #1234 from origin/feature/one", "origin/feature/one", null, null, 1234 },
9293
new object[] { "Merge pull request #1234 in feature/4.1/one", "feature/4.1/one", null, new SemanticVersion(4,1), 1234 },
9394
new object[] { "Merge pull request #1234 in V://10.10.10.10", "V://10.10.10.10", null, null, 1234 },
94-
95-
96-
//TODO: Investigate successful github merge messages that may be invalid
97-
// Should an empty PR number be valid?
98-
new object[] { "Merge pull request # from feature/one", "feature/one", null, null, 0 },
99-
// The branch name appears to be incorrect
100-
new object[] { "Merge pull request #1234 from feature/one into dev", "feature/one into dev", "dev", null, 1234 },
95+
new object[] { "Merge pull request #1234 from feature/one into dev", "feature/one", "dev", null, 1234 }
10196
};
10297

10398
[TestCaseSource(nameof(GitHubPullPullMergeMessages))]
@@ -112,31 +107,32 @@ public void ParsesGitHubPullMergeMessage(
112107
var sut = new MergeMessage(message, _config);
113108

114109
// Assert
110+
sut.MatchDefinition.ShouldBe("GitHubPull");
115111
sut.TargetBranch.ShouldBe(expectedTargetBranch);
116112
sut.MergedBranch.ShouldBe(expectedMergedBranch);
117113
sut.IsMergedPullRequest.ShouldBeTrue();
118114
sut.PullRequestNumber.ShouldBe(expectedPullRequestNumber);
119115
sut.Version.ShouldBe(expectedVersion);
120116
}
121-
117+
122118
private static readonly object[] BitBucketPullMergeMessages =
123119
{
124-
new object[] { "Merge pull request #1234 from feature/one from feature/two to dev", "feature/two", null, null, 1234 },
125-
new object[] { "Merge pull request #1234 in feature/one from feature/two to dev", "feature/two", null, null, 1234 },
126-
new object[] { "Merge pull request #1234 in v4.0.0 from v4.1.0 to dev", "v4.1.0", null, new SemanticVersion(4,1), 1234 },
127-
new object[] { "Merge pull request #1234 in V4.0.0 from V4.1.0 to dev", "V4.1.0", null, new SemanticVersion(4,1), 1234 },
128-
new object[] { "Merge pull request #1234 from origin/feature/one from origin/feature/4.2/two to dev", "origin/feature/4.2/two", null, new SemanticVersion(4,2), 1234 },
129-
new object[] { "Merge pull request #1234 in feature/4.1/one from feature/4.2/two to dev", "feature/4.2/two", null, new SemanticVersion(4,2), 1234 },
130-
new object[] { "Merge pull request #1234 in feature/4.1/one from feature/4.2/two to dev into master", "feature/4.2/two", "master", new SemanticVersion(4,2), 1234 },
131-
new object[] { "Merge pull request #1234 in V4.1.0 from V://10.10.10.10 to dev", "V://10.10.10.10", null, null, 1234 },
120+
new object[] { "Merge pull request #1234 from feature/one from feature/two to dev", "feature/two", "dev", null, 1234 },
121+
new object[] { "Merge pull request #1234 in feature/one from feature/two to dev", "feature/two", "dev", null, 1234 },
122+
new object[] { "Merge pull request #1234 in v4.0.0 from v4.1.0 to dev", "v4.1.0", "dev", new SemanticVersion(4,1), 1234 },
123+
new object[] { "Merge pull request #1234 in V4.0.0 from V4.1.0 to dev", "V4.1.0", "dev", new SemanticVersion(4,1), 1234 },
124+
new object[] { "Merge pull request #1234 from origin/feature/one from origin/feature/4.2/two to dev", "origin/feature/4.2/two", "dev", new SemanticVersion(4,2), 1234 },
125+
new object[] { "Merge pull request #1234 in feature/4.1/one from feature/4.2/two to dev", "feature/4.2/two", "dev", new SemanticVersion(4,2), 1234 },
126+
new object[] { "Merge pull request #1234 in feature/4.1/one from feature/4.2/two to dev", "feature/4.2/two", "dev", new SemanticVersion(4,2), 1234 },
127+
new object[] { "Merge pull request #1234 from feature/one from feature/two to master" , "feature/two", "master", null, 1234 },
128+
new object[] { "Merge pull request #1234 in V4.1.0 from V://10.10.10.10 to dev", "V://10.10.10.10", "dev", null, 1234 },
132129
//TODO: Investigate successful bitbucket merge messages that may be invalid
133130
// Regex has double 'from/in from' section. Is that correct?
134-
new object[] { "Merge pull request #1234 in feature/4.1/one from feature/4.2/two to dev", "feature/4.2/two", null, new SemanticVersion(4,2), 1234 },
135-
new object[] { "Merge pull request #1234 from feature/one from v4.0.0 to master", "v4.0.0", null, new SemanticVersion(4), 1234 },
136-
// target branch is not resolved from targetbranch group
137-
new object[] { "Merge pull request #1234 from feature/one from feature/two to master" , "feature/two", null, null, 1234 },
138-
// Should an empty PR number be valid?
139-
new object[] { "Merge pull request # in feature/one from feature/two to master" , "feature/two", null, null, 0 }
131+
new object[] { "Merge pull request #1234 in feature/4.1/one from feature/4.2/two to dev", "feature/4.2/two", "dev", new SemanticVersion(4,2), 1234 },
132+
new object[] { "Merge pull request #1234 from feature/one from v4.0.0 to master", "v4.0.0", "master", new SemanticVersion(4), 1234 }
133+
134+
135+
140136
};
141137

142138
[TestCaseSource(nameof(BitBucketPullMergeMessages))]
@@ -151,13 +147,15 @@ public void ParsesBitBucketPullMergeMessage(
151147
var sut = new MergeMessage(message, _config);
152148

153149
// Assert
150+
sut.MatchDefinition.ShouldBe("BitBucketPull");
154151
sut.TargetBranch.ShouldBe(expectedTargetBranch);
155152
sut.MergedBranch.ShouldBe(expectedMergedBranch);
156153
sut.IsMergedPullRequest.ShouldBeTrue();
157154
sut.PullRequestNumber.ShouldBe(expectedPullRequestNumber);
158155
sut.Version.ShouldBe(expectedVersion);
159156
}
160157

158+
161159
private static readonly object[] SmartGitMergeMessages =
162160
{
163161
new object[] { "Finish feature/one", "feature/one", null, null },
@@ -166,10 +164,7 @@ public void ParsesBitBucketPullMergeMessage(
166164
new object[] { "Finish feature/4.1/one", "feature/4.1/one", null, new SemanticVersion(4, 1) },
167165
new object[] { "Finish origin/4.1/feature/one", "origin/4.1/feature/one", null, new SemanticVersion(4, 1) },
168166
new object[] { "Finish V://10.10.10.10", "V://10.10.10.10", null, null },
169-
170-
//TODO: Investigate successful smart git merge messages that may be invalid
171-
// The branch name appears to be incorrect
172-
new object[] { "Finish V4.0.0 into master", "V4.0.0 into master", "master", new SemanticVersion(4) }
167+
new object[] { "Finish V4.0.0 into master", "V4.0.0", "master", new SemanticVersion(4) }
173168
};
174169

175170
[TestCaseSource(nameof(SmartGitMergeMessages))]
@@ -183,6 +178,7 @@ public void ParsesSmartGitMergeMessage(
183178
var sut = new MergeMessage(message, _config);
184179

185180
// Assert
181+
sut.MatchDefinition.ShouldBe("SmartGit");
186182
sut.TargetBranch.ShouldBe(expectedTargetBranch);
187183
sut.MergedBranch.ShouldBe(expectedMergedBranch);
188184
sut.IsMergedPullRequest.ShouldBeFalse();
@@ -212,11 +208,38 @@ public void ParsesRemoteTrackingMergeMessage(
212208
var sut = new MergeMessage(message, _config);
213209

214210
// Assert
211+
sut.MatchDefinition.ShouldBe("RemoteTracking");
215212
sut.TargetBranch.ShouldBe(expectedTargetBranch);
216213
sut.MergedBranch.ShouldBe(expectedMergedBranch);
217214
sut.IsMergedPullRequest.ShouldBeFalse();
218215
sut.PullRequestNumber.ShouldBeNull();
219216
sut.Version.ShouldBe(expectedVersion);
220217
}
218+
219+
private static readonly object[] InvalidMergeMessages =
220+
{
221+
new object[] { "Merge pull request # from feature/one", "", null, null, null },
222+
new object[] { "Merge pull request # in feature/one from feature/two to master" , "", null, null, null }
223+
};
224+
225+
[TestCaseSource(nameof(InvalidMergeMessages))]
226+
public void ParsesInvalidBitBucketPullMergeMessage(
227+
string message,
228+
string expectedMergedBranch,
229+
string expectedTargetBranch,
230+
SemanticVersion expectedVersion,
231+
int? expectedPullRequestNumber)
232+
{
233+
// Act
234+
var sut = new MergeMessage(message, _config);
235+
236+
// Assert
237+
sut.MatchDefinition.ShouldBeNull();
238+
sut.TargetBranch.ShouldBe(expectedTargetBranch);
239+
sut.MergedBranch.ShouldBe(expectedMergedBranch);
240+
sut.IsMergedPullRequest.ShouldBeFalse();
241+
sut.PullRequestNumber.ShouldBe(expectedPullRequestNumber);
242+
sut.Version.ShouldBe(expectedVersion);
243+
}
221244
}
222245
}

‎src/GitVersionCore/MergeMessage.cs

Lines changed: 53 additions & 105 deletions
Original file line numberDiff line numberDiff line change
@@ -1,138 +1,86 @@
11
using System;
2+
using System.Collections.Generic;
23
using System.Text.RegularExpressions;
34

45
namespace GitVersion
56
{
67
class MergeMessage
78
{
8-
static Regex parseMergeMessage = new Regex(
9-
@"^Merge (branch|tag) '(?<Branch>[^']*)'",
10-
RegexOptions.IgnoreCase | RegexOptions.Compiled);
11-
static Regex parseGitHubPullMergeMessage = new Regex(
12-
@"^Merge pull request #(?<PullRequestNumber>\d*) (from|in) (?<Source>.*)",
13-
RegexOptions.IgnoreCase | RegexOptions.Compiled);
14-
static Regex parseBitBucketPullMergeMessage = new Regex(
15-
@"^Merge pull request #(?<PullRequestNumber>\d*) (from|in) (?<Source>.*) from (?<SourceBranch>.*) to (?<TargetBranch>.*)",
16-
RegexOptions.IgnoreCase | RegexOptions.Compiled);
17-
static Regex smartGitMergeMessage = new Regex(
18-
@"^Finish (?<Branch>.*)",
19-
RegexOptions.IgnoreCase | RegexOptions.Compiled);
20-
static Regex parseRemoteTrackingMergeMessage = new Regex(
21-
@"^Merge remote-tracking branch '(?<SourceBranch>.*)'( into (?<TargetBranch>.*))?",
22-
RegexOptions.IgnoreCase | RegexOptions.Compiled);
23-
static Regex parseTfsMergeMessageEnglishUS = new Regex(
24-
@"^Merge (?<SourceBranch>.*) to (?<TargetBranch>.*)",
25-
RegexOptions.IgnoreCase | RegexOptions.Compiled);
26-
// Zusammengeführter PR \"9\": release/5.0.1 mit master mergen
27-
static Regex parseTfsMergeMessageGermanDE = new Regex(
28-
@"^Zusammengeführter PR ""(?<PullRequestNumber>\d*)""\: (?<SourceBranch>.*) mit (?<TargetBranch>.*) mergen",
29-
RegexOptions.IgnoreCase | RegexOptions.Compiled);
30-
31-
private string mergeMessage;
9+
private static readonly IList<MergeMessagePattern> Patterns = new List<MergeMessagePattern>
10+
{
11+
new MergeMessagePattern("Default", @"^Merge (branch|tag) '(?<SourceBranch>[^']*)'(?: into (?<TargetBranch>[^\s]*))*"),
12+
new MergeMessagePattern("SmartGit", @"^Finish (?<SourceBranch>[^\s]*)(?: into (?<TargetBranch>[^\s]*))*"),
13+
new MergeMessagePattern("BitBucketPull", @"^Merge pull request #(?<PullRequestNumber>\d+) (from|in) (?<Source>.*) from (?<SourceBranch>[^\s]*) to (?<TargetBranch>[^\s]*)"),
14+
new MergeMessagePattern("GitHubPull", @"^Merge pull request #(?<PullRequestNumber>\d+) (from|in) (?:(?<SourceBranch>[^\s]*))(?: into (?<TargetBranch>[^\s]*))*"),
15+
new MergeMessagePattern("RemoteTracking", @"^Merge remote-tracking branch '(?<SourceBranch>[^\s]*)'(?: into (?<TargetBranch>[^\s]*))*")
16+
};
3217

3318
public MergeMessage(string mergeMessage, Config config)
3419
{
35-
this.mergeMessage = mergeMessage;
20+
if (mergeMessage == null)
21+
throw new NullReferenceException();
3622

37-
var lastIndexOf = mergeMessage.LastIndexOf("into", StringComparison.OrdinalIgnoreCase);
38-
if (lastIndexOf != -1)
23+
foreach (var pattern in Patterns)
3924
{
40-
// If we have into in the merge message the rest should be the target branch
41-
TargetBranch = mergeMessage.Substring(lastIndexOf + 5);
42-
}
25+
var match = pattern.Format.Match(mergeMessage);
26+
if (match.Success)
27+
{
28+
MatchDefinition = pattern.Name;
29+
MergedBranch = match.Groups["SourceBranch"].Value;
4330

44-
MergedBranch = ParseBranch();
31+
if (match.Groups["TargetBranch"].Success)
32+
{
33+
TargetBranch = match.Groups["TargetBranch"].Value;
34+
}
4535

46-
// Remove remotes and branch prefixes like release/ feature/ hotfix/ etc
47-
var toMatch = Regex.Replace(MergedBranch, @"^(\w+[-/])*", "", RegexOptions.IgnoreCase);
48-
toMatch = Regex.Replace(toMatch, $"^{config.TagPrefix}", "");
49-
// We don't match if the version is likely an ip (i.e starts with http://)
50-
var versionMatch = new Regex(@"^(?<!://)\d+\.\d+(\.*\d+)*");
51-
var version = versionMatch.Match(toMatch);
36+
if (int.TryParse(match.Groups["PullRequestNumber"].Value, out var pullNumber))
37+
{
38+
PullRequestNumber = pullNumber;
39+
}
5240

53-
if (version.Success)
54-
{
55-
SemanticVersion val;
56-
if (SemanticVersion.TryParse(version.Value, config.TagPrefix, out val))
57-
{
58-
Version = val;
41+
Version = ParseVersion(MergedBranch, config.TagPrefix);
42+
43+
break;
5944
}
6045
}
6146
}
6247

63-
private string ParseBranch()
64-
{
65-
var match = parseMergeMessage.Match(mergeMessage);
66-
if (match.Success)
67-
{
68-
return match.Groups["Branch"].Value;
69-
}
70-
71-
match = smartGitMergeMessage.Match(mergeMessage);
72-
if (match.Success)
73-
{
74-
return match.Groups["Branch"].Value;
75-
}
76-
77-
match = parseBitBucketPullMergeMessage.Match(mergeMessage);
78-
if (match.Success)
79-
{
80-
IsMergedPullRequest = true;
81-
PullRequestNumber = GetPullRequestNumber(match);
82-
return match.Groups["SourceBranch"].Value;
83-
}
84-
85-
match = parseGitHubPullMergeMessage.Match(mergeMessage);
86-
if (match.Success)
87-
{
88-
IsMergedPullRequest = true;
89-
PullRequestNumber = GetPullRequestNumber(match);
90-
var from = match.Groups["Source"].Value;
91-
// TODO We could remove/separate the remote name at this point?
92-
return from;
93-
}
48+
public string MatchDefinition { get; }
49+
public string TargetBranch { get; }
50+
public string MergedBranch { get; } = "";
51+
public bool IsMergedPullRequest => PullRequestNumber != null;
52+
public int? PullRequestNumber { get; }
53+
public SemanticVersion Version { get; }
9454

95-
match = parseRemoteTrackingMergeMessage.Match(mergeMessage);
96-
if (match.Success)
97-
{
98-
var from = match.Groups["SourceBranch"].Value;
99-
// TODO We could remove/separate the remote name at this point?
100-
return from;
101-
}
10255

103-
match = parseTfsMergeMessageEnglishUS.Match(mergeMessage);
104-
if (match.Success)
105-
{
106-
IsMergedPullRequest = true;
107-
var from = match.Groups["SourceBranch"].Value;
108-
return from;
109-
}
56+
private SemanticVersion ParseVersion(string branchName, string tagPrefix)
57+
{
58+
// Remove remotes and branch prefixes like release/ feature/ hotfix/ etc
59+
var toMatch = Regex.Replace(MergedBranch, @"^(\w+[-/])*", "", RegexOptions.IgnoreCase);
60+
toMatch = Regex.Replace(toMatch, $"^{tagPrefix}", "");
61+
// We don't match if the version is likely an ip (i.e starts with http://)
62+
var versionMatch = new Regex(@"^(?<!://)\d+\.\d+(\.*\d+)*");
63+
var version = versionMatch.Match(toMatch);
11064

111-
match = parseTfsMergeMessageGermanDE.Match(mergeMessage);
112-
if (match.Success)
65+
if (version.Success && SemanticVersion.TryParse(version.Value, tagPrefix, out var val))
11366
{
114-
IsMergedPullRequest = true;
115-
var from = match.Groups["SourceBranch"].Value;
116-
return from;
67+
return val;
11768
}
11869

119-
return "";
70+
return null;
12071
}
12172

122-
private int GetPullRequestNumber(Match match)
73+
private class MergeMessagePattern
12374
{
124-
int pullNumber;
125-
if (int.TryParse(match.Groups["PullRequestNumber"].Value, out pullNumber))
75+
public MergeMessagePattern(string name, string format)
12676
{
127-
PullRequestNumber = pullNumber;
77+
Name = name;
78+
Format = new Regex(format, RegexOptions.IgnoreCase | RegexOptions.Compiled);
12879
}
129-
return pullNumber;
130-
}
13180

132-
public string TargetBranch { get; }
133-
public string MergedBranch { get; }
134-
public bool IsMergedPullRequest { get; private set; }
135-
public int? PullRequestNumber { get; private set; }
136-
public SemanticVersion Version { get; }
81+
public string Name { get; }
82+
83+
public Regex Format { get; }
84+
}
13785
}
13886
}

0 commit comments

Comments
 (0)