-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
fix #13564 - warn when fixtures get wrapped with a decorator #13745
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix #13564 - warn when fixtures get wrapped with a decorator #13745
Conversation
@RonnyPfannschmidt
RonnyPfannschmidt
commented
Sep 22, 2025
- closes Fixture definition check in parsefactories is too strict #13564
c879256 to
9e7e132
Compare
9e7e132 to
ac8d323
Compare
b79b0ed to
59a31ca
Compare
Refactor the wrapped fixture detection logic to safely handle: - Mock objects (which have _mock_name attribute) - Proxy objects with problematic __class__ properties - Wrapper loops (like in mock.call) - Objects that raise exceptions during isinstance checks The new _find_wrapped_fixture_def() method walks the wrapper chain more safely and avoids infinite loops and errors from special objects. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Prevent OOM crash when encountering objects with self-referential __wrapped__ attributes. The loop detection logic now checks object IDs at the start of each iteration and includes a maximum depth limit as an additional safeguard. The original code had a timing issue where it checked id(wrapped) against seen after fetching __wrapped__ but before updating current, which failed to detect self-referential objects properly. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds a warning mechanism to detect when pytest fixtures are wrapped with custom decorators, which prevents pytest from discovering them during collection. The warning helps developers understand why their fixtures might not be working as expected.
Key Changes:
- Added a warning system that detects fixtures wrapped in decorators using
functools.wraps - Implemented safe wrapper chain traversal with loop detection and special object handling
- Added comprehensive test coverage for the new warning functionality
Reviewed Changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/_pytest/fixtures.py |
Implements the core detection logic for wrapped fixtures and issues warnings when found |
testing/python/fixtures.py |
Adds test case to verify the warning is correctly issued for decorated fixtures |
changelog/13564.improvement.rst |
Documents the new warning feature for wrapped fixtures |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Copilot
AI
Oct 18, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The comment should clarify why mock objects need to be skipped. Consider: 'Skip mock objects (they have _mock_name attribute) to avoid false positives when traversing their wrapper chains.'
Copilot
AI
Oct 18, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The reference to 'issue #214' is unclear without context. Consider specifying the full issue reference (e.g., 'pytest issue #214') or linking to the relevant documentation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
redundant with start-of-loop logic