-
-
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -57,3 +57,4 @@ pip-wheel-metadata/ | |
|
|
||
| # pytest debug logs generated via --debug | ||
| pytestdebug.log | ||
| .claude/settings.local.json | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| Issue a warning when fixtures are wrapped with a decorator, as that excludes | ||
| them from being discovered safely by pytest. |
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -1271,7 +1271,9 @@ def __init__( | |||||||||||||
| def __repr__(self) -> str: | ||||||||||||||
| return f"<pytest_fixture({self._fixture_function})>" | ||||||||||||||
|
|
||||||||||||||
| def __get__(self, instance, owner=None): | ||||||||||||||
| def __get__( | ||||||||||||||
| self, instance: object, owner: type | None = None | ||||||||||||||
| ) -> FixtureFunctionDefinition: | ||||||||||||||
| """Behave like a method if the function it was applied to was a method.""" | ||||||||||||||
| return FixtureFunctionDefinition( | ||||||||||||||
| function=self._fixture_function, | ||||||||||||||
|
|
@@ -1765,6 +1767,80 @@ def _register_fixture( | |||||||||||||
| if autouse: | ||||||||||||||
| self._nodeid_autousenames.setdefault(nodeid or "", []).append(name) | ||||||||||||||
|
|
||||||||||||||
| def _find_wrapped_fixture_def( | ||||||||||||||
| self, obj: object | ||||||||||||||
| ) -> FixtureFunctionDefinition | None: | ||||||||||||||
| """Walk through wrapper chain to find a FixtureFunctionDefinition. | ||||||||||||||
|
|
||||||||||||||
| Returns the FixtureFunctionDefinition if found in the wrapper chain, | ||||||||||||||
| None otherwise. Handles loops and special objects safely. | ||||||||||||||
| """ | ||||||||||||||
| from _pytest.compat import safe_getattr | ||||||||||||||
|
|
||||||||||||||
| # Skip mock objects (they have _mock_name attribute) | ||||||||||||||
| if safe_getattr(obj, "_mock_name", None) is not None: | ||||||||||||||
| return None | ||||||||||||||
|
|
||||||||||||||
| current = obj | ||||||||||||||
| seen = set() # Track object IDs to detect loops | ||||||||||||||
| max_depth = 100 # Prevent infinite loops even if ID tracking fails | ||||||||||||||
|
|
||||||||||||||
| for _ in range(max_depth): | ||||||||||||||
| if current is None: | ||||||||||||||
| break | ||||||||||||||
|
|
||||||||||||||
| # Check for wrapper loops by object identity | ||||||||||||||
| current_id = id(current) | ||||||||||||||
| if current_id in seen: | ||||||||||||||
| return None | ||||||||||||||
| seen.add(current_id) | ||||||||||||||
|
|
||||||||||||||
| # Check if current is a FixtureFunctionDefinition | ||||||||||||||
| # Use try/except to handle objects with problematic __class__ properties | ||||||||||||||
| try: | ||||||||||||||
| if isinstance(current, FixtureFunctionDefinition): | ||||||||||||||
| return current | ||||||||||||||
| except Exception: | ||||||||||||||
| # Can't check isinstance - probably a proxy object | ||||||||||||||
| return None | ||||||||||||||
|
|
||||||||||||||
| # Try to get the next wrapped object using safe_getattr to handle | ||||||||||||||
| # "evil objects" that raise on attribute access (see issue #214) | ||||||||||||||
|
||||||||||||||
| # "evil objects" that raise on attribute access (see issue #214) | |
| # "evil objects" that raise on attribute access (see pytest issue #214: https://github.com/pytest-dev/pytest/issues/214) |
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.
| wrapped = safe_getattr(current, "__wrapped__", None) | |
| if wrapped is None: | |
| break | |
| current = wrapped | |
| current = safe_getattr(current, "__wrapped__", None) |
redundant with start-of-loop logic
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.'