Merge pull request #805 from alfhenrik/mergepullrequest-sha

Add SHA and make message optional for pull request merge
octokit · May 19, 2015 · bb57a4a · bb57a4a
2 parents bef91f7 + 6cdb1cf
commit bb57a4a
Show file tree

Hide file tree

Showing 4 changed files with 59 additions and 18 deletions.
diff --git a/Octokit.Tests.Integration/Clients/PullRequestsClientTests.cs b/Octokit.Tests.Integration/Clients/PullRequestsClientTests.cs
@@ -3,6 +3,7 @@
 using System.Linq;
 using System.Threading.Tasks;
 using Octokit;
+using Octokit.Tests.Helpers;
 using Octokit.Tests.Integration;
 using Xunit;
 
@@ -193,12 +194,54 @@ public async Task CanBeMerged()
         var newPullRequest = new NewPullRequest("a pull request", branchName, "master");
         var pullRequest = await _fixture.Create(Helper.UserName, _repository.Name, newPullRequest);
 
-        var merge = new MergePullRequest("thing the thing");
+        var merge = new MergePullRequest { Message = "thing the thing" };
         var result = await _fixture.Merge(Helper.UserName, _repository.Name, pullRequest.Number, merge);
 
         Assert.True(result.Merged);
     }
 
+    [IntegrationTest]
+    public async Task CanBeMergedWithNoOptionalInput()
+    {
+        await CreateTheWorld();
+
+        var newPullRequest = new NewPullRequest("a pull request", branchName, "master");
+        var pullRequest = await _fixture.Create(Helper.UserName, _repository.Name, newPullRequest);
+
+        var merge = new MergePullRequest();
+        var result = await _fixture.Merge(Helper.UserName, _repository.Name, pullRequest.Number, merge);
+
+        Assert.True(result.Merged);
+    }
+    [IntegrationTest]
+    public async Task CanBeMergedWithShaSpecified()
+    {
+        await CreateTheWorld();
+
+        var newPullRequest = new NewPullRequest("a pull request", branchName, "master");
+        var pullRequest = await _fixture.Create(Helper.UserName, _repository.Name, newPullRequest);
+
+        var merge = new MergePullRequest { Message = "thing the thing", Sha = pullRequest.Head.Sha };
+        var result = await _fixture.Merge(Helper.UserName, _repository.Name, pullRequest.Number, merge);
+
+        Assert.True(result.Merged);
+    }
+
+    [IntegrationTest]
+    public async Task CannotBeMerged()
+    {
+        await CreateTheWorld();
+        var fakeSha = new string('f', 40);
+
+        var newPullRequest = new NewPullRequest("a pull request", branchName, "master");
+        var pullRequest = await _fixture.Create(Helper.UserName, _repository.Name, newPullRequest);
+
+        var merge = new MergePullRequest { Sha = fakeSha };
+        var ex = await AssertEx.Throws<ApiException>(async () => await _fixture.Merge(Helper.UserName, _repository.Name, pullRequest.Number, merge));
+
+        Assert.True(ex.ApiError.Message.StartsWith("Head branch was modified"));
+    }
+
     [IntegrationTest]
     public async Task UpdatesMaster()
     {
@@ -207,7 +250,7 @@ public async Task UpdatesMaster()
         var newPullRequest = new NewPullRequest("a pull request", branchName, "master");
         var pullRequest = await _fixture.Create(Helper.UserName, _repository.Name, newPullRequest);
 
-        var merge = new MergePullRequest("thing the thing");
+        var merge = new MergePullRequest { Message = "thing the thing" };
         var result = await _fixture.Merge(Helper.UserName, _repository.Name, pullRequest.Number, merge);
 
         var master = await _client.GitDatabase.Reference.Get(Helper.UserName, _repository.Name, "heads/master");

diff --git a/Octokit.Tests/Clients/PullRequestsClientTests.cs b/Octokit.Tests/Clients/PullRequestsClientTests.cs
@@ -138,9 +138,9 @@ await AssertEx.Throws<ArgumentNullException>(() =>
         public class TheMergeMethod 
         {
             [Fact]
-            public void PutsToCorrectUrl() 
+            public void PutsToCorrectUrl()
             {
-                var mergePullRequest = new MergePullRequest("fake commit message");
+                var mergePullRequest = new MergePullRequest { Message = "fake commit message" };
                 var connection = Substitute.For<IApiConnection>();
                 var client = new PullRequestsClient(connection);
 
@@ -157,9 +157,9 @@ public async Task EnsuresArgumentsNotNull()
                 var client = new PullRequestsClient(connection);
 
                 await AssertEx.Throws<ArgumentNullException>(() =>
-                    client.Merge(null, "name", 42, new MergePullRequest("message")));
+                    client.Merge(null, "name", 42, new MergePullRequest { Message = "message" }));
                 await AssertEx.Throws<ArgumentNullException>(() =>
-                    client.Merge("owner", null, 42, new MergePullRequest("message")));
+                    client.Merge("owner", null, 42, new MergePullRequest { Message = "message" }));
                 await AssertEx.Throws<ArgumentNullException>(() =>
                     client.Merge("owner", "name", 42, null));
             }

diff --git a/Octokit.Tests/Reactive/ObservablePullRequestsClientTests.cs b/Octokit.Tests/Reactive/ObservablePullRequestsClientTests.cs
@@ -224,7 +224,7 @@ public class TheMergeMethod
             [Fact]
             public void MergesPullRequest()
             {
-                var mergePullRequest = new MergePullRequest("fake commit message");
+                var mergePullRequest = new MergePullRequest { Message = "fake commit message" };
                 var gitHubClient = Substitute.For<IGitHubClient>();
                 var client = new ObservablePullRequestsClient(gitHubClient);
 
@@ -240,9 +240,9 @@ public async Task EnsuresArgumentsNotNull()
                 var client = new PullRequestsClient(connection);
 
                 await AssertEx.Throws<ArgumentNullException>(async () => await
-                    client.Merge(null, "name", 42, new MergePullRequest("message")));
+                    client.Merge(null, "name", 42, new MergePullRequest { Message = "message" }));
                 await AssertEx.Throws<ArgumentNullException>(async () => await
-                    client.Merge("owner", null, 42, new MergePullRequest("message")));
+                    client.Merge("owner", null, 42, new MergePullRequest { Message = "message" }));
                 await AssertEx.Throws<ArgumentNullException>(async () => await
                     client.Merge("owner", "name", 42, null));
             }

diff --git a/Octokit/Models/Request/MergePullRequest.cs b/Octokit/Models/Request/MergePullRequest.cs
@@ -11,23 +11,21 @@ namespace Octokit
     [DebuggerDisplay("{DebuggerDisplay,nq}")]
     public class MergePullRequest
     {
-        public MergePullRequest(string message)
-        {
-            Ensure.ArgumentNotNull(message, "message");
-
-            Message = message;
-        }
-
         /// <summary>
         /// The message that will be used for the merge commit (optional)
         /// </summary>
-        public string Message { get; private set; }
+        public string Message { get; set; }
+
+        /// <summary>
+        /// The SHA that pull request head must match to allow merge (optional)
+        /// </summary>
+        public string Sha { get; set; }
 
         internal string DebuggerDisplay
         {
             get
             {
-                return String.Format(CultureInfo.InvariantCulture, "Message: {0}", Message);
+                return String.Format(CultureInfo.InvariantCulture, "Message: '{0}', Sha: '{1}'", Message, Sha);
             }
         }
     }