diff --git a/changelog/12446.bugfix.rst b/changelog/12446.bugfix.rst new file mode 100644 index 0000000000..2f591c48ee --- /dev/null +++ b/changelog/12446.bugfix.rst @@ -0,0 +1 @@ +Avoid calling ``@property`` (and other instance descriptors) during fixture discovery -- by :user:`asottile` diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py index aaa92c6372..6b882fa351 100644 --- a/src/_pytest/fixtures.py +++ b/src/_pytest/fixtures.py @@ -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 @@ -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 diff --git a/testing/python/collect.py b/testing/python/collect.py index 0638661127..530f1c340f 100644 --- a/testing/python/collect.py +++ b/testing/python/collect.py @@ -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(