-
Notifications
You must be signed in to change notification settings - Fork 554
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
Fix coverage rate of the parallel_tests #441
Fix coverage rate of the parallel_tests #441
Conversation
To be clear: you intend for this to be just adding specs + refactoring, not changing any behaviour? |
No, this Pull Request is changing the behavior a little bit. The specification of Coverage module are as below.
And the coverage of unloaded files are This Pull Request is intended to solve it. See also: |
ah, so I suspect this is a "dup" of #436 ? I don't have a preference as to which is a better solution - only reason I haven't merged the other one is that I was waiting for someone to confirm it actually fixed the issue :) |
#436 is insufficient to fix this problem. (#436 (comment)) |
It would be great if this could get merged. When I run simplecov as part of a normal rspec run I get around 80% test coverage, but for a while I'm down do around 48% when running as part of parallel_tests. With this branch I'm now back to 80%. |
That's the confirmation I was looking for :) |
That's why I posted it. 😁 |
I just used this change on my project, and it fixed our issue of multiple runs being merged incorrectly, leading to lower-than-actual results. It would be really nice to get this released in a new version of the gem. |
Just added Simplecov to a Rails 4.2 project. I was disturbed to see a much-lower-than-expected coverage percentage, then I figure out that all of my lines of comments were being reported as "missed". Using this branch fixes the problem. Please merge and release it! |
new_array[i] = local_value + other_value | ||
end | ||
pair = [element, new_array[i]] | ||
new_array[i] = if pair.any?(&:nil?) && pair.map(&:to_i).all?(&:zero?) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so, the change is
- result is `nil` if the element in both sets is nil
+ result is `nil` if either set contains nil AND both are each one of either nil, zero, or some non-integer?
and then the merge result is (LHS || 0) + (RHS || 0)
as the test shows
result1 = [nil, 0, nil, 0]
result2 = [nil, nil, 0, 0]
merge_result = [nil, nil, nil, 0]
@sinsoku can you rebase and force push your commits, or @colszowka wanna change the merge options? :) |
a45f7d3
to
54e60b7
Compare
@bf4 Thank you for the feedback. I rebased and force pushed. |
I just wanted to mention that I have the "incorrectly low coverage" problem but I am not using |
Thanks! On Mon, Apr 11, 2016 at 1:56 PM, Benjamin Fleischer <
|
release ? :) |
PR is simplecov-ruby/simplecov#441 undo this change when a new version of simplecov is released
0.12.0 2016-07-02 ([changes](simplecov-ruby/simplecov@v0.11.2...v0.12.0)) ================= ## Enhancements * Add support for JSON versions 2.x ## Bugfixes * Fix coverage rate of the parallel_tests. See [#441](simplecov-ruby/simplecov#441) (thanks @sinsoku) * Fix a regression on old rubies that failed to work with the recently introduced frozen VERSION string. See [#461](simplecov-ruby/simplecov#461) (thanks @leafle)
The coverage rate has decreased when used with
parallel_tests
. So, I fixed it.merge_resultset
This Pull Request is to change behaviors of 2 and 4.
Environment