Avoid premature failure with parallel_tests #706
Conversation
When running within [parallel_tests](https://github.com/grosser/parallel_tests/wiki), wait until all test processes complete before evaluating coverage statistics. This avoids a premature failure exit code if an early run finishes with coverage that does not meet the configured criteria.
f1sherman
force-pushed
the
fix-minimum-coverage-with-parallel-tests
branch
from
1137200 to
a3faac1
Compare
|
Any ETA for a release including this fix? |
There was a problem hiding this comment.
Looking good, I like the usage of ParallelTest APIs although that binds us to its API pretty much. I think this will have to be re-evaluated when a new gem doing this emerges... wasn't Rails 6 supposed to ship with something to that tune? Well, we'll see...
Thanks for taking the time to tackle this difficult issue 💚
Sorry for the long response time, I took a bit of a break from maintaining simplecov.
I'll address the things I found because I think they're easy to address no need to wait on you after you waited so long for me :)
| @@ -1,5 +1,5 @@ | |||
| # frozen_string_literal: true | |||
|
|
|||
| module SimpleCov | |||
| VERSION = "0.16.1".freeze | |||
| VERSION = "0.16.2".freeze | |||
There was a problem hiding this comment.
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 :)
| @@ -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 | |||
There was a problem hiding this comment.
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.
|
@tavlima I'll try to cut a new release in the coming days, no promises though. |
|
Merged as 318b212 |
* Update simeplecov gem to fix parallel_test coverage bug simplecov-ruby/simplecov#706 * don't chane bundle version
Update ruby-simplecov to 0.17.1. 0.17.1 (2019-09-16) =================== Bugfix release for problems with ParallelTests. ## Bugfixes * Avoid hanging with parallel_tests. See [#746](simplecov-ruby/simplecov#746) (thanks [@annaswims](https://github.com/annaswims)) 0.17.0 (2019-07-02) =================== Maintenance release with nice convenience features and important bugfixes. Notably this **will be the last release to support ruby versions that have reached their end of life**. Moving forward official CRuby support will be 2.4+ and JRuby support will be 9.1+. Older versions might still work but no guarantees. ## Enhancements * Per default filter hidden files and folders. See [#721](simplecov-ruby/simplecov#721) (thanks [Renuo AG](https://www.renuo.ch)) * Print the exit status explicitly when it's not a successful build so it's easier figure out SimpleCov failed the build in the output. See [#688](simplecov-ruby/simplecov#688) (thanks [@daemonsy](https://github.com/daemonsy)) ## Bugfixes * Avoid a premature failure exit code when setting `minimum_coverage` in combination with using [parallel_tests](https://github.com/grosser/parallel_tests). See [#706](simplecov-ruby/simplecov#706) (thanks [@f1sherman](https://github.com/f1sherman)) * Project roots with special characters no longer cause crashes. See [#717](simplecov-ruby/simplecov#717) (thanks [@deivid-rodriguez](https://github.com/deivid-rodriguez)) * Avoid continously overriding test results with manual `ResultMergere.store_results` usage. See [#674](simplecov-ruby/simplecov#674) (thanks [@tomeon](https://github.com/tomeon))
When running within parallel_tests, wait until all test processes complete before evaluating coverage statistics. This avoids a premature failure exit code if an early run finishes with coverage that does not meet the configured criteria. I believe this will fix #559. There are a few different approaches I could see to resolve this issue, feedback welcome of course!