From d8cef691b713fe1eae4de9940db25d1347935dfd Mon Sep 17 00:00:00 2001 From: Henrik Andersson Date: Tue, 19 May 2015 07:53:01 +1000 Subject: [PATCH 1/2] Add SHA and make message optional --- .../Clients/PullRequestsClientTests.cs | 4 ++-- Octokit.Tests/Clients/PullRequestsClientTests.cs | 8 ++++---- .../ObservablePullRequestsClientTests.cs | 6 +++--- Octokit/Models/Request/MergePullRequest.cs | 16 +++++++--------- 4 files changed, 16 insertions(+), 18 deletions(-) diff --git a/Octokit.Tests.Integration/Clients/PullRequestsClientTests.cs b/Octokit.Tests.Integration/Clients/PullRequestsClientTests.cs index 590a05a59b..c35caa3761 100644 --- a/Octokit.Tests.Integration/Clients/PullRequestsClientTests.cs +++ b/Octokit.Tests.Integration/Clients/PullRequestsClientTests.cs @@ -193,7 +193,7 @@ 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); @@ -207,7 +207,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 index b8ff769f88..34fc70c83b 100644 --- a/Octokit.Tests/Clients/PullRequestsClientTests.cs +++ b/Octokit.Tests/Clients/PullRequestsClientTests.cs @@ -138,9 +138,9 @@ await AssertEx.Throws(() => 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(); var client = new PullRequestsClient(connection); @@ -157,9 +157,9 @@ public async Task EnsuresArgumentsNotNull() var client = new PullRequestsClient(connection); await AssertEx.Throws(() => - client.Merge(null, "name", 42, new MergePullRequest("message"))); + client.Merge(null, "name", 42, new MergePullRequest { Message = "message" })); await AssertEx.Throws(() => - client.Merge("owner", null, 42, new MergePullRequest("message"))); + client.Merge("owner", null, 42, new MergePullRequest { Message = "message" })); await AssertEx.Throws(() => client.Merge("owner", "name", 42, null)); } diff --git a/Octokit.Tests/Reactive/ObservablePullRequestsClientTests.cs b/Octokit.Tests/Reactive/ObservablePullRequestsClientTests.cs index 8dac68aa46..e4a2c0106e 100644 --- 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(); var client = new ObservablePullRequestsClient(gitHubClient); @@ -240,9 +240,9 @@ public async Task EnsuresArgumentsNotNull() var client = new PullRequestsClient(connection); await AssertEx.Throws(async () => await - client.Merge(null, "name", 42, new MergePullRequest("message"))); + client.Merge(null, "name", 42, new MergePullRequest { Message = "message" })); await AssertEx.Throws(async () => await - client.Merge("owner", null, 42, new MergePullRequest("message"))); + client.Merge("owner", null, 42, new MergePullRequest { Message = "message" })); await AssertEx.Throws(async () => await client.Merge("owner", "name", 42, null)); } diff --git a/Octokit/Models/Request/MergePullRequest.cs b/Octokit/Models/Request/MergePullRequest.cs index 307422cef8..368331aef3 100644 --- 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; - } - /// /// The message that will be used for the merge commit (optional) /// - public string Message { get; private set; } + public string Message { get; set; } + + /// + /// The SHA that pull request head must match to allow merge (optional) + /// + 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); } } } From 6cdb1cf918dbd7d0390584ee437f1d484a712660 Mon Sep 17 00:00:00 2001 From: Henrik Andersson Date: Tue, 19 May 2015 08:36:46 +1000 Subject: [PATCH 2/2] Add integration tests --- .../Clients/PullRequestsClientTests.cs | 43 +++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/Octokit.Tests.Integration/Clients/PullRequestsClientTests.cs b/Octokit.Tests.Integration/Clients/PullRequestsClientTests.cs index c35caa3761..d98f8acb79 100644 --- 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; @@ -199,6 +200,48 @@ public async Task CanBeMerged() 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(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() {