From 38d96037ce5f9a6249f03a89c299611fd2ad4fb1 Mon Sep 17 00:00:00 2001 From: Pedro Algarvio Date: Wed, 17 Feb 2016 12:04:29 +0000 Subject: [PATCH 1/4] Catch and raise OutcomeException's instead of catching all exceptions --- _pytest/skipping.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/_pytest/skipping.py b/_pytest/skipping.py index 1b1c682676d..d23f577e36c 100644 --- a/_pytest/skipping.py +++ b/_pytest/skipping.py @@ -6,6 +6,7 @@ import py import pytest from _pytest.mark import MarkInfo +from _pytest.runner import OutcomeException def pytest_addoption(parser): @@ -88,6 +89,8 @@ def invalidraise(self, exc): def istrue(self): try: return self._istrue() + except OutcomeException: + raise except Exception: self.exc = sys.exc_info() if isinstance(self.exc[1], SyntaxError): From f2fc48face4306cfcfeff7582bf8f3429c50e6a5 Mon Sep 17 00:00:00 2001 From: Pedro Algarvio Date: Wed, 17 Feb 2016 12:43:11 +0000 Subject: [PATCH 2/4] Just catch `Skipped` since we trigger `Failed` on `_istrue()` evaluation --- _pytest/skipping.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/_pytest/skipping.py b/_pytest/skipping.py index d23f577e36c..513f64f16bd 100644 --- a/_pytest/skipping.py +++ b/_pytest/skipping.py @@ -6,7 +6,7 @@ import py import pytest from _pytest.mark import MarkInfo -from _pytest.runner import OutcomeException +from _pytest.runner import Skipped def pytest_addoption(parser): @@ -89,7 +89,7 @@ def invalidraise(self, exc): def istrue(self): try: return self._istrue() - except OutcomeException: + except Skipped: raise except Exception: self.exc = sys.exc_info() From a58ec32034860cb3a1e4924d0b66beed498a5dae Mon Sep 17 00:00:00 2001 From: Pedro Algarvio Date: Wed, 17 Feb 2016 14:00:04 +0000 Subject: [PATCH 3/4] Test proper handling of `pytest.skip` within a custom skip marker --- testing/test_skipping.py | 79 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 79 insertions(+) diff --git a/testing/test_skipping.py b/testing/test_skipping.py index 8e43914f0d6..0f5ddd492b2 100644 --- a/testing/test_skipping.py +++ b/testing/test_skipping.py @@ -126,6 +126,85 @@ def test_func(self): assert expl == "condition: config._hackxyz" +class TestCustomMarkEvaluator: + def test_custom_skipiffixture(self, testdir): + testdir.makeconftest(''' + import pytest + import _pytest.skipping + + class FixtureMarkEvaluator(_pytest.skipping.MarkEvaluator): + def _getglobals(self): + evaluation_globals = super(FixtureMarkEvaluator, self)._getglobals() + for fixture_name in self.holder.kwargs.get('fixtures', ()): + # Looking up fixture results will add some finalizers which, once/if tests are skipped + # by this marker, it will make pytest fail because of the added finalizer. + # We have 2 options, run the finalizer ourselves, or, just restore the previously + # existing finalizers ignoring any actions those added finalizers would do(for our + # current needs, we can ignore the finalizers) + + # Store the finalizers before getting the fixture value and restore it afterwards + original_finalizers = self.item._request.session._setupstate._finalizers.copy() + + # Inject the fixture value into the globals that should be passed on to evaluate the marker + try: + evaluation_globals[fixture_name] = self.item._request.getfuncargvalue(fixture_name) + finally: + # Restore previous finalizers state + self.item._request.session._setupstate._finalizers = original_finalizers + return evaluation_globals + + def istrue(self): + result = super(FixtureMarkEvaluator, self).istrue() + if self.holder: + try: + # We're evaluating against a dictionary which changes + # Remove the evaluated expr from the cache since the cache is + # not aware of the arguments used to get the result + self.item.config._evalcache.pop(self.expr) + except KeyError: + pass + return result + + @pytest.hookimpl(tryfirst=True) + def pytest_runtest_setup(item): + eval_fixture_skipif = FixtureMarkEvaluator(item, 'skipiffixture') + if eval_fixture_skipif.istrue(): + item._evalskip = eval_fixture_skipif + pytest.skip(eval_fixture_skipif.getexplanation()) + + HAS_LDAP = False + + @pytest.fixture(scope='function', params=('internal', 'ldap', 'ad')) + def auth_config(request): + if HAS_LDAP is False and request.param == 'ldap': + pytest.skip('ldap3 lib not installed') + return {'auth_config': request.param} + ''') + + testdir.makepyfile(''' + import pytest + + @pytest.mark.skipiffixture("auth_config['auth_config'] != 'internal'", + fixtures=('auth_config',), + reason='Only test internal') + def test_custom_skipiffixture(auth_config): + assert auth_config['auth_config'] == 'internal' + ''') + result = testdir.runpytest('-vsra') + result.stdout.fnmatch_lines([ + 'test_custom_skipiffixture.py::test_custom_skipiffixture[internal] PASSED', + 'test_custom_skipiffixture.py::test_custom_skipiffixture[ldap] SKIPPED', + 'test_custom_skipiffixture.py::test_custom_skipiffixture[ad] SKIPPED', + ]) + # The fnmatch_lines below need to be separate because they both share the same start + result.stdout.fnmatch_lines([ + 'SKIP * Only test internal', + ]) + result.stdout.fnmatch_lines([ + 'SKIP * ldap3 lib not installed' + ]) + + class TestXFail: def test_xfail_simple(self, testdir): item = testdir.getitem(""" From ebe45e9834075fca9627e8a8e7b0361c696eaecf Mon Sep 17 00:00:00 2001 From: Pedro Algarvio Date: Wed, 17 Feb 2016 14:32:20 +0000 Subject: [PATCH 4/4] Don't use super --- testing/test_skipping.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testing/test_skipping.py b/testing/test_skipping.py index 0f5ddd492b2..5ce84121165 100644 --- a/testing/test_skipping.py +++ b/testing/test_skipping.py @@ -154,7 +154,7 @@ def _getglobals(self): return evaluation_globals def istrue(self): - result = super(FixtureMarkEvaluator, self).istrue() + result = _pytest.skipping.MarkEvaluator.istrue(self) if self.holder: try: # We're evaluating against a dictionary which changes