Fix at_exit hook to cooperate with Minitest #756
Conversation
|
Fine with me. I never got minitest to work satisfactorily with simplecov. Sounds like you did, so kudos |
|
Would be nice to have some sort of regression test on this, though I'm not quite sure how. start a new process? and shell out to a file the opens with a required minitest/autorun ? |
Leave it with me. Maybe we can shell out and test the Simplecov output matches an expected coverage. |
Ironically once I've started testing it I can't seem to get it to reproduce. Running it against a real test suite it's fine tho. Going to keep looking at it. |
adam12
force-pushed
the
fix-at-exit-hook
branch
from
6d66944
to
c960b1c
Compare
adam12
force-pushed
the
fix-at-exit-hook
branch
2 times, most recently
from
a999aab
to
77fd1d2
Compare
|
rubocop is complaining: spec/faked_project/minitest/test_helper.rb:8:3: C: Style/StderrPuts: Use warn instead of $stderr.puts to allow such output to be disabled. $stderr.puts "No SimpleCov config file found!" I might get to fixing this up myself tomorrow or so but if you get to it before I do, ofc feel free! |
|
I have a fix locally but left it broken intentionally on remote as I’m still not 100% sure my fix works in all scenarios and didn’t want a premature merge.
Leave it with me :) going to get to the bottom of it eventually.
…Sent from my iPhone
On Dec 1, 2019, at 9:31 AM, Tobias Pfeiffer ***@***.***> wrote:
rubocop is complaining: spec/faked_project/minitest/test_helper.rb:8:3: C: Style/StderrPuts: Use warn instead of $stderr.puts to allow such output to be disabled.
$stderr.puts "No SimpleCov config file found!"
I might get to fixing this up myself tomorrow or so but if you get to it before I do, ofc feel free!
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or unsubscribe.
|
|
I'm still exploring if this solution is right in all cases. I had to re-work the original patch when writing the feature specs, but the original patch indeed worked locally when I used it on a project. I suspect that there's an issue with load order; when Minitest is loaded, the hook can be set correctly and everything functions well. If Minitest isn't loaded, the original A Minitest plugin might work better here, but that still needs to be explored. |
|
@adam12 thanks a lot for all the work you put into this! |
adam12
force-pushed
the
fix-at-exit-hook
branch
from
f1a25d2
to
945afed
Compare
|
All the tests except for the newest ruby version are failing with a slightly different coverage... which is an interesting edge case 🤔 Not sure what to do about it though... making it conditional, a range, trying to find a bug (I suspect it's in ruby itself...) - thougs @adam12 ? Also, thanks a lot! |
|
Yeah, I'm not too sure what to make of it right now. I've been pondering it all weekend. I'll probably have to establish which value I believe is correct and then work backwards from there. I'm guessing it's from changes I made because CI for master tests those older versions with correct coverage. I'll keep poking at it. |
|
@adam12 the failure is from your newly introduced feature test (well done for introducing it and thank you! 💚 ) - so in that sense it is from changes you made, but the actually measurement might always have been broken :) I'd suggest looking at the HTML report and see what's correct |
|
@adam12 Hey, how's it going? :) Do you wanna keep poking at it or should I see if I could/should take over? It'd be no bother for me, you already did a lot 💚 If you wanna keep poking at it I also understand and will do things elsewhere. I really want to have this as I believe it'll fix quite some issues for people 🚀 |
|
Hello @PragTob! I haven't forgotten about it but my free time ebbs and flows. Hoping to look at it again this week. If you're interested in taking a poke at the coverage values and seeing if you can spot the discrepancy I'd definitely not oppose :) Cheers. |
|
Hi @adam12 thank you! I'll first stick with the branch coverage so we can get that relased as a preview and then if you haven't fixed it I'll take a poke at it. Thanks so much! 👌 |
Looks like branch coverage might be our issue here as well?
Also, I'll be rebasing against master unless you've started working on this branch? LMK. |
|
@adam12 nope haven't started working on it. Rebase away! Na that's not branch coverage/branch coverage doesn't seem to be enabled. However, the number of relevant lines seems to have decreased and somewhat rightfully so. I wouldn't think |
|
#855 is there now trying to get this in. |
|
And #855 appears to be ready. Closing this in favor of it. |
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)
Ensures that the
at_exithook is triggered at the right time when run under Minitest, by using the Minitestafter_runhook for execution.Basically verbatim of @zenspider's suggested changes.