Skip to content
This repository was archived by the owner on Jul 19, 2025. It is now read-only.

QUA-360: Add skip diff coverage method #152

Merged
merged 3 commits into from
Jun 30, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 22 additions & 19 deletions lib/cc/pull_requests.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,29 +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)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason to pass the call_method as a symbol instead of a string? I see that we convert it as a string later in the receive_request method

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Honestly, there isn't a strong reason. I just felt like it was more readable, but I'm open to change it if you don't feel the same about it

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I personally like it as a symbol. I associate symbols with methods better 👍

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
receive_request("success", :update_coverage_status)
end

response
def receive_pull_request_diff_coverage
receive_request("skipped", :update_diff_coverage_status)
end

private
Expand Down Expand Up @@ -57,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
Expand Down Expand Up @@ -91,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
Expand Down
1 change: 1 addition & 0 deletions lib/cc/service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ def self.load_services
issue
pull_request
pull_request_coverage
Copy link
Contributor

@larkinscott larkinscott Jun 29, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just curious: is this the total_coverage event? Actually, never mind.

Copy link
Contributor Author

@camillof camillof Jun 29, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Were you able to find out why this is requested?? It's the available services names, so you can call it like

service = CC::Service::GitHubPullRequests.new(
  {
    oauth_token: ...,
  },
  name: "pull_request_diff_coverage",
  state: "skipped",
  github_slug: ...,
  number: ...,
  commit_sha: ...
)

pull_request_diff_coverage
quality
snapshot
test
Expand Down
5 changes: 5 additions & 0 deletions lib/cc/services/github_pull_requests.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
123 changes: 123 additions & 0 deletions spec/cc/pull_requests_spec.rb
Original file line number Diff line number Diff line change
@@ -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
20 changes: 20 additions & 0 deletions spec/cc/service/github_pull_requests_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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, {}, ""] }

Expand Down Expand Up @@ -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,
Expand Down