From 56fc85827f0dd03db52f8cc2d9d62ecd6e8bbc8f 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 | 5 +- lib/github_service.rb | 25 ++++++++- spec/lib/github_service_spec.rb | 56 +++++++++++++++++++ 3 files changed, 82 insertions(+), 4 deletions(-) create mode 100644 spec/lib/github_service_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..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..87ddb3e0 --- /dev/null +++ b/spec/lib/github_service_spec.rb @@ -0,0 +1,56 @@ +require 'spec_helper' + +RSpec.describe GithubService do + let(:repo) { "owner/repository" } + let(:sha) { "cd16q9z48b3b0xbb91wb311" } + let(:url) { "https://github.com/" } + let(:num) { 123456 } + + 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) { ["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) { |someblock| "somevalue" } + end + end + +end