From 3a0a0c2df93ecfcf4fb09a0f7c24bf3a06d1bae5 Mon Sep 17 00:00:00 2001
From: Ran Benita <ran234@gmail.com>
Date: Sun, 5 Feb 2017 23:55:39 +0200
Subject: [PATCH] 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.
---
 AUTHORS                   |  1 +
 CHANGELOG.rst             |  7 +++++++
 _pytest/fixtures.py       |  5 ++++-
 testing/python/collect.py | 10 ++++++++++
 4 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/AUTHORS b/AUTHORS
index fa1140d4dfe..b09516a1dd0 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -118,6 +118,7 @@ Piotr Banaszkiewicz
 Punyashloka Biswal
 Quentin Pradet
 Ralf Schmitt
+Ran Benita
 Raphael Pierzina
 Raquel Alegre
 Roberto Polli
diff --git a/CHANGELOG.rst b/CHANGELOG.rst
index d9ff2afd52b..c11200d6440 100644
--- a/CHANGELOG.rst
+++ b/CHANGELOG.rst
@@ -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`_).
   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
 
 .. _PEP-479: https://www.python.org/dev/peps/pep-0479/
diff --git a/_pytest/fixtures.py b/_pytest/fixtures.py
index c5bf7538648..248708f6ea9 100644
--- a/_pytest/fixtures.py
+++ b/_pytest/fixtures.py
@@ -14,6 +14,7 @@
     getfslineno, get_real_func,
     is_generator, isclass, getimfunc,
     getlocation, getfuncargnames,
+    safe_getattr,
 )
 
 def pytest_sessionstart(session):
@@ -1066,7 +1067,9 @@ def parsefactories(self, node_or_obj, nodeid=NOTSET, unittest=False):
         self._holderobjseen.add(holderobj)
         autousenames = []
         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)
             # or are "@pytest.fixture" marked
             marker = getfixturemarker(obj)
diff --git a/testing/python/collect.py b/testing/python/collect.py
index 1e69f2da939..cce934ddc8d 100644
--- a/testing/python/collect.py
+++ b/testing/python/collect.py
@@ -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):