diff --git a/AUTHORS b/AUTHORS index 229e4315078..e57481d2000 100644 --- a/AUTHORS +++ b/AUTHORS @@ -341,6 +341,7 @@ Mike Lundy Milan Lesnek minbang930 Miro HronĨok +Mohith Gajjela Mulat Mekonen mrbean-bremen Nathan Goldbaum diff --git a/changelog/14800.bugfix.rst b/changelog/14800.bugfix.rst new file mode 100644 index 00000000000..bd56027331b --- /dev/null +++ b/changelog/14800.bugfix.rst @@ -0,0 +1 @@ +Fixed an internal ``AssertionError`` when a :hook:`pytest_fixture_setup` hookimpl raises during the setup of a parametrized fixture: the remaining parameters of the same fixture now run normally instead of erroring out with a stale-finalizer assertion. This was a regression in pytest 9.1.0. diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py index 1b1591d1f83..0db1ce41c4c 100644 --- a/src/_pytest/fixtures.py +++ b/src/_pytest/fixtures.py @@ -1205,9 +1205,13 @@ 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 not self._finalizers: + # Already finished. + # Note: finalizers may be pending even without a cached result -- + # execute() registers the pytest_fixture_post_finalizer finalizer + # before running the pytest_fixture_setup hook, so if a hookimpl + # raises, the failure is not cached but the finalizer must still + # run (#14800). return exceptions: list[BaseException] = [] diff --git a/testing/python/fixtures.py b/testing/python/fixtures.py index 95fe5c389b8..2927e849817 100644 --- a/testing/python/fixtures.py +++ b/testing/python/fixtures.py @@ -4535,6 +4535,95 @@ def test_second(my_fixture): ) +@pytest.mark.parametrize( + "failure", ["pytest.skip('unavailable')", "raise RuntimeError('boom')"] +) +def test_fixture_setup_hook_failure_does_not_break_subsequent_params( + pytester: Pytester, failure: str +) -> None: + """A pytest_fixture_setup hookimpl raising before the result is cached + must not leave stale finalizers on the FixtureDef, which broke every + subsequent parameter of the same fixture with an internal AssertionError + (#14800). + """ + pytester.makeconftest( + """ + import pytest + + @pytest.hookimpl(tryfirst=True) + def pytest_fixture_setup(fixturedef, request): + param = getattr(request, "param", None) + if isinstance(param, str) and param.startswith("fixture:"): + request.param = request.getfixturevalue(param[len("fixture:"):]) + """ + ) + pytester.makepyfile( + f""" + import pytest + + @pytest.fixture + def failing_base(): + {failure} + + @pytest.fixture + def derived(failing_base): + return "derived" + + @pytest.mark.parametrize("val", ["fixture:derived", "plain-1", "plain-2"]) + def test_thing(val): + assert isinstance(val, str) + """ + ) + result = pytester.runpytest() + result.stdout.no_fnmatch_line("*AssertionError*") + if "skip" in failure: + result.assert_outcomes(passed=2, skipped=1) + else: + result.assert_outcomes(passed=2, errors=1) + result.stdout.fnmatch_lines(["*RuntimeError: boom*"]) + + +def test_fixture_post_finalizer_called_once_after_fixture_setup_hook_failure( + pytester: Pytester, +) -> None: + """Draining the stale finalizer of a failed setup must not cause duplicate + pytest_fixture_post_finalizer calls for later parameters (#5848, #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) + + def pytest_terminal_summary(terminalreporter): + terminalreporter.write_line(f"post_finalizer calls: {calls}") + """ + ) + pytester.makepyfile( + """ + import pytest + + @pytest.mark.parametrize("val", ["fail", "ok-1", "ok-2"]) + def test_thing(val): + assert val.startswith("ok") + """ + ) + result = pytester.runpytest() + result.assert_outcomes(passed=2, errors=1) + assert ( + "post_finalizer calls: " + "['test_thing[fail]', 'test_thing[ok-1]', 'test_thing[ok-2]']" + ) in result.stdout.str() + + class TestParamValueKey: """Unit tests for the equivalence key used by `reorder_items` (#8914)."""