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

pytest-cov + xdist + remote workers #232

Open
simon-weber opened this issue Oct 4, 2018 · 10 comments
Open

pytest-cov + xdist + remote workers #232

simon-weber opened this issue Oct 4, 2018 · 10 comments
Labels

Comments

@simon-weber
Copy link

Our CI uses an xdist setup with remote tcp workers (--tx socket=...) to run a large test suite. As far as I can tell, pytest-cov supports this. But, when I use --cov=., the report only contains files/lines that (I suspect) are imported by the master process, such as our conftest. Running without xdist reports the expected coverage.

So, before going too much further, I wanted to check: is this form of xdist supported? There are a few other issues, but they only refer to multi-process mode. If so, any ideas as to how I can go about debugging this? If not, any ideas on how I could still collect coverage with this configuration?

Here's some relevant package versions, in case it's helpful:

  • cov==2.6.0
  • coverage==4.4.1
  • pytest==3.6.0
  • pytest-cov==2.6.0
  • a slightly modified fork of xdist 1.22.2 described here
  • python 2.7
@blueyed
Copy link
Contributor

blueyed commented Oct 5, 2018

I am using it myself with --tx ssh=…, and it works.
I assume that socket would be handled similar to ssh, but cannot say really.
What options are you using with pytest-cov?
You can try using export EXECNET_DEBUG=2 to get more details.

@simon-weber
Copy link
Author

Interestingly, I've gotten it to report data inconsistently a few times now. All my successful samples so far have had EXECNET_DEBUG and PYTEST_DEBUG enabled for the master. My best guess is there's some kind of state I'm not accounting for (eg the report coverage file?), even though this is on CI and should have a clean working directory each time.

The successful invocations look like:

PYTEST_DEBUG=true EXECNET_DEBUG=2 pytest -d --tx socket=... --cov=. --cov-report="xml:..." --rsyncdir foo foo/test.py

I haven't noticed anything that stands out in the debug output so far. Forking pytest-cov and adding some debugging shows that the master is performing reasonably (eg it's running the distributed code and doesn't think it's colocated with the agent).

Next I think I'll see if there's any difference when running on fresh machines, and look into getting debug output from the agents too (I think I need EXECNET_DEBUG=1 there because of how the socketserver swallows the subprocess output).

@blueyed
Copy link
Contributor

blueyed commented Oct 5, 2018

Check if it works with 2.5.2.
Might be related to coverage combining (#222), e.g. when you run coverage combine yourself, but should use -a.
You can also use COVERAGE_DEBUG=sys,config,process,self,multiproc to get debug information from coveragepy.

@simon-weber
Copy link
Author

So far so good on 2.5.1. We're not interacting with coverage outside of pytest-cov.

I still haven't managed to consistently reproduce failures or successes on 2.6.0. I was able to find a case where the coverage file stuck around after a build, but the next run (which succeeded) didn't run on that same host.

@jmbowman
Copy link
Contributor

I just debugged a similar problem; it looks like there's a bug in how pytest-cov combines coverage from remote workers where the code is installed at a different base path than on the master. There is code in pytest-cov to automatically build a [paths] coverage configuration entry which would combine these correctly, but it isn't used because of this line which replaces the Coverage object using the updated configuration with an object created earlier that lacks it.

Maybe the [paths] configuration was supposed to be updated on self.combining_cov instead of self.cov?

@ionelmc
Copy link
Member

ionelmc commented Jan 22, 2019

@jmbowman that line is like that to force coverage to reload the coverage data (fixes other issues). Not sure what's the bug - what unused code are you referring to?

@jmbowman
Copy link
Contributor

There is code when the master initializes and when each worker shuts down that ensures the configuration for self.cov contains a mapping between source code paths on the master and on each worker, used when combining all the collected coverage data files. But this configuration is lost when self.cov is replaced with self.combining_cov just before combining the files.

@ionelmc
Copy link
Member

ionelmc commented Jan 23, 2019

So you're suggesting the paths aren't adjusted before saving the fist cov instance (that gets replaced)?

@blueyed
Copy link
Contributor

blueyed commented Jan 23, 2019

@jmbowman
btw: pressing y in a commit view will use the current hash in the URL (GitHub redirects you). This allows for source reference URLs to stay correct after e.g. master gets changed.

@jmbowman
Copy link
Contributor

@ionelmc Correct, the paths never get adjusted in the individual data files; coverage is always recorded with the actual paths used on the filesystem where the code is being run. The paths configuration is only used when running coverage combine (or in this case, the underling Coverage.combine() method) to generate a new coverage data file aggregating all of the individual ones. I used some debugging code to confirm that coverage was being measured correctly on each system, and all the files were being gathered on the master as intended, but the Coverage object used to combine them lacked the generated source entry in the paths configuration, and the combined file used the original paths from each system instead of mapping them all to the paths used on the master. This meant that any coverage recorded on the remote systems was ignored when reporting because it couldn't be mapped to actual files on the system generating the report.

The bug can be worked around if you know all of the remote machine base paths ahead of time by putting the correct paths configuration in .coveragerc; this is what I did in https://github.com/edx/edx-platform/pull/19630 to resolve my immediate problem. In the process, I discovered a more esoteric bug in coverage.py that @nedbat captured in nedbat/coveragepy#757 (I had multiple path lists in paths while debugging, and it wasn't choosing one according to the rules implied in the docs).

@blueyed Thanks for the tip!

@ionelmc ionelmc added the bug label Jan 23, 2019
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

4 participants