Merge pull request #56 from codeclimate/jy-do-not-allow-nil-branches

Give an OK of false if the PR comment exists

Author: Jon Yurek

lib/cc/service/invocation.rb

@@ -2,12 +2,14 @@
 require 'cc/service/invocation/with_retries'
 require 'cc/service/invocation/with_metrics'
 require 'cc/service/invocation/with_error_handling'
+require 'cc/service/invocation/with_return_values'
 
 class CC::Service::Invocation
   MIDDLEWARE = {
     retries: WithRetries,
     metrics: WithMetrics,
     error_handling: WithErrorHandling,
+    return_values: WithReturnValues,
   }
 
   attr_reader :result

lib/cc/service/invocation/with_return_values.rb

@@ -0,0 +1,18 @@
+class CC::Service::Invocation
+  class WithReturnValues
+    def initialize(invocation, message = nil)
+      @invocation = invocation
+      @message = message || "An internal error happened"
+    end
+
+    def call
+      result = @invocation.call
+      if result.nil?
+        { ok: false, message: @message }
+      else
+        result
+      end
+    end
+  end
+end
+

lib/cc/services/github_pull_requests.rb

@@ -85,19 +85,17 @@ def update_status_pending
   end
 
   def update_status(state, description)
-    if config.update_status
-      params = {
-        state:       state,
-        description: description,
-        target_url:  @payload["details_url"],
-        context:     "codeclimate"
-      }
-      service_post(status_url, params.to_json)
-    end
+    params = {
+      state:       state,
+      description: description,
+      target_url:  @payload["details_url"],
+      context:     "codeclimate"
+    }
+    service_post(status_url, params.to_json)
   end
 
   def add_comment
-    if config.add_comment && !comment_present?
+    if !comment_present?
       body = {
         body: COMMENT_BODY % @payload["compare_url"]
       }.to_json
@@ -106,6 +104,8 @@ def add_comment
         doc = JSON.parse(response.body)
         { id: doc["id"] }
       end
+    else
+      { ok: false, message: "Comment already present" }
     end
   end
 

test/github_pull_requests_test.rb

@@ -38,7 +38,6 @@ def test_pull_request_status_error
       commit_sha:  "abc123",
       state:       "error",
     })
-
   end
 
   def test_pull_request_status_test_success
@@ -115,11 +114,13 @@ def test_pull_request_comment_already_present
 
     # With no POST expectation, test will fail if request is made.
 
-    receive_pull_request({ add_comment: true }, {
+    rsp = receive_pull_request({ add_comment: true, update_status: false }, {
       github_slug: "pbrisbin/foo",
       number:      1,
       state:       "success",
     })
+    assert_not_nil rsp
+    assert_false rsp[:ok]
   end
 
 private

test/invocation_return_values_test.rb

@@ -0,0 +1,21 @@
+require File.expand_path('../helper', __FILE__)
+
+class InvocationReturnValuesTest < CC::Service::TestCase
+  def test_success_returns_upstream_result
+    handler = CC::Service::Invocation::WithReturnValues.new(
+      lambda { :return_value },
+      "error message"
+    )
+
+    assert_equal :return_value, handler.call
+  end
+
+  def test_empty_results_returns_hash
+    handler = CC::Service::Invocation::WithReturnValues.new(
+      lambda { nil },
+      "error message"
+    )
+
+    assert_equal( {ok: false, message: "error message"}, handler.call )
+  end
+end

test/invocation_test.rb

@@ -10,6 +10,28 @@ def test_success
     assert_equal :some_result, result
   end
 
+  def test_success_with_return_values
+    service = FakeService.new(:some_result)
+
+    result = CC::Service::Invocation.invoke(service) do |i|
+      i.with :return_values, "error"
+    end
+
+    assert_equal 1, service.receive_count
+    assert_equal :some_result, result
+  end
+
+  def test_failure_with_return_values
+    service = FakeService.new(nil)
+
+    result = CC::Service::Invocation.invoke(service) do |i|
+      i.with :return_values, "error"
+    end
+
+    assert_equal 1, service.receive_count
+    assert_equal( {ok: false, message: "error"}, result )
+  end
+
   def test_retries
     service = FakeService.new
     service.fake_error = RuntimeError.new