Skip to content
This repository was archived by the owner on Jul 19, 2025. It is now read-only.

Commit 5a2a4a3

Browse files
committed
Merge pull request #50 from codeclimate/nd-add-test-for-gh-pr-comment
Check config of GH PR service and test appropriately for status/comment ability
2 parents d2d0c50 + a4ce396 commit 5a2a4a3

File tree

3 files changed

+117
-16
lines changed

3 files changed

+117
-16
lines changed

lib/cc/services/github_pull_requests.rb

Lines changed: 58 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,25 @@ class Config < CC::Service::Config
1313
validates :oauth_token, presence: true
1414
end
1515

16+
class ResponseAggregator
17+
def initialize(status_response, comment_response)
18+
@status_response = status_response
19+
@comment_response = comment_response
20+
end
21+
22+
def response
23+
return @status_response if @status_response[:ok] && @comment_response[:ok]
24+
message = if !@status_response[:ok] && !@comment_response[:ok]
25+
"Unable to post comment or update status"
26+
elsif !@status_response[:ok]
27+
"Unable to update status: #{@status_response[:message]}"
28+
elsif !@comment_response[:ok]
29+
"Unable to post comment: #{@comment_response[:message]}"
30+
end
31+
{ ok: false, message: message }
32+
end
33+
end
34+
1635
self.title = "GitHub Pull Requests"
1736
self.description = "Update pull requests on GitHub"
1837

@@ -26,16 +45,13 @@ class Config < CC::Service::Config
2645
def receive_test
2746
setup_http
2847

29-
http_post(base_status_url("0" * 40), "{}")
30-
31-
rescue HTTPError => ex
32-
if ex.status == 422 # response message: "No commit found for SHA"
33-
{ ok: true, message: "OAuth token is valid" }
34-
else ex.status == 401 # response message: "Bad credentials"
35-
{ ok: false, message: ex.message }
48+
if config.update_status && config.add_comment
49+
ResponseAggregator.new(receive_test_status, receive_test_comment).response
50+
elsif config.update_status
51+
receive_test_status
52+
elsif config.add_comment
53+
receive_test_comment
3654
end
37-
rescue => ex
38-
{ ok: false, message: ex.message }
3955
end
4056

4157
def receive_pull_request
@@ -75,6 +91,31 @@ def add_comment
7591
end
7692
end
7793

94+
def receive_test_status
95+
http_post(base_status_url("0" * 40), "{}")
96+
97+
rescue HTTPError => ex
98+
if ex.status == 422 # response message: "No commit found for SHA"
99+
{ ok: true, message: "OAuth token is valid" }
100+
else ex.status == 401 # response message: "Bad credentials"
101+
{ ok: false, message: ex.message }
102+
end
103+
rescue => ex
104+
{ ok: false, message: ex.message }
105+
end
106+
107+
def receive_test_comment
108+
response = http_get(user_url)
109+
if response_includes_repo_scope?(response)
110+
{ ok: true, message: "OAuth token is valid" }
111+
else
112+
{ ok: false, message: "OAuth token requires 'repo' scope to post comments." }
113+
end
114+
115+
rescue => ex
116+
{ ok: false, message: ex.message }
117+
end
118+
78119
def comment_present?
79120
response = http_get(comments_url)
80121
comments = JSON.parse(response.body)
@@ -100,6 +141,10 @@ def comments_url
100141
"#{BASE_URL}/repos/#{github_slug}/issues/#{number}/comments"
101142
end
102143

144+
def user_url
145+
"#{BASE_URL}/user"
146+
end
147+
103148
def github_slug
104149
@payload.fetch("github_slug")
105150
end
@@ -112,4 +157,8 @@ def number
112157
@payload.fetch("number")
113158
end
114159

160+
def response_includes_repo_scope?(response)
161+
response.headers['x-oauth-scopes'] && response.headers['x-oauth-scopes'].split(/\s*,\s*/).include?("repo")
162+
end
163+
115164
end

service_test.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
# Example:
1212
#
1313
# $ SLACK_WEBHOOK_URL="http://..." bundle exec ruby service_test.rb
14-
# $ GITHUBPULLREQUESTS_OAUTH_TOKEN=06083a4a060d358ca709939b1f00645777661c44 bundle exec ruby service_test.rb
14+
# $ GITHUBPULLREQUESTS_UPDATE_STATUS=false GITHUBPULLREQUESTS_ADD_COMMENT=true GITHUBPULLREQUESTS_OAUTH_TOKEN=06083a4a060d358ca709939b1f00645777661c44 bundle exec ruby service_test.rb
1515
#
1616
# Other Environment variables used:
1717
#
@@ -80,4 +80,4 @@ def test_service(klass, config, payload)
8080
ServiceTest.new(CC::Service::Flowdock, :api_token).test
8181
ServiceTest.new(CC::Service::Jira, :username, :password, :domain, :project_id).test
8282
ServiceTest.new(CC::Service::Asana, :api_key, :workspace_id, :project_id).test
83-
ServiceTest.new(CC::Service::GitHubPullRequests, :oauth_token).test({ github_slug: "codeclimate/codeclimate" })
83+
ServiceTest.new(CC::Service::GitHubPullRequests, :oauth_token, :update_status, :add_comment).test({ github_slug: "codeclimate/codeclimate" })

test/github_pull_requests_test.rb

Lines changed: 57 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -27,16 +27,64 @@ def test_pull_request_status_success
2727
})
2828
end
2929

30-
def test_pull_request_test_success
30+
def test_pull_request_status_test_success
3131
@stubs.post("/repos/pbrisbin/foo/statuses/#{"0" * 40}") { |env| [422, {}, ""] }
3232

33-
assert receive_test({}, { github_slug: "pbrisbin/foo" })[:ok], "Expected test of pull request to be true"
33+
assert receive_test({ update_status: true }, { github_slug: "pbrisbin/foo" })[:ok], "Expected test of pull request to be true"
3434
end
3535

36-
def test_pull_request_test_failure
36+
def test_pull_request_status_test_failure
3737
@stubs.post("/repos/pbrisbin/foo/statuses/#{"0" * 40}") { |env| [401, {}, ""] }
3838

39-
assert !receive_test({}, { github_slug: "pbrisbin/foo" })[:ok], "Expected failed test of pull request"
39+
assert !receive_test({ update_status: true }, { github_slug: "pbrisbin/foo" })[:ok], "Expected failed test of pull request"
40+
end
41+
42+
def test_pull_request_comment_test_success
43+
@stubs.get("/user") { |env| [200, { "x-oauth-scopes" => "gist, user, repo" }, ""] }
44+
45+
assert receive_test({ add_comment: true })[:ok], "Expected test of pull request to be true"
46+
end
47+
48+
def test_pull_request_comment_test_failure_insufficient_permissions
49+
@stubs.get("/user") { |env| [200, { "x-oauth-scopes" => "gist, user" }, ""] }
50+
51+
assert !receive_test({ add_comment: true })[:ok], "Expected failed test of pull request"
52+
end
53+
54+
def test_pull_request_comment_test_failure_bad_token
55+
@stubs.get("/user") { |env| [401, {}, ""] }
56+
57+
assert !receive_test({ add_comment: true })[:ok], "Expected failed test of pull request"
58+
end
59+
60+
def test_pull_request_success_both
61+
@stubs.post("/repos/pbrisbin/foo/statuses/#{"0" * 40}") { |env| [422, {}, ""] }
62+
@stubs.get("/user") { |env| [200, { "x-oauth-scopes" => "gist, user, repo" }, ""] }
63+
64+
assert receive_test({ update_status: true, add_comment: true }, { github_slug: "pbrisbin/foo" })[:ok], "Expected test of pull request to be true"
65+
end
66+
67+
def test_response_aggregator_success
68+
response = aggregrate_response({ok: true, message: "OK"}, {ok: true, message: "OK 2"})
69+
assert_equal response, { ok: true, message: "OK" }
70+
end
71+
72+
def test_response_aggregator_failure_both
73+
response = aggregrate_response({ok: false, message: "Bad Token"}, {ok: false, message: "Bad Stuff"})
74+
assert_equal response, { ok: false, message: "Unable to post comment or update status" }
75+
end
76+
77+
def test_response_aggregator_failure_status
78+
response = aggregrate_response({ok: false, message: "Bad Token"}, {ok: true, message: "OK"})
79+
assert !response[:ok], "Expected invalid response because status response is invalid"
80+
assert_match /Bad Token/, response[:message]
81+
end
82+
83+
84+
def test_response_aggregator_failure_status
85+
response = aggregrate_response({ok: true, message: "OK"}, {ok: false, message: "Bad Stuff"})
86+
assert !response[:ok], "Expected invalid response because comment response is invalid"
87+
assert_match /Bad Stuff/, response[:message]
4088
end
4189

4290
def test_pull_request_comment
@@ -104,12 +152,16 @@ def receive_pull_request(config, event_data)
104152
)
105153
end
106154

107-
def receive_test(config, event_data)
155+
def receive_test(config, event_data = {})
108156
receive(
109157
CC::Service::GitHubPullRequests,
110158
{ oauth_token: "123" }.merge(config),
111159
{ name: "test" }.merge(event_data)
112160
)
113161
end
114162

163+
def aggregrate_response(status_response, comment_response)
164+
CC::Service::GitHubPullRequests::ResponseAggregator.new(status_response, comment_response).response
165+
end
166+
115167
end

0 commit comments

Comments
 (0)