Ignore errors raised from descriptors when collecting fixtures

Descriptors (e.g. properties) such as in the added test case are
triggered during collection, executing arbitrary code which can raise.
Previously, such exceptions were propagated and failed the collection.
Now these exceptions are caught and the corresponding attributes are
silently ignored.

A better solution would be to completely skip access to all custom
descriptors, such that the offending code doesn't even trigger. However
I think this requires manually going through the instance and all of its
MRO for each and every attribute checking if it might be a proper
fixture before accessing it. So I took the easy route here.

In other words, putting something like this in your test class is still
a bad idea...:

    @property
    def innocent(self):
        os.system('rm -rf /')

Fixes #2234.
This commit is contained in:
Ran Benita 2017-02-05 23:55:39 +02:00
parent 87fb689ab1
commit 3a0a0c2df9
4 changed files with 22 additions and 1 deletions

View File

@ -118,6 +118,7 @@ Piotr Banaszkiewicz
Punyashloka Biswal Punyashloka Biswal
Quentin Pradet Quentin Pradet
Ralf Schmitt Ralf Schmitt
Ran Benita
Raphael Pierzina Raphael Pierzina
Raquel Alegre Raquel Alegre
Roberto Polli Roberto Polli

View File

@ -3,6 +3,9 @@
* *
* Ignore exceptions raised from descriptors (e.g. properties) during Python test collection (`#2234`_).
Thanks to `@bluetech`_.
* Replace ``raise StopIteration`` usages in the code by simple ``returns`` to finish generators, in accordance to `PEP-479`_ (`#2160`_). * Replace ``raise StopIteration`` usages in the code by simple ``returns`` to finish generators, in accordance to `PEP-479`_ (`#2160`_).
Thanks `@tgoodlet`_ for the report and `@nicoddemus`_ for the PR. Thanks `@tgoodlet`_ for the report and `@nicoddemus`_ for the PR.
@ -16,6 +19,10 @@
* *
.. _#2234: https://github.com/pytest-dev/pytest/issues/2234
.. _@bluetech: https://github.com/bluetech
.. _#2160: https://github.com/pytest-dev/pytest/issues/2160 .. _#2160: https://github.com/pytest-dev/pytest/issues/2160
.. _PEP-479: https://www.python.org/dev/peps/pep-0479/ .. _PEP-479: https://www.python.org/dev/peps/pep-0479/

View File

@ -14,6 +14,7 @@ from _pytest.compat import (
getfslineno, get_real_func, getfslineno, get_real_func,
is_generator, isclass, getimfunc, is_generator, isclass, getimfunc,
getlocation, getfuncargnames, getlocation, getfuncargnames,
safe_getattr,
) )
def pytest_sessionstart(session): def pytest_sessionstart(session):
@ -1066,7 +1067,9 @@ class FixtureManager:
self._holderobjseen.add(holderobj) self._holderobjseen.add(holderobj)
autousenames = [] autousenames = []
for name in dir(holderobj): for name in dir(holderobj):
obj = getattr(holderobj, name, None) # 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)
# fixture functions have a pytest_funcarg__ prefix (pre-2.3 style) # fixture functions have a pytest_funcarg__ prefix (pre-2.3 style)
# or are "@pytest.fixture" marked # or are "@pytest.fixture" marked
marker = getfixturemarker(obj) marker = getfixturemarker(obj)

View File

@ -166,6 +166,16 @@ class TestClass:
"because it has a __new__ constructor*" "because it has a __new__ constructor*"
) )
def test_issue2234_property(self, testdir):
testdir.makepyfile("""
class TestCase(object):
@property
def prop(self):
raise NotImplementedError()
""")
result = testdir.runpytest()
assert result.ret == EXIT_NOTESTSCOLLECTED
class TestGenerator: class TestGenerator:
def test_generative_functions(self, testdir): def test_generative_functions(self, testdir):