From 3a81d2e01281358ddb7987863c1b8382fc293a64 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Mon, 20 Jun 2016 14:45:13 +0200 Subject: [PATCH 1/5] conftest files now use assertion rewriting Fix #1619 --- _pytest/assertion/__init__.py | 50 +++++++++++++++++++++--------- _pytest/assertion/rewrite.py | 57 +++++++++++++++++++++-------------- testing/test_assertrewrite.py | 34 +++++++++++++++++++++ testing/test_config.py | 11 +++++-- 4 files changed, 112 insertions(+), 40 deletions(-) diff --git a/_pytest/assertion/__init__.py b/_pytest/assertion/__init__.py index 6921deb2a60..3da99c63c8b 100644 --- a/_pytest/assertion/__init__.py +++ b/_pytest/assertion/__init__.py @@ -4,6 +4,8 @@ import py import os import sys + +from _pytest.config import hookimpl from _pytest.monkeypatch import monkeypatch from _pytest.assertion import util @@ -42,9 +44,13 @@ def __init__(self, config, mode): self.trace = config.trace.root.get("assertion") -def pytest_configure(config): - mode = config.getvalue("assertmode") - if config.getvalue("noassert") or config.getvalue("nomagic"): +@hookimpl(tryfirst=True) +def pytest_load_initial_conftests(early_config, parser, args): + ns, ns_unknown_args = parser.parse_known_and_unknown_args(args) + mode = ns.assertmode + no_assert = ns.noassert + no_magic = ns.nomagic + if no_assert or no_magic: mode = "plain" if mode == "rewrite": try: @@ -57,25 +63,30 @@ def pytest_configure(config): if (sys.platform.startswith('java') or sys.version_info[:3] == (2, 6, 0)): mode = "reinterp" + + early_config._assertstate = AssertionState(early_config, mode) + warn_about_missing_assertion(mode, early_config.pluginmanager) + if mode != "plain": _load_modules(mode) m = monkeypatch() - config._cleanup.append(m.undo) + early_config._cleanup.append(m.undo) m.setattr(py.builtin.builtins, 'AssertionError', reinterpret.AssertionError) # noqa + hook = None if mode == "rewrite": hook = rewrite.AssertionRewritingHook() # noqa + hook.set_config(early_config) sys.meta_path.insert(0, hook) - warn_about_missing_assertion(mode) - config._assertstate = AssertionState(config, mode) - config._assertstate.hook = hook - config._assertstate.trace("configured with mode set to %r" % (mode,)) + + early_config._assertstate.hook = hook + early_config._assertstate.trace("configured with mode set to %r" % (mode,)) def undo(): - hook = config._assertstate.hook + hook = early_config._assertstate.hook if hook is not None and hook in sys.meta_path: sys.meta_path.remove(hook) - config.add_cleanup(undo) + early_config.add_cleanup(undo) def pytest_collection(session): @@ -154,7 +165,8 @@ def _load_modules(mode): from _pytest.assertion import rewrite # noqa -def warn_about_missing_assertion(mode): +def warn_about_missing_assertion(mode, pluginmanager): + print('got here') try: assert False except AssertionError: @@ -166,10 +178,18 @@ def warn_about_missing_assertion(mode): else: specifically = "failing tests may report as passing" - sys.stderr.write("WARNING: " + specifically + - " because assert statements are not executed " - "by the underlying Python interpreter " - "(are you using python -O?)\n") + # temporarily disable capture so we can print our warning + capman = pluginmanager.getplugin('capturemanager') + try: + out, err = capman.suspendcapture() + sys.stderr.write("WARNING: " + specifically + + " because assert statements are not executed " + "by the underlying Python interpreter " + "(are you using python -O?)\n") + finally: + capman.resumecapture() + sys.stdout.write(out) + sys.stderr.write(err) # Expose this plugin's implementation for the pytest_assertrepr_compare hook diff --git a/_pytest/assertion/rewrite.py b/_pytest/assertion/rewrite.py index 14b8e49db2b..efddc89203e 100644 --- a/_pytest/assertion/rewrite.py +++ b/_pytest/assertion/rewrite.py @@ -50,14 +50,14 @@ def __init__(self): self._register_with_pkg_resources() def set_session(self, session): - self.fnpats = session.config.getini("python_files") self.session = session + def set_config(self, config): + self.config = config + self.fnpats = config.getini("python_files") + def find_module(self, name, path=None): - if self.session is None: - return None - sess = self.session - state = sess.config._assertstate + state = self.config._assertstate state.trace("find_module called for: %s" % name) names = name.rsplit(".", 1) lastname = names[-1] @@ -86,24 +86,11 @@ def find_module(self, name, path=None): return None else: fn = os.path.join(pth, name.rpartition(".")[2] + ".py") + fn_pypath = py.path.local(fn) - # Is this a test file? - if not sess.isinitpath(fn): - # We have to be very careful here because imports in this code can - # trigger a cycle. - self.session = None - try: - for pat in self.fnpats: - if fn_pypath.fnmatch(pat): - state.trace("matched test file %r" % (fn,)) - break - else: - return None - finally: - self.session = sess - else: - state.trace("matched test file (was specified on cmdline): %r" % - (fn,)) + if not self._should_rewrite(fn_pypath, state): + return + # The requested module looks like a test file, so rewrite it. This is # the most magical part of the process: load the source, rewrite the # asserts, and load the rewritten source. We also cache the rewritten @@ -151,6 +138,32 @@ def find_module(self, name, path=None): self.modules[name] = co, pyc return self + def _should_rewrite(self, fn_pypath, state): + # always rewrite conftest files + fn = str(fn_pypath) + if fn_pypath.basename == 'conftest.py': + state.trace("rewriting conftest file: %r" % (fn,)) + return True + elif self.session is not None: + if self.session.isinitpath(fn): + state.trace("matched test file (was specified on cmdline): %r" % + (fn,)) + return True + else: + # modules not passed explicitly on the command line are only + # rewritten if they match the naming convention for test files + session = self.session # avoid a cycle here + self.session = None + try: + for pat in self.fnpats: + if fn_pypath.fnmatch(pat): + state.trace("matched test file %r" % (fn,)) + return True + finally: + self.session = session + del session + return False + def load_module(self, name): # If there is an existing module object named 'fullname' in # sys.modules, the loader must use that existing module. (Otherwise, diff --git a/testing/test_assertrewrite.py b/testing/test_assertrewrite.py index f43c424ca94..8d16bfc66d1 100644 --- a/testing/test_assertrewrite.py +++ b/testing/test_assertrewrite.py @@ -694,6 +694,40 @@ def test_foo(self): result = testdir.runpytest() result.stdout.fnmatch_lines('*1 passed*') + @pytest.mark.parametrize('initial_conftest', [True, False]) + @pytest.mark.parametrize('mode', ['plain', 'rewrite', 'reinterp']) + def test_conftest_assertion_rewrite(self, testdir, initial_conftest, mode): + """Test that conftest files are using assertion rewrite on import. + (#1619) + """ + testdir.tmpdir.join('foo/tests').ensure(dir=1) + conftest_path = 'conftest.py' if initial_conftest else 'foo/conftest.py' + contents = { + conftest_path: """ + import pytest + @pytest.fixture + def check_first(): + def check(values, value): + assert values.pop(0) == value + return check + """, + 'foo/tests/test_foo.py': """ + def test(check_first): + check_first([10, 30], 30) + """ + } + testdir.makepyfile(**contents) + result = testdir.runpytest_subprocess('--assert=%s' % mode) + if mode == 'plain': + expected = 'E AssertionError' + elif mode == 'rewrite': + expected = '*assert 10 == 30*' + elif mode == 'reinterp': + expected = '*AssertionError:*was re-run*' + else: + assert 0 + result.stdout.fnmatch_lines([expected]) + def test_issue731(testdir): testdir.makepyfile(""" diff --git a/testing/test_config.py b/testing/test_config.py index fe06540173e..69bea4a9c88 100644 --- a/testing/test_config.py +++ b/testing/test_config.py @@ -485,9 +485,14 @@ def pytest_load_initial_conftests(self): pm.register(m) hc = pm.hook.pytest_load_initial_conftests l = hc._nonwrappers + hc._wrappers - assert l[-1].function.__module__ == "_pytest.capture" - assert l[-2].function == m.pytest_load_initial_conftests - assert l[-3].function.__module__ == "_pytest.config" + expected = [ + "_pytest.config", + 'test_config', + '_pytest.assertion', + '_pytest.capture', + ] + assert [x.function.__module__ for x in l] == expected + class TestWarning: def test_warn_config(self, testdir): From e0cb0468856eefb88bf16afdd233d9ac3ca1884d Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Mon, 20 Jun 2016 18:36:40 +0200 Subject: [PATCH 2/5] Update AUTHORS and CHANGELOG --- AUTHORS | 1 + CHANGELOG.rst | 13 +++++++++++++ 2 files changed, 14 insertions(+) diff --git a/AUTHORS b/AUTHORS index 0c40517cea8..b182048aa48 100644 --- a/AUTHORS +++ b/AUTHORS @@ -84,6 +84,7 @@ Ronny Pfannschmidt Ross Lawley Ryan Wooden Samuele Pedroni +Stephan Obermann Tareq Alayan Tom Viner Trevor Bekolay diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 26f78cec18e..60444e74bee 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -74,8 +74,19 @@ message to raise when no exception occurred. Thanks `@palaviv`_ for the complete PR (`#1616`_). +* ``conftest.py`` files now benefit from assertion rewriting; previously it + was only available for test modules. Thanks `@flub`_, `@sober7`_ and + `@nicoddemus`_ for the PR (`#1619`_). + +* + +* + +* + .. _@milliams: https://github.com/milliams .. _@csaftoiu: https://github.com/csaftoiu +.. _@flub: https://github.com/flub .. _@novas0x2a: https://github.com/novas0x2a .. _@kalekundert: https://github.com/kalekundert .. _@tareqalayan: https://github.com/tareqalayan @@ -83,6 +94,7 @@ .. _@palaviv: https://github.com/palaviv .. _@omarkohl: https://github.com/omarkohl .. _@mikofski: https://github.com/mikofski +.. _@sober7: https://github.com/sober7 .. _#1426: https://github.com/pytest-dev/pytest/issues/1426 .. _#1428: https://github.com/pytest-dev/pytest/pull/1428 @@ -95,6 +107,7 @@ .. _#1474: https://github.com/pytest-dev/pytest/pull/1474 .. _#1502: https://github.com/pytest-dev/pytest/pull/1502 .. _#1520: https://github.com/pytest-dev/pytest/pull/1520 +.. _#1619: https://github.com/pytest-dev/pytest/issues/1619 .. _#372: https://github.com/pytest-dev/pytest/issues/372 .. _#1544: https://github.com/pytest-dev/pytest/issues/1544 .. _#1616: https://github.com/pytest-dev/pytest/pull/1616 From 9118c0222f4d0bb4b20c579f9a20912f97f57a87 Mon Sep 17 00:00:00 2001 From: Floris Bruynooghe Date: Tue, 21 Jun 2016 09:28:10 +0200 Subject: [PATCH 3/5] Merge .set_config() into constructor --- _pytest/assertion/__init__.py | 3 +-- _pytest/assertion/rewrite.py | 8 +++----- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/_pytest/assertion/__init__.py b/_pytest/assertion/__init__.py index 3da99c63c8b..09f82c776c0 100644 --- a/_pytest/assertion/__init__.py +++ b/_pytest/assertion/__init__.py @@ -76,8 +76,7 @@ def pytest_load_initial_conftests(early_config, parser, args): hook = None if mode == "rewrite": - hook = rewrite.AssertionRewritingHook() # noqa - hook.set_config(early_config) + hook = rewrite.AssertionRewritingHook(early_config) # noqa sys.meta_path.insert(0, hook) early_config._assertstate.hook = hook diff --git a/_pytest/assertion/rewrite.py b/_pytest/assertion/rewrite.py index efddc89203e..b85ca686cf9 100644 --- a/_pytest/assertion/rewrite.py +++ b/_pytest/assertion/rewrite.py @@ -44,7 +44,9 @@ class AssertionRewritingHook(object): """PEP302 Import hook which rewrites asserts.""" - def __init__(self): + def __init__(self, config): + self.config = config + self.fnpats = config.getini("python_files") self.session = None self.modules = {} self._register_with_pkg_resources() @@ -52,10 +54,6 @@ def __init__(self): def set_session(self, session): self.session = session - def set_config(self, config): - self.config = config - self.fnpats = config.getini("python_files") - def find_module(self, name, path=None): state = self.config._assertstate state.trace("find_module called for: %s" % name) From 8b0fb47c799b79ba905d2ea5573b7cf6508e8ff2 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Tue, 21 Jun 2016 12:23:02 +0200 Subject: [PATCH 4/5] Remove print left by accident --- _pytest/assertion/__init__.py | 1 - 1 file changed, 1 deletion(-) diff --git a/_pytest/assertion/__init__.py b/_pytest/assertion/__init__.py index 09f82c776c0..099d5f0b4f2 100644 --- a/_pytest/assertion/__init__.py +++ b/_pytest/assertion/__init__.py @@ -165,7 +165,6 @@ def _load_modules(mode): def warn_about_missing_assertion(mode, pluginmanager): - print('got here') try: assert False except AssertionError: From 819942e96451ebc9664a6a82ef02e7f1745b8f1a Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Tue, 21 Jun 2016 12:28:36 +0200 Subject: [PATCH 5/5] Return explicit None from rewrite hook's find_module --- _pytest/assertion/rewrite.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/_pytest/assertion/rewrite.py b/_pytest/assertion/rewrite.py index b85ca686cf9..fd4f66cd0d5 100644 --- a/_pytest/assertion/rewrite.py +++ b/_pytest/assertion/rewrite.py @@ -87,7 +87,7 @@ def find_module(self, name, path=None): fn_pypath = py.path.local(fn) if not self._should_rewrite(fn_pypath, state): - return + return None # The requested module looks like a test file, so rewrite it. This is # the most magical part of the process: load the source, rewrite the