Pull request commit list now implemented / tested

Renamed and enhanced existing Merged tests.

Octokit.Reactive/Clients/IObservablePullRequestsClient.cs

@@ -84,5 +84,15 @@ namespace Octokit
         /// <param name="number">The pull request number</param>
         /// <returns></returns>
         IObservable<PullRequestMerge> Merged(string owner, string name, int number);
+
+        /// <summary>
+        /// Gets the list of commits on a pull request.
+        /// </summary>
+        /// <remarks>http://developer.github.com/v3/pulls/#list-commits-on-a-pull-request</remarks>
+        /// <param name="owner">The owner of the repository</param>
+        /// <param name="name">The name of the repository</param>
+        /// <param name="number">The pull request number</param>
+        /// <returns></returns>
+        IObservable<Commit> Commits(string owner, string name, int number);
     }
 }

Octokit.Reactive/Clients/ObservablePullRequestsClient.cs

@@ -1,4 +1,5 @@
 using System;
+using System.Collections.Generic;
 using System.Reactive;
 using System.Reactive.Threading.Tasks;
 using System.Threading.Tasks;
@@ -137,5 +138,21 @@ namespace Octokit.Reactive.Clients
 
             return _client.Merged(owner, name, number).ToObservable();
         }
+
+        /// <summary>
+        /// Gets the list of commits on a pull request.
+        /// </summary>
+        /// <remarks>http://developer.github.com/v3/pulls/#list-commits-on-a-pull-request</remarks>
+        /// <param name="owner">The owner of the repository</param>
+        /// <param name="name">The name of the repository</param>
+        /// <param name="number">The pull request number</param>
+        /// <returns></returns>
+        public IObservable<Commit> Commits(string owner, string name, int number) 
+        {
+            Ensure.ArgumentNotNullOrEmptyString(owner, "owner");
+            Ensure.ArgumentNotNullOrEmptyString(name, "name");
+
+            return _connection.GetAndFlattenAllPages<Commit>(ApiUrls.PullRequestCommits(owner, name, number));
+        }
     }
 }

Octokit.Tests/Clients/PullRequestsClientTests.cs

@@ -167,7 +167,7 @@ namespace Octokit.Tests.Clients
         public class TheMergedMethod 
         {
             [Fact]
-            public void PutsToCorrectUrl()
+            public void RequestsCorrectUrl()
             {
                 var connection = Substitute.For<IApiConnection>();
                 var client = new PullRequestsClient(connection);
@@ -183,10 +183,37 @@ namespace Octokit.Tests.Clients
                 var connection = Substitute.For<IApiConnection>();
                 var client = new PullRequestsClient(connection);
 
-                AssertEx.Throws<ArgumentNullException>(async () => await
+                await AssertEx.Throws<ArgumentNullException>(async () => await client.Merged(null, "name", 1));
-                    client.Merged(null, "name", 42));
+                await AssertEx.Throws<ArgumentNullException>(async () => await client.Merged("owner", null, 1));
-                AssertEx.Throws<ArgumentException>(async () => await
+                await AssertEx.Throws<ArgumentException>(async () => await client.Merged(null, "", 1));
-                    client.Merged("owner", null, 42));
+                await AssertEx.Throws<ArgumentException>(async () => await client.Merged("", null, 1));
+            }
+        }
+
+        public class TheCommitsMethod 
+        {
+            [Fact]
+            public async void RequestsCorrectUrl()
+            {
+                var connection = Substitute.For<IApiConnection>();
+                var client = new PullRequestsClient(connection);
+
+                client.Commits("fake", "repo", 42);
+
+                connection.Received().GetAll<Commit>(
+                    Arg.Is<Uri>(u => u.ToString() == "repos/fake/repo/pulls/42/commits"));
+            }
+
+            [Fact]
+            public async Task EnsuresArgumentsNotNull()
+            {
+                var connection = Substitute.For<IApiConnection>();
+                var client = new PullRequestsClient(connection);
+
+                await AssertEx.Throws<ArgumentNullException>(async () => await client.Commits(null, "name", 1));
+                await AssertEx.Throws<ArgumentNullException>(async () => await client.Commits("owner", null, 1));
+                await AssertEx.Throws<ArgumentException>(async () => await client.Commits(null, "", 1));
+                await AssertEx.Throws<ArgumentException>(async () => await client.Commits("", null, 1));
             }
         }
 

Octokit/Clients/IPullRequestsClient.cs

@@ -82,5 +82,15 @@ namespace Octokit
         /// <param name="number">The pull request number</param>
         /// <returns></returns>
         Task<PullRequestMerge> Merged(string owner, string name, int number);
+
+        /// <summary>
+        /// Gets the list of commits on a pull request.
+        /// </summary>
+        /// <remarks>http://developer.github.com/v3/pulls/#list-commits-on-a-pull-request</remarks>
+        /// <param name="owner">The owner of the repository</param>
+        /// <param name="name">The name of the repository</param>
+        /// <param name="number">The pull request number</param>
+        /// <returns></returns>
+        Task<IReadOnlyList<Commit>> Commits(string owner, string name, int number);
     }
 }

Octokit/Clients/PullRequestsClient.cs

@@ -130,5 +130,21 @@ namespace Octokit
 
             return ApiConnection.Get<PullRequestMerge>(ApiUrls.MergePullRequest(owner, name, number));
         }
+
+        /// <summary>
+        /// Gets the list of commits on a pull request.
+        /// </summary>
+        /// <remarks>http://developer.github.com/v3/pulls/#list-commits-on-a-pull-request</remarks>
+        /// <param name="owner">The owner of the repository</param>
+        /// <param name="name">The name of the repository</param>
+        /// <param name="number">The pull request number</param>
+        /// <returns></returns>
+        public Task<IReadOnlyList<Commit>> Commits(string owner, string name, int number) 
+        {
+            Ensure.ArgumentNotNullOrEmptyString(owner, "owner");
+            Ensure.ArgumentNotNullOrEmptyString(name, "name");
+
+            return ApiConnection.GetAll<Commit>(ApiUrls.PullRequestCommits(owner, name, number));
+        }
     }
 }

Octokit/Helpers/ApiUrls.cs

@@ -599,6 +599,17 @@ namespace Octokit
             return "repos/{0}/{1}/pulls/{2}/merge".FormatUri(owner, name, number);
         }
 
+        /// <summary>
+        /// Returns the <see cref="Uri"/> that returns the commits on a pull request.
+        /// </summary>
+        /// <param name="owner">The owner of the repository</param>
+        /// <param name="name">The name of the repository</param>
+        /// /// <param name="number">The pull request number</param>
+        public static Uri PullRequestCommits(string owner, string name, int number) 
+        {
+            return "repos/{0}/{1}/pulls/{2}/commits".FormatUri(owner, name, number);
+        }
+
         /// <summary>
         /// Returns the <see cref="Uri"/> for a spesific comment for the specified commit.
         /// </summary>