TST: relax codecov project threshold #12549
Conversation
|
Looks good to me, thanks. The main reason why failing for reducing overall code coverage is not a good idea is that it penalizes you for deleting code! |
|
Also, the measurements are simply incorrect. It's a difference between a PR and some random previous run. So turning it off completely is also fine if the check keeps failiing. |
|
And it did, just now. This didn't work it looks like, it's still trying to use 0.01%. IIRC we've tried this before and |
d3da1b8 to
fdabe85
Compare
|
Not surprised the API isn't responding to the yml. Trying one more thing now -- disabling the project status entirely, since it is really only the patch diff that is useful. |
|
Maybe you can copypaste the config from scipy --- it used to work before hitting codecov size limits... |
fdabe85 to
b5bac31
Compare
|
I've copied in the My other thought, because this is really confusing--what if |
|
A third possibility is web-hook only vs. github marketplace app for codecov. Pretty sure we use the former, not sure about SciPy but probaby web-hook there too given historical availability. |
|
The verbatim copy of the SciPy codecov config allows CI to pass, but seems to eliminate the most useful entry from the matrix (the patch diff coverage). Not a fan of that tradeoff myself, and not sure it is worth trying to experiment too much more if the docs aren't reliable. |
b5bac31 to
6bcaa0b
Compare
|
I've added a dummy function that is not covered by unit tests to this PR -- let's see if the SciPy codecov settings that are copied in provide some kind of indication that the new function is not covered. If there's a silent indicator of some sort, that might be a compromise worth discussing. |
|
With the SciPy settings you get complete silence on uncovered code additions in a PR, but the CI will have less false positive failures. I suppose reviewers could be trusted to click through the codecov link and take a peek at the patch diff. Not sure how many more issues might sneak though if CI is more quiet about coverage matters. |
|
This needs a rebase. I suggest to move |
* we have no idea why codecov API does not respond to codecov.yml settings as documented; we now use config from SciPy verbatim to avoid excessive coverage CI failures
6bcaa0b to
3ddc23c
Compare
|
The hidden status doesn't seem to matter -- copying the I was simply saying I wasn't fully satisfied with that fix, because now reviewers will have to click through a few pages to gauge the coverage of the patch diff manually. Anyway, I've rebased, force-pushed, and I'll remove the WIP tag if CI is all green. I suspect this will do what is desired--basically always produce a green CI as far as codecov is concerned with the tradeoffs already discussed. I suspect we can all agree that improvement on their end or a new competitor in the ecosystem would be most welcome. I suppose it may not be that hard to simply code in the patch cov diff % check to a CI stage manually & fail below some threshold we agree on & storing the coverage html artifact for the offending file. But maybe that is something a third-party service should really be doing for the ecosystem. |
|
core dump on Travis! codecov passing |
yes, saw that in your PR that removes lgtm, okay to remove the WIP status and merge? |
|
Ok, I've removed the WIP tag if this is what most devs want. |
|
3 approvals and all green (except for the unrelated segfault), merging. Thanks @tylerjereddy |
We have been observing far too many false positive project-level codecov failures due to < 0.01 % drops that are clearly not relevant. I've tried to adjust project
thresholdparameter based on the standard docs.This would allow a 0.05 % drop in total project test coverage to report as a success, but I think this is probably fine since a PR will still fail if its diff has poor coverage (there's a separate
thresholdwe could set for the diff--currently I think it is set to our base coverage of around 85 % by default, which is fairly solid on a PR).I know this is something that came up recently with @shoyer in an in person discussion.
I'm not always optimistic about codecov doing exactly what the
ymlshould be telling it to do, but maybe worth a try.