From 2fd3afbef3d26647fcbee46dbf11bee2bbaa7fbb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20T=C3=B3th?= Date: Wed, 7 Mar 2018 17:18:14 +0100 Subject: [PATCH] GitHub Status API Integration Commits are marked with [success/error] state depending on offence detection. If there are any offences in the code, the commit is marked with error state otherwise with success state. It is reflected at the end of PR in the check conclusion part. --- .../commit_range/rubocop_checker.rb | 13 ++++- lib/github_service.rb | 35 ++++++++++- spec/lib/github_service_spec.rb | 58 +++++++++++++++++++ .../commit_range/rubocop_checker_spec.rb | 44 ++++++++++++++ 4 files changed, 144 insertions(+), 6 deletions(-) create mode 100644 spec/workers/commit_monitor_handlers/commit_range/rubocop_checker_spec.rb 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