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

@@ -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() <pytest.MonkeyPatch.undo>`, the CWD might get restored.
+Use :func:`monkeypatch.context() <pytest.MonkeyPatch.context>` instead.

src/_pytest/pytester.py

@@ -625,14 +625,6 @@ class RunResult:
         )
 
 
-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 @@ class Pytester:
         #: 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 @@ class Pytester:
         """
         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 @@ class Pytester:
 
         This is done automatically upon instantiation.
         """
-        os.chdir(self.path)
+        self._monkeypatch.chdir(self.path)
 
     def _makefile(
         self,

testing/_py/test_local.py

@@ -1080,14 +1080,14 @@ class TestImport:
         name = "pointsback123"
         ModuleType = type(os)
         p = tmpdir.ensure(name + ".py")
+        with monkeypatch.context() as mp:
             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)
+                mp.setitem(sys.modules, name, mod)
                 newmod = p.pyimport()
                 assert mod == newmod
-        monkeypatch.undo()
         mod = ModuleType(name)
         pseudopath = tmpdir.ensure(name + "123.py")
         mod.__file__ = str(pseudopath)

testing/code/test_excinfo.py

@@ -854,7 +854,11 @@ raise ValueError()
         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 @@ raise ValueError()
         excinfo = pytest.raises(ValueError, mod.entry)
         from _pytest._code.code import Code
 
-        monkeypatch.setattr(Code, "path", "bogus")
+        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
-        monkeypatch.undo()
         assert lines[0] == "    func1()"
 
         assert last_lines[0] == '    raise ValueError("hello")'

testing/test_assertrewrite.py

@@ -895,7 +895,11 @@ def test_rewritten():
         )
 
     @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_rewritten():
         )
         p = make_numbered_dir(root=Path(pytester.path), prefix="runpytest-")
         tmp = "--basetemp=%s" % p
-        monkeypatch.setenv("PYTHONOPTIMIZE", "2")
+        with monkeypatch.context() as mp:
-        monkeypatch.delenv("PYTHONDONTWRITEBYTECODE", raising=False)
+            mp.setenv("PYTHONOPTIMIZE", "2")
-        monkeypatch.delenv("PYTHONPYCACHEPREFIX", raising=False)
+            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.undo()
         monkeypatch.delenv("PYTHONDONTWRITEBYTECODE", raising=False)
         monkeypatch.delenv("PYTHONPYCACHEPREFIX", raising=False)
         assert pytester.runpytest_subprocess(tmp).ret == 1

testing/test_pathlib.py

@@ -236,15 +236,15 @@ class TestImportPath:
         name = "pointsback123"
         p = tmp_path.joinpath(name + ".py")
         p.touch()
+        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)
-            monkeypatch.setitem(sys.modules, name, mod)
+                mp.setitem(sys.modules, name, mod)
                 newmod = import_path(p, root=tmp_path)
                 assert mod == newmod
-        monkeypatch.undo()
         mod = ModuleType(name)
         pseudopath = tmp_path.joinpath(name + "123.py")
         pseudopath.touch()

testing/test_pytester.py

@@ -2,7 +2,6 @@ import os
 import subprocess
 import sys
 import time
-from pathlib import Path
 from types import ModuleType
 from typing import List
 
@@ -11,7 +10,6 @@ import pytest
 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"