From 81192ca85f8e64ddf71a7c467c228be08440ebce Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Tue, 15 Aug 2023 22:09:00 +0300 Subject: [PATCH] pytester: use monkeypatch.chdir() for dir changing The current method as the following problem, described by Sadra Barikbin: The tests that request both `pytester` and `monkeypatch` and use `monkeypatch.chdir` without context, relying on `monkeypatch`'s teardown to restore cwd. This doesn't work because the following sequence of actions take place: - `monkeypatch` is set up. - `pytester` is set up. It saves the original cwd and changes it to a new one dedicated to the test function. - Test function calls `monkeypatch.chdir()` without context. `monkeypatch` saves cwd, which is not the original one, before changing it. - `pytester` is torn down. It restores the cwd to the original one. - `monkeypatch` is torn down. It restores cwd to what it has saved. The solution here is to have pytester use `monkeypatch.chdir()` itself, then everything is handled correctly. --- changelog/11315.trivial.rst | 3 +++ src/_pytest/pytester.py | 14 ++------------ testing/_py/test_local.py | 16 ++++++++-------- testing/code/test_excinfo.py | 22 +++++++++++++--------- testing/test_assertrewrite.py | 20 ++++++++++++-------- testing/test_pathlib.py | 18 +++++++++--------- testing/test_pytester.py | 13 ------------- 7 files changed, 47 insertions(+), 59 deletions(-) create mode 100644 changelog/11315.trivial.rst diff --git a/changelog/11315.trivial.rst b/changelog/11315.trivial.rst new file mode 100644 index 00000000000..309dccd8b40 --- /dev/null +++ b/changelog/11315.trivial.rst @@ -0,0 +1,3 @@ +The :fixture:`pytester` fixture now uses the :fixture:`monkeypatch` fixture to manage the current working directory. +If you use ``pytester`` in combination with :func:`monkeypatch.undo() `, the CWD might get restored. +Use :func:`monkeypatch.context() ` instead. diff --git a/src/_pytest/pytester.py b/src/_pytest/pytester.py index 854ee6834aa..ea1c302140e 100644 --- a/src/_pytest/pytester.py +++ b/src/_pytest/pytester.py @@ -625,14 +625,6 @@ def assert_outcomes( ) -class CwdSnapshot: - def __init__(self) -> None: - self.__saved = os.getcwd() - - def restore(self) -> None: - os.chdir(self.__saved) - - class SysModulesSnapshot: def __init__(self, preserve: Optional[Callable[[str], bool]] = None) -> None: self.__preserve = preserve @@ -696,15 +688,14 @@ def __init__( #: be added to the list. The type of items to add to the list depends on #: the method using them so refer to them for details. self.plugins: List[Union[str, _PluggyPlugin]] = [] - self._cwd_snapshot = CwdSnapshot() self._sys_path_snapshot = SysPathsSnapshot() self._sys_modules_snapshot = self.__take_sys_modules_snapshot() - self.chdir() self._request.addfinalizer(self._finalize) self._method = self._request.config.getoption("--runpytest") self._test_tmproot = tmp_path_factory.mktemp(f"tmp-{name}", numbered=True) self._monkeypatch = mp = monkeypatch + self.chdir() mp.setenv("PYTEST_DEBUG_TEMPROOT", str(self._test_tmproot)) # Ensure no unexpected caching via tox. mp.delenv("TOX_ENV_DIR", raising=False) @@ -735,7 +726,6 @@ def _finalize(self) -> None: """ self._sys_modules_snapshot.restore() self._sys_path_snapshot.restore() - self._cwd_snapshot.restore() def __take_sys_modules_snapshot(self) -> SysModulesSnapshot: # Some zope modules used by twisted-related tests keep internal state @@ -760,7 +750,7 @@ def chdir(self) -> None: This is done automatically upon instantiation. """ - os.chdir(self.path) + self._monkeypatch.chdir(self.path) def _makefile( self, diff --git a/testing/_py/test_local.py b/testing/_py/test_local.py index 57e2d91f900..aebee380cb9 100644 --- a/testing/_py/test_local.py +++ b/testing/_py/test_local.py @@ -1080,14 +1080,14 @@ def test_pyimport_check_filepath_consistency(self, monkeypatch, tmpdir): name = "pointsback123" ModuleType = type(os) p = tmpdir.ensure(name + ".py") - for ending in (".pyc", "$py.class", ".pyo"): - mod = ModuleType(name) - pseudopath = tmpdir.ensure(name + ending) - mod.__file__ = str(pseudopath) - monkeypatch.setitem(sys.modules, name, mod) - newmod = p.pyimport() - assert mod == newmod - monkeypatch.undo() + with monkeypatch.context() as mp: + for ending in (".pyc", "$py.class", ".pyo"): + mod = ModuleType(name) + pseudopath = tmpdir.ensure(name + ending) + mod.__file__ = str(pseudopath) + mp.setitem(sys.modules, name, mod) + newmod = p.pyimport() + assert mod == newmod mod = ModuleType(name) pseudopath = tmpdir.ensure(name + "123.py") mod.__file__ = str(pseudopath) diff --git a/testing/code/test_excinfo.py b/testing/code/test_excinfo.py index 89beefce547..22be51d407e 100644 --- a/testing/code/test_excinfo.py +++ b/testing/code/test_excinfo.py @@ -854,7 +854,11 @@ def entry(): reprtb = p.repr_traceback(excinfo) assert len(reprtb.reprentries) == 3 - def test_traceback_short_no_source(self, importasmod, monkeypatch) -> None: + def test_traceback_short_no_source( + self, + importasmod, + monkeypatch: pytest.MonkeyPatch, + ) -> None: mod = importasmod( """ def func1(): @@ -866,14 +870,14 @@ def entry(): excinfo = pytest.raises(ValueError, mod.entry) from _pytest._code.code import Code - monkeypatch.setattr(Code, "path", "bogus") - p = FormattedExcinfo(style="short") - reprtb = p.repr_traceback_entry(excinfo.traceback[-2]) - lines = reprtb.lines - last_p = FormattedExcinfo(style="short") - last_reprtb = last_p.repr_traceback_entry(excinfo.traceback[-1], excinfo) - last_lines = last_reprtb.lines - monkeypatch.undo() + with monkeypatch.context() as mp: + mp.setattr(Code, "path", "bogus") + p = FormattedExcinfo(style="short") + reprtb = p.repr_traceback_entry(excinfo.traceback[-2]) + lines = reprtb.lines + last_p = FormattedExcinfo(style="short") + last_reprtb = last_p.repr_traceback_entry(excinfo.traceback[-1], excinfo) + last_lines = last_reprtb.lines assert lines[0] == " func1()" assert last_lines[0] == ' raise ValueError("hello")' diff --git a/testing/test_assertrewrite.py b/testing/test_assertrewrite.py index b3fd0c2f2e7..d3cd6144404 100644 --- a/testing/test_assertrewrite.py +++ b/testing/test_assertrewrite.py @@ -895,7 +895,11 @@ def test_foo(): ) @pytest.mark.skipif('"__pypy__" in sys.modules') - def test_pyc_vs_pyo(self, pytester: Pytester, monkeypatch) -> None: + def test_pyc_vs_pyo( + self, + pytester: Pytester, + monkeypatch: pytest.MonkeyPatch, + ) -> None: pytester.makepyfile( """ import pytest @@ -905,13 +909,13 @@ def test_optimized(): ) p = make_numbered_dir(root=Path(pytester.path), prefix="runpytest-") tmp = "--basetemp=%s" % p - monkeypatch.setenv("PYTHONOPTIMIZE", "2") - monkeypatch.delenv("PYTHONDONTWRITEBYTECODE", raising=False) - monkeypatch.delenv("PYTHONPYCACHEPREFIX", raising=False) - assert pytester.runpytest_subprocess(tmp).ret == 0 - tagged = "test_pyc_vs_pyo." + PYTEST_TAG - assert tagged + ".pyo" in os.listdir("__pycache__") - monkeypatch.undo() + with monkeypatch.context() as mp: + mp.setenv("PYTHONOPTIMIZE", "2") + mp.delenv("PYTHONDONTWRITEBYTECODE", raising=False) + mp.delenv("PYTHONPYCACHEPREFIX", raising=False) + assert pytester.runpytest_subprocess(tmp).ret == 0 + tagged = "test_pyc_vs_pyo." + PYTEST_TAG + assert tagged + ".pyo" in os.listdir("__pycache__") monkeypatch.delenv("PYTHONDONTWRITEBYTECODE", raising=False) monkeypatch.delenv("PYTHONPYCACHEPREFIX", raising=False) assert pytester.runpytest_subprocess(tmp).ret == 1 diff --git a/testing/test_pathlib.py b/testing/test_pathlib.py index 2c5432cef0a..3e1d2265bb7 100644 --- a/testing/test_pathlib.py +++ b/testing/test_pathlib.py @@ -236,15 +236,15 @@ def test_check_filepath_consistency( name = "pointsback123" p = tmp_path.joinpath(name + ".py") p.touch() - for ending in (".pyc", ".pyo"): - mod = ModuleType(name) - pseudopath = tmp_path.joinpath(name + ending) - pseudopath.touch() - mod.__file__ = str(pseudopath) - monkeypatch.setitem(sys.modules, name, mod) - newmod = import_path(p, root=tmp_path) - assert mod == newmod - monkeypatch.undo() + with monkeypatch.context() as mp: + for ending in (".pyc", ".pyo"): + mod = ModuleType(name) + pseudopath = tmp_path.joinpath(name + ending) + pseudopath.touch() + mod.__file__ = str(pseudopath) + mp.setitem(sys.modules, name, mod) + newmod = import_path(p, root=tmp_path) + assert mod == newmod mod = ModuleType(name) pseudopath = tmp_path.joinpath(name + "123.py") pseudopath.touch() diff --git a/testing/test_pytester.py b/testing/test_pytester.py index 8f8b4d2914f..6fc6bd24519 100644 --- a/testing/test_pytester.py +++ b/testing/test_pytester.py @@ -2,7 +2,6 @@ import subprocess import sys import time -from pathlib import Path from types import ModuleType from typing import List @@ -11,7 +10,6 @@ from _pytest.config import ExitCode from _pytest.config import PytestPluginManager from _pytest.monkeypatch import MonkeyPatch -from _pytest.pytester import CwdSnapshot from _pytest.pytester import HookRecorder from _pytest.pytester import LineMatcher from _pytest.pytester import Pytester @@ -301,17 +299,6 @@ def test_assert_outcomes_after_pytest_error(pytester: Pytester) -> None: result.assert_outcomes(passed=0) -def test_cwd_snapshot(pytester: Pytester) -> None: - foo = pytester.mkdir("foo") - bar = pytester.mkdir("bar") - os.chdir(foo) - snapshot = CwdSnapshot() - os.chdir(bar) - assert Path().absolute() == bar - snapshot.restore() - assert Path().absolute() == foo - - class TestSysModulesSnapshot: key = "my-test-module"