-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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.
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 labels CRUD #256
Changes from 2 commits
8f44a22
2ab931b
891a353
dc9a059
18a3d39
0a484c0
7c64c17
4697c96
a3ff363
63da0a6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,101 @@ | ||
using System; | ||
using System.Collections.ObjectModel; | ||
using System.Linq; | ||
using System.Net.Http.Headers; | ||
using System.Threading; | ||
using System.Threading.Tasks; | ||
using Octokit; | ||
using Octokit.Tests.Integration; | ||
using Xunit; | ||
|
||
public class IssuesLabelsClientTests | ||
{ | ||
readonly IGitHubClient _gitHubClient; | ||
readonly IIssuesLabelsClient _issuesLabelsClient; | ||
readonly IIssuesClient _issuesClient; | ||
readonly Repository _repository; | ||
readonly string _repositoryOwner; | ||
readonly string _repositoryName; | ||
|
||
public IssuesLabelsClientTests() | ||
{ | ||
_gitHubClient = new GitHubClient(new ProductHeaderValue("OctokitTests")) | ||
{ | ||
Credentials = Helper.Credentials | ||
}; | ||
_issuesLabelsClient= _gitHubClient.Issue.Labels; | ||
_issuesClient = _gitHubClient.Issue; | ||
var repoName = Helper.MakeNameWithTimestamp("public-repo"); | ||
|
||
_repository = _gitHubClient.Repository.Create(new NewRepository { Name = repoName }).Result; | ||
_repositoryOwner = _repository.Owner.Login; | ||
_repositoryName = _repository.Name; | ||
} | ||
|
||
[IntegrationTest] | ||
public async Task CanListLabelsForAnIssue() | ||
{ | ||
var newIssue = new NewIssue("A test issue") { Body = "A new unassigned issue" }; | ||
var newLabel = new NewLabel("test label", "#FFFFFF"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The color value must not contain the leading Can we do some validation in here to ensure users supply this in the correct format? |
||
|
||
var label = _issuesLabelsClient.Create(_repositoryOwner, _repository.Name, newLabel).Result; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You can just |
||
var issue = await _issuesClient.Create(_repositoryOwner, _repositoryName, newIssue); | ||
|
||
var issueLabelsInfo = await _issuesLabelsClient.GetForIssue(_repositoryOwner, _repositoryName, issue.Number); | ||
Assert.Empty(issueLabelsInfo); | ||
|
||
var issueUpdate = new IssueUpdate(); | ||
issueUpdate.Labels.Add(label.Name); | ||
var updated = _issuesClient.Update(_repositoryOwner, _repository.Name, issue.Number, issueUpdate) | ||
.Result; | ||
Assert.NotNull(updated); | ||
issueLabelsInfo = await _issuesLabelsClient.GetForIssue(_repositoryOwner, _repositoryName, issue.Number); | ||
|
||
Assert.Equal(1, issueLabelsInfo.Count); | ||
Assert.Equal(newLabel.Color, issueLabelsInfo[0].Color); | ||
} | ||
|
||
[IntegrationTest] | ||
public async Task CanListIssueLabelsForARepository() | ||
{ | ||
// create 2 new issues | ||
var newIssue1 = new NewIssue("A test issue1") { Body = "Everything's coming up Millhouse" }; | ||
var newLabel1 = new NewLabel("test label 1", "#FFFFFF"); | ||
var newLabel2 = new NewLabel("test label 2", "#FFFFFF"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As above - drop the |
||
|
||
var issue1 = await _issuesClient.Create(_repositoryOwner, _repository.Name, newIssue1); | ||
|
||
var label1 = _issuesLabelsClient.Create(_repositoryOwner, _repository.Name, newLabel1).Result; | ||
var label2 = _issuesLabelsClient.Create(_repositoryOwner, _repository.Name, newLabel2).Result; | ||
|
||
// close and open issue1 | ||
var issueUpdate = new IssueUpdate(); | ||
issueUpdate.Labels.Add(label1.Name); | ||
issueUpdate.Labels.Add(label2.Name); | ||
var updated1 = _issuesClient.Update(_repositoryOwner, _repository.Name, issue1.Number, issueUpdate) | ||
.Result; | ||
Assert.NotNull(updated1); | ||
|
||
var issueLabels = await _issuesLabelsClient.GetForRepository(_repositoryOwner, _repositoryName); | ||
|
||
Assert.Equal(2, issueLabels.Count); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So there's something in here where a new repository has a bunch of default labels for issues. So we get 8 instead of 2 in this test. I'll look into whether there's a way to create an actually "empty" repository... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. cc @pengwynn I don't see an option here I can specify to make the repository not have the default labels There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No such option exists in the API or on the web site. |
||
} | ||
|
||
[IntegrationTest] | ||
public async Task CanRetrieveIssueLabelByName() | ||
{ | ||
var newLabel = new NewLabel("test label 1b", "#FFFFFF"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And one last |
||
var label = _issuesLabelsClient.Create(_repositoryOwner, _repository.Name, newLabel).Result; | ||
Assert.NotNull(label); | ||
|
||
var issueLabelLookupByName = await _issuesLabelsClient.Get(_repositoryOwner, _repositoryName, label.Name); | ||
|
||
Assert.Equal(label.Name, issueLabelLookupByName.Name); | ||
Assert.Equal(label.Color, issueLabelLookupByName.Color); | ||
} | ||
|
||
public void Dispose() | ||
{ | ||
Helper.DeleteRepo(_repository); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,83 @@ | ||
using System; | ||
using System.Threading.Tasks; | ||
using NSubstitute; | ||
using Octokit.Tests.Helpers; | ||
using Xunit; | ||
|
||
namespace Octokit.Tests.Clients | ||
{ | ||
public class IssuesLabelsClientTests | ||
{ | ||
public class TheGetForIssueMethod | ||
{ | ||
[Fact] | ||
public async Task RequestsCorrectUrl() | ||
{ | ||
var connection = Substitute.For<IApiConnection>(); | ||
var client = new IssuesLabelsClient(connection); | ||
|
||
await client.GetForIssue("fake", "repo", 42); | ||
|
||
connection.Received().GetAll<EventInfo>(Arg.Is<Uri>(u => u.ToString() == "repos/fake/repo/issues/42/labels")); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When you're testing NSubstitute mocks, you'll need to ensure the right generic parameters are specified. So these tests should look for |
||
} | ||
|
||
[Fact] | ||
public async Task EnsuresNonNullArguments() | ||
{ | ||
var client = new IssuesLabelsClient(Substitute.For<IApiConnection>()); | ||
|
||
await AssertEx.Throws<ArgumentNullException>(async () => await client.Get(null, "name", "label")); | ||
await AssertEx.Throws<ArgumentNullException>(async () => await client.Get("owner", null, "label")); | ||
await AssertEx.Throws<ArgumentNullException>(async () => await client.Get("owner", "name", null)); | ||
} | ||
} | ||
|
||
public class TheGetForRepositoryMethod | ||
{ | ||
[Fact] | ||
public async Task RequestsCorrectUrl() | ||
{ | ||
var connection = Substitute.For<IApiConnection>(); | ||
var client = new IssuesLabelsClient(connection); | ||
|
||
await client.GetForRepository("fake", "repo"); | ||
|
||
connection.Received().GetAll<IssueEvent>(Arg.Is<Uri>(u => u.ToString() == "repos/fake/repo/issues/labels")); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same as above - check for Oh, and the URL should be |
||
} | ||
|
||
[Fact] | ||
public async Task EnsuresNonNullArguments() | ||
{ | ||
var client = new IssuesLabelsClient(Substitute.For<IApiConnection>()); | ||
|
||
await AssertEx.Throws<ArgumentNullException>(async () => await client.Get(null, "name", "label")); | ||
await AssertEx.Throws<ArgumentNullException>(async () => await client.Get("owner", null, "label")); | ||
await AssertEx.Throws<ArgumentNullException>(async () => await client.Get("owner", "name", null)); | ||
} | ||
} | ||
|
||
public class TheGetMethod | ||
{ | ||
[Fact] | ||
public void RequestsCorrectUrl() | ||
{ | ||
var connection = Substitute.For<IApiConnection>(); | ||
var client = new IssuesLabelsClient(connection); | ||
|
||
client.Get("fake", "repo", "label"); | ||
|
||
connection.Received().Get<IssueEvent>(Arg.Is<Uri>(u => u.ToString() == "repos/fake/repo/issues/labels/label"), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Lucky last - switch to |
||
null); | ||
} | ||
|
||
[Fact] | ||
public async Task EnsuresNonNullArguments() | ||
{ | ||
var client = new IssuesLabelsClient(Substitute.For<IApiConnection>()); | ||
|
||
await AssertEx.Throws<ArgumentNullException>(async () => await client.Get(null, "name", "label")); | ||
await AssertEx.Throws<ArgumentNullException>(async () => await client.Get("owner", null, "label")); | ||
} | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,83 @@ | ||
using System.Collections.Generic; | ||
using System.Diagnostics.CodeAnalysis; | ||
using System.Threading.Tasks; | ||
|
||
namespace Octokit | ||
{ | ||
public interface IIssuesLabelsClient | ||
{ | ||
/// <summary> | ||
/// Gets all labels for the issue. | ||
/// </summary> | ||
/// <remarks> | ||
/// See the <a href="http://developer.github.com/v3/issues/labels/#list-labels-on-an-issue">API documentation</a> for more information. | ||
/// </remarks> | ||
/// <param name="owner">The owner of the repository</param> | ||
/// <param name="repo">The name of the repository</param> | ||
/// <param name="number">The number of the issue</param> | ||
/// <returns>The list of labels</returns> | ||
Task<IReadOnlyList<Label>> GetForIssue(string owner, string repo, int number); | ||
|
||
/// <summary> | ||
/// Gets all labels for the repository. | ||
/// </summary> | ||
/// <remarks> | ||
/// See the <a href="http://developer.github.com/v3/issues/labels/#list-all-labels-for-this-repository">API documentation</a> for more information. | ||
/// </remarks> | ||
/// <param name="owner">The owner of the repository</param> | ||
/// <param name="repo">The name of the repository</param> | ||
/// <returns>The list of labels</returns> | ||
Task<IReadOnlyList<Label>> GetForRepository(string owner, string repo); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It'd be great if you could document these signatures like how we document the other clients. There's probably things in here which aren't relevant to your use case...
|
||
|
||
/// <summary> | ||
/// Gets a single Label by name. | ||
/// </summary> | ||
/// <remarks> | ||
/// See the <a href="http://developer.github.com/v3/issues/labels/#get-a-single-label">API documentation</a> for more information. | ||
/// </remarks> | ||
/// <param name="owner">The owner of the repository</param> | ||
/// <param name="repo">The name of the repository</param> | ||
/// <param name="name">The name of the label</param> | ||
/// <returns>The label</returns> | ||
[SuppressMessage("Microsoft.Naming", "CA1716:IdentifiersShouldNotMatchKeywords", MessageId = "Get", | ||
Justification = "Method makes a network request")] | ||
Task<Label> Get(string owner, string repo, string name); | ||
|
||
/// <summary> | ||
/// Deletes a label. | ||
/// </summary> | ||
/// <remarks> | ||
/// See the <a href="http://developer.github.com/v3/issues/labels/#delete-a-label">API documentation</a> for more information. | ||
/// </remarks> | ||
/// <param name="owner">The owner of the repository</param> | ||
/// <param name="repo">The name of the repository</param> | ||
/// <param name="name">The name of the label</param> | ||
/// <returns></returns> | ||
Task Delete(string owner, string repo, string name); | ||
|
||
/// <summary> | ||
/// Creates a label. | ||
/// </summary> | ||
/// <remarks> | ||
/// See the <a href="http://developer.github.com/v3/issues/labels/#create-a-label">API documentation</a> for more information. | ||
/// </remarks> | ||
/// <param name="owner">The owner of the repository</param> | ||
/// <param name="repo">The name of the repository</param> | ||
/// <param name="newLabel">The data for the label to be created</param> | ||
/// <returns>The created label</returns> | ||
Task<Label> Create(string owner, string repo, NewLabel newLabel); | ||
|
||
/// <summary> | ||
/// Updates a label. | ||
/// </summary> | ||
/// <remarks> | ||
/// See the <a href="http://developer.github.com/v3/issues/labels/#update-a-label">API documentation</a> for more information. | ||
/// </remarks> | ||
/// <param name="owner">The owner of the repository</param> | ||
/// <param name="repo">The name of the repository</param> | ||
/// <param name="name">The name of the label</param> | ||
/// <param name="labelUpdate">The data for the label to be updated</param> | ||
/// <returns>The updated label</returns> | ||
Task<Label> Update(string owner, string repo, string name, LabelUpdate labelUpdate); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't forget to implement
IDisposable
so we clean up the repository correctly after the test (this is just an xUnit convention)