Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement changes to Checks API for Annotations models and re-request endpoint #1857

Merged
merged 7 commits into from
Sep 3, 2018
Merged

Implement changes to Checks API for Annotations models and re-request endpoint #1857

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
e432ca8
Attempt to handle both old and new annotations models so we support t…
ryangribble Aug 23, 2018
d50f5d4
adjust tests to remove use of obsoleted fields
ryangribble Aug 23, 2018
a99df42
fix a couple of other tests using unrelated obsoleted fields
ryangribble Aug 23, 2018
51a3914
Mark check suite Request method and request object as obsolete
ryangribble Aug 28, 2018
4baede9
Add Rerequest() method to normal and observable clients
ryangribble Aug 28, 2018
0488b47
add StartColumn and EndColumn as optional fields for CheckRunAnnotati…
ryangribble Aug 30, 2018
16feded
remove integration tests for Request() method as they no longer work …
ryangribble Sep 3, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions Octokit.Reactive/Clients/IObservableCheckSuitesClient.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,7 @@ public interface IObservableCheckSuitesClient
/// <param name="owner">The owner of the repository</param>
/// <param name="name">The name of the repository</param>
/// <param name="request">Details of the Check Suite request</param>
[Obsolete("This method has been deprecated in the GitHub Api, however can still be used on GitHub Enterprise 2.14")]
IObservable<bool> Request(string owner, string name, CheckSuiteTriggerRequest request);

/// <summary>
Expand All @@ -161,6 +162,28 @@ public interface IObservableCheckSuitesClient
/// </remarks>
/// <param name="repositoryId">The Id of the repository</param>
/// <param name="request">Details of the Check Suite request</param>
[Obsolete("This method has been deprecated in the GitHub Api, however can still be used on GitHub Enterprise 2.14")]
IObservable<bool> Request(long repositoryId, CheckSuiteTriggerRequest request);

/// <summary>
/// Triggers GitHub to rerequest an existing check suite, without pushing new code to a repository
/// </summary>
/// <remarks>
/// See the <a href="https://developer.github.com/v3/checks/suites/#request-check-suites">Check Suites API documentation</a> for more information.
/// </remarks>
/// <param name="owner">The owner of the repository</param>
/// <param name="name">The name of the repository</param>
/// <param name="checkSuiteId">The Id of the check suite</param>
IObservable<bool> Rerequest(string owner, string name, long checkSuiteId);

/// <summary>
/// Triggers GitHub to rerequest an existing check suite, without pushing new code to a repository
/// </summary>
/// <remarks>
/// See the <a href="https://developer.github.com/v3/checks/suites/#request-check-suites">Check Suites API documentation</a> for more information.
/// </remarks>
/// <param name="repositoryId">The Id of the repository</param>
/// <param name="checkSuiteId">The Id of the check suite</param>
IObservable<bool> Rerequest(long repositoryId, long checkSuiteId);
}
}
33 changes: 32 additions & 1 deletion Octokit.Reactive/Clients/ObservableCheckSuitesClient.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,7 @@ public IObservable<CheckSuite> Create(long repositoryId, NewCheckSuite newCheckS
/// <param name="owner">The owner of the repository</param>
/// <param name="name">The name of the repository</param>
/// <param name="request">Details of the Check Suite request</param>
[Obsolete("This method has been deprecated in the GitHub Api, however can still be used on GitHub Enterprise 2.14")]
public IObservable<bool> Request(string owner, string name, CheckSuiteTriggerRequest request)
{
Ensure.ArgumentNotNullOrEmptyString(owner, nameof(owner));
Expand All @@ -260,12 +261,42 @@ public IObservable<bool> Request(string owner, string name, CheckSuiteTriggerReq
/// </remarks>
/// <param name="repositoryId">The Id of the repository</param>
/// <param name="request">Details of the Check Suite request</param>

[Obsolete("This method has been deprecated in the GitHub Api, however can still be used on GitHub Enterprise 2.14")]
public IObservable<bool> Request(long repositoryId, CheckSuiteTriggerRequest request)
{
Ensure.ArgumentNotNull(request, nameof(request));

return _client.Request(repositoryId, request).ToObservable();
}

/// <summary>
/// Triggers GitHub to rerequest an existing check suite, without pushing new code to a repository
/// </summary>
/// <remarks>
/// See the <a href="https://developer.github.com/v3/checks/suites/#request-check-suites">Check Suites API documentation</a> for more information.
/// </remarks>
/// <param name="owner">The owner of the repository</param>
/// <param name="name">The name of the repository</param>
/// <param name="checkSuiteId">The Id of the check suite</param>
public IObservable<bool> Rerequest(string owner, string name, long checkSuiteId)
{
Ensure.ArgumentNotNullOrEmptyString(owner, nameof(owner));
Ensure.ArgumentNotNullOrEmptyString(name, nameof(name));

return _client.Rerequest(owner, name, checkSuiteId).ToObservable();
}

/// <summary>
/// Triggers GitHub to rerequest an existing check suite, without pushing new code to a repository
/// </summary>
/// <remarks>
/// See the <a href="https://developer.github.com/v3/checks/suites/#request-check-suites">Check Suites API documentation</a> for more information.
/// </remarks>
/// <param name="repositoryId">The Id of the repository</param>
/// <param name="checkSuiteId">The Id of the check suite</param>
public IObservable<bool> Rerequest(long repositoryId, long checkSuiteId)
{
return _client.Rerequest(repositoryId, checkSuiteId).ToObservable();
}
}
}
8 changes: 4 additions & 4 deletions Octokit.Tests.Integration/Clients/CheckRunsClientTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -409,7 +409,7 @@ public async Task GetsAllAnnotations()
{
Annotations = new[]
{
new NewCheckRunAnnotation("file.txt", "blob", 1, 1, CheckWarningLevel.Warning, "this is a warning")
new NewCheckRunAnnotation("file.txt", 1, 1, CheckAnnotationLevel.Warning, "this is a warning")
}
}
};
Expand All @@ -421,7 +421,7 @@ public async Task GetsAllAnnotations()
// Check result
Assert.Equal(1, annotations.Count);
Assert.Equal("this is a warning", annotations.First().Message);
Assert.Equal(CheckWarningLevel.Warning, annotations.First().WarningLevel);
Assert.Equal(CheckAnnotationLevel.Warning, annotations.First().AnnotationLevel);
}
}

Expand All @@ -442,7 +442,7 @@ public async Task GetsAllAnnotationsWithRepositoryId()
{
Annotations = new[]
{
new NewCheckRunAnnotation("file.txt", "blob", 1, 1, CheckWarningLevel.Warning, "this is a warning")
new NewCheckRunAnnotation("file.txt", 1, 1, CheckAnnotationLevel.Warning, "this is a warning")
}
}
};
Expand All @@ -454,7 +454,7 @@ public async Task GetsAllAnnotationsWithRepositoryId()
// Check result
Assert.Equal(1, annotations.Count);
Assert.Equal("this is a warning", annotations.First().Message);
Assert.Equal(CheckWarningLevel.Warning, annotations.First().WarningLevel);
Assert.Equal(CheckAnnotationLevel.Warning, annotations.First().AnnotationLevel);
}
}
}
Expand Down
20 changes: 13 additions & 7 deletions Octokit.Tests.Integration/Clients/CheckSuitesClientTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -213,12 +213,12 @@ public async Task CreatesCheckSuiteWithRepositoryId()
}
}

public class TheRequestMethod
public class TheRerequestMethod
{
IGitHubClient _github;
IGitHubClient _githubAppInstallation;

public TheRequestMethod()
public TheRerequestMethod()
{
_github = Helper.GetAuthenticatedClient();

Expand All @@ -227,26 +227,32 @@ public TheRequestMethod()
}

[GitHubAppsTest]
public async Task RequestsCheckSuite()
public async Task RerequestsCheckSuite()
{
using (var repoContext = await _github.CreateRepositoryContext(new NewRepository(Helper.MakeNameWithTimestamp("public-repo")) { AutoInit = true }))
{
var headCommit = await _github.Repository.Commit.Get(repoContext.RepositoryOwner, repoContext.RepositoryName, "master");
// Need to get a CheckSuiteId so we can test the Get method
var headCommit = await _github.Repository.Commit.Get(repoContext.RepositoryId, "master");
var checkSuite = (await _githubAppInstallation.Check.Suite.GetAllForReference(repoContext.RepositoryId, headCommit.Sha)).CheckSuites.First();

var result = await _githubAppInstallation.Check.Suite.Request(repoContext.RepositoryOwner, repoContext.RepositoryName, new CheckSuiteTriggerRequest(headCommit.Sha));
// Get Check Suite by Id
var result = await _githubAppInstallation.Check.Suite.Rerequest(repoContext.RepositoryOwner, repoContext.RepositoryName, checkSuite.Id);

Assert.True(result);
}
}

[GitHubAppsTest]
public async Task RequestsCheckSuiteWithRepositoryId()
public async Task RerequestsCheckSuiteWithRepositoryId()
{
using (var repoContext = await _github.CreateRepositoryContext(new NewRepository(Helper.MakeNameWithTimestamp("public-repo")) { AutoInit = true }))
{
// Need to get a CheckSuiteId so we can test the Get method
var headCommit = await _github.Repository.Commit.Get(repoContext.RepositoryId, "master");
var checkSuite = (await _githubAppInstallation.Check.Suite.GetAllForReference(repoContext.RepositoryId, headCommit.Sha)).CheckSuites.First();

var result = await _githubAppInstallation.Check.Suite.Request(repoContext.RepositoryId, new CheckSuiteTriggerRequest(headCommit.Sha));
// Get Check Suite by Id
var result = await _githubAppInstallation.Check.Suite.Rerequest(repoContext.RepositoryId, checkSuite.Id);

Assert.True(result);
}
Expand Down
8 changes: 4 additions & 4 deletions Octokit.Tests.Integration/Reactive/ObservableCheckRunsClientTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -410,7 +410,7 @@ public async Task GetsAllAnnotations()
{
Annotations = new[]
{
new NewCheckRunAnnotation("file.txt", "blob", 1, 1, CheckWarningLevel.Warning, "this is a warning")
new NewCheckRunAnnotation("file.txt", 1, 1, CheckAnnotationLevel.Warning, "this is a warning")
}
}
};
Expand All @@ -422,7 +422,7 @@ public async Task GetsAllAnnotations()
// Check result
Assert.Equal(1, annotations.Count);
Assert.Equal("this is a warning", annotations.First().Message);
Assert.Equal(CheckWarningLevel.Warning, annotations.First().WarningLevel);
Assert.Equal(CheckAnnotationLevel.Warning, annotations.First().AnnotationLevel);
}
}

Expand All @@ -443,7 +443,7 @@ public async Task GetsAllAnnotationsWithRepositoryId()
{
Annotations = new[]
{
new NewCheckRunAnnotation("file.txt", "blob", 1, 1, CheckWarningLevel.Warning, "this is a warning")
new NewCheckRunAnnotation("file.txt", 1, 1, CheckAnnotationLevel.Warning, "this is a warning")
}
}
};
Expand All @@ -455,7 +455,7 @@ public async Task GetsAllAnnotationsWithRepositoryId()
// Check result
Assert.Equal(1, annotations.Count);
Assert.Equal("this is a warning", annotations.First().Message);
Assert.Equal(CheckWarningLevel.Warning, annotations.First().WarningLevel);
Assert.Equal(CheckAnnotationLevel.Warning, annotations.First().AnnotationLevel);
}
}
}
Expand Down
18 changes: 11 additions & 7 deletions Octokit.Tests.Integration/Reactive/ObservableCheckSuitesClientTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -215,12 +215,12 @@ public async Task CreatesCheckSuiteWithRepositoryId()
}
}

public class TheRequestMethod
public class TheRerequestMethod
{
IObservableGitHubClient _github;
IObservableGitHubClient _githubAppInstallation;

public TheRequestMethod()
public TheRerequestMethod()
{
_github = new ObservableGitHubClient(Helper.GetAuthenticatedClient());

Expand All @@ -229,26 +229,30 @@ public TheRequestMethod()
}

[GitHubAppsTest]
public async Task RequestsCheckSuite()
public async Task RerequestsCheckSuite()
{
using (var repoContext = await _github.CreateRepositoryContext(new NewRepository(Helper.MakeNameWithTimestamp("public-repo")) { AutoInit = true }))
{
var headCommit = await _github.Repository.Commit.Get(repoContext.RepositoryOwner, repoContext.RepositoryName, "master");
// Need to get a CheckSuiteId so we can test the Get method
var headCommit = await _github.Repository.Commit.Get(repoContext.RepositoryId, "master");
var checkSuite = (await _githubAppInstallation.Check.Suite.GetAllForReference(repoContext.RepositoryId, headCommit.Sha)).CheckSuites.First();

var result = await _githubAppInstallation.Check.Suite.Request(repoContext.RepositoryOwner, repoContext.RepositoryName, new CheckSuiteTriggerRequest(headCommit.Sha));
var result = await _githubAppInstallation.Check.Suite.Rerequest(repoContext.RepositoryOwner, repoContext.RepositoryName, checkSuite.Id);

Assert.True(result);
}
}

[GitHubAppsTest]
public async Task RequestsCheckSuiteWithRepositoryId()
public async Task RerequestsCheckSuiteWithRepositoryId()
{
using (var repoContext = await _github.CreateRepositoryContext(new NewRepository(Helper.MakeNameWithTimestamp("public-repo")) { AutoInit = true }))
{
// Need to get a CheckSuiteId so we can test the Get method
var headCommit = await _github.Repository.Commit.Get(repoContext.RepositoryId, "master");
var checkSuite = (await _githubAppInstallation.Check.Suite.GetAllForReference(repoContext.RepositoryId, headCommit.Sha)).CheckSuites.First();

var result = await _githubAppInstallation.Check.Suite.Request(repoContext.RepositoryId, new CheckSuiteTriggerRequest(headCommit.Sha));
var result = await _githubAppInstallation.Check.Suite.Rerequest(repoContext.RepositoryId, checkSuite.Id);

Assert.True(result);
}
Expand Down
53 changes: 53 additions & 0 deletions Octokit.Tests/Clients/CheckSuitesClientTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -332,6 +332,7 @@ public async Task EnsuresNonEmptyArguments()
}
}

#pragma warning disable CS0618 // Type or member is obsolete
public class TheRequestMethod
{
[Fact]
Expand Down Expand Up @@ -393,5 +394,57 @@ public async Task EnsuresNonEmptyArguments()
await Assert.ThrowsAsync<ArgumentException>(() => client.Request("fake", "", request));
}
}
#pragma warning restore CS0618 // Type or member is obsolete

public class TheRerequestMethod
{
[Fact]
public async Task RequestsCorrectUrl()
{
var connection = MockedIApiConnection.PostReturnsHttpStatus(HttpStatusCode.Created);
var client = new CheckSuitesClient(connection);

await client.Rerequest("fake", "repo", 1);

connection.Connection.Received().Post(
Arg.Is<Uri>(u => u.ToString() == "repos/fake/repo/check-suites/1/rerequest"),
Args.Object,
"application/vnd.github.antiope-preview+json");
}

[Fact]
public async Task RequestsCorrectUrlWithRepositoryId()
{
var connection = MockedIApiConnection.PostReturnsHttpStatus(HttpStatusCode.Created);
var client = new CheckSuitesClient(connection);

await client.Rerequest(1, 1);

connection.Connection.Received().Post(
Arg.Is<Uri>(u => u.ToString() == "repositories/1/check-suites/1/rerequest"),
Args.Object,
"application/vnd.github.antiope-preview+json");
}

[Fact]
public async Task EnsuresNonNullArguments()
{
var connection = Substitute.For<IApiConnection>();
var client = new CheckSuitesClient(connection);

await Assert.ThrowsAsync<ArgumentNullException>(() => client.Rerequest(null, "repo", 1));
await Assert.ThrowsAsync<ArgumentNullException>(() => client.Rerequest("fake", null, 1));
}

[Fact]
public async Task EnsuresNonEmptyArguments()
{
var connection = Substitute.For<IApiConnection>();
var client = new CheckSuitesClient(connection);

await Assert.ThrowsAsync<ArgumentException>(() => client.Rerequest("", "repo", 1));
await Assert.ThrowsAsync<ArgumentException>(() => client.Rerequest("fake", "", 1));
}
}
}
}
10 changes: 8 additions & 2 deletions Octokit.Tests/Clients/ProjectCardsClientTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,10 @@ public async Task PostsToCorrectURL()
{
var connection = Substitute.For<IApiConnection>();
var client = new ProjectCardsClient(connection);
var updateCard = new ProjectCardUpdate("someNewNote");
var updateCard = new ProjectCardUpdate
{
Note = "someNewNote"
};

await client.Update(1, updateCard);

Expand All @@ -129,7 +132,10 @@ public async Task PostsToCorrectURL()
public async Task EnsuresNonNullArguments()
{
var client = new ProjectCardsClient(Substitute.For<IApiConnection>());
var updateCard = new ProjectCardUpdate("someNewNote");
var updateCard = new ProjectCardUpdate
{
Note = "someNewNote"
};

await Assert.ThrowsAsync<ArgumentNullException>(() => client.Update(1, null));
}
Expand Down
Loading