GitHub Status API Integration (rebased)

app/workers/commit_monitor_handlers/commit_range/rubocop_checker.rb

@@ -39,9 +39,16 @@ def rubocop_comments
 
   def replace_rubocop_comments
     logger.info("Updating PR #{pr_number} with rubocop comment.")
-    GithubService.replace_comments(fq_repo_name, pr_number, rubocop_comments) do |old_comment|
-      rubocop_comment?(old_comment)
-    end
+
+    type = if results["files"].none? { |f| f["offenses"].any? } # zero offenses - green status
+             :zero
+           elsif results["files"].any? { |f| f["offenses"].any? { |o| o["severity"] == "error" || o["severity"] == "fatal" } } # catastrophic offense(s) - red status
+             :bomb
+           else # informative offenses excluding catastrophic offense(s) - green status
+             :warn
+           end
+
+    GithubService.replace_comments(fq_repo_name, pr_number, rubocop_comments, type, commits.last) { |old_comment| rubocop_comment?(old_comment) }
   end
 
   def rubocop_comment?(comment)

lib/github_service.rb

@@ -12,11 +12,35 @@ module GithubService
   #
   class << self
     def add_comments(fq_repo_name, issue_number, comments)
-      Array(comments).each do |comment|
+      Array(comments).map do |comment|
         add_comment(fq_repo_name, issue_number, comment)
       end
     end
 
+    # https://octokit.github.io/octokit.rb/Octokit/Client/Statuses.html#create_status-instance_method
+    def add_status(fq_repo_name, commit_sha, comment_url, commit_state)
+      repo    = fq_repo_name
+      sha     = commit_sha
+      options = {
+        "context"    => Settings.github_credentials.username,
+        "target_url" => comment_url,
+      }
+
+      case commit_state
+      when :zero
+        state = "success"
+        options["description"] = "Everything looks fine."
+      when :warn
+        state = "success"
+        options["description"] = "Some offenses detected."
+      when :bomb
+        state = "error"
+        options["description"] = "Something went wrong."
+      end
+
+      create_status(repo, sha, state, options)
+    end
+
     def delete_comments(fq_repo_name, comment_ids)
       Array(comment_ids).each do |comment_id|
         delete_comment(fq_repo_name, comment_id)
@@ -25,12 +49,17 @@ def delete_comments(fq_repo_name, comment_ids)
 
     # Deletes the issue comments found by the provided block, then creates new
     # issue comments from those provided.
-    def replace_comments(fq_repo_name, issue_number, new_comments)
+    def replace_comments(fq_repo_name, issue_number, new_comments, status = nil, commit_sha = nil)
       raise "no block given" unless block_given?
 
       to_delete = issue_comments(fq_repo_name, issue_number).select { |c| yield c }
       delete_comments(fq_repo_name, to_delete.map(&:id))
-      add_comments(fq_repo_name, issue_number, new_comments)
+      first_comment = add_comments(fq_repo_name, issue_number, new_comments).first
+
+      # add_status creates a commit status pointing to the first comment URL
+      if first_comment && status && commit_sha
+        add_status(fq_repo_name, commit_sha, first_comment["html_url"], status)
+      end
     end
 
     def issue(*args)

spec/lib/github_service_spec.rb

@@ -1,4 +1,9 @@
 describe GithubService do
+  let(:repo) { "owner/repository" }
+  let(:sha) { "b0e6911a4b7cc8dcf6aa4ed28244a1d5fb90d051" }
+  let(:url) { "https://github.com/" }
+  let(:num) { 42 }
+
   describe "#username_lookup" do
     let(:lookup_username) { "NickLaMuro" }
     let(:lookup_status)   { 200 }
@@ -60,4 +65,57 @@ def lookup_cache
       end
     end
   end
+
+  describe ".add_status" do
+    let(:username) { "name" }
+
+    before do
+      stub_settings(Hash(:github_credentials => {:username => username}))
+    end
+
+    it "should create a success state (zero offenses)" do
+      expect(described_class).to receive(:create_status).with(repo, sha, "success", Hash("context" => username, "target_url" => url, "description" => "Everything looks fine."))
+
+      described_class.add_status(repo, sha, url, :zero)
+    end
+
+    it "should create an success state (some offenses)" do
+      expect(described_class).to receive(:create_status).with(repo, sha, "success", Hash("context" => username, "target_url" => url, "description" => "Some offenses detected."))
+
+      described_class.add_status(repo, sha, url, :warn)
+    end
+
+    it "should create an error state (at least one bomb offense)" do
+      expect(described_class).to receive(:create_status).with(repo, sha, "error", Hash("context" => username, "target_url" => url, "description" => "Something went wrong."))
+
+      described_class.add_status(repo, sha, url, :bomb)
+    end
+  end
+
+  describe ".add_comments" do
+    let(:comment_in) { "input_comment" }
+    let(:comments_in) { [comment_in, comment_in, comment_in] }
+    let(:comment_out) { Hash("html_url"=> url) }
+
+    it "should return an array of comments" do
+      expect(described_class).to receive(:add_comment).with(repo, num, comment_in).and_return(comment_out).exactly(comments_in.count)
+      expect(described_class.add_comments(repo, num, comments_in)).to match_array([comment_out, comment_out, comment_out])
+    end
+  end
+
+  describe ".replace_comments" do
+    let(:comments_in) { %w(input_comment input_comment input_comment) }
+    let(:comment_to_delete) { double("comment_to_delete", :id => "comment_id") }
+    let(:comments_to_delete) { [comment_to_delete, comment_to_delete, comment_to_delete] }
+    let(:comments_out) { [Hash("html_url"=> url), Hash("key"=> "value")] }
+
+    it "should add commit status" do
+      expect(described_class).to receive(:issue_comments).with(repo, num).and_return(comments_to_delete)
+      expect(described_class).to receive(:delete_comments).with(repo, comments_to_delete.map(&:id))
+      expect(described_class).to receive(:add_comments).with(repo, num, comments_in).and_return(comments_out)
+      expect(described_class).to receive(:add_status).with(repo, sha, url, true).once
+
+      described_class.replace_comments(repo, num, comments_in, true, sha) { "something" }
+    end
+  end
 end

spec/workers/commit_monitor_handlers/commit_range/rubocop_checker_spec.rb

@@ -0,0 +1,44 @@
+require 'spec_helper'
+
+describe CommitMonitorHandlers::CommitRange::RubocopChecker do
+  describe "#replace_rubocop_comments" do
+    let(:num) { 42 }
+    let(:repo) { "owner/repository" }
+    let(:comments) { %w(comment1 comment2) }
+    let(:sha) { "b0e6911a4b7cc8dcf6aa4ed28244a1d5fb90d051" }
+    let(:commits) { ["x", "y", sha] }
+
+    let(:results_no_status) { Hash("files"=> [Hash("offenses"=> [Hash("severity"=>"warn"), Hash("severity"=>"info")]), Hash("offenses"=> [Hash("severity"=>"unknown"), Hash("severity"=>"info")])]) }
+    let(:results_red_status) { Hash("files"=> [Hash("offenses"=> [Hash("severity"=>"warn"), Hash("severity"=>"error")]), Hash("offenses"=> [Hash("severity"=>"fatal"), Hash("severity"=>"info")])]) }
+    let(:results_green_status) { Hash("files"=> [Hash("offenses"=> []), Hash("offenses"=> [])]) }
+
+    before do
+      allow(subject).to receive(:fq_repo_name).and_return(repo)
+      allow(subject).to receive(:pr_number).and_return(num)
+      allow(subject).to receive(:rubocop_comments).and_return(comments)
+      allow(subject).to receive(:commits).and_return(commits)
+    end
+
+    after do
+      subject.send(:replace_rubocop_comments)
+    end
+
+    it "should call replace_comments with :zero (green - success) status" do
+      allow(subject).to receive(:results).and_return(results_green_status)
+
+      expect(GithubService).to receive(:replace_comments).with(repo, num, comments, :zero, sha).once
+    end
+
+    it "should call replace_comments with :bomb (red - error) status" do
+      allow(subject).to receive(:results).and_return(results_red_status)
+
+      expect(GithubService).to receive(:replace_comments).with(repo, num, comments, :bomb, sha).once
+    end
+
+    it "should call replace_comments with :warn (green - success) status" do
+      allow(subject).to receive(:results).and_return(results_no_status)
+
+      expect(GithubService).to receive(:replace_comments).with(repo, num, comments, :warn, sha).once
+    end
+  end
+end