From 9e68c1777fcca208eb651ad31146c919a3d59f9e Mon Sep 17 00:00:00 2001 From: Camillo Facello <52419977+camillof@users.noreply.github.com> Date: Tue, 28 Jun 2022 16:38:30 -0300 Subject: [PATCH 1/3] Add pull_request_diff_coverage_skipped method --- lib/cc/pull_requests.rb | 13 +++++++++++++ lib/cc/service.rb | 1 + lib/cc/services/github_pull_requests.rb | 5 +++++ spec/cc/service/github_pull_requests_spec.rb | 20 ++++++++++++++++++++ 4 files changed, 39 insertions(+) diff --git a/lib/cc/pull_requests.rb b/lib/cc/pull_requests.rb index c8a8ce6..c7afc69 100644 --- a/lib/cc/pull_requests.rb +++ b/lib/cc/pull_requests.rb @@ -31,6 +31,19 @@ def receive_pull_request_coverage response end + def receive_pull_request_diff_coverage + setup_http + state = @payload["state"] + + if state == "skipped" && report_status? + update_diff_coverage_status_skipped + else + @response = simple_failure("Unknown state") + end + + response + end + private def report_status? diff --git a/lib/cc/service.rb b/lib/cc/service.rb index 8873122..7cfa78a 100644 --- a/lib/cc/service.rb +++ b/lib/cc/service.rb @@ -36,6 +36,7 @@ def self.load_services issue pull_request pull_request_coverage + pull_request_diff_coverage quality snapshot test diff --git a/lib/cc/services/github_pull_requests.rb b/lib/cc/services/github_pull_requests.rb index 3238842..ab8570b 100644 --- a/lib/cc/services/github_pull_requests.rb +++ b/lib/cc/services/github_pull_requests.rb @@ -71,6 +71,11 @@ def update_coverage_status_success update_status("success", presenter.coverage_message, "#{config.context}/coverage") end + def update_diff_coverage_status_skipped + update_status("success", presenter.skipped_message, "#{config.context}/diff-coverage") + update_status("success", presenter.skipped_message, "#{config.context}/total-coverage") + end + def update_status_failure update_status("failure", presenter.success_message) end diff --git a/spec/cc/service/github_pull_requests_spec.rb b/spec/cc/service/github_pull_requests_spec.rb index a72fbb1..9c3e0a1 100644 --- a/spec/cc/service/github_pull_requests_spec.rb +++ b/spec/cc/service/github_pull_requests_spec.rb @@ -82,6 +82,18 @@ covered_percent_delta: 2.0) end + it "pull request diff coverage skipped" do + expect_status_update("pbrisbin/foo", "abc123", "state" => "success", + "description" => /skipped analysis/, "context" => /diff-coverage/) + expect_status_update("pbrisbin/foo", "abc123", "state" => "success", + "description" => /skipped analysis/, "context" => /total-coverage/) + + receive_pull_request_diff_coverage({}, + github_slug: "pbrisbin/foo", + commit_sha: "abc123", + state: "skipped") + end + it "pull request status test success" do http_stubs.post("/repos/pbrisbin/foo/statuses/#{"0" * 40}") { |_env| [422, {}, ""] } @@ -213,6 +225,14 @@ def receive_pull_request_coverage(config, event_data) ) end + def receive_pull_request_diff_coverage(config, event_data) + service_receive( + CC::Service::GitHubPullRequests, + { oauth_token: "123" }.merge(config), + { name: "pull_request_diff_coverage" }.merge(event_data), + ) + end + def receive_test(config, event_data = {}) service_receive( CC::Service::GitHubPullRequests, From 264841241bf6a13e1cab56c93b36bf68200405b4 Mon Sep 17 00:00:00 2001 From: Camillo Facello <52419977+camillof@users.noreply.github.com> Date: Wed, 29 Jun 2022 00:51:39 -0300 Subject: [PATCH 2/3] Add CC::PullRequests tests --- spec/cc/pull_requests_spec.rb | 123 ++++++++++++++++++++++++++++++++++ 1 file changed, 123 insertions(+) create mode 100644 spec/cc/pull_requests_spec.rb diff --git a/spec/cc/pull_requests_spec.rb b/spec/cc/pull_requests_spec.rb new file mode 100644 index 0000000..eab0d51 --- /dev/null +++ b/spec/cc/pull_requests_spec.rb @@ -0,0 +1,123 @@ +require "spec_helper" + +describe CC::PullRequests do + shared_examples "receive method" do + before do + allow(instance).to receive(:report_status?).and_return(true) + expect(instance).to receive(:setup_http) + end + + context "when the status is valid" do + let(:instance) { CC::PullRequests.new({}, name: "test", state: payload_status) } + let(:response) do + { + ok: true, + state: 201, + message: "Success", + } + end + + it "calls the corresponding method" do + expect(instance).to receive(expected_method_name) do + instance.instance_variable_set(:@response, response) + end + result = instance.send(method_to_call) + + expect(result).to eq(response) + end + + context "when report_status? is false" do + before { expect(instance).to receive(:report_status?).and_return(false) } + + it "returns unknown status message" do + expect(instance).not_to receive(expected_method_name) + result = instance.send(method_to_call) + + expect(result).to eq({ ok: false, message: "Unknown state" }) + end + end + end + + context "when the status is not valid" do + let(:instance) { CC::PullRequests.new({}, name: "test", status: "invalid_status") } + + it "returns unknown status message" do + expect(instance).not_to receive(expected_method_name) + result = instance.send(method_to_call) + + expect(result).to eq({ ok: false, message: "Unknown state" }) + end + end + end + + describe "#receive_test" do + let(:instance) { CC::PullRequests.new({}, name: "test") } + + before do + expect(instance).to receive(:base_status_url) do |param| + "some_url" + param + end + expect(instance).to receive(:setup_http) + end + + it "makes a raw http test post" do + expect_any_instance_of(CC::Service::HTTP).to receive(:raw_post).with( + "some_url" + ("0" * 40), + { state: "success" }.to_json + ) + + instance.receive_test + end + + context "when raising an HTTPError" do + context "when message is equal to test_status_code" do + it "returns an ok message" do + expect(instance).to receive(:test_status_code) { 777 } + expect(instance).to receive(:raw_post). + and_raise(CC::Service::HTTPError.new("error", status: 777)) + + result = instance.receive_test + expect(result).to include( + ok: true, + status: 777, + message: "Access token is valid" + ) + end + end + + context "when message is different than test_status_code" do + it "raises the error" do + expect(instance).to receive(:test_status_code) { 777 } + expect(instance).to receive(:raw_post). + and_raise(CC::Service::HTTPError.new("error", status: 000)) + + expect { instance.receive_test }.to raise_error + end + end + end + end + + describe "#receive_pull_request" do + let(:payload_status) { "skipped" } + let(:expected_method_name) { :update_status_skipped } + let(:method_to_call) { :receive_pull_request } + + it_behaves_like "receive method" + end + + describe "#receive_pull_request_coverage" do + let(:payload_status) { "success" } + let(:expected_method_name) { :update_coverage_status_success } + let(:method_to_call) { :receive_pull_request_coverage } + + it_behaves_like "receive method" + end + + describe "#receive_pull_request_diff_coverage" do + let(:payload_status) { "skipped" } + let(:expected_method_name) { :update_diff_coverage_status_skipped } + let(:method_to_call) { :receive_pull_request_diff_coverage } + + it_behaves_like "receive method" + end +end From b3222a97eecff1bf8c946a3cd97b438a5d528707 Mon Sep 17 00:00:00 2001 From: Camillo Facello <52419977+camillof@users.noreply.github.com> Date: Wed, 29 Jun 2022 00:53:31 -0300 Subject: [PATCH 3/3] Refactor CC::PullRequests --- lib/cc/pull_requests.rb | 50 +++++++++++++++++------------------------ 1 file changed, 20 insertions(+), 30 deletions(-) diff --git a/lib/cc/pull_requests.rb b/lib/cc/pull_requests.rb index c7afc69..79bee31 100644 --- a/lib/cc/pull_requests.rb +++ b/lib/cc/pull_requests.rb @@ -6,42 +6,15 @@ def receive_test end def receive_pull_request - setup_http - state = @payload["state"] - - if %w[pending success failure skipped error].include?(state) && report_status? - send("update_status_#{state}") - else - @response = simple_failure("Unknown state") - end - - response + receive_request(%w[pending success failure skipped error], :update_status) end def receive_pull_request_coverage - setup_http - state = @payload["state"] - - if state == "success" && report_status? - update_coverage_status_success - else - @response = simple_failure("Unknown state") - end - - response + receive_request("success", :update_coverage_status) end def receive_pull_request_diff_coverage - setup_http - state = @payload["state"] - - if state == "skipped" && report_status? - update_diff_coverage_status_skipped - else - @response = simple_failure("Unknown state") - end - - response + receive_request("skipped", :update_diff_coverage_status) end private @@ -70,6 +43,10 @@ def update_coverage_status_success raise NotImplementedError end + def update_diff_coverage_status_skipped + raise NotImplementedError + end + def update_status_failure raise NotImplementedError end @@ -104,6 +81,19 @@ def receive_test_status end end + def receive_request(*permitted_statuses, call_method) + setup_http + state = @payload["state"] + + if permitted_statuses.flatten.include?(state) && report_status? + send(call_method.to_s + "_#{state}") + else + @response = simple_failure("Unknown state") + end + + response + end + def presenter CC::Service::PullRequestsPresenter.new(@payload) end