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

Require code coverage not drop in a PR #38

Closed
brettcannon opened this issue Feb 25, 2017 · 8 comments
Closed

Require code coverage not drop in a PR #38

brettcannon opened this issue Feb 25, 2017 · 8 comments
Labels

Comments

@brettcannon
Copy link
Member

Codecov allows for PRs to be blocked from merging if the code coverage drops too much. As of right now I have seen coverage for things like pydoc vary too much to do that yet. We should probably work towards figuring out what is leading to that coverage variance and fix it so we can require coverage never gets worse.

If people do come across modules whose coverage seems to drop unexpectedly, please mention them here so we know what modules need to have their tests fixed.

@ncoghlan
Copy link
Contributor

ncoghlan commented Feb 25, 2017

For python/cpython#85 the problem wasn't with coverage variance, but rather with the fact that PR tweaked the details of code flows that currently only run on Mac OS X. Although now I realise I was actually a bit hasty on that front - the reason the flows were tweaked were because the exact spelling went from being a private API to a public API, which means there should now be a cross-platform test case. I've reopened the related issue and set it to "test needed": http://bugs.python.org/issue24241#msg288541 :)

@ncoghlan
Copy link
Contributor

python/cpython#289 looks like a legitimate case of code not being covered in CI - the code in question only runs inside an X environment, which I'm pretty sure isn't available on the Travis builders.

@dstufft
Copy link
Member

dstufft commented Feb 28, 2017

Hmm, we can install whatever apt packages we want. Surely there's some sort of headless X environment we can install?

@warsaw
Copy link
Member

warsaw commented Feb 28, 2017

$ apt-cache show xvfb
...
Description-en: Virtual Framebuffer 'fake' X server
 Xvfb provides an X server that can run on machines with no display hardware
 and no physical input devices. It emulates a dumb framebuffer using virtual
 memory. The primary use of this server was intended to be server testing,
 but other novel uses for it have been found, including testing clients
 against unusual depths and screen configurations, doing batch processing with
 Xvfb as a background rendering engine, load testing, as an aid to porting the
 X server to a new platform, and providing an unobtrusive way to run
 applications that don't really need an X server but insist on having one
 anyway.
 .
 This package also contains a convenience script called xvfb-run which
 simplifies the automated execution of X clients in a virtual server
 environment. This convenience script requires the use of the xauth
 program.
 .
 More information about X.Org can be found at:
 <URL:http://www.X.org>
 .
 This package is built from the X.org xserver module.
...

@ncoghlan
Copy link
Contributor

ncoghlan commented Mar 3, 2017

+1 for trying to get the X tests running in CI (presumably the tkinter tests are currently being skipped entirely).

The diff coverage results on python/cpython#360 show that we're going to need to address #18 as well (the remaining uncovered lines are the new module level code in contextlib)

@brettcannon
Copy link
Member Author

I was thinking about this the other day and I was wondering if some of this comes from randomness in tests that is introduced to try and pick up unexpected issues. If that's the case we may want to add a "randomness" resource or something so that the coverage run can exclude those tests. That way we can make the test runs more deterministic.

But then again I might be wrong and this might not really be an issue. 😄

@nightlark
Copy link

Is this issue and #18 still relevant? Since the removal of the coverage job on Travis, https://app.codecov.io/gh/python/cpython/ has stopped getting new data, but I noticed that one of the Azure Pipelines yaml files has some lines related to coverage (though it doesn't appear to be running/uploading data -- intentionally or a bug?).

@brettcannon
Copy link
Member Author

I think we have given up on watching coverage this closely since people didn't notice when coverage broke for quite some time a while back.

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

No branches or pull requests

5 participants