Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid premature failure with parallel_tests #706

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 7 additions & 0 deletions CHANGELOG.md
@@ -1,3 +1,10 @@
0.16.2 (TBD)
===================

## Bugfixes

* Avoid a premature failure exit code when setting `minimum_coverage` in combination with using [parallel_tests](https://github.com/grosser/parallel_tests)

0.16.1 (2018-03-16)
===================

Expand Down
18 changes: 17 additions & 1 deletion lib/simplecov.rb
Expand Up @@ -90,6 +90,7 @@ def result
# If we're using merging of results, store the current result
# first (if there is one), then merge the results and return those
if use_merging
wait_for_other_processes
SimpleCov::ResultMerger.store_result(@result) if result?
@result = SimpleCov::ResultMerger.merged_result
end
Expand Down Expand Up @@ -220,7 +221,7 @@ def process_result(result, exit_status)
if result_exit_status == SimpleCov::ExitCodes::SUCCESS # No result errors
write_last_run(covered_percent)
end
result_exit_status
final_result_process? ? result_exit_status : SimpleCov::ExitCodes::SUCCESS
Copy link
Collaborator

Choose a reason for hiding this comment

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

I wonder if this could be simplified. I guess the thinking here is that we'll always say it's success unless we were the last process? Might be nice to say in a comment or something.

end

# @api private
Expand Down Expand Up @@ -248,6 +249,21 @@ def result_exit_status(result, covered_percent)
end
# rubocop:enable Metrics/MethodLength

#
# @api private
#
def final_result_process?
!defined?(ParallelTests) || ParallelTests.last_process?
end

#
# @api private
#
def wait_for_other_processes
return unless defined?(ParallelTests) && final_result_process?
ParallelTests.wait_for_other_processes_to_finish
end

#
# @api private
#
Expand Down
2 changes: 1 addition & 1 deletion lib/simplecov/version.rb
@@ -1,5 +1,5 @@
# frozen_string_literal: true

module SimpleCov
VERSION = "0.16.1".freeze
VERSION = "0.16.2".freeze
Copy link
Collaborator

Choose a reason for hiding this comment

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

Bumping versions is usually more of a maintainer choice (with a lot of the other fixes I'll probably go for 0.17.0) so I think it's fine to keep as is before result :)

end
23 changes: 23 additions & 0 deletions spec/simplecov_spec.rb
Expand Up @@ -13,6 +13,7 @@
context "with merging disabled" do
before do
allow(SimpleCov).to receive(:use_merging).once.and_return(false)
expect(SimpleCov).to_not receive(:wait_for_other_processes)
end

context "when not running" do
Expand Down Expand Up @@ -64,6 +65,7 @@
allow(SimpleCov).to receive(:use_merging).once.and_return(true)
allow(SimpleCov::ResultMerger).to receive(:store_result).once
allow(SimpleCov::ResultMerger).to receive(:merged_result).once.and_return(the_merged_result)
expect(SimpleCov).to receive(:wait_for_other_processes)
end

context "when not running" do
Expand Down Expand Up @@ -170,5 +172,26 @@
end
end
end

describe ".process_result" do
before do
expect(SimpleCov).to receive(:result_exit_status).and_return SimpleCov::ExitCodes::MINIMUM_COVERAGE
expect(SimpleCov).to receive(:result?).and_return true
end
context "when the final result process" do
let(:result) { double(SimpleCov::Result, :covered_percent => 0.0) }
before { expect(SimpleCov).to receive(:final_result_process?).and_return true }
it "returns the exit code from .result_exit_status" do
expect(SimpleCov.process_result(result, SimpleCov::ExitCodes::SUCCESS)).to eq SimpleCov::ExitCodes::MINIMUM_COVERAGE
end
end
context "when not the final result process" do
let(:result) { double(SimpleCov::Result, :covered_percent => 0.0) }
before { expect(SimpleCov).to receive(:final_result_process?).and_return false }
it "returns the success exit code" do
expect(SimpleCov.process_result(result, SimpleCov::ExitCodes::SUCCESS)).to eq SimpleCov::ExitCodes::SUCCESS
end
end
end
end
end