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.

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

Merged
merged 10 commits into from
Dec 8, 2013
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
using Octokit.Tests.Integration;
using Xunit;

public class IssuesEventsClientTests
public class IssuesEventsClientTests : IDisposable
{
readonly IGitHubClient _gitHubClient;
readonly IIssuesEventsClient _issuesEventsClientClient;
Expand Down
101 changes: 101 additions & 0 deletions Octokit.Tests.Integration/Clients/IssuesLabelsClientTests.cs
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 : IDisposable
{
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");
Copy link
Member

Choose a reason for hiding this comment

The 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;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can just await on these - given we're inside an async Task test

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");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As above - drop the # from these tests


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);
Copy link
Member

Choose a reason for hiding this comment

The 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...

Copy link
Member

Choose a reason for hiding this comment

The 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

http://developer.github.com/v3/repos/#create

Copy link

Choose a reason for hiding this comment

The 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");
Copy link
Member

Choose a reason for hiding this comment

The 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);
}
}
1 change: 1 addition & 0 deletions Octokit.Tests.Integration/Octokit.Tests.Integration.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@
<Compile Include="Clients\BlobClientTests.cs" />
<Compile Include="Clients\CommitsClientTests.cs" />
<Compile Include="Clients\CommitStatusClientTests.cs" />
<Compile Include="Clients\IssuesLabelsClientTests.cs" />
<Compile Include="Clients\GistsClientTests.cs" />
<Compile Include="Clients\IssuesEventsClientTests.cs" />
<Compile Include="Clients\MilestonesClientTests.cs" />
Expand Down
83 changes: 83 additions & 0 deletions Octokit.Tests/Clients/IssuesLabelsClientTests.cs
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"));
Copy link
Member

Choose a reason for hiding this comment

The 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 <Label> - that's one of the reasons why these tests fail

}

[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"));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above - check for <Label> here.

Oh, and the URL should be /repos/:owner/:repo/labels - drop the /issues/ part

}

[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"),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lucky last - switch to <Label> and drop /issues/ from the URL

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"));
}
}
}
}
1 change: 1 addition & 0 deletions Octokit.Tests/Octokit.Tests.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@
<Compile Include="Clients\GistCommentsClientTests.cs" />
<Compile Include="Clients\GistsClientTests.cs" />
<Compile Include="Clients\BlobClientTests.cs" />
<Compile Include="Clients\IssuesLabelsClientTests.cs" />
<Compile Include="Clients\ReferencesClientTests.cs" />
<Compile Include="Clients\RepoCollaboratorsClientTests.cs" />
<Compile Include="Clients\EventsClientTests.cs" />
Expand Down
5 changes: 5 additions & 0 deletions Octokit/Clients/IIssuesClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,11 @@ public interface IIssuesClient
/// </summary>
IMilestonesClient Milestone { get; }

/// <summary>
/// Client for managing labels.
/// </summary>
IIssuesLabelsClient Labels { get; }

/// <summary>
/// Client for managing comments.
/// </summary>
Expand Down
83 changes: 83 additions & 0 deletions Octokit/Clients/IIssuesLabelsClient.cs
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);
Copy link
Member

Choose a reason for hiding this comment

The 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 specific <see cref="Authorization"/> for the authenticated user.
/// </summary>
/// <remarks>
/// This method requires authentication.
/// See the <a href="http://developer.github.com/v3/oauth/#get-a-single-authorization">API documentation</a> for more information.
/// </remarks>
/// <param name="id">The ID of the <see cref="Authorization"/> to get</param>
/// <exception cref="AuthorizationException">
/// Thrown when the current user does not have permission to make this request.
/// </exception>
/// <exception cref="ApiException">Thrown when a general API error occurs.</exception>
/// <returns>The specified <see cref="Authorization"/>.</returns>


/// <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);
}
}
2 changes: 2 additions & 0 deletions Octokit/Clients/IssuesClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,14 @@ public IssuesClient(IApiConnection apiConnection) : base(apiConnection)
{
Assignee = new AssigneesClient(apiConnection);
Events = new IssuesEventsClient(apiConnection);
Labels = new IssuesLabelsClient(apiConnection);
Milestone = new MilestonesClient(apiConnection);
Comment = new IssueCommentsClient(apiConnection);
}

public IAssigneesClient Assignee { get; private set; }
public IIssuesEventsClient Events { get; private set; }
public IIssuesLabelsClient Labels { get; private set; }
public IMilestonesClient Milestone { get; private set; }
public IIssueCommentsClient Comment { get; private set; }

Expand Down
Loading