From b939cb4ef12c1c826bd83ea63196d9300fbedb20 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 21 Jun 2026 14:36:15 +0000 Subject: [PATCH 1/3] Initial plan From 3fdbf0b06a74facefa4ddf168089c4ac5a9fdbe1 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 21 Jun 2026 14:41:10 +0000 Subject: [PATCH 2/3] Fix recursive find_spec crash with PYTHON_LAZY_IMPORTS=all (#14632) --- src/_pytest/assertion/rewrite.py | 69 ++++++++++++++++++-------------- testing/test_assertrewrite.py | 45 +++++++++++++++++++++ 2 files changed, 85 insertions(+), 29 deletions(-) diff --git a/src/_pytest/assertion/rewrite.py b/src/_pytest/assertion/rewrite.py index 99815b70cf1..49e8cbc9436 100644 --- a/src/_pytest/assertion/rewrite.py +++ b/src/_pytest/assertion/rewrite.py @@ -88,6 +88,9 @@ def __init__(self, config: Config) -> None: # flag to guard against trying to rewrite a pyc file while we are already writing another pyc file, # which might result in infinite recursion (#3506) self._writing_pyc = False + # flag to guard against recursive find_spec calls, e.g. triggered by PYTHON_LAZY_IMPORTS=all + # where accessing a lazily-imported name inside find_spec triggers another find_spec call (#14632) + self._in_find_spec = False self._basenames_to_check_rewrite = {"conftest"} self._marked_for_rewrite_cache: dict[str, bool] = {} self._session_paths_checked = False @@ -107,38 +110,46 @@ def find_spec( ) -> importlib.machinery.ModuleSpec | None: if self._writing_pyc: return None - state = self.config.stash[assertstate_key] - if self._early_rewrite_bailout(name, state): - return None - state.trace(f"find_module called for: {name}") - - # Type ignored because mypy is confused about the `self` binding here. - spec = self._find_spec(name, path) # type: ignore - - if ( - # the import machinery could not find a file to import - spec is None - # this is a namespace package (without `__init__.py`) - # there's nothing to rewrite there - or spec.origin is None - # we can only rewrite source files - or not isinstance(spec.loader, importlib.machinery.SourceFileLoader) - # if the file doesn't exist, we can't rewrite it - or not os.path.exists(spec.origin) - ): + if self._in_find_spec: + # Guard against recursive find_spec calls, e.g. triggered by PYTHON_LAZY_IMPORTS=all + # where accessing a lazily-imported name inside find_spec triggers another find_spec call. return None - else: - fn = spec.origin + self._in_find_spec = True + try: + state = self.config.stash[assertstate_key] + if self._early_rewrite_bailout(name, state): + return None + state.trace(f"find_module called for: {name}") + + # Type ignored because mypy is confused about the `self` binding here. + spec = self._find_spec(name, path) # type: ignore + + if ( + # the import machinery could not find a file to import + spec is None + # this is a namespace package (without `__init__.py`) + # there's nothing to rewrite there + or spec.origin is None + # we can only rewrite source files + or not isinstance(spec.loader, importlib.machinery.SourceFileLoader) + # if the file doesn't exist, we can't rewrite it + or not os.path.exists(spec.origin) + ): + return None + else: + fn = spec.origin - if not self._should_rewrite(name, fn, state): - return None + if not self._should_rewrite(name, fn, state): + return None - return importlib.util.spec_from_file_location( - name, - fn, - loader=self, - submodule_search_locations=spec.submodule_search_locations, - ) + return importlib.util.spec_from_file_location( + name, + fn, + loader=self, + submodule_search_locations=spec.submodule_search_locations, + ) + finally: + self._in_find_spec = False def create_module( self, spec: importlib.machinery.ModuleSpec diff --git a/testing/test_assertrewrite.py b/testing/test_assertrewrite.py index e11863547ba..4902094fc6c 100644 --- a/testing/test_assertrewrite.py +++ b/testing/test_assertrewrite.py @@ -2424,3 +2424,48 @@ def test(): ) reprec = pytester.inline_run("-p", "no:terminalreporter") reprec.assertoutcome(passed=1) + + +def test_rewrite_hook_reentrancy_guard(pytestconfig, pytester: Pytester) -> None: + """AssertionRewritingHook.find_spec returns None when called recursively, + e.g. when PYTHON_LAZY_IMPORTS=all triggers an import inside find_spec (#14632).""" + hook = AssertionRewritingHook(pytestconfig) + + # Simulate a recursive find_spec call from inside the hook body, as would happen + # when PYTHON_LAZY_IMPORTS=all causes a lazy-import resolution mid-find_spec. + original_early_bailout = hook._early_rewrite_bailout + recursive_call_returned_none = [] + + def spy_early_bailout(name, state): + # Attempt a nested find_spec call; the re-entrancy guard must return None. + result = hook.find_spec("fnmatch") + recursive_call_returned_none.append(result is None) + return original_early_bailout(name, state) + + hook._early_rewrite_bailout = spy_early_bailout # type: ignore[method-assign] + pytester.syspathinsert() + pytester.makepyfile(test_foo="def test_foo(): pass") + hook.find_spec("test_foo") + + assert recursive_call_returned_none == [True], ( + "Recursive find_spec call should return None (re-entrancy guard)" + ) + + +@pytest.mark.skipif( + sys.version_info < (3, 15), + reason="PYTHON_LAZY_IMPORTS requires Python 3.15+", +) +def test_lazy_imports_all_does_not_crash_pytest( + pytester: Pytester, monkeypatch: pytest.MonkeyPatch +) -> None: + """pytest does not crash with PYTHON_LAZY_IMPORTS=all (#14632).""" + pytester.makepyfile( + """ + def test_foo(): + assert 1 == 1 + """ + ) + monkeypatch.setenv("PYTHON_LAZY_IMPORTS", "all") + result = pytester.runpytest_subprocess() + assert result.ret == 0 From f50d6958cf6d1aecb1660438f4c94a3dc10dee28 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 21 Jun 2026 15:13:51 +0000 Subject: [PATCH 3/3] Apply remaining changes --- src/_pytest/assertion/rewrite.py | 75 +++++++++++++++----------------- testing/test_assertrewrite.py | 33 +++++--------- 2 files changed, 45 insertions(+), 63 deletions(-) diff --git a/src/_pytest/assertion/rewrite.py b/src/_pytest/assertion/rewrite.py index 49e8cbc9436..169ab6d7028 100644 --- a/src/_pytest/assertion/rewrite.py +++ b/src/_pytest/assertion/rewrite.py @@ -88,9 +88,6 @@ def __init__(self, config: Config) -> None: # flag to guard against trying to rewrite a pyc file while we are already writing another pyc file, # which might result in infinite recursion (#3506) self._writing_pyc = False - # flag to guard against recursive find_spec calls, e.g. triggered by PYTHON_LAZY_IMPORTS=all - # where accessing a lazily-imported name inside find_spec triggers another find_spec call (#14632) - self._in_find_spec = False self._basenames_to_check_rewrite = {"conftest"} self._marked_for_rewrite_cache: dict[str, bool] = {} self._session_paths_checked = False @@ -110,46 +107,38 @@ def find_spec( ) -> importlib.machinery.ModuleSpec | None: if self._writing_pyc: return None - if self._in_find_spec: - # Guard against recursive find_spec calls, e.g. triggered by PYTHON_LAZY_IMPORTS=all - # where accessing a lazily-imported name inside find_spec triggers another find_spec call. + state = self.config.stash[assertstate_key] + if self._early_rewrite_bailout(name, state): return None - self._in_find_spec = True - try: - state = self.config.stash[assertstate_key] - if self._early_rewrite_bailout(name, state): - return None - state.trace(f"find_module called for: {name}") - - # Type ignored because mypy is confused about the `self` binding here. - spec = self._find_spec(name, path) # type: ignore - - if ( - # the import machinery could not find a file to import - spec is None - # this is a namespace package (without `__init__.py`) - # there's nothing to rewrite there - or spec.origin is None - # we can only rewrite source files - or not isinstance(spec.loader, importlib.machinery.SourceFileLoader) - # if the file doesn't exist, we can't rewrite it - or not os.path.exists(spec.origin) - ): - return None - else: - fn = spec.origin + state.trace(f"find_module called for: {name}") + + # Type ignored because mypy is confused about the `self` binding here. + spec = self._find_spec(name, path) # type: ignore + + if ( + # the import machinery could not find a file to import + spec is None + # this is a namespace package (without `__init__.py`) + # there's nothing to rewrite there + or spec.origin is None + # we can only rewrite source files + or not isinstance(spec.loader, importlib.machinery.SourceFileLoader) + # if the file doesn't exist, we can't rewrite it + or not os.path.exists(spec.origin) + ): + return None + else: + fn = spec.origin - if not self._should_rewrite(name, fn, state): - return None + if not self._should_rewrite(name, fn, state): + return None - return importlib.util.spec_from_file_location( - name, - fn, - loader=self, - submodule_search_locations=spec.submodule_search_locations, - ) - finally: - self._in_find_spec = False + return importlib.util.spec_from_file_location( + name, + fn, + loader=self, + submodule_search_locations=spec.submodule_search_locations, + ) def create_module( self, spec: importlib.machinery.ModuleSpec @@ -206,6 +195,12 @@ def _early_rewrite_bailout(self, name: str, state: AssertionState) -> bool: tries to filter what we're sure won't be rewritten before getting to it. """ + # stdlib modules are never rewritten; bail out early to avoid calling + # fnmatch_ex, which can trigger lazy import resolution and cause + # recursion with PYTHON_LAZY_IMPORTS=all (#14632). + if name.partition(".")[0] in sys.stdlib_module_names: + return True + if self.session is not None and not self._session_paths_checked: self._session_paths_checked = True for initial_path in self.session._initialpaths: diff --git a/testing/test_assertrewrite.py b/testing/test_assertrewrite.py index 4902094fc6c..023ef4322cb 100644 --- a/testing/test_assertrewrite.py +++ b/testing/test_assertrewrite.py @@ -2426,30 +2426,17 @@ def test(): reprec.assertoutcome(passed=1) -def test_rewrite_hook_reentrancy_guard(pytestconfig, pytester: Pytester) -> None: - """AssertionRewritingHook.find_spec returns None when called recursively, - e.g. when PYTHON_LAZY_IMPORTS=all triggers an import inside find_spec (#14632).""" +def test_rewrite_hook_stdlib_modules_skipped(pytestconfig: pytest.Config) -> None: + """AssertionRewritingHook.find_spec returns None for stdlib modules early + (via sys.stdlib_module_names in _early_rewrite_bailout) to prevent + recursion with PYTHON_LAZY_IMPORTS=all (#14632).""" hook = AssertionRewritingHook(pytestconfig) - - # Simulate a recursive find_spec call from inside the hook body, as would happen - # when PYTHON_LAZY_IMPORTS=all causes a lazy-import resolution mid-find_spec. - original_early_bailout = hook._early_rewrite_bailout - recursive_call_returned_none = [] - - def spy_early_bailout(name, state): - # Attempt a nested find_spec call; the re-entrancy guard must return None. - result = hook.find_spec("fnmatch") - recursive_call_returned_none.append(result is None) - return original_early_bailout(name, state) - - hook._early_rewrite_bailout = spy_early_bailout # type: ignore[method-assign] - pytester.syspathinsert() - pytester.makepyfile(test_foo="def test_foo(): pass") - hook.find_spec("test_foo") - - assert recursive_call_returned_none == [True], ( - "Recursive find_spec call should return None (re-entrancy guard)" - ) + # stdlib modules are always skipped; this also breaks the recursion that + # PYTHON_LAZY_IMPORTS=all would cause when fnmatch resolves lazily inside + # _early_rewrite_bailout. + assert hook.find_spec("fnmatch") is None + assert hook.find_spec("os") is None + assert hook.find_spec("re") is None @pytest.mark.skipif(