Fix stale finalizers when a pytest_fixture_setup hookimpl raises - #14801
Open
Mohith26 wants to merge 2 commits into
Open
Fix stale finalizers when a pytest_fixture_setup hookimpl raises#14801Mohith26 wants to merge 2 commits into
Mohith26 wants to merge 2 commits into
Conversation
If a pytest_fixture_setup hookimpl raised before the default implementation cached the result (e.g. pytest-lazy-fixtures resolving a fixture whose dependency chain skips), FixtureDef.finish() early-returned on `cached_result is None` without draining `_finalizers`. The pytest_fixture_post_finalizer finalizer registered by execute() before the hook call was left stale, so every subsequent parameter of the same fixture tripped the internal `assert not self._finalizers`. Only take the early-return (added in pytest-dev#5848 to avoid duplicate pytest_fixture_post_finalizer calls) when there is also nothing left to finalize, so a failed setup still gets drained. The already-finished case still short-circuits, since finish() clears both cached_result and _finalizers. Fixes pytest-dev#14800. Regression in 9.1.0 (interaction of pytest-dev#5848 and pytest-dev#14114). Co-authored-by: Claude <noreply@anthropic.com>
Cover a tryfirst pytest_fixture_setup hookimpl raising (both Skipped and a plain exception) during setup of a parametrized fixture, asserting the remaining parameters still run and the real failure is attributed to the right parameter. Also assert pytest_fixture_post_finalizer is still called exactly once per parameter, preserving pytest-dev#5848. Co-authored-by: Claude <noreply@anthropic.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.
Fixes #14800, reported by @jclerman.
When a
pytest_fixture_setuphookimpl raises before the fixture result is cached (as pytest-lazy-fixtures does when a lazy fixture's dependency skips),FixtureDef.finish()returned early without draining thepytest_fixture_post_finalizerfinalizer thatexecute()had already registered. The stale finalizer then made every subsequent parameter of the same parametrized fixture fail with an internalAssertionErrorinstead of running. This is a 9.1.0 regression from the interaction of #5848 and #14114.The fix makes
finish()'s early return conditional on there also being no pending finalizers, so failed setups are drained while the duplicate-post-finalizer protection from #5848 is preserved. The issue's alternative (registering the finalizer later) was rejected because it would break the post-finalizer's executed-last ordering.Includes a changelog fragment and AUTHORS entry. Three regression tests (skip variant, RuntimeError variant, and a post-finalizer-called-exactly-once guard for #5848) fail without the fix; the fixtures and runner suites pass with zero new failures (310 passed, 4 xfailed).