Skip to content

Commit 3644c78

Browse files
committed
Fixing the last bugs and tests
1 parent 578a47f commit 3644c78

File tree

4 files changed

+56
-33
lines changed

4 files changed

+56
-33
lines changed

app/config/github.yml

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,3 +29,10 @@ parameters:
2929
- app.subscriber.status_change_by_comment_subscriber
3030
- app.subscriber.needs_review_new_pr_subscriber
3131
- app.subscriber.bug_label_new_issue_subscriber
32+
33+
# used in a functional test
34+
weaverryan/symfony:
35+
subscribers:
36+
- app.subscriber.status_change_by_comment_subscriber
37+
- app.subscriber.needs_review_new_pr_subscriber
38+
- app.subscriber.bug_label_new_issue_subscriber

app/config/services.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ services:
3232

3333
app.github.request_handler:
3434
class: AppBundle\Issues\GitHubRequestHandler
35-
arguments: ['@app.github.labels_api', '@event_dispatcher', '@app.repository_provider', '@service_container']
35+
arguments: ['@event_dispatcher', '@app.repository_provider', '@service_container']
3636

3737
app.repository_provider:
3838
class: AppBundle\Repository\Provider\InMemoryRepositoryProvider

src/AppBundle/Tests/Issues/GitHub/CachedLabelsApiTest.php

Lines changed: 26 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
namespace AppBundle\Tests\Issues\GitHub;
44

55
use AppBundle\Issues\GitHub\CachedLabelsApi;
6+
use AppBundle\Repository\Repository;
67
use Github\Api\Issue\Labels;
78

89
/**
@@ -24,12 +25,20 @@ class CachedLabelsApiTest extends \PHPUnit_Framework_TestCase
2425
*/
2526
private $api;
2627

28+
private $repository;
29+
2730
protected function setUp()
2831
{
2932
$this->backendApi = $this->getMockBuilder('Github\Api\Issue\Labels')
3033
->disableOriginalConstructor()
3134
->getMock();
32-
$this->api = new CachedLabelsApi($this->backendApi, self::USER_NAME.'/'.self::REPO_NAME);
35+
$this->api = new CachedLabelsApi($this->backendApi);
36+
$this->repository = new Repository(
37+
self::USER_NAME,
38+
self::REPO_NAME,
39+
[],
40+
null
41+
);
3342
}
3443

3544
public function testGetIssueLabels()
@@ -43,10 +52,10 @@ public function testGetIssueLabels()
4352
array('name' => 'c'),
4453
));
4554

46-
$this->assertSame(array('a', 'b', 'c'), $this->api->getIssueLabels(1234));
55+
$this->assertSame(array('a', 'b', 'c'), $this->api->getIssueLabels(1234, $this->repository));
4756

4857
// Subsequent access goes to cache
49-
$this->assertSame(array('a', 'b', 'c'), $this->api->getIssueLabels(1234));
58+
$this->assertSame(array('a', 'b', 'c'), $this->api->getIssueLabels(1234, $this->repository));
5059
}
5160

5261
public function testAddIssueLabel()
@@ -58,7 +67,7 @@ public function testAddIssueLabel()
5867
->method('add')
5968
->with(self::USER_NAME, self::REPO_NAME, 1234, 'a');
6069

61-
$this->api->addIssueLabel(1234, 'a');
70+
$this->api->addIssueLabel(1234, 'a', $this->repository);
6271
}
6372

6473
public function testAddIssueLabelUpdatesCache()
@@ -76,11 +85,11 @@ public function testAddIssueLabelUpdatesCache()
7685
->method('add')
7786
->with(self::USER_NAME, self::REPO_NAME, 1234, 'd');
7887

79-
$this->assertSame(array('a', 'b', 'c'), $this->api->getIssueLabels(1234));
88+
$this->assertSame(array('a', 'b', 'c'), $this->api->getIssueLabels(1234, $this->repository));
8089

81-
$this->api->addIssueLabel(1234, 'd');
90+
$this->api->addIssueLabel(1234, 'd', $this->repository);
8291

83-
$this->assertSame(array('a', 'b', 'c', 'd'), $this->api->getIssueLabels(1234));
92+
$this->assertSame(array('a', 'b', 'c', 'd'), $this->api->getIssueLabels(1234, $this->repository));
8493
}
8594

8695
public function testAddIssueLabelIgnoresDuplicate()
@@ -97,11 +106,11 @@ public function testAddIssueLabelIgnoresDuplicate()
97106
$this->backendApi->expects($this->never())
98107
->method('add');
99108

100-
$this->assertSame(array('a', 'b', 'c'), $this->api->getIssueLabels(1234));
109+
$this->assertSame(array('a', 'b', 'c'), $this->api->getIssueLabels(1234, $this->repository));
101110

102-
$this->api->addIssueLabel(1234, 'c');
111+
$this->api->addIssueLabel(1234, 'c', $this->repository);
103112

104-
$this->assertSame(array('a', 'b', 'c'), $this->api->getIssueLabels(1234));
113+
$this->assertSame(array('a', 'b', 'c'), $this->api->getIssueLabels(1234, $this->repository));
105114
}
106115

107116
public function testRemoveIssueLabel()
@@ -113,7 +122,7 @@ public function testRemoveIssueLabel()
113122
->method('remove')
114123
->with(self::USER_NAME, self::REPO_NAME, 1234, 'a');
115124

116-
$this->api->removeIssueLabel(1234, 'a');
125+
$this->api->removeIssueLabel(1234, 'a', $this->repository);
117126
}
118127

119128
public function testRemoveIssueLabelUpdatesCache()
@@ -131,11 +140,11 @@ public function testRemoveIssueLabelUpdatesCache()
131140
->method('remove')
132141
->with(self::USER_NAME, self::REPO_NAME, 1234, 'a');
133142

134-
$this->assertSame(array('a', 'b', 'c'), $this->api->getIssueLabels(1234));
143+
$this->assertSame(array('a', 'b', 'c'), $this->api->getIssueLabels(1234, $this->repository));
135144

136-
$this->api->removeIssueLabel(1234, 'a');
145+
$this->api->removeIssueLabel(1234, 'a', $this->repository);
137146

138-
$this->assertSame(array('b', 'c'), $this->api->getIssueLabels(1234));
147+
$this->assertSame(array('b', 'c'), $this->api->getIssueLabels(1234, $this->repository));
139148
}
140149

141150
public function testRemoveIssueLabelIgnoresUnsetLabel()
@@ -152,10 +161,10 @@ public function testRemoveIssueLabelIgnoresUnsetLabel()
152161
$this->backendApi->expects($this->never())
153162
->method('remove');
154163

155-
$this->assertSame(array('a', 'b', 'c'), $this->api->getIssueLabels(1234));
164+
$this->assertSame(array('a', 'b', 'c'), $this->api->getIssueLabels(1234, $this->repository));
156165

157-
$this->api->removeIssueLabel(1234, 'd');
166+
$this->api->removeIssueLabel(1234, 'd', $this->repository);
158167

159-
$this->assertSame(array('a', 'b', 'c'), $this->api->getIssueLabels(1234));
168+
$this->assertSame(array('a', 'b', 'c'), $this->api->getIssueLabels(1234, $this->repository));
160169
}
161170
}

src/AppBundle/Tests/Issues/GitHub/GitHubStatusApiTest.php

Lines changed: 22 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
use AppBundle\Issues\GitHub\CachedLabelsApi;
66
use AppBundle\Issues\GitHub\GitHubStatusApi;
77
use AppBundle\Issues\Status;
8+
use AppBundle\Repository\Repository;
89

910
/**
1011
* @author Bernhard Schussek <bschussek@gmail.com>
@@ -25,12 +26,23 @@ class GitHubStatusApiTest extends \PHPUnit_Framework_TestCase
2526
*/
2627
private $api;
2728

29+
/**
30+
* @var Repository
31+
*/
32+
private $repository;
33+
2834
protected function setUp()
2935
{
3036
$this->labelsApi = $this->getMockBuilder('AppBundle\Issues\GitHub\CachedLabelsApi')
3137
->disableOriginalConstructor()
3238
->getMock();
33-
$this->api = new GitHubStatusApi($this->labelsApi, self::USER_NAME.'/'.self::REPO_NAME);
39+
$this->api = new GitHubStatusApi($this->labelsApi);
40+
$this->repository = new Repository(
41+
self::USER_NAME,
42+
self::REPO_NAME,
43+
[],
44+
null
45+
);
3446
}
3547

3648
public function testSetIssueStatus()
@@ -48,7 +60,7 @@ public function testSetIssueStatus()
4860
->method('addIssueLabel')
4961
->with(1234, 'Status: Reviewed');
5062

51-
$this->api->setIssueStatus(1234, Status::REVIEWED);
63+
$this->api->setIssueStatus(1234, Status::REVIEWED, $this->repository);
5264
}
5365

5466
public function testSetIssueStatusWithoutPreviousStatus()
@@ -65,7 +77,7 @@ public function testSetIssueStatusWithoutPreviousStatus()
6577
->method('addIssueLabel')
6678
->with(1234, 'Status: Reviewed');
6779

68-
$this->api->setIssueStatus(1234, Status::REVIEWED);
80+
$this->api->setIssueStatus(1234, Status::REVIEWED, $this->repository);
6981
}
7082

7183
public function testSetIssueStatusRemovesExcessStatuses()
@@ -87,7 +99,7 @@ public function testSetIssueStatusRemovesExcessStatuses()
8799
->method('addIssueLabel')
88100
->with(1234, 'Status: Reviewed');
89101

90-
$this->api->setIssueStatus(1234, Status::REVIEWED);
102+
$this->api->setIssueStatus(1234, Status::REVIEWED, $this->repository);
91103
}
92104

93105
public function testSetIssueStatusDoesNothingIfAlreadySet()
@@ -103,7 +115,7 @@ public function testSetIssueStatusDoesNothingIfAlreadySet()
103115
$this->labelsApi->expects($this->never())
104116
->method('addIssueLabel');
105117

106-
$this->api->setIssueStatus(1234, Status::NEEDS_REVIEW);
118+
$this->api->setIssueStatus(1234, Status::NEEDS_REVIEW, $this->repository);
107119
}
108120

109121
public function testSetIssueStatusRemovesExcessLabelsIfAlreadySet()
@@ -120,7 +132,7 @@ public function testSetIssueStatusRemovesExcessLabelsIfAlreadySet()
120132
$this->labelsApi->expects($this->never())
121133
->method('addIssueLabel');
122134

123-
$this->api->setIssueStatus(1234, Status::REVIEWED);
135+
$this->api->setIssueStatus(1234, Status::REVIEWED, $this->repository);
124136
}
125137

126138
public function testSetIssueStatusRemovesUnconfirmedWhenBugIsReviewed()
@@ -142,7 +154,7 @@ public function testSetIssueStatusRemovesUnconfirmedWhenBugIsReviewed()
142154
->method('addIssueLabel')
143155
->with(1234, 'Status: Reviewed');
144156

145-
$this->api->setIssueStatus(1234, Status::REVIEWED);
157+
$this->api->setIssueStatus(1234, Status::REVIEWED, $this->repository);
146158
}
147159

148160
public function testGetIssueStatus()
@@ -152,7 +164,7 @@ public function testGetIssueStatus()
152164
->with(1234)
153165
->willReturn(array('Bug', 'Status: Needs Review'));
154166

155-
$this->assertSame(Status::NEEDS_REVIEW, $this->api->getIssueStatus(1234));
167+
$this->assertSame(Status::NEEDS_REVIEW, $this->api->getIssueStatus(1234, $this->repository));
156168
}
157169

158170
public function testGetIssueStatusReturnsFirst()
@@ -162,7 +174,7 @@ public function testGetIssueStatusReturnsFirst()
162174
->with(1234)
163175
->willReturn(array('Bug', 'Status: Needs Review', 'Status: Reviewed'));
164176

165-
$this->assertSame(Status::NEEDS_REVIEW, $this->api->getIssueStatus(1234));
177+
$this->assertSame(Status::NEEDS_REVIEW, $this->api->getIssueStatus(1234, $this->repository));
166178
}
167179

168180
public function testGetIssueStatusReturnsNullIfNoneSet()
@@ -172,11 +184,6 @@ public function testGetIssueStatusReturnsNullIfNoneSet()
172184
->with(1234)
173185
->willReturn(array('Bug'));
174186

175-
$this->assertNull($this->api->getIssueStatus(1234));
176-
}
177-
178-
public function testGetNeedsReviewUrl()
179-
{
180-
$this->assertSame('https://github.com/weaverryan/carson/labels/Status%3A%20Needs%20Review', $this->api->getNeedsReviewUrl());
187+
$this->assertNull($this->api->getIssueStatus(1234, $this->repository));
181188
}
182189
}

0 commit comments

Comments
 (0)