From b157c452a24cae8cd08bedaff79cc64c54647a64 Mon Sep 17 00:00:00 2001 From: "aedampir@gmail.com" Date: Tue, 15 Mar 2016 17:10:08 +0700 Subject: [PATCH 01/23] Class ApiOptionsSingleton was implemented in order to support proper unit tests. Now ApiOptionsSingleton.Instance is used as ApiOptions.None static member instead of creation of new ApiOptions class each time when ApiOptions.None is invoked. Singleton pattern will allow write proper tests for methods that use ApiOptions.None as their parameter For instance, now "Arg.Is(options => options == ApiOptions.None)" construction can be used. Also, usage of sigleton has some posititve performance impact. --- Octokit/Models/Request/ApiOptions.cs | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/Octokit/Models/Request/ApiOptions.cs b/Octokit/Models/Request/ApiOptions.cs index ef149633c9..764dd45fba 100644 --- a/Octokit/Models/Request/ApiOptions.cs +++ b/Octokit/Models/Request/ApiOptions.cs @@ -7,9 +7,32 @@ namespace Octokit [DebuggerDisplay("{DebuggerDisplay,nq}")] public class ApiOptions { + private class ApiOptionsSingleton + { + private static readonly ApiOptions _instance = new ApiOptions(); + + // Explicit static constructor to tell C# compiler + // not to mark type as beforefieldinit + static ApiOptionsSingleton() + { + } + + private ApiOptionsSingleton() + { + } + + public static ApiOptions Instance + { + get + { + return _instance; + } + } + } + public static ApiOptions None { - get { return new ApiOptions(); } + get { return ApiOptionsSingleton.Instance; } } /// From ec5de110ad201b7599a445c4141c404819ff5bad Mon Sep 17 00:00:00 2001 From: "aedampir@gmail.com" Date: Tue, 15 Mar 2016 17:15:04 +0700 Subject: [PATCH 02/23] The overloaded version of needed methods for IUserEmailsClient interface was added and impelemented. Base methods: Task> GetAll() Overload methods: Task> GetAll(ApiOptions options); --- Octokit/Clients/IUserEmailsClient.cs | 11 +++++++++++ Octokit/Clients/UserEmailsClient.cs | 16 +++++++++++++++- 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/Octokit/Clients/IUserEmailsClient.cs b/Octokit/Clients/IUserEmailsClient.cs index b653d218da..4fb9fb1232 100644 --- a/Octokit/Clients/IUserEmailsClient.cs +++ b/Octokit/Clients/IUserEmailsClient.cs @@ -22,6 +22,17 @@ public interface IUserEmailsClient [SuppressMessage("Microsoft.Design", "CA1024:UsePropertiesWhereAppropriate")] Task> GetAll(); + /// + /// Gets all email addresses for the authenticated user. + /// + /// + /// http://developer.github.com/v3/users/emails/#list-email-addresses-for-a-user + /// + /// Options for changing the API response + /// The es for the authenticated user. + [SuppressMessage("Microsoft.Design", "CA1024:UsePropertiesWhereAppropriate")] + Task> GetAll(ApiOptions options); + /// /// Adds email addresses for the authenticated user. /// diff --git a/Octokit/Clients/UserEmailsClient.cs b/Octokit/Clients/UserEmailsClient.cs index d503e85370..fc638cd060 100644 --- a/Octokit/Clients/UserEmailsClient.cs +++ b/Octokit/Clients/UserEmailsClient.cs @@ -30,7 +30,21 @@ public UserEmailsClient(IApiConnection apiConnection) /// The es for the authenticated user. public Task> GetAll() { - return ApiConnection.GetAll(ApiUrls.Emails()); + return GetAll(ApiOptions.None); + } + + /// + /// Gets all email addresses for the authenticated user. + /// + /// + /// http://developer.github.com/v3/users/emails/#list-email-addresses-for-a-user + /// + /// The es for the authenticated user. + public Task> GetAll(ApiOptions options) + { + Ensure.ArgumentNotNull(options, "options"); + + return ApiConnection.GetAll(ApiUrls.Emails(), options); } /// From b920bcdfcec241705505710666eaec5fafabde02 Mon Sep 17 00:00:00 2001 From: "aedampir@gmail.com" Date: Tue, 15 Mar 2016 17:17:33 +0700 Subject: [PATCH 03/23] A bunch of unit and integration tests for overloaded version of needed methods for IUserEmailsClient interface were added and impelemented. Base methods: Task> GetAll() Overload methods: Task> GetAll(ApiOptions options); --- .../Clients/UserEmailsClientTests.cs | 8 +++++++ .../Clients/UserEmailsClientTests.cs | 23 +++++++++++++++++-- 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/Octokit.Tests.Integration/Clients/UserEmailsClientTests.cs b/Octokit.Tests.Integration/Clients/UserEmailsClientTests.cs index f0af752c69..9b54fb7642 100644 --- a/Octokit.Tests.Integration/Clients/UserEmailsClientTests.cs +++ b/Octokit.Tests.Integration/Clients/UserEmailsClientTests.cs @@ -15,6 +15,14 @@ public async Task CanGetEmail() Assert.NotEmpty(emails); } + [IntegrationTest] + public async Task CanGetEmailWithApiOptions() + { + var github = Helper.GetAuthenticatedClient(); + var emails = await github.User.Email.GetAll(ApiOptions.None); + Assert.NotEmpty(emails); + } + const string testEmailAddress = "hahaha-not-a-real-email@foo.com"; [IntegrationTest(Skip = "this isn't passing in CI - i hate past me right now")] diff --git a/Octokit.Tests/Clients/UserEmailsClientTests.cs b/Octokit.Tests/Clients/UserEmailsClientTests.cs index b356479600..d2642c124c 100644 --- a/Octokit.Tests/Clients/UserEmailsClientTests.cs +++ b/Octokit.Tests/Clients/UserEmailsClientTests.cs @@ -8,7 +8,7 @@ namespace Octokit.Tests.Clients { public class UserEmailsClientTests { - public class TheGetAllMethod + public class TheGetAllMethods { [Fact] public void GetsCorrectUrl() @@ -19,7 +19,26 @@ public void GetsCorrectUrl() client.GetAll(); connection.Received(1) - .GetAll(Arg.Is(u => u.ToString() == "user/emails")); + .GetAll(Arg.Is(u => u.ToString() == "user/emails"), + Arg.Is(ApiOptions.None)); + } + + [Fact] + public void GetsCorrectUrlWithApiOptions() + { + var options = new ApiOptions + { + StartPage = 1, + PageCount = 1 + }; + + var connection = Substitute.For(); + var client = new UserEmailsClient(connection); + + client.GetAll(options); + + connection.Received(1) + .GetAll(Arg.Is(u => u.ToString() == "user/emails"), Arg.Is(apiOptions => apiOptions == options)); } } From 9d4f0077b9b7979d1e2a21cf37b0469514e84582 Mon Sep 17 00:00:00 2001 From: "aedampir@gmail.com" Date: Tue, 15 Mar 2016 17:40:22 +0700 Subject: [PATCH 04/23] ApiOptionsSingleton implementation was changed in order to pass FoxCop analysis. --- Octokit/Models/Request/ApiOptions.cs | 21 ++++++--------------- 1 file changed, 6 insertions(+), 15 deletions(-) diff --git a/Octokit/Models/Request/ApiOptions.cs b/Octokit/Models/Request/ApiOptions.cs index 764dd45fba..e7ed1c4095 100644 --- a/Octokit/Models/Request/ApiOptions.cs +++ b/Octokit/Models/Request/ApiOptions.cs @@ -7,29 +7,21 @@ namespace Octokit [DebuggerDisplay("{DebuggerDisplay,nq}")] public class ApiOptions { - private class ApiOptionsSingleton + private sealed class ApiOptionsSingleton { - private static readonly ApiOptions _instance = new ApiOptions(); + private static readonly Lazy _lazy = + new Lazy(() => new ApiOptions()); - // Explicit static constructor to tell C# compiler - // not to mark type as beforefieldinit - static ApiOptionsSingleton() + public static ApiOptions Instance { + get { return _lazy.Value; } } private ApiOptionsSingleton() { } - - public static ApiOptions Instance - { - get - { - return _instance; - } - } } - + public static ApiOptions None { get { return ApiOptionsSingleton.Instance; } @@ -81,5 +73,4 @@ internal string DebuggerDisplay } } } - } From bc3dc18045db183fbd45cec4518a4257c02193a2 Mon Sep 17 00:00:00 2001 From: "aedampir@gmail.com" Date: Tue, 15 Mar 2016 18:56:06 +0700 Subject: [PATCH 05/23] Add new overloaded extension method Task> GetResponse(this IConnection connection, Uri uri, Dictionary parameters) to work with IConnection. --- Octokit/Helpers/ApiExtensions.cs | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/Octokit/Helpers/ApiExtensions.cs b/Octokit/Helpers/ApiExtensions.cs index 81b1ab8e6f..d25027e058 100644 --- a/Octokit/Helpers/ApiExtensions.cs +++ b/Octokit/Helpers/ApiExtensions.cs @@ -74,6 +74,23 @@ public static Task> GetResponse(this IConnection connection, return connection.Get(uri, null, null); } + /// + /// Gets the API resource at the specified URI. + /// + /// Type of the API resource to get. + /// The connection to use + /// URI of the API resource to get + /// Querystring parameters for the request + /// The API resource. + /// Thrown when an API error occurs. + public static Task> GetResponse(this IConnection connection, Uri uri, Dictionary parameters) + { + Ensure.ArgumentNotNull(connection, "connection"); + Ensure.ArgumentNotNull(uri, "uri"); + + return connection.Get(uri, parameters, null); + } + [Obsolete("Octokit's HTTP library now follows redirects by default - this API will be removed in a future release")] public static Task> GetRedirect(this IConnection connection, Uri uri) { From 7a01f22ee9ba539e0c9ca5133400358df3f45453 Mon Sep 17 00:00:00 2001 From: "aedampir@gmail.com" Date: Tue, 15 Mar 2016 18:58:10 +0700 Subject: [PATCH 06/23] New overloaded method IObservable GetAll(ApiOptions options) has been added and implemented on IObservableUserEmailsClient/ObservableUserEmailsClient. --- .../Clients/IObservableUserEmailsClient.cs | 11 +++++++++++ .../Clients/ObservableUserEmailsClient.cs | 17 ++++++++++++++++- 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/Octokit.Reactive/Clients/IObservableUserEmailsClient.cs b/Octokit.Reactive/Clients/IObservableUserEmailsClient.cs index 5198ef1ad0..dd1c99c19a 100644 --- a/Octokit.Reactive/Clients/IObservableUserEmailsClient.cs +++ b/Octokit.Reactive/Clients/IObservableUserEmailsClient.cs @@ -22,6 +22,17 @@ public interface IObservableUserEmailsClient [SuppressMessage("Microsoft.Design", "CA1024:UsePropertiesWhereAppropriate")] IObservable GetAll(); + /// + /// Gets all email addresses for the authenticated user. + /// + /// + /// http://developer.github.com/v3/users/emails/#list-email-addresses-for-a-user + /// + /// Options for changing the API response + /// The es for the authenticated user. + [SuppressMessage("Microsoft.Design", "CA1024:UsePropertiesWhereAppropriate")] + IObservable GetAll(ApiOptions options); + /// /// Adds email addresses for the authenticated user. /// diff --git a/Octokit.Reactive/Clients/ObservableUserEmailsClient.cs b/Octokit.Reactive/Clients/ObservableUserEmailsClient.cs index 2a2d10712f..96076c307e 100644 --- a/Octokit.Reactive/Clients/ObservableUserEmailsClient.cs +++ b/Octokit.Reactive/Clients/ObservableUserEmailsClient.cs @@ -34,7 +34,22 @@ public ObservableUserEmailsClient(IGitHubClient client) /// The es for the authenticated user. public IObservable GetAll() { - return _connection.GetAndFlattenAllPages(ApiUrls.Emails()); + return GetAll(ApiOptions.None); + } + + /// + /// Gets all email addresses for the authenticated user. + /// + /// + /// http://developer.github.com/v3/users/emails/#list-email-addresses-for-a-user + /// + /// Options for changing the API response + /// The es for the authenticated user. + public IObservable GetAll(ApiOptions options) + { + Ensure.ArgumentNotNull(options, "options"); + + return _connection.GetAndFlattenAllPages(ApiUrls.Emails(), options); } /// From dd969f24368f0eab5ecc0254015a62409063e52d Mon Sep 17 00:00:00 2001 From: "aedampir@gmail.com" Date: Tue, 15 Mar 2016 18:58:57 +0700 Subject: [PATCH 07/23] New unit and integration tests were added in order to test IObservable GetAll(ApiOptions options) method. --- .../ObservableUserEmailsClientTests.cs | 11 ++++++ .../ObservableUserEmailsClientTests.cs | 34 ++++++++++++++----- 2 files changed, 36 insertions(+), 9 deletions(-) diff --git a/Octokit.Tests.Integration/Reactive/ObservableUserEmailsClientTests.cs b/Octokit.Tests.Integration/Reactive/ObservableUserEmailsClientTests.cs index f3cd8b8147..a1fa327f62 100644 --- a/Octokit.Tests.Integration/Reactive/ObservableUserEmailsClientTests.cs +++ b/Octokit.Tests.Integration/Reactive/ObservableUserEmailsClientTests.cs @@ -17,5 +17,16 @@ public async Task CanGetEmail() var email = await client.GetAll(); Assert.NotNull(email); } + + [IntegrationTest] + public async Task CanGetEmailWithApiOptions() + { + var github = Helper.GetAuthenticatedClient(); + + var client = new ObservableUserEmailsClient(github); + + var email = await client.GetAll(ApiOptions.None); + Assert.NotNull(email); + } } } diff --git a/Octokit.Tests/Reactive/ObservableUserEmailsClientTests.cs b/Octokit.Tests/Reactive/ObservableUserEmailsClientTests.cs index 1e62a86b10..22a03e5704 100644 --- a/Octokit.Tests/Reactive/ObservableUserEmailsClientTests.cs +++ b/Octokit.Tests/Reactive/ObservableUserEmailsClientTests.cs @@ -1,7 +1,7 @@ -using NSubstitute; -using Octokit.Reactive; -using System; +using System; using System.Collections.Generic; +using NSubstitute; +using Octokit.Reactive; using Xunit; namespace Octokit.Tests @@ -18,25 +18,41 @@ private static ObservableUserEmailsClient CreateFixtureWithNonReactiveClient() public class TheGetAllMethod { + private static readonly Uri _expectedUri = new Uri("user/emails", UriKind.Relative); + [Fact] public void GetsCorrectUrl() { - var expectedUri = new Uri("user/emails", UriKind.Relative); var github = Substitute.For(); var client = new ObservableUserEmailsClient(github); client.GetAll(); - github.Connection.Received(1).GetResponse>(expectedUri); + github.Connection.Received(1).GetResponse>(_expectedUri, + Arg.Is>(dictionary => dictionary.Count == 0)); + } + + [Fact] + public void GetsCorrectUrlWithApiOption() + { + var github = Substitute.For(); + var client = new ObservableUserEmailsClient(github); + + var options = new ApiOptions + { + StartPage = 1, + PageCount = 1 + }; + + client.GetAll(options); + + github.Connection.Received(1).GetResponse>(_expectedUri, + Arg.Is>(dictionary => dictionary.Count == 1)); } } public class TheAddMethod { - public IGitHubClient GitHubClient; - - public ObservableUserEmailsClient Client; - [Fact] public void CallsAddOnClient() { From 531792ebe2e991fb2fc52c460701a6d5cf3e85a0 Mon Sep 17 00:00:00 2001 From: "aedampir@gmail.com" Date: Tue, 15 Mar 2016 19:02:11 +0700 Subject: [PATCH 08/23] Some minor changes in logic of GetsCorrectUrlWithApiOptions test. Use ApiOptions.None instead of setted up class. --- Octokit.Tests/Clients/UserEmailsClientTests.cs | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/Octokit.Tests/Clients/UserEmailsClientTests.cs b/Octokit.Tests/Clients/UserEmailsClientTests.cs index d2642c124c..5f3f6b24ba 100644 --- a/Octokit.Tests/Clients/UserEmailsClientTests.cs +++ b/Octokit.Tests/Clients/UserEmailsClientTests.cs @@ -26,19 +26,13 @@ public void GetsCorrectUrl() [Fact] public void GetsCorrectUrlWithApiOptions() { - var options = new ApiOptions - { - StartPage = 1, - PageCount = 1 - }; - var connection = Substitute.For(); var client = new UserEmailsClient(connection); - client.GetAll(options); + client.GetAll(ApiOptions.None); connection.Received(1) - .GetAll(Arg.Is(u => u.ToString() == "user/emails"), Arg.Is(apiOptions => apiOptions == options)); + .GetAll(Arg.Is(u => u.ToString() == "user/emails"), Arg.Is(apiOptions => apiOptions == ApiOptions.None)); } } From 9fd3c9b730fffe4281fdcc027ba1d02707a6f3e7 Mon Sep 17 00:00:00 2001 From: "aedampir@gmail.com" Date: Tue, 15 Mar 2016 19:05:03 +0700 Subject: [PATCH 09/23] Some minor changes in ObservableUserEmailsClientTests.GetsCorrectUrlWithApiOption. Now ApiOptions.None is used instead of pre-configured class. --- .../Reactive/ObservableUserEmailsClientTests.cs | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/Octokit.Tests/Reactive/ObservableUserEmailsClientTests.cs b/Octokit.Tests/Reactive/ObservableUserEmailsClientTests.cs index 22a03e5704..267f3029de 100644 --- a/Octokit.Tests/Reactive/ObservableUserEmailsClientTests.cs +++ b/Octokit.Tests/Reactive/ObservableUserEmailsClientTests.cs @@ -38,16 +38,10 @@ public void GetsCorrectUrlWithApiOption() var github = Substitute.For(); var client = new ObservableUserEmailsClient(github); - var options = new ApiOptions - { - StartPage = 1, - PageCount = 1 - }; - - client.GetAll(options); + client.GetAll(ApiOptions.None); github.Connection.Received(1).GetResponse>(_expectedUri, - Arg.Is>(dictionary => dictionary.Count == 1)); + Arg.Is>(dictionary => dictionary.Count == 0)); } } From f16b20d28d1cc5479f6e0d8192794d8f60a10f4d Mon Sep 17 00:00:00 2001 From: "aedampir@gmail.com" Date: Tue, 15 Mar 2016 19:32:42 +0700 Subject: [PATCH 10/23] Some new integration tests were added in order to test Pagination Support on IUserEmailsClient and IObservableUserEmailsClient. --- .../Clients/UserEmailsClientTests.cs | 60 ++++++++++++++++++ .../ObservableUserEmailsClientTests.cs | 63 ++++++++++++++++--- 2 files changed, 116 insertions(+), 7 deletions(-) diff --git a/Octokit.Tests.Integration/Clients/UserEmailsClientTests.cs b/Octokit.Tests.Integration/Clients/UserEmailsClientTests.cs index 9b54fb7642..0a1c5888f8 100644 --- a/Octokit.Tests.Integration/Clients/UserEmailsClientTests.cs +++ b/Octokit.Tests.Integration/Clients/UserEmailsClientTests.cs @@ -6,6 +6,14 @@ namespace Octokit.Tests.Integration.Clients { public class UserEmailsClientTests { + private readonly IUserEmailsClient _emailClient; + + public UserEmailsClientTests() + { + var github = Helper.GetAuthenticatedClient(); + _emailClient = github.User.Email; + } + [IntegrationTest] public async Task CanGetEmail() { @@ -23,6 +31,58 @@ public async Task CanGetEmailWithApiOptions() Assert.NotEmpty(emails); } + [IntegrationTest] + public async Task ReturnsCorrectCountOfEmailsWithoutStart() + { + var options = new ApiOptions + { + PageSize = 5, + PageCount = 1 + }; + + var emails = await _emailClient.GetAll(options); + + Assert.Equal(1, emails.Count); + } + + [IntegrationTest] + public async Task ReturnsCorrectCountOfEmailsWithStart() + { + var options = new ApiOptions + { + PageSize = 5, + PageCount = 1, + StartPage = 2 + }; + + var emails = await _emailClient.GetAll(options); + + Assert.Equal(0, emails.Count); + } + + //[IntegrationTest] + //public async Task ReturnsDistinctResultsBasedOnStartPage() + //{ + // var startOptions = new ApiOptions + // { + // PageSize = 5, + // PageCount = 1 + // }; + + // var firstPage = await _emailClient.GetAll(startOptions); + + // var skipStartOptions = new ApiOptions + // { + // PageSize = 5, + // PageCount = 1, + // StartPage = 2 + // }; + + // var secondPage = await _emailClient.GetAll(skipStartOptions); + + // Assert.Equal(firstPage[0].Email, secondPage[0].Email); + //} + const string testEmailAddress = "hahaha-not-a-real-email@foo.com"; [IntegrationTest(Skip = "this isn't passing in CI - i hate past me right now")] diff --git a/Octokit.Tests.Integration/Reactive/ObservableUserEmailsClientTests.cs b/Octokit.Tests.Integration/Reactive/ObservableUserEmailsClientTests.cs index a1fa327f62..66811c5241 100644 --- a/Octokit.Tests.Integration/Reactive/ObservableUserEmailsClientTests.cs +++ b/Octokit.Tests.Integration/Reactive/ObservableUserEmailsClientTests.cs @@ -7,12 +7,13 @@ namespace Octokit.Tests.Integration { public class ObservableUserEmailsClientTests { + private readonly ObservableUserEmailsClient _emailClient + = new ObservableUserEmailsClient(Helper.GetAuthenticatedClient()); + [IntegrationTest] public async Task CanGetEmail() { - var github = Helper.GetAuthenticatedClient(); - - var client = new ObservableUserEmailsClient(github); + var client = new ObservableUserEmailsClient(Helper.GetAuthenticatedClient()); var email = await client.GetAll(); Assert.NotNull(email); @@ -21,12 +22,60 @@ public async Task CanGetEmail() [IntegrationTest] public async Task CanGetEmailWithApiOptions() { - var github = Helper.GetAuthenticatedClient(); + var email = await _emailClient.GetAll(ApiOptions.None); + Assert.NotNull(email); + } + + [IntegrationTest] + public async Task ReturnsCorrectCountOfEmailsWithoutStart() + { + var options = new ApiOptions + { + PageSize = 5, + PageCount = 1 + }; - var client = new ObservableUserEmailsClient(github); + var emails = await _emailClient.GetAll(options).ToList(); - var email = await client.GetAll(ApiOptions.None); - Assert.NotNull(email); + Assert.Equal(1, emails.Count); } + + [IntegrationTest] + public async Task ReturnsCorrectCountOfEmailsWithStart() + { + var options = new ApiOptions + { + PageSize = 5, + PageCount = 1, + StartPage = 2 + }; + + var emails = await _emailClient.GetAll(options).ToList(); + + Assert.Equal(0, emails.Count); + } + + //[IntegrationTest] + //public async Task ReturnsDistinctResultsBasedOnStartPage() + //{ + // var startOptions = new ApiOptions + // { + // PageSize = 5, + // PageCount = 1 + // }; + + // var firstPage = await _emailClient.GetAll(startOptions); + + // var skipStartOptions = new ApiOptions + // { + // PageSize = 5, + // PageCount = 1, + // StartPage = 2 + // }; + + // var secondPage = await _emailClient.GetAll(skipStartOptions); + + // Assert.Equal(firstPage[0].Email, secondPage[0].Email); + //} } } From ad3b7823be91954e520201094a6514668182605d Mon Sep 17 00:00:00 2001 From: "aedampir@gmail.com" Date: Tue, 15 Mar 2016 19:42:12 +0700 Subject: [PATCH 11/23] Replace usage Arg.Is with lambda to Arg.Any. --- Octokit.Tests/Clients/UserEmailsClientTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Octokit.Tests/Clients/UserEmailsClientTests.cs b/Octokit.Tests/Clients/UserEmailsClientTests.cs index 5f3f6b24ba..dc38ba1ba0 100644 --- a/Octokit.Tests/Clients/UserEmailsClientTests.cs +++ b/Octokit.Tests/Clients/UserEmailsClientTests.cs @@ -32,7 +32,7 @@ public void GetsCorrectUrlWithApiOptions() client.GetAll(ApiOptions.None); connection.Received(1) - .GetAll(Arg.Is(u => u.ToString() == "user/emails"), Arg.Is(apiOptions => apiOptions == ApiOptions.None)); + .GetAll(Arg.Is(u => u.ToString() == "user/emails"), Arg.Any()); } } From 6760aca2b7a8ddee256269a855654d3322bca029 Mon Sep 17 00:00:00 2001 From: "aedampir@gmail.com" Date: Tue, 15 Mar 2016 23:56:49 +0700 Subject: [PATCH 12/23] Cosmetic changes UserEmailsClientTests. Changed Arg.Any to Args.ApiOptions. --- Octokit.Tests/Clients/UserEmailsClientTests.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Octokit.Tests/Clients/UserEmailsClientTests.cs b/Octokit.Tests/Clients/UserEmailsClientTests.cs index dc38ba1ba0..462852cc11 100644 --- a/Octokit.Tests/Clients/UserEmailsClientTests.cs +++ b/Octokit.Tests/Clients/UserEmailsClientTests.cs @@ -1,7 +1,7 @@ -using NSubstitute; -using System; +using System; using System.Collections.Generic; using System.Threading.Tasks; +using NSubstitute; using Xunit; namespace Octokit.Tests.Clients @@ -19,8 +19,8 @@ public void GetsCorrectUrl() client.GetAll(); connection.Received(1) - .GetAll(Arg.Is(u => u.ToString() == "user/emails"), - Arg.Is(ApiOptions.None)); + .GetAll(Arg.Is(u => u.ToString() == "user/emails"), + Arg.Is(ApiOptions.None)); } [Fact] @@ -32,7 +32,7 @@ public void GetsCorrectUrlWithApiOptions() client.GetAll(ApiOptions.None); connection.Received(1) - .GetAll(Arg.Is(u => u.ToString() == "user/emails"), Arg.Any()); + .GetAll(Arg.Is(u => u.ToString() == "user/emails"), Args.ApiOptions); } } From 7c4185af8ea3b39422957d3b21f2e7e02a2cc9c6 Mon Sep 17 00:00:00 2001 From: "aedampir@gmail.com" Date: Wed, 16 Mar 2016 12:13:47 +0700 Subject: [PATCH 13/23] Now GetsCorrectUrl test accept any ApiOptions as valid. --- Octokit.Tests/Clients/UserEmailsClientTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Octokit.Tests/Clients/UserEmailsClientTests.cs b/Octokit.Tests/Clients/UserEmailsClientTests.cs index 462852cc11..a66d5983fc 100644 --- a/Octokit.Tests/Clients/UserEmailsClientTests.cs +++ b/Octokit.Tests/Clients/UserEmailsClientTests.cs @@ -20,7 +20,7 @@ public void GetsCorrectUrl() connection.Received(1) .GetAll(Arg.Is(u => u.ToString() == "user/emails"), - Arg.Is(ApiOptions.None)); + Args.ApiOptions); } [Fact] From 8c03c90bfc80e2f1137cd9cea9c84128280c658e Mon Sep 17 00:00:00 2001 From: "aedampir@gmail.com" Date: Wed, 16 Mar 2016 12:17:53 +0700 Subject: [PATCH 14/23] Tests have been rewritten in order to support "Received" check with ApiOptions parameter. --- Octokit.Tests/Reactive/ObservableUserEmailsClientTests.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Octokit.Tests/Reactive/ObservableUserEmailsClientTests.cs b/Octokit.Tests/Reactive/ObservableUserEmailsClientTests.cs index 267f3029de..ea7dc6886e 100644 --- a/Octokit.Tests/Reactive/ObservableUserEmailsClientTests.cs +++ b/Octokit.Tests/Reactive/ObservableUserEmailsClientTests.cs @@ -28,8 +28,8 @@ public void GetsCorrectUrl() client.GetAll(); - github.Connection.Received(1).GetResponse>(_expectedUri, - Arg.Is>(dictionary => dictionary.Count == 0)); + github.Connection.Received(1).Get>(_expectedUri, + Arg.Is>(dictionary => dictionary.Count == 0), null); } [Fact] @@ -40,8 +40,8 @@ public void GetsCorrectUrlWithApiOption() client.GetAll(ApiOptions.None); - github.Connection.Received(1).GetResponse>(_expectedUri, - Arg.Is>(dictionary => dictionary.Count == 0)); + github.Connection.Received(1).Get>(_expectedUri, + Arg.Is>(dictionary => dictionary.Count == 0), null); } } From 833767cde41081e635c7177187d95af069f7b3c1 Mon Sep 17 00:00:00 2001 From: "aedampir@gmail.com" Date: Wed, 16 Mar 2016 12:19:23 +0700 Subject: [PATCH 15/23] An ApiExtensions.GetResponse overload has been removed. --- Octokit/Helpers/ApiExtensions.cs | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/Octokit/Helpers/ApiExtensions.cs b/Octokit/Helpers/ApiExtensions.cs index d25027e058..81b1ab8e6f 100644 --- a/Octokit/Helpers/ApiExtensions.cs +++ b/Octokit/Helpers/ApiExtensions.cs @@ -74,23 +74,6 @@ public static Task> GetResponse(this IConnection connection, return connection.Get(uri, null, null); } - /// - /// Gets the API resource at the specified URI. - /// - /// Type of the API resource to get. - /// The connection to use - /// URI of the API resource to get - /// Querystring parameters for the request - /// The API resource. - /// Thrown when an API error occurs. - public static Task> GetResponse(this IConnection connection, Uri uri, Dictionary parameters) - { - Ensure.ArgumentNotNull(connection, "connection"); - Ensure.ArgumentNotNull(uri, "uri"); - - return connection.Get(uri, parameters, null); - } - [Obsolete("Octokit's HTTP library now follows redirects by default - this API will be removed in a future release")] public static Task> GetRedirect(this IConnection connection, Uri uri) { From a1341c99bed07f15ad3adf319b03d11bda6ec9b0 Mon Sep 17 00:00:00 2001 From: "aedampir@gmail.com" Date: Wed, 16 Mar 2016 12:20:01 +0700 Subject: [PATCH 16/23] Unused commented test has been removed. --- .../ObservableUserEmailsClientTests.cs | 23 ------------------- 1 file changed, 23 deletions(-) diff --git a/Octokit.Tests.Integration/Reactive/ObservableUserEmailsClientTests.cs b/Octokit.Tests.Integration/Reactive/ObservableUserEmailsClientTests.cs index 66811c5241..352fce92cf 100644 --- a/Octokit.Tests.Integration/Reactive/ObservableUserEmailsClientTests.cs +++ b/Octokit.Tests.Integration/Reactive/ObservableUserEmailsClientTests.cs @@ -54,28 +54,5 @@ public async Task ReturnsCorrectCountOfEmailsWithStart() Assert.Equal(0, emails.Count); } - - //[IntegrationTest] - //public async Task ReturnsDistinctResultsBasedOnStartPage() - //{ - // var startOptions = new ApiOptions - // { - // PageSize = 5, - // PageCount = 1 - // }; - - // var firstPage = await _emailClient.GetAll(startOptions); - - // var skipStartOptions = new ApiOptions - // { - // PageSize = 5, - // PageCount = 1, - // StartPage = 2 - // }; - - // var secondPage = await _emailClient.GetAll(skipStartOptions); - - // Assert.Equal(firstPage[0].Email, secondPage[0].Email); - //} } } From 0fd34d9edc1dc00d622495dcdbacc0db2aaab03f Mon Sep 17 00:00:00 2001 From: "aedampir@gmail.com" Date: Wed, 16 Mar 2016 12:23:38 +0700 Subject: [PATCH 17/23] Singleton ApiOptions instance has been removed. --- Octokit/Models/Request/ApiOptions.cs | 19 ++----------------- 1 file changed, 2 insertions(+), 17 deletions(-) diff --git a/Octokit/Models/Request/ApiOptions.cs b/Octokit/Models/Request/ApiOptions.cs index e7ed1c4095..af369d105c 100644 --- a/Octokit/Models/Request/ApiOptions.cs +++ b/Octokit/Models/Request/ApiOptions.cs @@ -5,26 +5,11 @@ namespace Octokit { [DebuggerDisplay("{DebuggerDisplay,nq}")] - public class ApiOptions + public class ApiOptions { - private sealed class ApiOptionsSingleton - { - private static readonly Lazy _lazy = - new Lazy(() => new ApiOptions()); - - public static ApiOptions Instance - { - get { return _lazy.Value; } - } - - private ApiOptionsSingleton() - { - } - } - public static ApiOptions None { - get { return ApiOptionsSingleton.Instance; } + get { return new ApiOptions(); } } /// From e2cb829f2639eb5fd5e709416c0e9174c318282a Mon Sep 17 00:00:00 2001 From: "aedampir@gmail.com" Date: Wed, 16 Mar 2016 12:24:30 +0700 Subject: [PATCH 18/23] Unused commented code has been removed. Minor changes. --- .../Clients/UserEmailsClientTests.cs | 30 ++----------------- 1 file changed, 2 insertions(+), 28 deletions(-) diff --git a/Octokit.Tests.Integration/Clients/UserEmailsClientTests.cs b/Octokit.Tests.Integration/Clients/UserEmailsClientTests.cs index 0a1c5888f8..03c03f67c8 100644 --- a/Octokit.Tests.Integration/Clients/UserEmailsClientTests.cs +++ b/Octokit.Tests.Integration/Clients/UserEmailsClientTests.cs @@ -17,17 +17,14 @@ public UserEmailsClientTests() [IntegrationTest] public async Task CanGetEmail() { - var github = Helper.GetAuthenticatedClient(); - - var emails = await github.User.Email.GetAll(); + var emails = await _emailClient.GetAll(); Assert.NotEmpty(emails); } [IntegrationTest] public async Task CanGetEmailWithApiOptions() { - var github = Helper.GetAuthenticatedClient(); - var emails = await github.User.Email.GetAll(ApiOptions.None); + var emails = await _emailClient.GetAll(ApiOptions.None); Assert.NotEmpty(emails); } @@ -60,29 +57,6 @@ public async Task ReturnsCorrectCountOfEmailsWithStart() Assert.Equal(0, emails.Count); } - //[IntegrationTest] - //public async Task ReturnsDistinctResultsBasedOnStartPage() - //{ - // var startOptions = new ApiOptions - // { - // PageSize = 5, - // PageCount = 1 - // }; - - // var firstPage = await _emailClient.GetAll(startOptions); - - // var skipStartOptions = new ApiOptions - // { - // PageSize = 5, - // PageCount = 1, - // StartPage = 2 - // }; - - // var secondPage = await _emailClient.GetAll(skipStartOptions); - - // Assert.Equal(firstPage[0].Email, secondPage[0].Email); - //} - const string testEmailAddress = "hahaha-not-a-real-email@foo.com"; [IntegrationTest(Skip = "this isn't passing in CI - i hate past me right now")] From 632dd6218bec026c1fb07cd974c957611eae2d63 Mon Sep 17 00:00:00 2001 From: "aedampir@gmail.com" Date: Wed, 16 Mar 2016 13:07:22 +0700 Subject: [PATCH 19/23] Some tests have been rewritten according to @shiftkey remarks. --- .../Clients/UserEmailsClientTests.cs | 19 ++----------------- 1 file changed, 2 insertions(+), 17 deletions(-) diff --git a/Octokit.Tests.Integration/Clients/UserEmailsClientTests.cs b/Octokit.Tests.Integration/Clients/UserEmailsClientTests.cs index 03c03f67c8..12ac23e496 100644 --- a/Octokit.Tests.Integration/Clients/UserEmailsClientTests.cs +++ b/Octokit.Tests.Integration/Clients/UserEmailsClientTests.cs @@ -39,24 +39,9 @@ public async Task ReturnsCorrectCountOfEmailsWithoutStart() var emails = await _emailClient.GetAll(options); - Assert.Equal(1, emails.Count); - } - - [IntegrationTest] - public async Task ReturnsCorrectCountOfEmailsWithStart() - { - var options = new ApiOptions - { - PageSize = 5, - PageCount = 1, - StartPage = 2 - }; - - var emails = await _emailClient.GetAll(options); - - Assert.Equal(0, emails.Count); + Assert.NotEmpty(emails); } - + const string testEmailAddress = "hahaha-not-a-real-email@foo.com"; [IntegrationTest(Skip = "this isn't passing in CI - i hate past me right now")] From fdcd0f95ec73ace5c4edb2201315e688e269be07 Mon Sep 17 00:00:00 2001 From: "aedampir@gmail.com" Date: Wed, 16 Mar 2016 13:08:57 +0700 Subject: [PATCH 20/23] Some integration tests have been rewritten according to @shiftkey remarks. --- .../Reactive/ObservableUserEmailsClientTests.cs | 17 +---------------- 1 file changed, 1 insertion(+), 16 deletions(-) diff --git a/Octokit.Tests.Integration/Reactive/ObservableUserEmailsClientTests.cs b/Octokit.Tests.Integration/Reactive/ObservableUserEmailsClientTests.cs index 352fce92cf..98a4cef4f1 100644 --- a/Octokit.Tests.Integration/Reactive/ObservableUserEmailsClientTests.cs +++ b/Octokit.Tests.Integration/Reactive/ObservableUserEmailsClientTests.cs @@ -37,22 +37,7 @@ public async Task ReturnsCorrectCountOfEmailsWithoutStart() var emails = await _emailClient.GetAll(options).ToList(); - Assert.Equal(1, emails.Count); - } - - [IntegrationTest] - public async Task ReturnsCorrectCountOfEmailsWithStart() - { - var options = new ApiOptions - { - PageSize = 5, - PageCount = 1, - StartPage = 2 - }; - - var emails = await _emailClient.GetAll(options).ToList(); - - Assert.Equal(0, emails.Count); + Assert.NotEmpty(emails); } } } From 2ae5c1d6aa4fbaf07fa4bb93f8d80c4e72093070 Mon Sep 17 00:00:00 2001 From: "aedampir@gmail.com" Date: Wed, 16 Mar 2016 20:15:07 +0700 Subject: [PATCH 21/23] Name of test class have been changed in order to support naming conventions. --- Octokit.Tests/Clients/UserEmailsClientTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Octokit.Tests/Clients/UserEmailsClientTests.cs b/Octokit.Tests/Clients/UserEmailsClientTests.cs index a66d5983fc..34d3110661 100644 --- a/Octokit.Tests/Clients/UserEmailsClientTests.cs +++ b/Octokit.Tests/Clients/UserEmailsClientTests.cs @@ -8,7 +8,7 @@ namespace Octokit.Tests.Clients { public class UserEmailsClientTests { - public class TheGetAllMethods + public class TheGetAllMethod { [Fact] public void GetsCorrectUrl() From 4b2f52f89b33c9ebf98ec4e310fb2f52bc7f80a9 Mon Sep 17 00:00:00 2001 From: "aedampir@gmail.com" Date: Wed, 16 Mar 2016 20:16:03 +0700 Subject: [PATCH 22/23] Extra whitespace have been removed. --- Octokit/Models/Request/ApiOptions.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Octokit/Models/Request/ApiOptions.cs b/Octokit/Models/Request/ApiOptions.cs index af369d105c..cdc3cbf379 100644 --- a/Octokit/Models/Request/ApiOptions.cs +++ b/Octokit/Models/Request/ApiOptions.cs @@ -5,7 +5,7 @@ namespace Octokit { [DebuggerDisplay("{DebuggerDisplay,nq}")] - public class ApiOptions + public class ApiOptions { public static ApiOptions None { From 5c835574b018e8c095264b117a9523abdbd2a41b Mon Sep 17 00:00:00 2001 From: "aedampir@gmail.com" Date: Wed, 16 Mar 2016 20:23:21 +0700 Subject: [PATCH 23/23] ObservableUserEmailsClientTests have been rewritten in order to support project coding styles. --- .../Reactive/ObservableUserEmailsClientTests.cs | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/Octokit.Tests.Integration/Reactive/ObservableUserEmailsClientTests.cs b/Octokit.Tests.Integration/Reactive/ObservableUserEmailsClientTests.cs index 98a4cef4f1..9e74629949 100644 --- a/Octokit.Tests.Integration/Reactive/ObservableUserEmailsClientTests.cs +++ b/Octokit.Tests.Integration/Reactive/ObservableUserEmailsClientTests.cs @@ -7,15 +7,19 @@ namespace Octokit.Tests.Integration { public class ObservableUserEmailsClientTests { - private readonly ObservableUserEmailsClient _emailClient - = new ObservableUserEmailsClient(Helper.GetAuthenticatedClient()); + readonly ObservableUserEmailsClient _emailClient; + + public ObservableUserEmailsClientTests() + { + var github = Helper.GetAuthenticatedClient(); + + _emailClient = new ObservableUserEmailsClient(github); + } [IntegrationTest] public async Task CanGetEmail() { - var client = new ObservableUserEmailsClient(Helper.GetAuthenticatedClient()); - - var email = await client.GetAll(); + var email = await _emailClient.GetAll(); Assert.NotNull(email); } @@ -36,7 +40,6 @@ public async Task ReturnsCorrectCountOfEmailsWithoutStart() }; var emails = await _emailClient.GetAll(options).ToList(); - Assert.NotEmpty(emails); } }