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

No report generated using RSpec Rails / SimpleCov 0.18.3 #873

Closed
odlp opened this issue Feb 24, 2020 · 12 comments
Closed

No report generated using RSpec Rails / SimpleCov 0.18.3 #873

odlp opened this issue Feb 24, 2020 · 12 comments
Labels

Comments

@odlp
Copy link
Contributor

odlp commented Feb 24, 2020

Hello,

First of all thank-you to everyone for all the hard work making the 0.18 release!

I think I've encountered a bug where no report is generated at the end of the test suite. This is in a Rails application tested using RSpec (using the rspec-rails gem), with SimpleCov 0.18.3.

Reproduction

I've made a minimal reproduction of the issue here:
https://github.com/odlp/simplecov-at-exit-bug

Tracing the cause

When ActiveSupport::TestCase is required, by RSpec Rails for example:

https://github.com/rspec/rspec-rails/blob/e23efbb2cd6b40aaa8c6f33a36fe7ed4e724e2c7/spec/rspec/rails/matchers/redirect_to_spec.rb#L2

This ends up requiring minitest:

https://github.com/rails/rails/blob/c70112e74f4d2ef517f4036fe6e2888cc30fc952/activesupport/lib/active_support/test_case.rb#L4

Unfortunately Minitest is a dependency of ActiveSupport:

https://github.com/rails/rails/blob/c70112e74f4d2ef517f4036fe6e2888cc30fc952/activesupport/activesupport.gemspec#L38

By having defined Minitest, this means the SimpleCov at_exit hook will skip
the 'at exit behaviour' hook:

https://github.com/colszowka/simplecov/blob/a179ec6dc419c43bce472c2426f30f24cc49b42f/lib/simplecov/defaults.rb#L24-L29

I've verified this by placing a breakpoint in the at_exit hook and stepping over the execution.

PRs which introduced this behaviour:

Temporary Workaround

RSpec.configure do |config|
  config.after(:suite) { SimpleCov.at_exit_behavior }
end

Or use versions '<= 0.18.2'

@odlp
Copy link
Contributor Author

odlp commented Feb 24, 2020

Forgot to mention I'm happy to contribute a PR to fix this, but would appreciate confirmation of the issue & any guidance on any preferred direction of the solution.

My initial thoughts for a fix would be to define a after(:suite) callback to trigger SimpleCov.at_exit_behavior if RSpec is defined, similar to the Minitest hook. Unfortunately if both RSpec and Minitest are defined I don't believe we can tell which is actually running (I haven't spotted a global flag or such in either library).

@nathanvda
Copy link

I can confirm, just started a new rails 6 project with rspec and simplecov did not generate a report, and adding the (temporary) workaround mentioned by @odlp works for me 👏 👏

@PragTob PragTob added the Bug label Feb 24, 2020
@PragTob
Copy link
Collaborator

PragTob commented Feb 24, 2020

👋

Hi @odlp and thank you so much for your bug report and your in depth investigation tracking it down, already providing a sample repo! 💚 This is truly great! 🚀

The bugs will never stop coming with this release will they 😢

Anyhow. I also don't know how to best fix this. Our command guesser seems to first check for RSpec and then Minitest probably for that very reason.

We might also on initial start check the command name that is being run (although that gets muddy/murky with parallel tests I believe). So... I'm saying this seems like a hard problem :/ Querying Minitest for whether it's really running is probably the preferred solution (if possible) as I don't believe we want to check every other test framework for whether it's in used so maybe not minitest.

Other implementation notes, damn myself for not already introducing the full rails projects. We have a test_projects folder where you should be able to almost drop in your test project and then we can use it to write a cuke against it fixing the behavior. And make sure this never happens again 😞

You PRing it would be mightily welcome, this is big enough of a bug though that I might try to clear some time in the coming days so a fix can be published ASAP. If you get there first, of course much welcome. I'll put a comment in this issue when I start working on it.

also cc: @adam12 as he seemed to know lots of things about minitest so might be of help :)

Thanks again for the great & nice report, it's greatly appreciated! 💚

Ghost approves:
IMG_20171221_144657_Bokeh

@adam12
Copy link
Contributor

adam12 commented Feb 24, 2020

Instead of using the call to defined?, maybe we can setup a boolean accessor on SimpleCov that the Minitest plugin sets to true and that value is used as the guard instead.

@adam12
Copy link
Contributor

adam12 commented Feb 24, 2020

I'll work on a PR for this.

@adam12
Copy link
Contributor

adam12 commented Feb 24, 2020

@odlp @nathanvda Would you mind trying my branch which uses a different guard method?

https://github.com/adam12/simplecov/tree/alternative-at-exit-guard

I might be wrong and we might need to add the RSpec hook similar to what was mentioned above, but this seemed inline with the changes we've already made.

@odlp
Copy link
Contributor Author

odlp commented Feb 24, 2020

@adam12 I can confirm that branch restores the expected behaviour in the Rails+RSpec codebase I've been testing with 👍

@adam12
Copy link
Contributor

adam12 commented Feb 24, 2020

Thanks @odlp

@PragTob I'll cleanup and submit as PR. Were you thinking about adding a Rails project for a test case?

@PragTob
Copy link
Collaborator

PragTob commented Feb 24, 2020

@adam12 yes, actually multiple. One with rspec, one with minitest etc... to make sure it all works in what's probably our must common use case. If you PR the changes I'm super happy adding the test projects myself 👌

@PragTob
Copy link
Collaborator

PragTob commented Feb 24, 2020

@adam12 if you're interested though, I imagine it like https://github.com/colszowka/simplecov/tree/master/test_projects/parallel_tests and the accompanying https://github.com/colszowka/simplecov/blob/master/features/parallel_tests.feature (minus the wips due to the other issue 😅 )

Long term vision is to have these for lots of common setups. Like Rails 5.2, Rails 6, some amount of different test frameworks. Rails + parallel tests.

All the fun to avoid these regressions in the future and also to have a framework to more easily reproduce bugs and write features/specs against it.

@PragTob
Copy link
Collaborator

PragTob commented Feb 24, 2020

Also: @adam12 thank you 💚

IMG_20171230_130536

@PragTob
Copy link
Collaborator

PragTob commented Feb 24, 2020

Fixed by #875 / #874 - releasing now. Thank you a lot everyone!

@PragTob PragTob closed this as completed Feb 24, 2020
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this issue Mar 8, 2020
Update ruby-simplecov to 0.18.5.


0.18.5 (2020-02-25)
===================

Can you guess? Another bugfix release!

## Bugfixes
* minitest won't crash if SimpleCov isn't loaded - aka don't execute SimpleCov code in the minitest plugin if SimpleCov isn't loaded. Thanks to [@edariedl](https://github.com/edariedl) for the report of the peculiar problem in [#877](simplecov-ruby/simplecov#877).

0.18.4 (2020-02-24)
===================

Another small bugfix release 🙈 Fixes SimpleCov running with rspec-rails, which was broken due to our fixed minitest integration.

## Bugfixes
* SimpleCov will run again correctly when used with rspec-rails. The excellent bug report [#873](simplecov-ruby/simplecov#873) by [@odlp](https://github.com/odlp) perfectly details what went wrong. Thanks to [@adam12](https://github.com/adam12) for the fix [#874](simplecov-ruby/simplecov#874).


0.18.3 (2020-02-23)
===========

Small bugfix release. It's especially recommended to upgrade simplecov-html as well because of bugs in the 0.12.0 release.

## Bugfixes
* Fix a regression related to file encodings as special characters were missing. Furthermore we now respect the magic `# encoding: ...` comment and read files in the right encoding. Thanks ([@Tietew](https://github.com/Tietew)) - see [#866](simplecov-ruby/simplecov#866)
* Use `Minitest.after_run` hook to trigger post-run hooks if `Minitest` is present. See [#756](simplecov-ruby/simplecov#756) and [#855](simplecov-ruby/simplecov#855) thanks ([@adam12](https://github.com/adam12))

0.18.2 (2020-02-12)
===================

Small release just to allow you to use the new simplecov-html.

## Enhancements
* Relax simplecov-html requirement so that you're able to use [0.12.0](https://github.com/colszowka/simplecov-html/blob/master/CHANGELOG.md#0120-2020-02-12)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants