From 3d18c9c1c6e58e30da97e74fc79efb91f1aae24e Mon Sep 17 00:00:00 2001 From: Barney Gale Date: Sun, 18 Dec 2016 21:30:00 +0000 Subject: [PATCH 1/8] 'xfail' markers without a condition no longer rely on the underlying `Item` deriving from `PyobjMixin` --- _pytest/skipping.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/_pytest/skipping.py b/_pytest/skipping.py index a8eaea98a..cfaed2fa7 100644 --- a/_pytest/skipping.py +++ b/_pytest/skipping.py @@ -119,7 +119,6 @@ class MarkEvaluator: if hasattr(self, 'result'): return self.result if self.holder: - d = self._getglobals() if self.holder.args or 'condition' in self.holder.kwargs: self.result = False # "holder" might be a MarkInfo or a MarkDecorator; only @@ -135,6 +134,7 @@ class MarkEvaluator: for expr in args: self.expr = expr if isinstance(expr, py.builtin._basestring): + d = self._getglobals() result = cached_eval(self.item.config, expr, d) else: if "reason" not in kwargs: From 8db9915374f60d00780b758b98ada8d3f80af9bb Mon Sep 17 00:00:00 2001 From: Barney Gale Date: Sun, 18 Dec 2016 21:39:35 +0000 Subject: [PATCH 2/8] Update AUTHORS, CHANGELOG --- AUTHORS | 1 + CHANGELOG.rst | 5 ++++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/AUTHORS b/AUTHORS index 8c6fa1b5d..3999db9b3 100644 --- a/AUTHORS +++ b/AUTHORS @@ -16,6 +16,7 @@ Antony Lee Armin Rigo Aron Curzon Aviv Palivoda +Barney Gale Ben Webb Benjamin Peterson Bernard Pratz diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 7c3df0327..64453f259 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -1,7 +1,10 @@ 3.0.6.dev0 (unreleased) ======================= -* +* Conditionless ``xfail`` markers no longer rely on the underlying test item + being an instance of ``PyobjMixin``, and can therefore apply to tests not + collected by the built-in python test collector. Thanks `@barneygale`_ for the + PR. * pytest no longer recognizes coroutine functions as yield tests (`#2129`_). Thanks to `@malinoff`_ for the PR. From df409a0c0ee9948d3725ee79c4a0a91960466af7 Mon Sep 17 00:00:00 2001 From: Barney Gale Date: Mon, 2 Jan 2017 22:01:40 +0000 Subject: [PATCH 3/8] Fix CHANGELOG.rst --- CHANGELOG.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 64453f259..eca520bbc 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -19,6 +19,7 @@ * +.. _@barneygale: https://github.com/barneygale .. _@lesteve: https://github.com/lesteve .. _@malinoff: https://github.com/malinoff .. _@pelme: https://github.com/pelme From bad261279c5af15e2126023a03f08389c189b2d5 Mon Sep 17 00:00:00 2001 From: Vidar Tonaas Fauske Date: Fri, 3 Feb 2017 16:04:34 +0100 Subject: [PATCH 4/8] Do not asssume `Item.obj` in 'skipping' plugin See #2231 for discussion. --- _pytest/skipping.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/_pytest/skipping.py b/_pytest/skipping.py index 91a34169f..591977efd 100644 --- a/_pytest/skipping.py +++ b/_pytest/skipping.py @@ -112,7 +112,8 @@ class MarkEvaluator: def _getglobals(self): d = {'os': os, 'sys': sys, 'config': self.item.config} - d.update(self.item.obj.__globals__) + if hasattr(self.item, 'obj'): + d.update(self.item.obj.__globals__) return d def _istrue(self): From 1a88a91c7a04ed996bee3c8957e07d9a0c735de1 Mon Sep 17 00:00:00 2001 From: Vidar Tonaas Fauske Date: Fri, 3 Feb 2017 16:29:43 +0100 Subject: [PATCH 5/8] Update authors/history --- AUTHORS | 1 + CHANGELOG.rst | 7 ++++++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/AUTHORS b/AUTHORS index c2400f508..d1a4aa675 100644 --- a/AUTHORS +++ b/AUTHORS @@ -141,5 +141,6 @@ Trevor Bekolay Tyler Goodlet Vasily Kuznetsov Victor Uriarte +Vidar T. Fauske Wouter van Ackooy Xuecong Liao diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 612ee0f3b..b81803324 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -6,7 +6,8 @@ * 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. -* +* Skipping plugin now also works with test items generated by custom collectors (`#2231`_). + Thanks to `@vidartf`_. * @@ -16,6 +17,10 @@ .. _PEP-479: https://www.python.org/dev/peps/pep-0479/ +.. _#2231: https://github.com/pytest-dev/pytest/issues/2231 + +.. _@vidartf: https://github.com/vidartf + 3.0.6 (2017-01-22) ================== From 832c89dd5fa7c310ae05ddcffc81025bed7a3c14 Mon Sep 17 00:00:00 2001 From: Vidar Tonaas Fauske Date: Fri, 3 Feb 2017 16:59:30 +0100 Subject: [PATCH 6/8] Test for `pytest.mark.xfail` with non-Python Item --- testing/test_skipping.py | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/testing/test_skipping.py b/testing/test_skipping.py index 2e7868d3a..ac4412fcb 100644 --- a/testing/test_skipping.py +++ b/testing/test_skipping.py @@ -969,3 +969,26 @@ def test_module_level_skip_error(testdir): result.stdout.fnmatch_lines( "*Using pytest.skip outside of a test is not allowed*" ) + + +def test_mark_xfail_item(testdir): + # Ensure pytest.mark.xfail works with non-Python Item + testdir.makeconftest(""" + import pytest + + class MyItem(pytest.Item): + nodeid = 'foo' + def setup(self): + marker = pytest.mark.xfail(True, reason="Expected failure") + self.add_marker(marker) + def runtest(self): + assert False + + def pytest_collect_file(path, parent): + return MyItem("foo", parent) + """) + result = testdir.inline_run() + passed, skipped, failed = result.listoutcomes() + assert not failed + xfailed = [r for r in skipped if hasattr(r, 'wasxfail')] + assert xfailed From 87fb689ab1d05935bfd75ff29fcbc1de4161f606 Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Tue, 7 Feb 2017 14:00:13 +0200 Subject: [PATCH 7/8] Remove an unneeded `except KeyboardInterrupt` KeyboardInterrupt is a subclass of BaseException, but not of Exception. Hence if we remove this except, KeyboardInterrupts will still be raised so the behavior stays the same. --- _pytest/fixtures.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/_pytest/fixtures.py b/_pytest/fixtures.py index 28bcd4d8d..c5bf75386 100644 --- a/_pytest/fixtures.py +++ b/_pytest/fixtures.py @@ -124,8 +124,6 @@ def getfixturemarker(obj): exceptions.""" try: return getattr(obj, "_pytestfixturefunction", None) - except KeyboardInterrupt: - raise except Exception: # some objects raise errors like request (from flask import request) # we don't expect them to be fixture functions From 3a0a0c2df93ecfcf4fb09a0f7c24bf3a06d1bae5 Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Sun, 5 Feb 2017 23:55:39 +0200 Subject: [PATCH 8/8] 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 fa1140d4d..b09516a1d 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 d9ff2afd5..c11200d64 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 c5bf75386..248708f6e 100644 --- a/_pytest/fixtures.py +++ b/_pytest/fixtures.py @@ -14,6 +14,7 @@ from _pytest.compat import ( getfslineno, get_real_func, is_generator, isclass, getimfunc, getlocation, getfuncargnames, + safe_getattr, ) def pytest_sessionstart(session): @@ -1066,7 +1067,9 @@ class FixtureManager: 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 1e69f2da9..cce934ddc 100644 --- a/testing/python/collect.py +++ b/testing/python/collect.py @@ -166,6 +166,16 @@ class TestClass: "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):