Cache fixture setup failures that happen before the fixture function runs#14777
Open
Gooh456 wants to merge 1 commit into
Open
Cache fixture setup failures that happen before the fixture function runs#14777Gooh456 wants to merge 1 commit into
Gooh456 wants to merge 1 commit into
Conversation
…runs If resolve_fixture_function() (or the async-fixture check) raises before call_fixture_func() is reached, pytest_fixture_setup() never wrote a cached_result for the FixtureDef. The exception still propagates and fails the current test correctly, but the FixtureDef is left in a half-registered state: the pytest_fixture_post_finalizer callback added in execute() is never popped, because that only happens via finish(), and finish() for a class/module/session-scoped fixture doesn't run until the owning scope's node tears down. The next test that requests the same fixture instance then hits `assert not self._finalizers` in FixtureDef.execute() and blows up with an internal AssertionError instead of reporting the original problem. This is easy to trigger now that resolving a class-scoped fixture defined as an instance method emits a warning (deprecations.rst), which becomes an exception under -W error before the fixture body ever runs. Move fixturefunc = resolve_fixture_function(...) and the async-fixture check inside the existing try/except TEST_OUTCOME block so any failure during setup, not just failures inside the fixture body, gets recorded in cached_result the same way. Subsequent requests then correctly re-raise the cached failure instead of tripping the internal assert. Fixes pytest-dev#14775 Signed-off-by: Kyue <164024549+Gooh456@users.noreply.github.com>
psf-chronographer
Bot
added
the
bot:chronographer:provided
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
yeah this one's a real crash, not just a bad error message.
execute()infixtures.pyregisters thepytest_fixture_post_finalizercallback intoself._finalizers(around line 1223) before callingpytest_fixture_setup(). Butpytest_fixture_setup()only wrapscall_fixture_func()intry/except TEST_OUTCOME—resolve_fixture_function()and the async-fixturefail()check both run outside that block. So if either of those raises,cached_resultnever gets set, and the finalizer already sitting inself._finalizersnever gets popped (that only happens viafinish(), which for a class-scoped fixture doesn't run until the class itself tears down, i.e. after the next test in the class already tried to set the fixture up again).Next test hits
assert not self._finalizersinexecute()and pytest crashes internally instead of reporting the actual problem.Easiest way to hit this right now: a class-scoped fixture defined as an instance method warns (the new deprecation from #14764), and under
-W errorthat warning becomes an exception insideresolve_fixture_function(), beforecall_fixture_func()is ever reached. Repro is basically verbatim from the issue.Fix: move
fixturefunc = resolve_fixture_function(...)and the async check inside the existingtry/except, so any setup failure — not just ones from inside the fixture body — gets cached the same waycall_fixture_funcfailures already are. The second test then correctly re-raises the cached failure instead of tripping the assert.Added a regression test in
TestErrors(testing/python/fixtures.py) using the exact repro from the issue, asserting two clean errors instead of one clean error + one internalAssertionError. Checked it fails on main and passes with the fix by stashing/unstashing the fixtures.py change.Ran
testing/python/andtesting/deprecated_test.pylocally, all green. Didn't run the full tox suite (mypy/pyright not installed here) — the diff is just a reindent/reorder of existing statements into the try block, no new types involved, but flagging that I haven't run those specific checks.Fixes #14775