simplecov-ruby / simplecov

Code coverage for Ruby with a powerful configuration library and automatic merging of coverage across test suites
MIT License
4.75k stars 552 forks source link

Regression: exit code 0 when loaded after minitest #465

Open searls opened 8 years ago

searls commented 8 years ago

This seems like a regression of #269

In a very simple project if I have this Rakefile:

require 'rake/testtask'

Rake::TestTask.new do |t|
  t.libs.push 'test'
  t.pattern = 'test/**/test_*.rb'
end

task :default => :test

And this test helper:

require "minitest/autorun"

require 'simplecov'
SimpleCov.start

And an error occurs, the process exits code 0. However, if I correct my configuration to load simplecov before minitest/autorun:

require 'simplecov'
SimpleCov.start

require "minitest/autorun"

exits non-zero.

Obviously, I had simplecov misconfigured, but I got no feedback at all I was doing it wrong, and my build was passing erroneously. Is there anything simplecov can do to guard against this or at least warn me?

bf4 commented 8 years ago

That's because minitest/autorun is kind of insane and doesn't start until the program ends. Yes, that's right, minitest autorun runs your entire test suite in an at_exit block. So, that's why you needed to require simplecov first. Otherwise, minitest will finish after simplecov and your final exit code will be that of minitest. :rage:

Requiring simplecov first loads the (default) at_exit hooks. I believe you can start simplecov after minitest as long as you have required simplecov first, but would want to confirm.

I ran into a lot of trouble with this in https://github.com/rails-api/active_model_serializers/pull/1386#discussion_r48422288 and https://github.com/rails-api/active_model_serializers/pull/1109 which referencs http://blog.arkency.com/2013/06/are-we-abusing-at-exit/

bf4 commented 8 years ago

So, I don't think it's a regression as that issue had to do with simplecov mishandling the error code. See my comment here on how I confirmed that https://github.com/colszowka/simplecov/issues/281#issuecomment-49255746

If you can confirm, this would make a good PR to the docs.

searls commented 8 years ago

Wow, @bf4, thanks for the background. I won't try speculating about who/what/where and ways to improve things here, but I'll make a note of it.

bf4 commented 8 years ago

Thanks. As an aside, I think we still wanted to get the rake task to eager load, which could help

On Mon, Feb 22, 2016 at 7:55 AM Justin Searls notifications@github.com wrote:

Wow, @bf4 https://github.com/bf4, thanks for the background. I won't try speculating about who/what/where and ways to improve things here, but I'll make a note of it.

— Reply to this email directly or view it on GitHub https://github.com/colszowka/simplecov/issues/465#issuecomment-187185614 .

PragTob commented 4 years ago

Another thing that I hope #756 might solve

zenspider commented 2 months ago

I think this issue could be closed... One suggestion might be to add a warning:

warn "WARNING: minitest should be loaded AFTER simplecov" if defined?(Minitest)

but I don't know if you want that "infecting" your project. Happy to whip up a PR if so.