pytester now requests monkeypatch instead of creating its own instance by nicoddemus · Pull Request #9714 · pytest-dev/pytest

nicoddemus · 2022-02-25T11:28:26Z

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 · 2022-02-25T11:29:47Z

testing/test_pytester.py

-    pytester: Pytester = request.getfixturevalue("pytester")
+    _: Pytester = request.getfixturevalue("pytester")
    assert "PYTEST_ADDOPTS" not in os.environ
-    pytester._finalize()


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 · 2022-02-25T11:45:08Z

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

asottile

asottile · 2022-02-25T17:52:49Z

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 · 2022-02-25T18:13:44Z

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

asottile · 2022-02-25T18:46:19Z

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 · 2022-02-25T21:28:02Z

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 · 2022-02-26T17:10:55Z

@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

nicoddemus requested a review from asottile February 25, 2022 11:28

nicoddemus commented Feb 25, 2022

View reviewed changes

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

asottile approved these changes Feb 25, 2022

View reviewed changes

asottile mentioned this pull request Mar 1, 2022

prevent import-time side-effects from setuptools bundled importlib-metadata #9719

Merged

asottile force-pushed the pytester-request-monkeypatch-9708 branch from 232c79a to f943d19 Compare March 4, 2022 13:58

asottile merged commit 51b86b4 into pytest-dev:main Mar 4, 2022

nicoddemus deleted the pytester-request-monkeypatch-9708 branch March 4, 2022 15:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Comments

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

pytester now requests monkeypatch instead of creating its own instance#9714
asottile merged 1 commit intopytest-dev:mainfrom
nicoddemus:pytester-request-monkeypatch-9708

nicoddemus commented Feb 25, 2022

Uh oh!

nicoddemus Feb 25, 2022

Uh oh!

nicoddemus commented Feb 25, 2022

Uh oh!

asottile left a comment

Uh oh!

asottile commented Feb 25, 2022

Uh oh!

asottile commented Feb 25, 2022

Uh oh!

asottile commented Feb 25, 2022

Uh oh!

jaraco commented Feb 25, 2022

Uh oh!

RonnyPfannschmidt commented Feb 26, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Comments

Conversation

nicoddemus commented Feb 25, 2022

Uh oh!

nicoddemus Feb 25, 2022

Choose a reason for hiding this comment

Uh oh!

nicoddemus commented Feb 25, 2022

Uh oh!

asottile left a comment

Choose a reason for hiding this comment

Uh oh!

asottile commented Feb 25, 2022

Uh oh!

asottile commented Feb 25, 2022

Uh oh!

asottile commented Feb 25, 2022

Uh oh!

jaraco commented Feb 25, 2022

Uh oh!

RonnyPfannschmidt commented Feb 26, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants