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..67ee4b8a 100644 --- a/app/workers/commit_monitor_handlers/commit_range/rubocop_checker.rb +++ b/app/workers/commit_monitor_handlers/commit_range/rubocop_checker.rb @@ -39,7 +39,10 @@ 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| + + status = results["files"].select { |f| f["offenses"].any? }.empty? + + GithubService.replace_comments(fq_repo_name, pr_number, rubocop_comments, status, commits.last) do |old_comment| rubocop_comment?(old_comment) end end diff --git a/lib/github_service.rb b/lib/github_service.rb index 133b8d6c..2ebe8226 100644 --- a/lib/github_service.rb +++ b/lib/github_service.rb @@ -12,11 +12,25 @@ 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 + state = commit_state ? "success" : "error" + options = { + "context" => Settings.github_credentials.username, + "target_url" => comment_url, + "description" => commit_state ? "Everything looks fine." : "Something went wrong." + } + + 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 +39,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 + unless first_comment.nil? && status.nil? + 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 new file mode 100644 index 00000000..29341ca9 --- /dev/null +++ b/spec/lib/github_service_spec.rb @@ -0,0 +1,55 @@ +require 'spec_helper' + +RSpec.describe GithubService do + let(:repo) { "owner/repository" } + let(:sha) { "cd16q9z48b3b0xbb91wb311" } + let(:url) { "https://github.com/" } + let(:num) { 42 } + + describe ".add_status" do + let(:username) { "name" } + + before do + stub_settings(Hash(:github_credentials => {:username => username})) + end + + it "should create a success state" 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, true) + end + + it "should create an error state" 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, false) + 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.rb b/spec/workers/commit_monitor_handlers/commit_range/rubocop_checker.rb new file mode 100644 index 00000000..9c76ebd8 --- /dev/null +++ b/spec/workers/commit_monitor_handlers/commit_range/rubocop_checker.rb @@ -0,0 +1,39 @@ +require 'spec_helper' + +describe CommitMonitorHandlers::CommitRange::RubocopChecker do + describe "#replace_rubocop_comments" do + let(:num) { 42 } + let(:repo) { "owner/repository" } + let(:comments) { ["comment1","comment2"] } + let(:sha) { "cd16q9z48b3b0xbb91wb311" } + let(:commits) { ["x","y",sha] } + let(:results_true) { Hash("files"=> [ Hash("offenses"=> []), Hash("offenses"=> [])]) } + let(:results_false) { Hash("files"=> [ Hash("offenses"=> ["a","b"]), Hash("offenses"=> ["c","d"])]) } + + it "should call replace_comments with true status" do + allow(subject).to receive(:results).and_return(results_true) + + 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) + + expect(GithubService).to receive(:replace_comments).with(repo, num, comments, true, sha).once + + subject.send(:replace_rubocop_comments) + end + + it "should call replace_comments with true status" do + allow(subject).to receive(:results).and_return(results_false) + + 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) + + expect(GithubService).to receive(:replace_comments).with(repo, num, comments, false, sha).once + + subject.send(:replace_rubocop_comments) + end + end +end