diff --git a/app/workers/commit_monitor_handlers/commit_range/rubocop_checker.rb b/app/workers/commit_monitor_handlers/commit_range/rubocop_checker.rb index 23fdd432..1392ca45 100644 --- a/app/workers/commit_monitor_handlers/commit_range/rubocop_checker.rb +++ b/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) diff --git a/lib/github_service.rb b/lib/github_service.rb index 8e17597d..98f74383 100644 --- a/lib/github_service.rb +++ b/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) diff --git a/spec/lib/github_service_spec.rb b/spec/lib/github_service_spec.rb index efac6e8a..45683759 100644 --- a/spec/lib/github_service_spec.rb +++ b/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 diff --git a/spec/workers/commit_monitor_handlers/commit_range/rubocop_checker_spec.rb b/spec/workers/commit_monitor_handlers/commit_range/rubocop_checker_spec.rb new file mode 100644 index 00000000..e11c719e --- /dev/null +++ b/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