Skip to content

Commit

Permalink
Don't execute class properties when collecting Python test classes
Browse files Browse the repository at this point in the history
Previously, properties such as in the added test case were triggered
during collection, possibly executing unintended code. Let's skip them
instead.

We should probably skip all custom descriptors, however I am not sure
how to do this cleanly, so for now just skip properties.

Fixes pytest-dev#2234.
  • Loading branch information
bluetech committed Feb 6, 2017
1 parent ccf9877 commit 34718c6
Show file tree
Hide file tree
Showing 4 changed files with 18 additions and 0 deletions.
1 change: 1 addition & 0 deletions AUTHORS
Expand Up @@ -118,6 +118,7 @@ Piotr Banaszkiewicz
Punyashloka Biswal
Quentin Pradet
Ralf Schmitt
Ran Benita
Raphael Pierzina
Raquel Alegre
Roberto Polli
Expand Down
4 changes: 4 additions & 0 deletions CHANGELOG.rst
Expand Up @@ -3,6 +3,8 @@

*

* Don't trigger execution of class properties in Python test classes during collection (`#2234`_).

* 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.

Expand All @@ -16,6 +18,8 @@

*

.. _#2234: https://github.com/pytest-dev/pytest/issues/2234

.. _#2160: https://github.com/pytest-dev/pytest/issues/2160

.. _PEP-479: https://www.python.org/dev/peps/pep-0479/
Expand Down
3 changes: 3 additions & 0 deletions _pytest/fixtures.py
Expand Up @@ -1068,6 +1068,9 @@ def parsefactories(self, node_or_obj, nodeid=NOTSET, unittest=False):
self._holderobjseen.add(holderobj)
autousenames = []
for name in dir(holderobj):
# Skip properties before the following getattr executes them.
if isinstance(type(holderobj).__dict__.get(name), property):
continue
obj = getattr(holderobj, name, None)
# fixture functions have a pytest_funcarg__ prefix (pre-2.3 style)
# or are "@pytest.fixture" marked
Expand Down
10 changes: 10 additions & 0 deletions testing/python/collect.py
Expand Up @@ -166,6 +166,16 @@ def test_issue1579_namedtuple(self, testdir):
"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:
def test_generative_functions(self, testdir):
Expand Down

0 comments on commit 34718c6

Please sign in to comment.