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.

Sign up for GitHub

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

Jump to bottom

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

Closed
ssbarnea opened this issue Oct 26, 2022 · 11 comments · Fixed by #558
Closed

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

ssbarnea opened this issue Oct 26, 2022 · 11 comments · Fixed by #558

Comments

@ssbarnea
Copy link
Member

Summary

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

Code

pytest-cov/src/pytest_cov/engine.py

Line 263 in f7fced5

self.config.option.rsyncdir.append(self.cov_config)

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.
This was referenced Oct 26, 2022
Closed
Closed
ssbarnea added a commit that referenced this issue Oct 26, 2022
@ssbarnea
Remove use of rsyncdir
0242835 
Fixes: #557
@ssbarnea ssbarnea mentioned this issue Oct 26, 2022
Merged
ssbarnea added a commit to ssbarnea/pytest-cov that referenced this issue Oct 26, 2022
@ssbarnea
Remove use of rsyncdir
4865bec 
Fixes: pytest-dev#557
@skirpichev skirpichev mentioned this issue Oct 30, 2022
Merged
WarrenWeckesser added a commit to WarrenWeckesser/scipy that referenced this issue Nov 3, 2022
@WarrenWeckesser
CI: On Azure, pin pytest-xdist to version 2.5.0
ae9bd52 
The Azure job "Main prerelease_deps_coverage_64bit_blas" is
failing because of an incompatibility between ptest-cov and
pytest-xdist 3.0.2, so use ptest-xdist 2.5.0 until the issue
pytest-dev/pytest-cov#557
is resolved.

[skip cirrus] [skip actions] [skip circle]
@WarrenWeckesser WarrenWeckesser mentioned this issue Nov 3, 2022
Merged
WarrenWeckesser added a commit to WarrenWeckesser/scipy that referenced this issue Nov 3, 2022
@WarrenWeckesser
CI: On Azure, pin pytest-xdist to version 2.5.0
f1e5dc9 
The Azure job "Main prerelease_deps_coverage_64bit_blas" is
failing because of an incompatibility between pytest-cov and
pytest-xdist 3.0.2, so use ptest-xdist 2.5.0 until the issue
pytest-dev/pytest-cov#557
is resolved.

[skip cirrus] [skip actions] [skip circle]
WarrenWeckesser added a commit to WarrenWeckesser/scipy that referenced this issue Nov 3, 2022
@WarrenWeckesser
CI: On Azure, pin pytest-xdist to version 2.5.0
3d5ead9 
The Azure job "Main prerelease_deps_coverage_64bit_blas" is
failing because of an incompatibility between pytest-cov and
pytest-xdist 3.0.2, so use pytest-xdist 2.5.0 until the issue
pytest-dev/pytest-cov#557
is resolved.

[skip cirrus] [skip actions] [skip circle]
webknjaz added a commit to sphinx-contrib/sphinxcontrib-towncrier that referenced this issue Nov 4, 2022
@webknjaz
Ignore pytest-xdist's depracation warning
50c3912 
It is coming from `pytest-cov`'s usage of the deprecated `rsyncdir`
setting use.

This patch can be reverted once
pytest-dev/pytest-cov#558 is in and released.

Ref: pytest-dev/pytest-cov#557
@dstansby dstansby mentioned this issue Nov 22, 2022
Merged
portante added a commit to portante/pbench that referenced this issue Jan 12, 2023
@portante
Silence warnings display by python unit tests
0400233 
There were two categories of warnings being displayed, those related
to the xdist plugin, and those related to SQLAlchemy 2.0 migration.

The xdist plugin warnings are due to a bug in pytest-cov, see:
    pytest-dev/pytest-cov#557
We just ignore them entirely for now, as that issue seems to be quiet.

The SQLAlchemy 2.0 migration warnings have 2 sub-categories.  The
first being that `declarative_base` has moved modules, and that warning
is removed by using the proper import.

However, once that `declarative_base` warning is removed, one
encounters:

    .../pbench/server/database/models/datasets.py:875:
        RemovedIn20Warning: "Metadata" object is being merged into a
        Session along the backref cascade path for relationship
        "Dataset.metadatas"; in SQLAlchemy 2.0, this reverse cascade
        will not take place.  Set cascade_backrefs to False in either
        the relationship() or backref() function for the 2.0 behavior;
        or to set globally for the whole Session, set the future=True
        flag (Background on SQLAlchemy 2.0 at:
        https://sqlalche.me/e/b8d9)
            meta = Metadata(**kwargs)

Since we are no going to migrate to SQLAlchemy 2.0 any time soon, it
seemed appropriate to just use the big hammer and turn the warnings
off entirely.
@portante portante mentioned this issue Jan 12, 2023
Merged
portante added a commit to portante/pbench that referenced this issue Jan 12, 2023
@portante
Silence warnings displayed by python unit tests
ff22deb 
There were two categories of warnings being displayed, those related
to the xdist plugin, and those related to SQLAlchemy 2.0 migration.

The xdist plugin warnings are due to a bug in pytest-cov, see:
    pytest-dev/pytest-cov#557
We just ignore them entirely for now, as that issue seems to be quiet.

The SQLAlchemy 2.0 migration warnings have 2 sub-categories.  The
first being that `declarative_base` has moved modules, and that warning
is removed by using the proper import.

However, once that `declarative_base` warning is removed, one
encounters:

    .../pbench/server/database/models/datasets.py:875:
        RemovedIn20Warning: "Metadata" object is being merged into a
        Session along the backref cascade path for relationship
        "Dataset.metadatas"; in SQLAlchemy 2.0, this reverse cascade
        will not take place.  Set cascade_backrefs to False in either
        the relationship() or backref() function for the 2.0 behavior;
        or to set globally for the whole Session, set the future=True
        flag (Background on SQLAlchemy 2.0 at:
        https://sqlalche.me/e/b8d9)
            meta = Metadata(**kwargs)

Since we are no going to migrate to SQLAlchemy 2.0 any time soon, it
seemed appropriate to just use the big hammer and turn the warnings
off entirely.
portante added a commit to portante/pbench that referenced this issue Jan 12, 2023
@portante
Silence warnings displayed by python unit tests
55a0571 
There were two categories of warnings being displayed, those related
to the xdist plugin, and those related to SQLAlchemy 2.0 migration.

The xdist plugin warnings are due to a bug in pytest-cov, see:
    pytest-dev/pytest-cov#557
We just ignore them entirely for now, as that issue seems to be quiet.

The SQLAlchemy 2.0 migration warnings have 2 sub-categories.  The
first being that `declarative_base` has moved modules, and that warning
is removed by using the proper import.

However, once that `declarative_base` warning is removed, one
encounters:

    .../pbench/server/database/models/datasets.py:875:
        RemovedIn20Warning: "Metadata" object is being merged into a
        Session along the backref cascade path for relationship
        "Dataset.metadatas"; in SQLAlchemy 2.0, this reverse cascade
        will not take place.  Set cascade_backrefs to False in either
        the relationship() or backref() function for the 2.0 behavior;
        or to set globally for the whole Session, set the future=True
        flag (Background on SQLAlchemy 2.0 at:
        https://sqlalche.me/e/b8d9)
            meta = Metadata(**kwargs)

Since we are no going to migrate to SQLAlchemy 2.0 any time soon, it
seemed appropriate to just use the big hammer and turn the warnings
off entirely.
portante added a commit to portante/pbench that referenced this issue Jan 12, 2023
@portante
Silence warnings displayed by python unit tests
284f23a 
There were two categories of warnings being displayed, those related
to the xdist plugin, and those related to SQLAlchemy 2.0 migration.

The xdist plugin warnings are due to a bug in pytest-cov, see:
    pytest-dev/pytest-cov#557
We just ignore them entirely for now, as that issue seems to be quiet.

The SQLAlchemy 2.0 migration warnings have 2 sub-categories.  The
first being that `declarative_base` has moved modules, and that warning
is removed by using the proper import.

However, once that `declarative_base` warning is removed, one
encounters:

    .../pbench/server/database/models/datasets.py:875:
        RemovedIn20Warning: "Metadata" object is being merged into a
        Session along the backref cascade path for relationship
        "Dataset.metadatas"; in SQLAlchemy 2.0, this reverse cascade
        will not take place.  Set cascade_backrefs to False in either
        the relationship() or backref() function for the 2.0 behavior;
        or to set globally for the whole Session, set the future=True
        flag (Background on SQLAlchemy 2.0 at:
        https://sqlalche.me/e/b8d9)
            meta = Metadata(**kwargs)

Since we are no going to migrate to SQLAlchemy 2.0 any time soon, it
seemed appropriate to just use the big hammer and turn the warnings
off entirely.
portante added a commit to portante/pbench that referenced this issue Jan 12, 2023
@portante
Silence warnings displayed by python unit tests
1b16727 
There were two categories of warnings being displayed, those related
to the xdist plugin, and those related to SQLAlchemy 2.0 migration.

The xdist plugin warnings are due to a bug in pytest-cov, see:
    pytest-dev/pytest-cov#557
We just ignore them entirely for now, as that issue seems to be quiet.

The SQLAlchemy 2.0 migration warnings have 2 sub-categories.  The
first being that `declarative_base` has moved modules, and that warning
is removed by using the proper import.

However, once that `declarative_base` warning is removed, one
encounters:

    .../pbench/server/database/models/datasets.py:875:
        RemovedIn20Warning: "Metadata" object is being merged into a
        Session along the backref cascade path for relationship
        "Dataset.metadatas"; in SQLAlchemy 2.0, this reverse cascade
        will not take place.  Set cascade_backrefs to False in either
        the relationship() or backref() function for the 2.0 behavior;
        or to set globally for the whole Session, set the future=True
        flag (Background on SQLAlchemy 2.0 at:
        https://sqlalche.me/e/b8d9)
            meta = Metadata(**kwargs)

Since we are no going to migrate to SQLAlchemy 2.0 any time soon, it
seemed appropriate to just use the big hammer and turn the warnings
off entirely.
portante added a commit to distributed-system-analysis/pbench that referenced this issue Jan 12, 2023
@portante
Silence warnings displayed by python unit tests
b9de4ba 
There were two categories of warnings being displayed, those related
to the xdist plugin, and those related to SQLAlchemy 2.0 migration.

The xdist plugin warnings are due to a bug in pytest-cov, see:
    pytest-dev/pytest-cov#557
We just ignore them entirely for now, as that issue seems to be quiet.

The SQLAlchemy 2.0 migration warnings have 2 sub-categories.  The
first being that `declarative_base` has moved modules, and that warning
is removed by using the proper import.

However, once that `declarative_base` warning is removed, one
encounters:

    .../pbench/server/database/models/datasets.py:875:
        RemovedIn20Warning: "Metadata" object is being merged into a
        Session along the backref cascade path for relationship
        "Dataset.metadatas"; in SQLAlchemy 2.0, this reverse cascade
        will not take place.  Set cascade_backrefs to False in either
        the relationship() or backref() function for the 2.0 behavior;
        or to set globally for the whole Session, set the future=True
        flag (Background on SQLAlchemy 2.0 at:
        https://sqlalche.me/e/b8d9)
            meta = Metadata(**kwargs)

Since we are no going to migrate to SQLAlchemy 2.0 any time soon, it
seemed appropriate to just use the big hammer and turn the warnings
off entirely.
ionelmc pushed a commit that referenced this issue Feb 28, 2023
@ssbarnea @ionelmc
Remove use of rsyncdir
2ea9442 
Fixes: #557
@DanielNoord
Copy link

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

@psi29a
Copy link

psi29a commented May 26, 2023

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
Copy link
Member

ionelmc commented May 26, 2023

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
Copy link

psi29a commented May 26, 2023

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
Copy link
Member

ionelmc commented May 26, 2023

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
Copy link

it doesn't deal with the warning

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

@DanielNoord
Copy link

@ionelmc Could you help us out here?

@ionelmc
Copy link
Member

ionelmc commented Jun 6, 2023

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
Copy link

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

@skirpichev skirpichev mentioned this issue Aug 18, 2023
Closed
skirpichev added a commit to skirpichev/diofant that referenced this issue Aug 18, 2023
@skirpichev
Revert 4bb2911
dd326d7 
Use COVERAGE_RCFILE environment variable instead of --cov-config.
See pytest-dev/pytest-cov#557.
@skirpichev
Copy link

@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).

@RomanBredehoft RomanBredehoft mentioned this issue Sep 26, 2023
Merged
@parthea parthea mentioned this issue Nov 28, 2023
Open
This was referenced Nov 28, 2023
Closed
Closed
@lesteve lesteve mentioned this issue Jan 8, 2024
Merged
@kasium
Copy link

kasium commented Mar 13, 2024

Hi guys,

there is still the use case of an explicit coverage file which seems to reply on rsync

pytest-cov/src/pytest_cov/engine.py

Lines 268 to 272 in 2c9f217

# Ensure coverage rc file rsynced if appropriate.
if self.cov_config and os.path.exists(self.cov_config):
# rsyncdir is going away in pytest-xdist 4.0, already deprecated
if hasattr(self.config.option, 'rsyncdir'):
self.config.option.rsyncdir.append(self.cov_config)

Any plan to allow this w/o using rsync?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Remove use of rsyncdir ssbarnea/pytest-cov
7 participants
@ssbarnea @ionelmc @psi29a @skirpichev @mdantonio @DanielNoord @kasium