Skip to content

Commit

Permalink
Do not discover properties when iterating fixtures (pytest-dev#12781)
Browse files Browse the repository at this point in the history
  • Loading branch information
asottile committed Sep 8, 2024
1 parent d35b802 commit c6a5290
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 3 deletions.
1 change: 1 addition & 0 deletions changelog/12446.bugfix.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Avoid calling ``@property`` (and other instance descriptors) during fixture discovery -- by :user:`asottile`
16 changes: 13 additions & 3 deletions src/_pytest/fixtures.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
from _pytest.compat import NOTSET
from _pytest.compat import NotSetType
from _pytest.compat import safe_getattr
from _pytest.compat import safe_isclass
from _pytest.config import _PluggyPlugin
from _pytest.config import Config
from _pytest.config import ExitCode
Expand Down Expand Up @@ -1724,17 +1725,26 @@ def parsefactories(
if holderobj in self._holderobjseen:
return

# Avoid accessing `@property` (and other descriptors) when iterating fixtures.
if not safe_isclass(holderobj) and not isinstance(holderobj, types.ModuleType):
holderobj_tp: object = type(holderobj)
else:
holderobj_tp = holderobj

self._holderobjseen.add(holderobj)
for name in dir(holderobj):
# The attribute can be an arbitrary descriptor, so the attribute
# access below can raise. safe_getatt() ignores such exceptions.
obj = safe_getattr(holderobj, name, None)
marker = getfixturemarker(obj)
# access below can raise. safe_getattr() ignores such exceptions.
obj_ub = safe_getattr(holderobj_tp, name, None)
marker = getfixturemarker(obj_ub)
if not isinstance(marker, FixtureFunctionMarker):
# Magic globals with __getattr__ might have got us a wrong
# fixture attribute.
continue

# OK we know it is a fixture -- now safe to look up on the _instance_.
obj = getattr(holderobj, name)

if marker.name:
name = marker.name

Expand Down
37 changes: 37 additions & 0 deletions testing/python/collect.py
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,43 @@ def prop(self):
result = pytester.runpytest()
assert result.ret == ExitCode.NO_TESTS_COLLECTED

def test_does_not_discover_properties(self, pytester: Pytester) -> None:
"""Regression test for #12446."""
pytester.makepyfile(
"""\
class TestCase:
@property
def oops(self):
raise SystemExit('do not call me!')
"""
)
result = pytester.runpytest()
assert result.ret == ExitCode.NO_TESTS_COLLECTED

def test_does_not_discover_instance_descriptors(self, pytester: Pytester) -> None:
"""Regression test for #12446."""
pytester.makepyfile(
"""\
# not `@property`, but it acts like one
# this should cover the case of things like `@cached_property` / etc.
class MyProperty:
def __init__(self, func):
self._func = func
def __get__(self, inst, owner):
if inst is None:
return self
else:
return self._func.__get__(inst, owner)()
class TestCase:
@MyProperty
def oops(self):
raise SystemExit('do not call me!')
"""
)
result = pytester.runpytest()
assert result.ret == ExitCode.NO_TESTS_COLLECTED

def test_abstract_class_is_not_collected(self, pytester: Pytester) -> None:
"""Regression test for #12275 (non-unittest version)."""
pytester.makepyfile(
Expand Down

0 comments on commit c6a5290

Please sign in to comment.