pytest-dev / pytest-cov

Coverage plugin for pytest.
MIT License
1.72k stars 211 forks source link

pytest-cov is incompatible with pytest-xdist 3.x due to injection of rsync option #557

Closed ssbarnea closed 1 year ago

ssbarnea commented 1 year ago

Summary

See https://github.com/pytest-dev/pytest-xdist/issues/825#issuecomment-1292306864 which reports a deprecation option related to rsyncdir(s) options. This breaks the builds for anyone that run with warnings-as-errors.

Code

https://github.com/pytest-dev/pytest-cov/blob/f7fced579e36b72b57e14768026467e4c4511a40/src/pytest_cov/engine.py#L263 Link to your repository, gist, pastebin or just paste raw code that illustrates the issue.

Solution

We need to stop adding this option on newer versions as it was already deprecated and will be removed on next version of xdist.

DanielNoord commented 1 year ago

We're still seeing this DeprecationWarning being raised on the new release. What can still trigger it after the PR got merged?

psi29a commented 1 year ago

We're also still getting this message.

Our workaround was to not use --cov-config and instead pass the env variable COVERAGE_RCFILE.

COVERAGE_RCFILE=.coveragerc pytest --cov
ionelmc commented 1 year ago

Oh damn I worded the changelog incorrectly. There's a feature test in pytest-cov for rsyncdir support. Basically the change allows pytest-cov to continute to work with xdist 4, it doesn't deal with the warning.

psi29a commented 1 year ago

It's strange, because passing this env variable tells coverage to use the file I want... if we use this:

pytest -n logical --maxprocesses=4 --dist loadgroup --cov --cov-config=../configuration/.coveragerc --cov-append --cov-branch

We get this:

INTERNALERROR> DeprecationWarning: The --rsyncdir command line argument and rsyncdirs config variable are deprecated.

INTERNALERROR> The rsync feature will be removed in pytest-xdist 4.0.

This does not make sense, as we don't use rsync at all.

Doing this however:

COVERAGE_RCFILE=.coveragerc pytest -n logical --maxprocesses=4 --dist loadgroup --cov --cov-append --cov-branch

then we can run without issues and no warning.

Seems like an issue to me, can you verify please? :)

ionelmc commented 1 year ago

Works as expected - because you have specified the config file via pytest-cov and enabled xdist it's being sent over xdist's file sharing mechanism. It may not be what you want but it is what it's implemented.

mdantonio commented 1 year ago

it doesn't deal with the warning

I'm a bit confused here 😖 Is another issue needed to deal with the warning?

DanielNoord commented 1 year ago

@ionelmc Could you help us out here?

ionelmc commented 1 year ago

Well I guess there could be another pytest-cov release outright removing the rsync stuff. Don't really have an opinion here. Would that make everyone happy?

mdantonio commented 1 year ago

I think so, if the warning is solved I think no one would complain again! (for sure I would be super happy! 🙏)

skirpichev commented 1 year ago

@ionelmc, unfortunately this solution doesn't work for all: with diofant/diofant@dd326d72ad I got this build failure (problem coming from measuring coverage from sub-processes).

While removing the option completely (see skirpichev/pytest-cov@3e96ac3) does work for me, see diofant/diofant#1345 (i.e. no coverage regressions).

kasium commented 6 months ago

Hi guys,

there is still the use case of an explicit coverage file which seems to reply on rsync https://github.com/pytest-dev/pytest-cov/blob/2c9f2170d8575b21bafb6402eb30ca7de31e20b9/src/pytest_cov/engine.py#L268-L272

Any plan to allow this w/o using rsync?