From 4b657d22b287d7541b3ad3eb3db0868f2d220488 Mon Sep 17 00:00:00 2001 From: Krishan Subudhi Date: Fri, 9 Oct 2026 17:25:48 +0000 Subject: [PATCH 1/2] Restore sys.exc_info() in pytest_fixture_post_finalizer Since pytest 8.1, a teardown error is caught and stored before pytest_fixture_post_finalizer runs, so the hook could no longer see it via sys.exc_info() the way it could in 8.0. Call the hook from FixtureDef.finish() again, in a ``finally`` around re-raising the teardown error(s), instead of registering it as the first finalizer. The cache reset moves into a nested ``finally`` so a raising hook still cannot leave the fixture half torn down (#14114), and the existing early return still keeps the hook to one call per teardown. Closes #12306 Co-authored-by: Claude Opus 5.5 --- AUTHORS | 1 + changelog/12306.bugfix.rst | 1 + src/_pytest/fixtures.py | 37 +++++++++++++++++------------------ testing/python/fixtures.py | 40 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 60 insertions(+), 19 deletions(-) create mode 100644 changelog/12306.bugfix.rst diff --git a/AUTHORS b/AUTHORS index f621dd6ddc8..a3dc5d80af8 100644 --- a/AUTHORS +++ b/AUTHORS @@ -279,6 +279,7 @@ Kim Soo Kodi B. Arfer Kojo Idrissa Kostis Anagnostopoulos +Krishan Subudhi Kristoffer Nordström Kyle Altendorf Lawrence Mitchell diff --git a/changelog/12306.bugfix.rst b/changelog/12306.bugfix.rst new file mode 100644 index 00000000000..2b5c51f4d21 --- /dev/null +++ b/changelog/12306.bugfix.rst @@ -0,0 +1 @@ +Exceptions raised during fixture teardown are once again available via :func:`sys.exc_info` inside :hook:`pytest_fixture_post_finalizer`, as they were before pytest 8.1. diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py index 20e43f89132..34ab8016d8b 100644 --- a/src/_pytest/fixtures.py +++ b/src/_pytest/fixtures.py @@ -1217,16 +1217,24 @@ def finish(self, request: SubRequest) -> None: except BaseException as e: exceptions.append(e) node = request.node - # Even if finalization fails, we invalidate the cached fixture - # value and remove all finalizers because they may be bound methods - # which will keep instances alive. - self.cached_result = None - self._finalizers.clear() - if len(exceptions) == 1: - raise exceptions[0] - elif len(exceptions) > 1: - msg = f'errors while tearing down fixture "{self.argname}" of {node}' - raise BaseExceptionGroup(msg, exceptions[::-1]) + try: + if len(exceptions) == 1: + raise exceptions[0] + elif len(exceptions) > 1: + msg = f'errors while tearing down fixture "{self.argname}" of {node}' + raise BaseExceptionGroup(msg, exceptions[::-1]) + finally: + # Any teardown error is still being raised here, for sys.exc_info(). + try: + node.ihook.pytest_fixture_post_finalizer( + fixturedef=self, request=request + ) + finally: + # Even if finalization fails, we invalidate the cached fixture + # value and remove all finalizers because they may be bound methods + # which will keep instances alive. + self.cached_result = None + self._finalizers.clear() def execute(self, request: SubRequest) -> FixtureValue: """Return the value of this fixture, executing it if not cached.""" @@ -1276,15 +1284,6 @@ def execute(self, request: SubRequest) -> FixtureValue: for parent_fixture in requested_fixtures_that_should_finalize_us: parent_fixture.addfinalizer(finalizer) - # Register the pytest_fixture_post_finalizer as the first finalizer, - # which is executed last. - assert not self._finalizers - self.addfinalizer( - lambda: request.node.ihook.pytest_fixture_post_finalizer( - fixturedef=self, request=request - ) - ) - ihook = request.node.ihook try: # Setup the fixture, run the code in it, and cache the value diff --git a/testing/python/fixtures.py b/testing/python/fixtures.py index cb2f1daab63..48cd31004b8 100644 --- a/testing/python/fixtures.py +++ b/testing/python/fixtures.py @@ -4638,6 +4638,46 @@ def test_second(my_fixture): ) +def test_fixture_post_finalizer_sees_teardown_exception(pytester: Pytester) -> None: + """The teardown error is visible via sys.exc_info() in the hook (#12306).""" + pytester.makeconftest( + """ + import sys + import pytest + + def pytest_fixture_post_finalizer(fixturedef, request): + if fixturedef.argname in ("failing", "passing"): + print(f"\\n{fixturedef.argname}: exc_info={sys.exc_info()[1]!r}") + + @pytest.fixture + def failing(): + yield + raise RuntimeError("teardown failed") + + @pytest.fixture + def passing(): + yield + """ + ) + pytester.makepyfile( + """ + def test_failing(failing): + pass + + def test_passing(passing): + pass + """ + ) + result = pytester.runpytest("-s") + result.assert_outcomes(passed=2, errors=1) + result.stdout.fnmatch_lines( + [ + "failing: exc_info=RuntimeError('teardown failed')", + "*passing: exc_info=None", + ] + ) + + class TestParamValueKey: """Unit tests for the equivalence key used by `reorder_items` (#8914).""" From f3b9ce10bea54dff5d05d6cf815c4172f4d28642 Mon Sep 17 00:00:00 2001 From: Krishan Subudhi Date: Fri, 9 Oct 2026 18:35:13 +0000 Subject: [PATCH 2/2] Pass teardown error to pytest_fixture_post_finalizer explicitly Per review, stop relying on sys.exc_info() inside the hook: - Add a ``teardown_exception`` argument to pytest_fixture_post_finalizer: the single teardown exception, a BaseExceptionGroup when several finalizers failed, or None. Existing hookimpls that do not accept it keep working. - Call the hook directly from FixtureDef.finish() after the finalizers have run; exceptions raised by the hook itself propagate. - finish() now passes the request that set the fixture up, also when a parametrized fixture is torn down to switch to another param. - Run the hook (once) for a setup that failed before a result was cached, and tear such a setup down before the next execute() (#14800). --- changelog/12306.bugfix.rst | 1 - changelog/12306.feature.rst | 3 ++ changelog/14800.bugfix.rst | 1 + src/_pytest/fixtures.py | 57 ++++++++++++-------- src/_pytest/hookspec.py | 25 +++++++-- testing/python/fixtures.py | 102 ++++++++++++++++++++++++++++++++---- 6 files changed, 151 insertions(+), 38 deletions(-) delete mode 100644 changelog/12306.bugfix.rst create mode 100644 changelog/12306.feature.rst create mode 100644 changelog/14800.bugfix.rst diff --git a/changelog/12306.bugfix.rst b/changelog/12306.bugfix.rst deleted file mode 100644 index 2b5c51f4d21..00000000000 --- a/changelog/12306.bugfix.rst +++ /dev/null @@ -1 +0,0 @@ -Exceptions raised during fixture teardown are once again available via :func:`sys.exc_info` inside :hook:`pytest_fixture_post_finalizer`, as they were before pytest 8.1. diff --git a/changelog/12306.feature.rst b/changelog/12306.feature.rst new file mode 100644 index 00000000000..01a3b248971 --- /dev/null +++ b/changelog/12306.feature.rst @@ -0,0 +1,3 @@ +:hook:`pytest_fixture_post_finalizer` now receives a ``teardown_exception`` argument with the exception raised while tearing down the fixture: the exception itself, a :class:`BaseExceptionGroup` if several finalizers of the fixture failed, or ``None`` if teardown succeeded. Use it instead of :func:`sys.exc_info`, which has not reliably reflected teardown errors inside this hook since pytest 8.1. + +The hook now also always receives the request that set the fixture up, and exceptions raised by implementations of the hook propagate instead of being reported as fixture teardown errors. diff --git a/changelog/14800.bugfix.rst b/changelog/14800.bugfix.rst new file mode 100644 index 00000000000..431cffd9ff0 --- /dev/null +++ b/changelog/14800.bugfix.rst @@ -0,0 +1 @@ +Fixed an internal ``AssertionError`` when a :hook:`pytest_fixture_setup` implementation raises before the fixture result is cached: the remaining parameters of the same fixture now run normally, and :hook:`pytest_fixture_post_finalizer` is still called for the failed setup. This was a regression in pytest 9.1.0. diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py index 34ab8016d8b..a4f94b0e421 100644 --- a/src/_pytest/fixtures.py +++ b/src/_pytest/fixtures.py @@ -1186,6 +1186,9 @@ def __init__( # Can change if the fixture is executed with different parameters. self.cached_result: _FixtureCachedResult[FixtureValue] | None = None self._finalizers: Final[list[Callable[[], object]]] = [] + # The request the fixture is currently set up with, if any; passed to + # pytest_fixture_post_finalizer on teardown. + self._setup_request: SubRequest | None = None # only used to emit a deprecationwarning, can be removed in pytest9 self._autouse = _autouse @@ -1204,10 +1207,14 @@ def addfinalizer(self, finalizer: Callable[[], object]) -> None: self._finalizers.append(finalizer) def finish(self, request: SubRequest) -> None: - if self.cached_result is None: - # Already finished. It is assumed that finalizers cannot be added in - # this state. + if self.cached_result is None and self._setup_request is None: + # Already finished (or never set up). It is assumed that finalizers + # cannot be added in this state. return + # The hook must see the request that set the fixture up, not the one + # that may be tearing it down to switch to a new param (#12306). + request = self._setup_request or request + self._setup_request = None exceptions: list[BaseException] = [] while self._finalizers: @@ -1217,24 +1224,26 @@ def finish(self, request: SubRequest) -> None: except BaseException as e: exceptions.append(e) node = request.node + teardown_exception: BaseException | None = None + if len(exceptions) == 1: + teardown_exception = exceptions[0] + elif len(exceptions) > 1: + msg = f'errors while tearing down fixture "{self.argname}" of {node}' + teardown_exception = BaseExceptionGroup(msg, exceptions[::-1]) try: - if len(exceptions) == 1: - raise exceptions[0] - elif len(exceptions) > 1: - msg = f'errors while tearing down fixture "{self.argname}" of {node}' - raise BaseExceptionGroup(msg, exceptions[::-1]) + # An exception raised by the hook itself is a plugin bug and + # propagates as-is. + node.ihook.pytest_fixture_post_finalizer( + fixturedef=self, request=request, teardown_exception=teardown_exception + ) finally: - # Any teardown error is still being raised here, for sys.exc_info(). - try: - node.ihook.pytest_fixture_post_finalizer( - fixturedef=self, request=request - ) - finally: - # Even if finalization fails, we invalidate the cached fixture - # value and remove all finalizers because they may be bound methods - # which will keep instances alive. - self.cached_result = None - self._finalizers.clear() + # Even if finalization fails, we invalidate the cached fixture + # value and remove all finalizers because they may be bound methods + # which will keep instances alive. + self.cached_result = None + self._finalizers.clear() + if teardown_exception is not None: + raise teardown_exception def execute(self, request: SubRequest) -> FixtureValue: """Return the value of this fixture, executing it if not cached.""" @@ -1272,10 +1281,11 @@ def execute(self, request: SubRequest) -> FixtureValue: raise exc.with_traceback(exc_tb) else: return self.cached_result[0] - # We have a previous but differently parametrized fixture instance - # so we need to tear it down before creating a new one. - self.finish(request) - assert self.cached_result is None + # We may have a previous but differently parametrized fixture instance, + # or a setup that failed before caching a result; tear it down before + # creating a new one (no-op if there is nothing to tear down). + self.finish(request) + assert self.cached_result is None # Add finalizer to requested fixtures we saved previously. # We make sure to do this after checking for cached value to avoid @@ -1285,6 +1295,7 @@ def execute(self, request: SubRequest) -> FixtureValue: parent_fixture.addfinalizer(finalizer) ihook = request.node.ihook + self._setup_request = request try: # Setup the fixture, run the code in it, and cache the value # in self.cached_result. diff --git a/src/_pytest/hookspec.py b/src/_pytest/hookspec.py index 0f4caa024c2..c190b4067c2 100644 --- a/src/_pytest/hookspec.py +++ b/src/_pytest/hookspec.py @@ -864,16 +864,33 @@ def pytest_fixture_setup( def pytest_fixture_post_finalizer( - fixturedef: FixtureDef[Any], request: SubRequest + fixturedef: FixtureDef[Any], + request: SubRequest, + teardown_exception: BaseException | None, ) -> None: """Called after fixture teardown, but before the cache is cleared, so - the fixture result ``fixturedef.cached_result`` is still available (not - ``None``). + the fixture result ``fixturedef.cached_result`` is still available. + + Also called when the fixture setup failed before a result was cached + (for example, a :hook:`pytest_fixture_setup` implementation raised), in + which case ``fixturedef.cached_result`` is ``None``. + + An exception raised by an implementation of this hook is a bug in that + implementation: it is not swallowed or merged into + ``teardown_exception``, but propagates immediately. :param fixturedef: The fixture definition object. :param request: - The fixture request object. + The fixture request object that set up the fixture. + :param teardown_exception: + The exception raised while tearing down the fixture, or ``None`` if + teardown succeeded. If several finalizers of the fixture failed, this + is a :class:`BaseExceptionGroup` holding all of their exceptions. + Use this argument rather than :func:`sys.exc_info` to inspect + teardown failures. + + .. versionadded:: 9.2 Use in conftest plugins ======================= diff --git a/testing/python/fixtures.py b/testing/python/fixtures.py index 48cd31004b8..f71c2d479be 100644 --- a/testing/python/fixtures.py +++ b/testing/python/fixtures.py @@ -4585,7 +4585,7 @@ def test_second(foo, bar, baz): def test_fixture_post_finalizer_hook_exception(pytester: Pytester) -> None: - """Test that exceptions in pytest_fixture_post_finalizer hook are caught. + """Test that exceptions in pytest_fixture_post_finalizer hook propagate. Also verifies that the fixture cache is properly reset even when the post_finalizer hook raises an exception, so the fixture can be rebuilt @@ -4638,22 +4638,27 @@ def test_second(my_fixture): ) -def test_fixture_post_finalizer_sees_teardown_exception(pytester: Pytester) -> None: - """The teardown error is visible via sys.exc_info() in the hook (#12306).""" +def test_fixture_post_finalizer_teardown_exception(pytester: Pytester) -> None: + """The teardown error is passed to the hook explicitly (#12306).""" pytester.makeconftest( """ - import sys import pytest - def pytest_fixture_post_finalizer(fixturedef, request): - if fixturedef.argname in ("failing", "passing"): - print(f"\\n{fixturedef.argname}: exc_info={sys.exc_info()[1]!r}") + def pytest_fixture_post_finalizer(fixturedef, request, teardown_exception): + if fixturedef.argname in ("failing", "failing_twice", "passing"): + print(f"\\n{fixturedef.argname}: {teardown_exception!r}") @pytest.fixture def failing(): yield raise RuntimeError("teardown failed") + @pytest.fixture + def failing_twice(request): + request.addfinalizer(lambda: 1 / 0) + yield + raise RuntimeError("teardown failed") + @pytest.fixture def passing(): yield @@ -4664,16 +4669,93 @@ def passing(): def test_failing(failing): pass + def test_failing_twice(failing_twice): + pass + def test_passing(passing): pass """ ) result = pytester.runpytest("-s") - result.assert_outcomes(passed=2, errors=1) + result.assert_outcomes(passed=3, errors=2) + result.stdout.fnmatch_lines( + [ + "failing: RuntimeError('teardown failed')", + "*failing_twice: ExceptionGroup('errors while tearing down fixture " + "\"failing_twice\"*ZeroDivisionError('division by zero'), " + "RuntimeError('teardown failed')*", + "*passing: None", + ] + ) + + +def test_fixture_post_finalizer_gets_setup_request_on_param_switch( + pytester: Pytester, +) -> None: + """When a parametrized fixture is torn down because the next test needs a + different param, the hook gets the request that set it up (#12306).""" + pytester.makeconftest( + """ + def pytest_fixture_post_finalizer(fixturedef, request): + if fixturedef.argname == "fix": + print(f"\\npost_finalizer: {fixturedef.cached_result[0]} {request.param}") + """ + ) + pytester.makepyfile( + """ + import pytest + + @pytest.fixture(scope="module", params=[1, 2]) + def fix(request): + return request.param + + def test_it(fix): + pass + """ + ) + result = pytester.runpytest("-s") + result.assert_outcomes(passed=2) + result.stdout.fnmatch_lines(["post_finalizer: 1 1", "*post_finalizer: 2 2"]) + + +def test_fixture_post_finalizer_after_setup_hook_failure(pytester: Pytester) -> None: + """The hook runs once for a setup that failed before a result was cached, + and the following params of the fixture still run (#14800).""" + pytester.makeconftest( + """ + import pytest + + calls = [] + + @pytest.hookimpl(tryfirst=True) + def pytest_fixture_setup(fixturedef, request): + if getattr(request, "param", None) == "fail": + raise RuntimeError("setup hook failed") + + def pytest_fixture_post_finalizer(fixturedef, request): + if fixturedef.argname == "val": + calls.append((request.node.name, fixturedef.cached_result)) + + def pytest_terminal_summary(terminalreporter): + terminalreporter.write_line(f"post_finalizer calls: {calls}") + """ + ) + pytester.makepyfile( + """ + import pytest + + @pytest.mark.parametrize("val", ["fail", "ok"]) + def test_thing(val): + assert val == "ok" + """ + ) + result = pytester.runpytest() + result.assert_outcomes(passed=1, errors=1) + result.stdout.no_fnmatch_line("*AssertionError*") result.stdout.fnmatch_lines( [ - "failing: exc_info=RuntimeError('teardown failed')", - "*passing: exc_info=None", + "post_finalizer calls: [('test_thing[fail]', None), " + "('test_thing[ok]', ('ok', 'ok', None))]" ] )