Skip to content

pytester now requests monkeypatch instead of creating its own instance #9714

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

Merged

Conversation

nicoddemus
Copy link
Member

It is tempting to use monkeypatch to replace the other mechanisms in pytester which change global
state: CwdSnapshot, SysModulesSnapshot, SysPathsSnapshot, however those are more delicate
than they look at first glance so leaving those alone for now.

Close #9708

@nicoddemus nicoddemus requested a review from asottile February 25, 2022 11:28
assert "PYTEST_ADDOPTS" not in os.environ
pytester._finalize()
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was testing that PYTEST_ADDOPTS was being undone by pytester, but we now delegate it to monkeypatch. The rest was testing monkeypatch itself, which is redundant and can be removed.

@nicoddemus nicoddemus force-pushed the pytester-request-monkeypatch-9708 branch 3 times, most recently from e394a16 to 232c79a Compare February 25, 2022 11:44
@nicoddemus
Copy link
Member Author

Failures seem related at first glance however they are also happening on main. Cannot reproduce them locally. 🤔

Copy link
Member

@asottile asottile left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@asottile
Copy link
Member

Failures seem related at first glance however they are also happening on main. Cannot reproduce them locally. 🤔

ahah! my tool to the rescue again!

here's the minimal set, I'll see if I can fix those:

pytest testing/test_doctest.py::TestDoctests::test_valid_setup_py testing/test_helpconfig.py::test_version_verbose
$ pytest testing/test_doctest.py::TestDoctests::test_valid_setup_py testing/test_helpconfig.py::test_version_verbose
============================= test session starts ==============================
platform linux -- Python 3.8.10, pytest-7.1.0.dev235+g5f3d94c47, pluggy-1.0.0
rootdir: /tmp/pytest, configfile: pyproject.toml
plugins: hypothesis-6.37.2
collected 2 items                                                              

testing/test_doctest.py .                                                [ 50%]
testing/test_helpconfig.py F                                             [100%]

=================================== FAILURES ===================================
_____________________________ test_version_verbose _____________________________

pytester = <Pytester PosixPath('/tmp/pytest-of-asottile/pytest-21/test_version_verbose0')>
pytestconfig = <_pytest.config.Config object at 0x7f60e706e460>
monkeypatch = <_pytest.monkeypatch.MonkeyPatch object at 0x7f60e6de67c0>

    def test_version_verbose(pytester: Pytester, pytestconfig, monkeypatch) -> None:
        monkeypatch.delenv("PYTEST_DISABLE_PLUGIN_AUTOLOAD")
        result = pytester.runpytest("--version", "--version")
        assert result.ret == 0
        result.stdout.fnmatch_lines([f"*pytest*{pytest.__version__}*imported from*"])
        if pytestconfig.pluginmanager.list_plugin_distinfo():
>           result.stdout.fnmatch_lines(["*setuptools registered plugins:", "*at*"])
E           Failed: nomatch: '*setuptools registered plugins:'
E               and: 'This is pytest version 7.1.0.dev235+g5f3d94c47, imported from /tmp/pytest/src/pytest/__init__.py'
E           remains unmatched: '*setuptools registered plugins:'

/tmp/pytest/testing/test_helpconfig.py:12: Failed
----------------------------- Captured stdout call -----------------------------
This is pytest version 7.1.0.dev235+g5f3d94c47, imported from /tmp/pytest/src/pytest/__init__.py
=========================== short test summary info ============================
FAILED testing/test_helpconfig.py::test_version_verbose - Failed: nomatch: '*...
========================= 1 failed, 1 passed in 0.43s ==========================

@asottile
Copy link
Member

appears to be caused by setuptools 60.9 as well, looking into why

@asottile
Copy link
Member

alright -- this is the minimal reproduction -- factored out pytest but I have the same "modules snapshot" code in there:

import importlib.metadata
import sys

class SysPathsSnapshot:
    def __init__(self) -> None:
        self.__saved = list(sys.path), list(sys.meta_path)

    def __enter__(self):
        return self

    def __exit__(self, *a):
        sys.path[:], sys.meta_path[:] = self.__saved


for dist in importlib.metadata.distributions():
    for ep in dist.entry_points:
        if ep.group == 'console_scripts' and ep.name == 'pip':
            print(ep)

with SysPathsSnapshot():
    import setuptools

print('===')

for dist in importlib.metadata.distributions():
    for ep in dist.entry_points:
        if ep.group == 'console_scripts' and ep.name == 'pip':
            print(ep)

@jaraco this appears to be directly caused by setuptools vendoring importlib-metadata

@jaraco
Copy link
Contributor

jaraco commented Feb 25, 2022

I can confirm the behavior is different when running asottile's test against Python 3.9:

draft $ cat test.py
__requires__ = ['setuptools>=60.9']


import importlib.metadata
import sys


class SysPathsSnapshot:
    def __init__(self) -> None:
        self.__saved = list(sys.path), list(sys.meta_path)

    def __enter__(self):
        return self

    def __exit__(self, *a):
        sys.path[:], sys.meta_path[:] = self.__saved


for dist in importlib.metadata.distributions():
    for ep in dist.entry_points:
        if ep.group == 'console_scripts' and ep.name == 'pip':
            print(ep)


with SysPathsSnapshot():
    __import__('setuptools')


print('===')


for dist in importlib.metadata.distributions():
    for ep in dist.entry_points:
        if ep.group == 'console_scripts' and ep.name == 'pip':
            print(ep)
draft $ pip-run -- test.py
Collecting setuptools
  Using cached setuptools-60.9.3-py3-none-any.whl (1.1 MB)
Installing collected packages: setuptools
Successfully installed setuptools-60.9.3
EntryPoint(name='pip', value='pip._internal.cli.main:main', group='console_scripts')
===
EntryPoint(name='pip', value='pip._internal.cli.main:main', group='console_scripts')
draft $ py -3.9 -m pip-run -- test.py
Collecting setuptools>=60.9
  Using cached setuptools-60.9.3-py3-none-any.whl (1.1 MB)
Installing collected packages: setuptools
Successfully installed setuptools-60.9.3
EntryPoint(name='pip', value='pip._internal.cli.main:main', group='console_scripts')
===

I can also confirm the same behavior will occur with the importlib_metadata backport even on python 3.10:

draft $ cat test.py
__requires__ = ['importlib_metadata']


import importlib.metadata
import sys


class SysPathsSnapshot:
    def __init__(self) -> None:
        self.__saved = list(sys.path), list(sys.meta_path)

    def __enter__(self):
        return self

    def __exit__(self, *a):
        sys.path[:], sys.meta_path[:] = self.__saved


def find_pip():
    for dist in importlib.metadata.distributions():
        for ep in dist.entry_points:
            if ep.group == 'console_scripts' and ep.name == 'pip':
                print(ep)


find_pip()

with SysPathsSnapshot():
    __import__('importlib_metadata')


print('===')
find_pip()
draft $ pip-run -- test.py
Collecting importlib_metadata
  Using cached importlib_metadata-4.11.1-py3-none-any.whl (17 kB)
Collecting zipp>=0.5
  Using cached zipp-3.7.0-py3-none-any.whl (5.3 kB)
Installing collected packages: zipp, importlib_metadata
Successfully installed importlib_metadata-4.11.1 zipp-3.7.0
EntryPoint(name='pip', value='pip._internal.cli.main:main', group='console_scripts')
===

The issues are that:

  • When imported, importlib_metadata installs a hook to enable discovery of PathDistributions.
  • When imported, importlib_metadata supersedes importlib.metadata for that hook.
  • Setuptools must vendor importlib_metadata (Python 3.9 and earlier), so gets this behavior also.

I'm not sure if a fix is possible given these constraints.

@RonnyPfannschmidt
Copy link
Member

@jaraco would it be possible to work around the constraints from the pytest side at least (aka restoring importlib metadata/setuptools hooks in some manner to ensure finding works as one would hope (after all sys.path/sys.meta_path setup/cleanup is a bit of a mess, albeit necessary for pytester as of now)

It is tempting to use `monkeypatch` to replace the other mechanisms in pytester which change global
state: `CwdSnapshot`, `SysModulesSnapshot`, `SysPathsSnapshot`, however those are more delicate
than they look at first glance so leaving those alone for now.

Close pytest-dev#9708
@asottile asottile force-pushed the pytester-request-monkeypatch-9708 branch from 232c79a to f943d19 Compare March 4, 2022 13:58
@asottile asottile merged commit 51b86b4 into pytest-dev:main Mar 4, 2022
@nicoddemus nicoddemus deleted the pytester-request-monkeypatch-9708 branch March 4, 2022 15:44
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

Successfully merging this pull request may close these issues.

pytester (?) causes monkeypatch to not tear down correctly
4 participants