Use aggregate response if both comment/status are checked

Author: noahd1

lib/cc/services/github_pull_requests.rb

@@ -13,6 +13,25 @@ class Config < CC::Service::Config
     validates :oauth_token, presence: true
   end
 
+  class ResponseAggregator
+    def initialize(status_response, comment_response)
+      @status_response = status_response
+      @comment_response = comment_response
+    end
+
+    def response
+      return @status_response if @status_response[:ok] && @comment_response[:ok]
+      message = if !@status_response[:ok] && !@comment_response[:ok]
+        "Unable to post comment or update status"
+      elsif !@status_response[:ok]
+        "Unable to update status: #{@status_response[:message]}"
+      elsif !@comment_response[:ok]
+        "Unable to post comment: #{@comment_response[:message]}"
+      end
+      { ok: false, message: message }
+    end
+  end
+
   self.title = "GitHub Pull Requests"
   self.description = "Update pull requests on GitHub"
 
@@ -24,7 +43,9 @@ class Config < CC::Service::Config
   # additional information (github-slug, PR number, etc) we can't test much
   # else.
   def receive_test
-    if config.update_status
+    if config.update_status && config.add_comment
+      ResponseAggregator.new(receive_test_status, receive_test_comment).response
+    elsif config.update_status
       receive_test_status
     elsif config.add_comment
       receive_test_comment

test/github_pull_requests_test.rb

@@ -57,6 +57,36 @@ def test_pull_request_comment_test_failure_bad_token
     assert !receive_test({ add_comment: true })[:ok], "Expected failed test of pull request"
   end
 
+  def test_pull_request_success_both
+    @stubs.post("/repos/pbrisbin/foo/statuses/#{"0" * 40}") { |env| [422, {}, ""] }
+    @stubs.get("/user") { |env| [200, { "x-oauth-scopes" => "gist, user, repo" }, ""] }
+
+    assert receive_test({ update_status: true, add_comment: true }, { github_slug: "pbrisbin/foo" })[:ok], "Expected test of pull request to be true"
+  end
+
+  def test_response_aggregator_success
+    response = aggregrate_response({ok: true, message: "OK"}, {ok: true, message: "OK 2"})
+    assert_equal response, { ok: true, message: "OK" }
+  end
+
+  def test_response_aggregator_failure_both
+    response = aggregrate_response({ok: false, message: "Bad Token"}, {ok: false, message: "Bad Stuff"})
+    assert_equal response, { ok: false, message: "Unable to post comment or update status" }
+  end
+
+  def test_response_aggregator_failure_status
+    response = aggregrate_response({ok: false, message: "Bad Token"}, {ok: true, message: "OK"})
+    assert !response[:ok], "Expected invalid response because status response is invalid"
+    assert_match /Bad Token/, response[:message]
+  end
+
+
+  def test_response_aggregator_failure_status
+    response = aggregrate_response({ok: true, message: "OK"}, {ok: false, message: "Bad Stuff"})
+    assert !response[:ok], "Expected invalid response because comment response is invalid"
+    assert_match /Bad Stuff/, response[:message]
+  end
+
   def test_pull_request_comment
     stub_existing_comments("pbrisbin/foo", 1, %w[Hey Yo])
 
@@ -130,4 +160,8 @@ def receive_test(config, event_data = {})
     )
   end
 
+  def aggregrate_response(status_response, comment_response)
+    CC::Service::GitHubPullRequests::ResponseAggregator.new(status_response, comment_response).response
+  end
+
 end