From e328911fc6cc85f2ba28eba33fe7502caa82f5fb Mon Sep 17 00:00:00 2001 From: "aedampir@gmail.com" Date: Fri, 25 Mar 2016 15:24:07 +0700 Subject: [PATCH 01/14] Method GetAll(string owner, string name, ApiOptions options) was implemented. --- Octokit/Clients/DeploymentsClient.cs | 19 ++++++++++++++++++- Octokit/Clients/IDeploymentsClient.cs | 13 +++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/Octokit/Clients/DeploymentsClient.cs b/Octokit/Clients/DeploymentsClient.cs index 4ccc2948..4f63e510 100644 --- a/Octokit/Clients/DeploymentsClient.cs +++ b/Octokit/Clients/DeploymentsClient.cs @@ -33,11 +33,28 @@ namespace Octokit /// The name of the repository /// All the s for the specified repository. public Task> GetAll(string owner, string name) + { + return GetAll(owner, name, ApiOptions.None); + } + + /// + /// Gets all the deployments for the specified repository. Any user with pull access + /// to a repository can view deployments. + /// + /// + /// http://developer.github.com/v3/repos/deployments/#list-deployments + /// + /// The owner of the repository + /// The name of the repository + /// Options for changing the API response + /// All the s for the specified repository. + public Task> GetAll(string owner, string name, ApiOptions options) { Ensure.ArgumentNotNullOrEmptyString(owner, "login"); Ensure.ArgumentNotNullOrEmptyString(name, "name"); + Ensure.ArgumentNotNull(options, "options"); - return ApiConnection.GetAll(ApiUrls.Deployments(owner, name)); + return ApiConnection.GetAll(ApiUrls.Deployments(owner, name), options); } /// diff --git a/Octokit/Clients/IDeploymentsClient.cs b/Octokit/Clients/IDeploymentsClient.cs index 97a8b245..3cbcd80f 100644 --- a/Octokit/Clients/IDeploymentsClient.cs +++ b/Octokit/Clients/IDeploymentsClient.cs @@ -24,6 +24,19 @@ namespace Octokit /// All the s for the specified repository. Task> GetAll(string owner, string name); + /// + /// Gets all the deployments for the specified repository. Any user with pull access + /// to a repository can view deployments. + /// + /// + /// http://developer.github.com/v3/repos/deployments/#list-deployments + /// + /// The owner of the repository + /// The name of the repository + /// Options for changing the API response + /// All the s for the specified repository. + Task> GetAll(string owner, string name, ApiOptions options); + /// /// Creates a new deployment for the specified repository. /// Users with push access can create a deployment for a given ref. From ad15e05471ca144572e50685b34277d5ab3d40cc Mon Sep 17 00:00:00 2001 From: "aedampir@gmail.com" Date: Fri, 25 Mar 2016 15:51:37 +0700 Subject: [PATCH 02/14] New GetAll overload added for ObservableDeploymentsClient --- .../Clients/IObservableDeploymentsClient.cs | 13 ++++++++++++ .../Clients/ObservableDeploymentsClient.cs | 21 +++++++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/Octokit.Reactive/Clients/IObservableDeploymentsClient.cs b/Octokit.Reactive/Clients/IObservableDeploymentsClient.cs index 3a77e846..23fbee4a 100644 --- a/Octokit.Reactive/Clients/IObservableDeploymentsClient.cs +++ b/Octokit.Reactive/Clients/IObservableDeploymentsClient.cs @@ -16,6 +16,19 @@ namespace Octokit.Reactive /// All the s for the specified repository. IObservable GetAll(string owner, string name); + /// + /// Gets all the deployments for the specified repository. Any user with pull access + /// to a repository can view deployments. + /// + /// + /// http://developer.github.com/v3/repos/deployments/#list-deployments + /// + /// The owner of the repository + /// The name of the repository + /// Options for changing the API response + /// All the s for the specified repository. + IObservable GetAll(string owner, string name, ApiOptions options); + /// /// Creates a new deployment for the specified repository. /// Users with push access can create a deployment for a given ref. diff --git a/Octokit.Reactive/Clients/ObservableDeploymentsClient.cs b/Octokit.Reactive/Clients/ObservableDeploymentsClient.cs index 3d2ab9f1..1851b719 100644 --- a/Octokit.Reactive/Clients/ObservableDeploymentsClient.cs +++ b/Octokit.Reactive/Clients/ObservableDeploymentsClient.cs @@ -38,6 +38,27 @@ namespace Octokit.Reactive.Clients ApiUrls.Deployments(owner, name)); } + /// + /// Gets all the deployments for the specified repository. Any user with pull access + /// to a repository can view deployments. + /// + /// + /// http://developer.github.com/v3/repos/deployments/#list-deployments + /// + /// The owner of the repository + /// The name of the repository + /// Options for changing the API response + /// All the s for the specified repository. + public IObservable GetAll(string owner, string name, ApiOptions options) + { + Ensure.ArgumentNotNullOrEmptyString(owner, "owner"); + Ensure.ArgumentNotNullOrEmptyString(name, "name"); + Ensure.ArgumentNotNull(options, "options"); + + return _connection.GetAndFlattenAllPages( + ApiUrls.Deployments(owner, name), options); + } + /// /// Creates a new deployment for the specified repository. /// Users with push access can create a deployment for a given ref. From 7d5b56618bced60b1fdb4b371def7b20bbf0991d Mon Sep 17 00:00:00 2001 From: "aedampir@gmail.com" Date: Fri, 25 Mar 2016 15:51:56 +0700 Subject: [PATCH 03/14] DeploymentsClientTests were updated --- .../Clients/DeploymentsClientTests.cs | 48 +++++++++++++------ 1 file changed, 34 insertions(+), 14 deletions(-) diff --git a/Octokit.Tests/Clients/DeploymentsClientTests.cs b/Octokit.Tests/Clients/DeploymentsClientTests.cs index 3eb59569..c0bd008d 100644 --- a/Octokit.Tests/Clients/DeploymentsClientTests.cs +++ b/Octokit.Tests/Clients/DeploymentsClientTests.cs @@ -2,6 +2,7 @@ using System.Threading.Tasks; using NSubstitute; using Octokit; +using Octokit.Tests; using Xunit; public class DeploymentsClientTests @@ -15,6 +16,7 @@ public class DeploymentsClientTests await Assert.ThrowsAsync(() => client.GetAll(null, "name")); await Assert.ThrowsAsync(() => client.GetAll("owner", null)); + await Assert.ThrowsAsync(() => client.GetAll("owner", "name", null)); } [Fact] @@ -45,24 +47,42 @@ public class DeploymentsClientTests { var connection = Substitute.For(); var client = new DeploymentsClient(connection); - var expectedUrl = "repos/owner/name/deployments"; + var expectedUrl = ApiUrls.Deployments("owner", "name"); client.GetAll("owner", "name"); - connection.Received(1).GetAll(Arg.Is(u => u.ToString() == expectedUrl)); + connection.Received(1).GetAll(Arg.Is(u => u == expectedUrl), Args.ApiOptions); + } + + [Fact] + public void RequestsCorrectUrlWithApiOptions() + { + var connection = Substitute.For(); + var client = new DeploymentsClient(connection); + var expectedUrl = ApiUrls.Deployments("owner", "name"); + + var options = new ApiOptions + { + PageSize = 1, + PageCount = 1, + StartPage = 1 + }; + + client.GetAll("owner", "name"); + connection.Received(1).GetAll(Arg.Is(u => u == expectedUrl), options); } } public class TheCreateMethod { - readonly NewDeployment newDeployment = new NewDeployment("aRef"); + private readonly NewDeployment _newDeployment = new NewDeployment("aRef"); [Fact] public async Task EnsuresNonNullArguments() { var client = new DeploymentsClient(Substitute.For()); - await Assert.ThrowsAsync(() => client.Create(null, "name", newDeployment)); - await Assert.ThrowsAsync(() => client.Create("owner", null, newDeployment)); + await Assert.ThrowsAsync(() => client.Create(null, "name", _newDeployment)); + await Assert.ThrowsAsync(() => client.Create("owner", null, _newDeployment)); await Assert.ThrowsAsync(() => client.Create("owner", "name", null)); } @@ -71,8 +91,8 @@ public class DeploymentsClientTests { var client = new DeploymentsClient(Substitute.For()); - await Assert.ThrowsAsync(() => client.Create("", "name", newDeployment)); - await Assert.ThrowsAsync(() => client.Create("owner", "", newDeployment)); + await Assert.ThrowsAsync(() => client.Create("", "name", _newDeployment)); + await Assert.ThrowsAsync(() => client.Create("owner", "", _newDeployment)); } [Theory] @@ -85,8 +105,8 @@ public class DeploymentsClientTests { var client = new DeploymentsClient(Substitute.For()); - await Assert.ThrowsAsync(() => client.Create(whitespace, "name", newDeployment)); - await Assert.ThrowsAsync(() => client.Create("owner", whitespace, newDeployment)); + await Assert.ThrowsAsync(() => client.Create(whitespace, "name", _newDeployment)); + await Assert.ThrowsAsync(() => client.Create("owner", whitespace, _newDeployment)); } [Fact] @@ -94,11 +114,11 @@ public class DeploymentsClientTests { var connection = Substitute.For(); var client = new DeploymentsClient(connection); - var expectedUrl = "repos/owner/name/deployments"; + var expectedUrl = ApiUrls.Deployments("owner", "name"); - client.Create("owner", "name", newDeployment); + client.Create("owner", "name", _newDeployment); - connection.Received(1).Post(Arg.Is(u => u.ToString() == expectedUrl), + connection.Received(1).Post(Arg.Is(u => u == expectedUrl), Arg.Any()); } @@ -108,10 +128,10 @@ public class DeploymentsClientTests var connection = Substitute.For(); var client = new DeploymentsClient(connection); - client.Create("owner", "name", newDeployment); + client.Create("owner", "name", _newDeployment); connection.Received(1).Post(Arg.Any(), - newDeployment); + _newDeployment); } } From 1ebdfbfb0acae585dd093f246bf05f003497278d Mon Sep 17 00:00:00 2001 From: "aedampir@gmail.com" Date: Fri, 25 Mar 2016 15:52:12 +0700 Subject: [PATCH 04/14] ObservableDeploymentsClientTests were updated --- .../ObservableDeploymentsClientTests.cs | 32 +++++++++++++++---- 1 file changed, 26 insertions(+), 6 deletions(-) diff --git a/Octokit.Tests/Reactive/ObservableDeploymentsClientTests.cs b/Octokit.Tests/Reactive/ObservableDeploymentsClientTests.cs index 9005cc7e..4f63e92d 100644 --- a/Octokit.Tests/Reactive/ObservableDeploymentsClientTests.cs +++ b/Octokit.Tests/Reactive/ObservableDeploymentsClientTests.cs @@ -13,8 +13,8 @@ namespace Octokit.Tests.Reactive { public class TheGetAllMethod { - readonly IGitHubClient _githubClient; - readonly ObservableDeploymentsClient _client; + private readonly IGitHubClient _githubClient; + private readonly ObservableDeploymentsClient _client; public TheGetAllMethod() { @@ -27,6 +27,7 @@ namespace Octokit.Tests.Reactive { Assert.Throws(() => _client.GetAll(null, "repo")); Assert.Throws(() => _client.GetAll("owner", null)); + Assert.Throws(() => _client.GetAll("owner", "repo", null)); } [Fact] @@ -46,7 +47,7 @@ namespace Octokit.Tests.Reactive } [Fact] - public void CallsDeploymentsUrl() + public void RequestsCorrectUrl() { var expectedUri = ApiUrls.Deployments("owner", "repo"); @@ -54,14 +55,33 @@ namespace Octokit.Tests.Reactive _githubClient.Connection .Received(1) .Get>(Arg.Is(expectedUri), - Arg.Any>(), Arg.Any()); + Arg.Is>(dictionary => dictionary.Count == 0), Arg.Any()); + } + + [Fact] + public void RequestsCorrectUrlWithApiOptions() + { + var expectedUri = ApiUrls.Deployments("owner", "repo"); + + var options = new ApiOptions + { + StartPage = 1, + PageCount = 1, + PageSize = 1 + }; + + _client.GetAll("owner", "repo", options); + _githubClient.Connection + .Received(1) + .Get>(Arg.Is(expectedUri), + Arg.Is>(dictionary => dictionary.Count == 3), Arg.Any()); } } public class TheCreateMethod { - IGitHubClient _githubClient; - ObservableDeploymentsClient _client; + private readonly IGitHubClient _githubClient; + private ObservableDeploymentsClient _client; public TheCreateMethod() { From a1e9a283e90c2f506ff9a7380839f90169734f9c Mon Sep 17 00:00:00 2001 From: "aedampir@gmail.com" Date: Fri, 25 Mar 2016 16:15:27 +0700 Subject: [PATCH 05/14] Small fixes in DeploymentsClientTests --- .../Clients/DeploymentsClientTests.cs | 25 +++++++++++-------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/Octokit.Tests/Clients/DeploymentsClientTests.cs b/Octokit.Tests/Clients/DeploymentsClientTests.cs index c0bd008d..e7f20ecc 100644 --- a/Octokit.Tests/Clients/DeploymentsClientTests.cs +++ b/Octokit.Tests/Clients/DeploymentsClientTests.cs @@ -9,14 +9,17 @@ public class DeploymentsClientTests { public class TheGetAllMethod { + private const string name = "name"; + private const string owner = "name"; + [Fact] public async Task EnsuresNonNullArguments() { var client = new DeploymentsClient(Substitute.For()); - await Assert.ThrowsAsync(() => client.GetAll(null, "name")); - await Assert.ThrowsAsync(() => client.GetAll("owner", null)); - await Assert.ThrowsAsync(() => client.GetAll("owner", "name", null)); + await Assert.ThrowsAsync(() => client.GetAll(null, name)); + await Assert.ThrowsAsync(() => client.GetAll(owner, null)); + await Assert.ThrowsAsync(() => client.GetAll(owner, name, null)); } [Fact] @@ -24,8 +27,8 @@ public class DeploymentsClientTests { var client = new DeploymentsClient(Substitute.For()); - await Assert.ThrowsAsync(() => client.GetAll("", "name")); - await Assert.ThrowsAsync(() => client.GetAll("owner", "")); + await Assert.ThrowsAsync(() => client.GetAll("", name)); + await Assert.ThrowsAsync(() => client.GetAll(owner, "")); } [Theory] @@ -38,8 +41,8 @@ public class DeploymentsClientTests { var client = new DeploymentsClient(Substitute.For()); - await Assert.ThrowsAsync(() => client.GetAll(whitespace, "name")); - await Assert.ThrowsAsync(() => client.GetAll("owner", whitespace)); + await Assert.ThrowsAsync(() => client.GetAll(whitespace, name)); + await Assert.ThrowsAsync(() => client.GetAll(owner, whitespace)); } [Fact] @@ -47,9 +50,9 @@ public class DeploymentsClientTests { var connection = Substitute.For(); var client = new DeploymentsClient(connection); - var expectedUrl = ApiUrls.Deployments("owner", "name"); + var expectedUrl = ApiUrls.Deployments(owner, name); - client.GetAll("owner", "name"); + client.GetAll(owner, name); connection.Received(1).GetAll(Arg.Is(u => u == expectedUrl), Args.ApiOptions); } @@ -58,7 +61,7 @@ public class DeploymentsClientTests { var connection = Substitute.For(); var client = new DeploymentsClient(connection); - var expectedUrl = ApiUrls.Deployments("owner", "name"); + var expectedUrl = ApiUrls.Deployments(owner, name); var options = new ApiOptions { @@ -67,7 +70,7 @@ public class DeploymentsClientTests StartPage = 1 }; - client.GetAll("owner", "name"); + client.GetAll(owner, name); connection.Received(1).GetAll(Arg.Is(u => u == expectedUrl), options); } } From 38dfa82100ae0c71a4684f403242a52cdfa5de78 Mon Sep 17 00:00:00 2001 From: "aedampir@gmail.com" Date: Fri, 25 Mar 2016 16:30:10 +0700 Subject: [PATCH 06/14] Fix for prev commit. --- Octokit.Tests/Clients/DeploymentsClientTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Octokit.Tests/Clients/DeploymentsClientTests.cs b/Octokit.Tests/Clients/DeploymentsClientTests.cs index e7f20ecc..af4ad63b 100644 --- a/Octokit.Tests/Clients/DeploymentsClientTests.cs +++ b/Octokit.Tests/Clients/DeploymentsClientTests.cs @@ -10,7 +10,7 @@ public class DeploymentsClientTests public class TheGetAllMethod { private const string name = "name"; - private const string owner = "name"; + private const string owner = "owner"; [Fact] public async Task EnsuresNonNullArguments() From 6aa617a3edf844916f01e8adc0670cc614b8a343 Mon Sep 17 00:00:00 2001 From: "aedampir@gmail.com" Date: Fri, 25 Mar 2016 16:31:09 +0700 Subject: [PATCH 07/14] Some additional fixes for ObservableDeploymentsClient. --- Octokit.Reactive/Clients/ObservableDeploymentsClient.cs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/Octokit.Reactive/Clients/ObservableDeploymentsClient.cs b/Octokit.Reactive/Clients/ObservableDeploymentsClient.cs index 1851b719..3d382af5 100644 --- a/Octokit.Reactive/Clients/ObservableDeploymentsClient.cs +++ b/Octokit.Reactive/Clients/ObservableDeploymentsClient.cs @@ -31,11 +31,7 @@ namespace Octokit.Reactive.Clients /// All the s for the specified repository. public IObservable GetAll(string owner, string name) { - Ensure.ArgumentNotNullOrEmptyString(owner, "owner"); - Ensure.ArgumentNotNullOrEmptyString(name, "name"); - - return _connection.GetAndFlattenAllPages( - ApiUrls.Deployments(owner, name)); + return GetAll(owner, name, ApiOptions.None); } /// From 18150cf152df5f0430a3b7ffe79fca7eb2679637 Mon Sep 17 00:00:00 2001 From: "aedampir@gmail.com" Date: Fri, 25 Mar 2016 16:35:59 +0700 Subject: [PATCH 08/14] Some small refactorings in ObservableDeploymentsClientTests. --- .../ObservableDeploymentsClientTests.cs | 24 ++++++++++--------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/Octokit.Tests/Reactive/ObservableDeploymentsClientTests.cs b/Octokit.Tests/Reactive/ObservableDeploymentsClientTests.cs index 4f63e92d..ddbe4cd7 100644 --- a/Octokit.Tests/Reactive/ObservableDeploymentsClientTests.cs +++ b/Octokit.Tests/Reactive/ObservableDeploymentsClientTests.cs @@ -15,6 +15,8 @@ namespace Octokit.Tests.Reactive { private readonly IGitHubClient _githubClient; private readonly ObservableDeploymentsClient _client; + private const string owner = "owner"; + private const string name = "name"; public TheGetAllMethod() { @@ -25,33 +27,33 @@ namespace Octokit.Tests.Reactive [Fact] public void EnsuresNonNullArguments() { - Assert.Throws(() => _client.GetAll(null, "repo")); - Assert.Throws(() => _client.GetAll("owner", null)); - Assert.Throws(() => _client.GetAll("owner", "repo", null)); + Assert.Throws(() => _client.GetAll(null, name)); + Assert.Throws(() => _client.GetAll(owner, null)); + Assert.Throws(() => _client.GetAll(owner, name, null)); } [Fact] public void EnsuresNonEmptyArguments() { - Assert.Throws(() => _client.GetAll("", "repo")); - Assert.Throws(() => _client.GetAll("owner", "")); + Assert.Throws(() => _client.GetAll("", name)); + Assert.Throws(() => _client.GetAll(owner, "")); } [Fact] public async Task EnsuresNonWhitespaceArguments() { await AssertEx.ThrowsWhenGivenWhitespaceArgument( - async whitespace => await _client.GetAll(whitespace, "repo")); + async whitespace => await _client.GetAll(whitespace, name)); await AssertEx.ThrowsWhenGivenWhitespaceArgument( - async whitespace => await _client.GetAll("owner", whitespace)); + async whitespace => await _client.GetAll(owner, whitespace)); } [Fact] public void RequestsCorrectUrl() { - var expectedUri = ApiUrls.Deployments("owner", "repo"); + var expectedUri = ApiUrls.Deployments(owner, name); - _client.GetAll("owner", "repo"); + _client.GetAll(owner, name); _githubClient.Connection .Received(1) .Get>(Arg.Is(expectedUri), @@ -61,7 +63,7 @@ namespace Octokit.Tests.Reactive [Fact] public void RequestsCorrectUrlWithApiOptions() { - var expectedUri = ApiUrls.Deployments("owner", "repo"); + var expectedUri = ApiUrls.Deployments(owner, name); var options = new ApiOptions { @@ -70,7 +72,7 @@ namespace Octokit.Tests.Reactive PageSize = 1 }; - _client.GetAll("owner", "repo", options); + _client.GetAll(owner, name, options); _githubClient.Connection .Received(1) .Get>(Arg.Is(expectedUri), From 9943af7ef169da310e9c261128c0a47a663bce38 Mon Sep 17 00:00:00 2001 From: "aedampir@gmail.com" Date: Fri, 25 Mar 2016 17:11:50 +0700 Subject: [PATCH 09/14] Integration tests for ApiOptions were added. --- .../Clients/DeploymentsClientTests.cs | 119 +++++++++++++++--- 1 file changed, 99 insertions(+), 20 deletions(-) diff --git a/Octokit.Tests.Integration/Clients/DeploymentsClientTests.cs b/Octokit.Tests.Integration/Clients/DeploymentsClientTests.cs index af308d6f..b2ab7abe 100644 --- a/Octokit.Tests.Integration/Clients/DeploymentsClientTests.cs +++ b/Octokit.Tests.Integration/Clients/DeploymentsClientTests.cs @@ -1,4 +1,6 @@ using System; +using System.Collections.Generic; +using System.Linq; using System.Threading.Tasks; using Octokit; using Octokit.Tests.Integration; @@ -9,7 +11,7 @@ public class DeploymentsClientTests : IDisposable { private readonly IDeploymentsClient _deploymentsClient; private readonly RepositoryContext _context; - private readonly Commit _commit; + private readonly ICollection _commits; public DeploymentsClientTests() { @@ -17,33 +19,38 @@ public class DeploymentsClientTests : IDisposable _deploymentsClient = github.Repository.Deployment; _context = github.CreateRepositoryContext("public-repo").Result; + _commits = new List(); - var blob = new NewBlob + for (int i = 0; i < 6; i++) { - Content = "Hello World!", - Encoding = EncodingType.Utf8 - }; + var blob = new NewBlob + { + Content = string.Format("Hello World {0}!", i), + Encoding = EncodingType.Utf8 + }; - var blobResult = github.Git.Blob.Create(_context.RepositoryOwner, _context.RepositoryName, blob).Result; + var blobResult = github.Git.Blob.Create(_context.RepositoryOwner, _context.RepositoryName, blob).Result; - var newTree = new NewTree(); - newTree.Tree.Add(new NewTreeItem - { - Type = TreeType.Blob, - Mode = FileMode.File, - Path = "README.md", - Sha = blobResult.Sha - }); + var newTree = new NewTree(); + newTree.Tree.Add(new NewTreeItem + { + Type = TreeType.Blob, + Mode = FileMode.File, + Path = "README.md", + Sha = blobResult.Sha + }); - var treeResult = github.Git.Tree.Create(_context.RepositoryOwner, _context.RepositoryName, newTree).Result; - var newCommit = new NewCommit("test-commit", treeResult.Sha); - _commit = github.Git.Commit.Create(_context.RepositoryOwner, _context.RepositoryName, newCommit).Result; + var treeResult = github.Git.Tree.Create(_context.RepositoryOwner, _context.RepositoryName, newTree).Result; + var newCommit = new NewCommit("test-commit", treeResult.Sha); + var commit = github.Git.Commit.Create(_context.RepositoryOwner, _context.RepositoryName, newCommit).Result; + _commits.Add(commit); + } } [IntegrationTest] public async Task CanCreateDeployment() { - var newDeployment = new NewDeployment(_commit.Sha) { AutoMerge = false }; + var newDeployment = new NewDeployment(_commits.First().Sha) { AutoMerge = false }; var deployment = await _deploymentsClient.Create(_context.RepositoryOwner, _context.RepositoryName, newDeployment); @@ -51,9 +58,9 @@ public class DeploymentsClientTests : IDisposable } [IntegrationTest] - public async Task CanGetDeployments() + public async Task ReturnsDeployments() { - var newDeployment = new NewDeployment(_commit.Sha) { AutoMerge = false }; + var newDeployment = new NewDeployment(_commits.First().Sha) { AutoMerge = false }; await _deploymentsClient.Create(_context.RepositoryOwner, _context.RepositoryName, newDeployment); var deployments = await _deploymentsClient.GetAll(_context.RepositoryOwner, _context.RepositoryName); @@ -61,6 +68,78 @@ public class DeploymentsClientTests : IDisposable Assert.NotEmpty(deployments); } + [IntegrationTest] + public async Task ReturnsCorrectCountOfDeploymentsWithoutStart() + { + foreach (var commit in _commits) + { + var newDeployment = new NewDeployment(commit.Sha) { AutoMerge = false }; + await _deploymentsClient.Create(_context.RepositoryOwner, _context.RepositoryName, newDeployment); + } + + var options = new ApiOptions + { + PageSize = 5, + PageCount = 1 + }; + + var releases = await _deploymentsClient.GetAll(_context.RepositoryOwner, _context.RepositoryName, options); + + Assert.Equal(5, releases.Count); + } + + [IntegrationTest] + public async Task ReturnsCorrectCountOfDeploymentsWithStart() + { + foreach (var commit in _commits) + { + var newDeployment = new NewDeployment(commit.Sha) { AutoMerge = false }; + await _deploymentsClient.Create(_context.RepositoryOwner, _context.RepositoryName, newDeployment); + } + + var options = new ApiOptions + { + PageSize = 3, + PageCount = 1, + StartPage = 2 + }; + + var releases = await _deploymentsClient.GetAll(_context.RepositoryOwner, _context.RepositoryName, options); + + Assert.Equal(3, releases.Count); + } + + [IntegrationTest] + public async Task ReturnsDistinctResultsBasedOnStartPage() + { + foreach (var commit in _commits) + { + var newDeployment = new NewDeployment(commit.Sha) { AutoMerge = false }; + await _deploymentsClient.Create(_context.RepositoryOwner, _context.RepositoryName, newDeployment); + } + + var startOptions = new ApiOptions + { + PageSize = 3, + PageCount = 1 + }; + + var firstPage = await _deploymentsClient.GetAll(_context.RepositoryOwner, _context.RepositoryName, startOptions); + + var skipStartOptions = new ApiOptions + { + PageSize = 3, + PageCount = 1, + StartPage = 2 + }; + + var secondPage = await _deploymentsClient.GetAll(_context.RepositoryOwner, _context.RepositoryName, skipStartOptions); + + Assert.NotEqual(firstPage[0].Id, secondPage[0].Id); + Assert.NotEqual(firstPage[1].Id, secondPage[1].Id); + Assert.NotEqual(firstPage[2].Id, secondPage[2].Id); + } + public void Dispose() { _context.Dispose(); From 9ec431f95e3113fc7d4d73729822e64b34ff27be Mon Sep 17 00:00:00 2001 From: "aedampir@gmail.com" Date: Fri, 25 Mar 2016 23:19:01 +0700 Subject: [PATCH 10/14] Fix red test. --- Octokit.Tests/Clients/DeploymentsClientTests.cs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/Octokit.Tests/Clients/DeploymentsClientTests.cs b/Octokit.Tests/Clients/DeploymentsClientTests.cs index af4ad63b..7597989b 100644 --- a/Octokit.Tests/Clients/DeploymentsClientTests.cs +++ b/Octokit.Tests/Clients/DeploymentsClientTests.cs @@ -53,7 +53,8 @@ public class DeploymentsClientTests var expectedUrl = ApiUrls.Deployments(owner, name); client.GetAll(owner, name); - connection.Received(1).GetAll(Arg.Is(u => u == expectedUrl), Args.ApiOptions); + connection.Received(1) + .GetAll(Arg.Is(u => u == expectedUrl), Args.ApiOptions); } [Fact] @@ -70,8 +71,9 @@ public class DeploymentsClientTests StartPage = 1 }; - client.GetAll(owner, name); - connection.Received(1).GetAll(Arg.Is(u => u == expectedUrl), options); + client.GetAll(owner, name, options); + connection.Received(1) + .GetAll(Arg.Is(u => u == expectedUrl), options); } } From b54922b855de980a89b42545a8a72ed61ef4b60c Mon Sep 17 00:00:00 2001 From: "aedampir@gmail.com" Date: Fri, 25 Mar 2016 23:51:08 +0700 Subject: [PATCH 11/14] In order to test ApiOptions we should check count of recived parameters. Fixed. --- .../ObservableDeploymentsClientTests.cs | 27 ++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/Octokit.Tests/Reactive/ObservableDeploymentsClientTests.cs b/Octokit.Tests/Reactive/ObservableDeploymentsClientTests.cs index ddbe4cd7..e3d815b9 100644 --- a/Octokit.Tests/Reactive/ObservableDeploymentsClientTests.cs +++ b/Octokit.Tests/Reactive/ObservableDeploymentsClientTests.cs @@ -65,6 +65,7 @@ namespace Octokit.Tests.Reactive { var expectedUri = ApiUrls.Deployments(owner, name); + // all properties are setted => only 2 options (StartPage, PageSize) in Dictionary var options = new ApiOptions { StartPage = 1, @@ -76,7 +77,31 @@ namespace Octokit.Tests.Reactive _githubClient.Connection .Received(1) .Get>(Arg.Is(expectedUri), - Arg.Is>(dictionary => dictionary.Count == 3), Arg.Any()); + Arg.Is>(dictionary => dictionary.Count == 2), null); + + // StartPage is setted => only 1 option (StartPage) in Dictionary + options = new ApiOptions + { + StartPage = 1 + }; + + _client.GetAll(owner, name, options); + _githubClient.Connection + .Received(1) + .Get>(Arg.Is(expectedUri), + Arg.Is>(dictionary => dictionary.Count == 1), null); + + // PageCount is setted => none of options in Dictionary + options = new ApiOptions + { + PageCount = 1 + }; + + _client.GetAll(owner, name, options); + _githubClient.Connection + .Received(1) + .Get>(Arg.Is(expectedUri), + Arg.Is>(dictionary => dictionary.Count == 0), null); } } From b225b25483caf16b43944ab4d0ed8d93f76acd46 Mon Sep 17 00:00:00 2001 From: "aedampir@gmail.com" Date: Sat, 26 Mar 2016 13:02:45 +0700 Subject: [PATCH 12/14] Some useful fixes in DeploymentsClientTests. --- Octokit.Tests/Clients/DeploymentsClientTests.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Octokit.Tests/Clients/DeploymentsClientTests.cs b/Octokit.Tests/Clients/DeploymentsClientTests.cs index 7597989b..ef69431c 100644 --- a/Octokit.Tests/Clients/DeploymentsClientTests.cs +++ b/Octokit.Tests/Clients/DeploymentsClientTests.cs @@ -119,11 +119,11 @@ public class DeploymentsClientTests { var connection = Substitute.For(); var client = new DeploymentsClient(connection); - var expectedUrl = ApiUrls.Deployments("owner", "name"); + var expectedUrl = "repos/owner/name/deployments"; client.Create("owner", "name", _newDeployment); - connection.Received(1).Post(Arg.Is(u => u == expectedUrl), + connection.Received(1).Post(Arg.Is(u => u.ToString() == expectedUrl), Arg.Any()); } From b34ba547a7ab9828cee2190a568a07cfe7ba3bd6 Mon Sep 17 00:00:00 2001 From: "aedampir@gmail.com" Date: Mon, 28 Mar 2016 12:33:26 +0700 Subject: [PATCH 13/14] Small refactoring of DeploymentsClientTests. --- .../Clients/DeploymentsClientTests.cs | 53 +++++++++++++++---- 1 file changed, 42 insertions(+), 11 deletions(-) diff --git a/Octokit.Tests.Integration/Clients/DeploymentsClientTests.cs b/Octokit.Tests.Integration/Clients/DeploymentsClientTests.cs index b2ab7abe..8cc015ff 100644 --- a/Octokit.Tests.Integration/Clients/DeploymentsClientTests.cs +++ b/Octokit.Tests.Integration/Clients/DeploymentsClientTests.cs @@ -1,17 +1,16 @@ using System; using System.Collections.Generic; -using System.Linq; using System.Threading.Tasks; using Octokit; using Octokit.Tests.Integration; -using Xunit; using Octokit.Tests.Integration.Helpers; +using Xunit; public class DeploymentsClientTests : IDisposable { private readonly IDeploymentsClient _deploymentsClient; private readonly RepositoryContext _context; - private readonly ICollection _commits; + private readonly Commit _commit; public DeploymentsClientTests() { @@ -19,9 +18,36 @@ public class DeploymentsClientTests : IDisposable _deploymentsClient = github.Repository.Deployment; _context = github.CreateRepositoryContext("public-repo").Result; - _commits = new List(); - for (int i = 0; i < 6; i++) + var blob = new NewBlob + { + Content = "Hello World!", + Encoding = EncodingType.Utf8 + }; + + var blobResult = github.Git.Blob.Create(_context.RepositoryOwner, _context.RepositoryName, blob).Result; + + var newTree = new NewTree(); + newTree.Tree.Add(new NewTreeItem + { + Type = TreeType.Blob, + Mode = FileMode.File, + Path = "README.md", + Sha = blobResult.Sha + }); + + var treeResult = github.Git.Tree.Create(_context.RepositoryOwner, _context.RepositoryName, newTree).Result; + var newCommit = new NewCommit("test-commit", treeResult.Sha); + _commit = github.Git.Commit.Create(_context.RepositoryOwner, _context.RepositoryName, newCommit).Result; + } + + private IEnumerable CreateCommits(int commitCount) + { + var github = Helper.GetAuthenticatedClient(); + + var list = new List(); + + for (int i = 0; i < commitCount; i++) { var blob = new NewBlob { @@ -43,14 +69,16 @@ public class DeploymentsClientTests : IDisposable var treeResult = github.Git.Tree.Create(_context.RepositoryOwner, _context.RepositoryName, newTree).Result; var newCommit = new NewCommit("test-commit", treeResult.Sha); var commit = github.Git.Commit.Create(_context.RepositoryOwner, _context.RepositoryName, newCommit).Result; - _commits.Add(commit); + list.Add(commit); } + + return list; } [IntegrationTest] public async Task CanCreateDeployment() { - var newDeployment = new NewDeployment(_commits.First().Sha) { AutoMerge = false }; + var newDeployment = new NewDeployment(_commit.Sha) { AutoMerge = false }; var deployment = await _deploymentsClient.Create(_context.RepositoryOwner, _context.RepositoryName, newDeployment); @@ -60,7 +88,7 @@ public class DeploymentsClientTests : IDisposable [IntegrationTest] public async Task ReturnsDeployments() { - var newDeployment = new NewDeployment(_commits.First().Sha) { AutoMerge = false }; + var newDeployment = new NewDeployment(_commit.Sha) { AutoMerge = false }; await _deploymentsClient.Create(_context.RepositoryOwner, _context.RepositoryName, newDeployment); var deployments = await _deploymentsClient.GetAll(_context.RepositoryOwner, _context.RepositoryName); @@ -71,7 +99,8 @@ public class DeploymentsClientTests : IDisposable [IntegrationTest] public async Task ReturnsCorrectCountOfDeploymentsWithoutStart() { - foreach (var commit in _commits) + var commits = CreateCommits(6); + foreach (var commit in commits) { var newDeployment = new NewDeployment(commit.Sha) { AutoMerge = false }; await _deploymentsClient.Create(_context.RepositoryOwner, _context.RepositoryName, newDeployment); @@ -91,7 +120,8 @@ public class DeploymentsClientTests : IDisposable [IntegrationTest] public async Task ReturnsCorrectCountOfDeploymentsWithStart() { - foreach (var commit in _commits) + var commits = CreateCommits(6); + foreach (var commit in commits) { var newDeployment = new NewDeployment(commit.Sha) { AutoMerge = false }; await _deploymentsClient.Create(_context.RepositoryOwner, _context.RepositoryName, newDeployment); @@ -112,7 +142,8 @@ public class DeploymentsClientTests : IDisposable [IntegrationTest] public async Task ReturnsDistinctResultsBasedOnStartPage() { - foreach (var commit in _commits) + var commits = CreateCommits(6); + foreach (var commit in commits) { var newDeployment = new NewDeployment(commit.Sha) { AutoMerge = false }; await _deploymentsClient.Create(_context.RepositoryOwner, _context.RepositoryName, newDeployment); From d1376c8f02b8ac61d165ea4f73adc4c3a7128f6d Mon Sep 17 00:00:00 2001 From: "aedampir@gmail.com" Date: Wed, 30 Mar 2016 12:04:50 +0700 Subject: [PATCH 14/14] All remarks were fixed --- .../Clients/ObservableDeploymentsClient.cs | 3 ++ .../Clients/DeploymentsClientTests.cs | 28 +++++----- .../ObservableDeploymentsClientTests.cs | 51 ++++++++++--------- Octokit/Clients/DeploymentsClient.cs | 5 +- 4 files changed, 47 insertions(+), 40 deletions(-) diff --git a/Octokit.Reactive/Clients/ObservableDeploymentsClient.cs b/Octokit.Reactive/Clients/ObservableDeploymentsClient.cs index 3d382af5..9a32ace1 100644 --- a/Octokit.Reactive/Clients/ObservableDeploymentsClient.cs +++ b/Octokit.Reactive/Clients/ObservableDeploymentsClient.cs @@ -31,6 +31,9 @@ namespace Octokit.Reactive.Clients /// All the s for the specified repository. public IObservable GetAll(string owner, string name) { + Ensure.ArgumentNotNullOrEmptyString(owner, "owner"); + Ensure.ArgumentNotNullOrEmptyString(name, "name"); + return GetAll(owner, name, ApiOptions.None); } diff --git a/Octokit.Tests/Clients/DeploymentsClientTests.cs b/Octokit.Tests/Clients/DeploymentsClientTests.cs index ef69431c..804835cc 100644 --- a/Octokit.Tests/Clients/DeploymentsClientTests.cs +++ b/Octokit.Tests/Clients/DeploymentsClientTests.cs @@ -50,11 +50,11 @@ public class DeploymentsClientTests { var connection = Substitute.For(); var client = new DeploymentsClient(connection); - var expectedUrl = ApiUrls.Deployments(owner, name); + var expectedUrl = string.Format("repos/{0}/{1}/deployments", owner, name); client.GetAll(owner, name); connection.Received(1) - .GetAll(Arg.Is(u => u == expectedUrl), Args.ApiOptions); + .GetAll(Arg.Is(u => u.ToString() == expectedUrl), Args.ApiOptions); } [Fact] @@ -62,7 +62,7 @@ public class DeploymentsClientTests { var connection = Substitute.For(); var client = new DeploymentsClient(connection); - var expectedUrl = ApiUrls.Deployments(owner, name); + var expectedUrl = string.Format("repos/{0}/{1}/deployments", owner, name); var options = new ApiOptions { @@ -73,21 +73,21 @@ public class DeploymentsClientTests client.GetAll(owner, name, options); connection.Received(1) - .GetAll(Arg.Is(u => u == expectedUrl), options); + .GetAll(Arg.Is(u => u.ToString() == expectedUrl), options); } } public class TheCreateMethod { - private readonly NewDeployment _newDeployment = new NewDeployment("aRef"); + private readonly NewDeployment newDeployment = new NewDeployment("aRef"); [Fact] public async Task EnsuresNonNullArguments() { var client = new DeploymentsClient(Substitute.For()); - await Assert.ThrowsAsync(() => client.Create(null, "name", _newDeployment)); - await Assert.ThrowsAsync(() => client.Create("owner", null, _newDeployment)); + await Assert.ThrowsAsync(() => client.Create(null, "name", newDeployment)); + await Assert.ThrowsAsync(() => client.Create("owner", null, newDeployment)); await Assert.ThrowsAsync(() => client.Create("owner", "name", null)); } @@ -96,8 +96,8 @@ public class DeploymentsClientTests { var client = new DeploymentsClient(Substitute.For()); - await Assert.ThrowsAsync(() => client.Create("", "name", _newDeployment)); - await Assert.ThrowsAsync(() => client.Create("owner", "", _newDeployment)); + await Assert.ThrowsAsync(() => client.Create("", "name", newDeployment)); + await Assert.ThrowsAsync(() => client.Create("owner", "", newDeployment)); } [Theory] @@ -110,8 +110,8 @@ public class DeploymentsClientTests { var client = new DeploymentsClient(Substitute.For()); - await Assert.ThrowsAsync(() => client.Create(whitespace, "name", _newDeployment)); - await Assert.ThrowsAsync(() => client.Create("owner", whitespace, _newDeployment)); + await Assert.ThrowsAsync(() => client.Create(whitespace, "name", newDeployment)); + await Assert.ThrowsAsync(() => client.Create("owner", whitespace, newDeployment)); } [Fact] @@ -121,7 +121,7 @@ public class DeploymentsClientTests var client = new DeploymentsClient(connection); var expectedUrl = "repos/owner/name/deployments"; - client.Create("owner", "name", _newDeployment); + client.Create("owner", "name", newDeployment); connection.Received(1).Post(Arg.Is(u => u.ToString() == expectedUrl), Arg.Any()); @@ -133,10 +133,10 @@ public class DeploymentsClientTests var connection = Substitute.For(); var client = new DeploymentsClient(connection); - client.Create("owner", "name", _newDeployment); + client.Create("owner", "name", newDeployment); connection.Received(1).Post(Arg.Any(), - _newDeployment); + newDeployment); } } diff --git a/Octokit.Tests/Reactive/ObservableDeploymentsClientTests.cs b/Octokit.Tests/Reactive/ObservableDeploymentsClientTests.cs index e3d815b9..477d1d78 100644 --- a/Octokit.Tests/Reactive/ObservableDeploymentsClientTests.cs +++ b/Octokit.Tests/Reactive/ObservableDeploymentsClientTests.cs @@ -51,21 +51,21 @@ namespace Octokit.Tests.Reactive [Fact] public void RequestsCorrectUrl() { - var expectedUri = ApiUrls.Deployments(owner, name); + var expectedUrl = string.Format("repos/{0}/{1}/deployments", owner, name); _client.GetAll(owner, name); - _githubClient.Connection - .Received(1) - .Get>(Arg.Is(expectedUri), - Arg.Is>(dictionary => dictionary.Count == 0), Arg.Any()); + _githubClient.Connection.Received(1) + .Get>(Arg.Is(u => u.ToString() == expectedUrl), + Arg.Is>(dictionary => dictionary.Count == 0), + Arg.Any()); } [Fact] public void RequestsCorrectUrlWithApiOptions() { - var expectedUri = ApiUrls.Deployments(owner, name); - - // all properties are setted => only 2 options (StartPage, PageSize) in Dictionary + var expectedUrl = string.Format("repos/{0}/{1}/deployments", owner, name); + + // all properties are setted => only 2 options (StartPage, PageSize) in dictionary var options = new ApiOptions { StartPage = 1, @@ -74,34 +74,34 @@ namespace Octokit.Tests.Reactive }; _client.GetAll(owner, name, options); - _githubClient.Connection - .Received(1) - .Get>(Arg.Is(expectedUri), - Arg.Is>(dictionary => dictionary.Count == 2), null); + _githubClient.Connection.Received(1) + .Get>(Arg.Is(u => u.ToString() == expectedUrl), + Arg.Is>(dictionary => dictionary.Count == 2), + null); - // StartPage is setted => only 1 option (StartPage) in Dictionary + // StartPage is setted => only 1 option (StartPage) in dictionary options = new ApiOptions { StartPage = 1 }; _client.GetAll(owner, name, options); - _githubClient.Connection - .Received(1) - .Get>(Arg.Is(expectedUri), - Arg.Is>(dictionary => dictionary.Count == 1), null); + _githubClient.Connection.Received(1) + .Get>(Arg.Is(u => u.ToString() == expectedUrl), + Arg.Is>(dictionary => dictionary.Count == 1), + null); - // PageCount is setted => none of options in Dictionary + // PageCount is setted => none of options in dictionary options = new ApiOptions { PageCount = 1 }; _client.GetAll(owner, name, options); - _githubClient.Connection - .Received(1) - .Get>(Arg.Is(expectedUri), - Arg.Is>(dictionary => dictionary.Count == 0), null); + _githubClient.Connection.Received(1) + .Get>(Arg.Is(u => u.ToString() == expectedUrl), + Arg.Is>(dictionary => dictionary.Count == 0), + null); } } @@ -164,9 +164,10 @@ namespace Octokit.Tests.Reactive var newDeployment = new NewDeployment("ref"); _client.Create("owner", "repo", newDeployment); + _githubClient.Repository.Deployment.Received(1).Create(Arg.Is("owner"), - Arg.Is("repo"), - Arg.Is(newDeployment)); + Arg.Is("repo"), + Arg.Is(newDeployment)); } } @@ -179,4 +180,4 @@ namespace Octokit.Tests.Reactive } } } -} \ No newline at end of file +} diff --git a/Octokit/Clients/DeploymentsClient.cs b/Octokit/Clients/DeploymentsClient.cs index 4f63e510..db42b424 100644 --- a/Octokit/Clients/DeploymentsClient.cs +++ b/Octokit/Clients/DeploymentsClient.cs @@ -34,6 +34,9 @@ namespace Octokit /// All the s for the specified repository. public Task> GetAll(string owner, string name) { + Ensure.ArgumentNotNullOrEmptyString(owner, "owner"); + Ensure.ArgumentNotNullOrEmptyString(name, "name"); + return GetAll(owner, name, ApiOptions.None); } @@ -50,7 +53,7 @@ namespace Octokit /// All the s for the specified repository. public Task> GetAll(string owner, string name, ApiOptions options) { - Ensure.ArgumentNotNullOrEmptyString(owner, "login"); + Ensure.ArgumentNotNullOrEmptyString(owner, "owner"); Ensure.ArgumentNotNullOrEmptyString(name, "name"); Ensure.ArgumentNotNull(options, "options");