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

Allow ignoring individual lines for coverage analysis #5375

Merged
merged 1 commit into from
May 19, 2017

Conversation

CyberShadow
Copy link
Member

This adds a work-around to the non-deterministic coverage fluctuations
in std.parallelism. Lines with nocoverage on them will be excluded
from coverage analysis, and be considered as not containing any code.

This adds a work-around to the non-deterministic coverage fluctuations
in std.parallelism. Lines with `nocoverage` on them will be excluded
from coverage analysis, and be considered as not containing any code.
@JackStouffer
Copy link
Member

Seems like treating the symptom and not the cause. Why not fix the tests?

@wilzbach
Copy link
Member

wilzbach commented May 7, 2017

Seems like treating the symptom and not the cause. Why not fix the tests?

Well, feel free to have a go at it (warning: it's not as easy as it looks!)!
Of course if you can provide a test that always covers this line this would be preferred ...

@CyberShadow
Copy link
Member Author

I've been told people have tried and didn't see an obvious simple fix, but feel free to have a go at it.

Either way, there will inevitably be non-deterministic conditions and error conditions that are too difficult to simulate, so it's good to have a mechanism to handle that in a semi-reasonable way without breaking the CI status of half of all PRs randomly.

Copy link
Member

@wilzbach wilzbach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As mentioned imho this is a very valuable addition as it avoids a CodeCov failure for a transient line(s) and thus the overall red cross for PRs, which is very confusing and frustrating for contributors.

@wilzbach
Copy link
Member

I am inclined to merge this as (1) it's a very simple solution for a long outstanding problem about which many contributors have complained as it's very hard to figure out that the PR is displayed has "failed" due to a unrelated line in std.parallelism being only sometimes hit. Moreover, (2) except for @JackStouffer no one seems to care much about this addition, so please if someone has a better solution in mind that he would like to do instead, please raise your voice, otherwise imho we should should go with this in the next days ;-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants