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

Regression in 2.6.0: running coverage combine after pytest-cov now errors out #222

Open
njsmith opened this issue Sep 5, 2018 · 16 comments

Comments

@njsmith
Copy link

njsmith commented Sep 5, 2018

We run coverage in parallel=True mode, because we have some tests that spawn subprocesses, and we want to capture coverage for all of them. To get a complete report for codecov, the CI does:

pytest --cov ...
coverage combine
bash <(curl -s https://codecov.io/bash)

As far as I know, this was mandatory on pytest-cov 2.5.1 and earlier. Since 2.6.0 came out yesterday, our tests have been failing because coverage combine says No data to combine and it errors out, failing the test suite.

I think this may be happening because pytest-cov is now effectively running coverage combine itself, so that instead of being mandatory to run coverage combine, it's now mandatory to not to run coverage combine? Possibly because of changes in #178? For now that seems to be working for us. But it'd be nice to confirm that our understanding is correct, and I figured you'd rather have a heads-up than not.

Our bug: python-trio/trio#646

anthrotype added a commit to anthrotype/afdko that referenced this issue Sep 5, 2018
to work around 'No data to combine error'

The latest pytest-cov 2.6.0 seems to be running coverage combine itself:
pytest-dev/pytest-cov#222

we may revert this later if we find a better solution
@ionelmc ionelmc changed the title Regression in 3.6.0: running coverage combine after pytest-cov now errors out Regression in 2.6.0: running coverage combine after pytest-cov now errors out Sep 5, 2018
@ionelmc
Copy link
Member

ionelmc commented Sep 5, 2018

Yeah sorry, it was an unforeseen consequence of fixing the combining problems at the end of the run (#130). Unfortunately coverage 4.3 made coverage combine strict - nedbat/coveragepy#412

Perhaps we could persuade @nedbat to revert that behavior, or have a config switch for it?

@ionelmc
Copy link
Member

ionelmc commented Sep 5, 2018

Maybe we could save the coverage file with a suffix as a workaround (then combine wouldn't fail). For how many ppl is this regression a problem?

@njsmith
Copy link
Author

njsmith commented Sep 5, 2018

It's not causing a problem for us anymore, though if you decide to switch back to requiring combine then we'd appreciate a heads up :-).

It does seem reasonable for pytest-cov to take care of this step in principle... it's one less obscure trick for people to learn.

Maybe coverage combine should detect when there are no files to combine, and there is already a single no-pid file, and in that case print a warning ("did you already run combine?") and exit without error?

miguelsousa pushed a commit to adobe-type-tools/afdko that referenced this issue Sep 6, 2018
to work around 'No data to combine error'

The latest pytest-cov 2.6.0 seems to be running coverage combine itself:
pytest-dev/pytest-cov#222

we may revert this later if we find a better solution
miguelsousa pushed a commit to adobe-type-tools/afdko that referenced this issue Sep 7, 2018
to work around 'No data to combine error'

The latest pytest-cov 2.6.0 seems to be running coverage combine itself:
pytest-dev/pytest-cov#222

we may revert this later if we find a better solution
@nedbat
Copy link
Collaborator

nedbat commented Sep 7, 2018

@ionelmc Hello! I'm also struggling with how to make 2.6.0 work for me. The issue says that now pytest-cov is doing it's own combine? But it seems to not be picking up the [paths] setting? Issue #130 seems to be talking about a similar problem.

miguelsousa pushed a commit to adobe-type-tools/afdko that referenced this issue Sep 7, 2018
to work around 'No data to combine error'

The latest pytest-cov 2.6.0 seems to be running coverage combine itself:
pytest-dev/pytest-cov#222

we may revert this later if we find a better solution
miguelsousa pushed a commit to adobe-type-tools/afdko that referenced this issue Sep 7, 2018
to work around 'No data to combine error'

The latest pytest-cov 2.6.0 seems to be running coverage combine itself:
pytest-dev/pytest-cov#222

we may revert this later if we find a better solution
@ionelmc
Copy link
Member

ionelmc commented Sep 7, 2018

@nedbat ooof ... 2.6.0 was supposed to fix #130 - do you have an example where 2.6.0 don't work as expected?

@nedbat
Copy link
Collaborator

nedbat commented Sep 7, 2018

As you might expect, this is in a large distributed test suite. I've worked around it by adding mv .coverage .coverage.1; coverage combine before my coverage html command.

I would like to get a small example of the behavior though, and I'm still working on that.

jbenden added a commit to jbenden/deployer that referenced this issue Sep 9, 2018
This patch is a work-around for an issue with `pytest-cov`. For
additional information, please see:

pytest-dev/pytest-cov#222
@blueyed
Copy link
Contributor

blueyed commented Sep 16, 2018

Update: I was missing the --cov (created #223 to fix the docs). But even with this (and as used before) coverage is dropping significantly when upgrading pytest-cov.. see pytest-dev/pytest-django#651 for details (look at the codecov report for the commits). Will report this separately after investigating, but the main question remains for now:

Is the combining meant to happen always now?

I'm a bit confused that the combining is not being done for me with 2.6.0..

.coveragerc:

[run]
parallel = true
source = .
branch = true
[report]
include = pytest_django/*,pytest_django_test/*,tests/*

tox.ini:

setenv =
    PYTHONPATH = {toxinidir}:{env:PYTHONPATH:}
    COV_CORE_SOURCE={toxinidir}
    COV_CORE_CONFIG={toxinidir}/.coveragerc
    COV_CORE_DATAFILE={toxinidir}/.coverage.eager
passenv = PYTEST_ADDOPTS

I set PYTEST_ADDOPTS='--cov-append --cov-report=xml --cov-report=term-missing:skip-covered'

I have all of the .coverage.eager.* files afterwards, and have to run coverage combine like before.

(I am investigating why pytest-cov 2.6.0 causes coverage to drop on codecov for pytest-django by 74% (pytest-dev/pytest-django#650))

@blueyed
Copy link
Contributor

blueyed commented Sep 21, 2018

Just for reference: only a single/smaller .coverage.eager.* file was left (apparently created after pytest-cov's internal combining), and running coverage combine (without -a) then caused coverage to drop.
This is due to using the eager approach, so this might also be a good workaround to make coverage combine work (but then use -a). Related PR for pytest-django: https://github.com/pytest-dev/pytest-django/pull/658/files.

@ionelmc
Copy link
Member

ionelmc commented Mar 24, 2019

Reverting the combining would make these tests fail:

  • test_central_with_path_aliasing
  • test_subprocess_with_path_aliasing

@nedbat so I've looked over the code in coverage 4.5.2 - it looks like the problem lies in the fact that neither of these methods applies aliasing to the current data:

  • coverage.control.Coverage.stop
  • coverage.control.Coverage.save
  • coverage.control.Coverage.combine - only applies aliasing to data on disk (as .coverage files), not to the current data (self.data)

So if you want this change in pytest-cov reverted then please advise of a way to correctly apply aliasing to current data, or fix the Coverage object to apply full aliasing in either of those methods.

@cjw296
Copy link

cjw296 commented Jan 18, 2020

This auto-combine is problematic when you have several runs of pytest (say across different versions of python) and want to combine them. Please at least add an option to prevent pytest-cov doing this combining.

@ionelmc
Copy link
Member

ionelmc commented Jan 19, 2020

@cjw296 --cov-append should solve that problem. If not then explain in more detail your usecase.

@cjw296
Copy link

cjw296 commented Jan 20, 2020

@ionelmc - I don't think it will. I run CI in docker containers, one for each python version, with parallel=True in the coverage config. So, each container should spit out a bunch of .coverage.whatever files, these are written to a volume outside the container. A separate job then runs coverage combine, coverage report, etc on the whole set of files.

Because each run with pytest-cov ends up producing only a .coverage file, the workflow fails. In my case, it's easy enough to just not use pytest-cov, but wanted to let you know...

@pawamoy
Copy link

pawamoy commented Sep 8, 2020

I'm having a similar issue. I run pytest with coverage multiple times and want to combine all results. If I don't use --cov-append, then between each run the .coverage file is deleted and then re-combined. At the end I get the coverage for the last run only. If I use --cov-append, now I have to explicitly delete the .coverage file myself before the runs.

In both cases it's one more command to run for me, but I honestly prefer combining explicitly myself, it feels less magic 🙂 (I had to run experiments to be sure of what happens with or without --cov-append). I'd say it's an argument in favor of #337.

@ionelmc
Copy link
Member

ionelmc commented Sep 28, 2020

Would a config solution as in #435 (setting a COVERAGE_FILE env var) solve the combining problems here?

@cjw296
Copy link

cjw296 commented Sep 29, 2020

Something that that could be put in .coveragerc or pytest.ini would be ideal.

@jiridanek
Copy link

My take on this is that while pytest users tend to naturally gravitate to pytest-cov, it is actually not necessary and running coverage.py directly works just as well, and the whole setup has less moving parts https://coverage.readthedocs.io/en/stable/#quick-start. So, unless you need pytest-cov to achieve something specific, I suggest not to use it "just because we use pytest".

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

No branches or pull requests

7 participants