From 36dc6718435c423f768df8334e93d7f06d752784 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Fri, 12 Oct 2018 09:37:54 -0300 Subject: [PATCH 1/4] New ExceptionInfo.getrepr 'chain' parameter to be able to suppress chained exceptions --- src/_pytest/_code/code.py | 42 ++++++++++++++++++++++++++++++------ testing/code/test_excinfo.py | 20 ++++++++++++----- 2 files changed, 50 insertions(+), 12 deletions(-) diff --git a/src/_pytest/_code/code.py b/src/_pytest/_code/code.py index 2662e432016..b2321c02d4b 100644 --- a/src/_pytest/_code/code.py +++ b/src/_pytest/_code/code.py @@ -451,13 +451,35 @@ def getrepr( tbfilter=True, funcargs=False, truncate_locals=True, + chain=True, ): - """ return str()able representation of this exception info. - showlocals: show locals per traceback entry - style: long|short|no|native traceback style - tbfilter: hide entries (where __tracebackhide__ is true) + """ + Return str()able representation of this exception info. + + :param bool showlocals: + Show locals per traceback entry. + Ignored if ``style=="native"``. + + :param str style: long|short|no|native traceback style + + :param bool abspath: + If paths should be changed to absolute or left unchanged. + + :param bool tbfilter: + Hide entries that contain a local variable ``__tracebackhide__==True``. + Ignored if ``style=="native"``. - in case of style==native, tbfilter and showlocals is ignored. + :param bool funcargs: + Show fixtures ("funcargs" for legacy purposes) per traceback entry. + + :param bool truncate_locals: + With ``showlocals==True``, make sure locals can be safely represented as strings. + + :param bool chain: if chained exceptions in Python 3 should be shown. + + .. versionchanged:: 3.9 + + Added the ``chain`` parameter. """ if style == "native": return ReprExceptionInfo( @@ -476,6 +498,7 @@ def getrepr( tbfilter=tbfilter, funcargs=funcargs, truncate_locals=truncate_locals, + chain=chain, ) return fmt.repr_excinfo(self) @@ -516,6 +539,7 @@ class FormattedExcinfo(object): tbfilter = attr.ib(default=True) funcargs = attr.ib(default=False) truncate_locals = attr.ib(default=True) + chain = attr.ib(default=True) astcache = attr.ib(default=attr.Factory(dict), init=False, repr=False) def _getindent(self, source): @@ -735,7 +759,7 @@ def repr_excinfo(self, excinfo): reprcrash = None repr_chain += [(reprtraceback, reprcrash, descr)] - if e.__cause__ is not None: + if e.__cause__ is not None and self.chain: e = e.__cause__ excinfo = ( ExceptionInfo((type(e), e, e.__traceback__)) @@ -743,7 +767,11 @@ def repr_excinfo(self, excinfo): else None ) descr = "The above exception was the direct cause of the following exception:" - elif e.__context__ is not None and not e.__suppress_context__: + elif ( + e.__context__ is not None + and not e.__suppress_context__ + and self.chain + ): e = e.__context__ excinfo = ( ExceptionInfo((type(e), e, e.__traceback__)) diff --git a/testing/code/test_excinfo.py b/testing/code/test_excinfo.py index 72b1a78abc6..92460cd2912 100644 --- a/testing/code/test_excinfo.py +++ b/testing/code/test_excinfo.py @@ -1184,20 +1184,28 @@ def h(): assert tw.lines[47] == ":15: AttributeError" @pytest.mark.skipif("sys.version_info[0] < 3") - def test_exc_repr_with_raise_from_none_chain_suppression(self, importasmod): + @pytest.mark.parametrize("mode", ["from_none", "explicit_suppress"]) + def test_exc_repr_chain_suppression(self, importasmod, mode): + """Check that exc repr does not show chained exceptions in Python 3. + - When the exception is raised with "from None" + - Explicitly suppressed with "chain=False" to ExceptionInfo.getrepr(). + """ + raise_suffix = " from None" if mode == "from_none" else "" mod = importasmod( """ def f(): try: g() except Exception: - raise AttributeError() from None + raise AttributeError(){raise_suffix} def g(): raise ValueError() - """ + """.format( + raise_suffix=raise_suffix + ) ) excinfo = pytest.raises(AttributeError, mod.f) - r = excinfo.getrepr(style="long") + r = excinfo.getrepr(style="long", chain=mode != "explicit_suppress") tw = TWMock() r.toterminal(tw) for line in tw.lines: @@ -1207,7 +1215,9 @@ def g(): assert tw.lines[2] == " try:" assert tw.lines[3] == " g()" assert tw.lines[4] == " except Exception:" - assert tw.lines[5] == "> raise AttributeError() from None" + assert tw.lines[5] == "> raise AttributeError(){}".format( + raise_suffix + ) assert tw.lines[6] == "E AttributeError" assert tw.lines[7] == "" line = tw.get_write_msg(8) From 8e11fe530487e560ac52f49ec50bda1f49718554 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Fri, 12 Oct 2018 10:01:30 -0300 Subject: [PATCH 2/4] Improve tracebacks for ImportErrors in conftest.py files Fix #3332 --- changelog/3332.feature.rst | 4 ++++ src/_pytest/config/__init__.py | 18 ++++++++++++++++-- testing/acceptance_test.py | 30 +++++++++++++++++++++++++----- 3 files changed, 45 insertions(+), 7 deletions(-) create mode 100644 changelog/3332.feature.rst diff --git a/changelog/3332.feature.rst b/changelog/3332.feature.rst new file mode 100644 index 00000000000..e0110c451dc --- /dev/null +++ b/changelog/3332.feature.rst @@ -0,0 +1,4 @@ +Improve the error displayed when a ``conftest.py`` file could not be imported. + +In order to implement this, a new ``chain`` parameter was added to ``ExceptionInfo.getrepr`` +to show or hide chained tracebacks in Python 3 (defaults to ``True``). diff --git a/src/_pytest/config/__init__.py b/src/_pytest/config/__init__.py index efc5e123550..363412abb67 100644 --- a/src/_pytest/config/__init__.py +++ b/src/_pytest/config/__init__.py @@ -57,10 +57,24 @@ def main(args=None, plugins=None): try: config = _prepareconfig(args, plugins) except ConftestImportFailure as e: + from _pytest._code import ExceptionInfo + + exc_info = ExceptionInfo(e.excinfo) tw = py.io.TerminalWriter(sys.stderr) - for line in traceback.format_exception(*e.excinfo): + tw.line( + "ImportError while loading conftest '{e.path}'.".format(e=e), red=True + ) + from _pytest.python import filter_traceback + + exc_info.traceback = exc_info.traceback.filter(filter_traceback) + exc_repr = ( + exc_info.getrepr(style="short", chain=False) + if exc_info.traceback + else exc_info.exconly() + ) + formatted_tb = safe_str(exc_repr) + for line in formatted_tb.splitlines(): tw.line(line.rstrip(), red=True) - tw.line("ERROR: could not load %s\n" % (e.path,), red=True) return 4 else: try: diff --git a/testing/acceptance_test.py b/testing/acceptance_test.py index 332af27b5f2..1728f1ff487 100644 --- a/testing/acceptance_test.py +++ b/testing/acceptance_test.py @@ -133,9 +133,16 @@ def test_not_collectable_arguments(self, testdir): assert result.ret result.stderr.fnmatch_lines(["*ERROR: not found:*{}".format(p2.basename)]) - def test_issue486_better_reporting_on_conftest_load_failure(self, testdir): + def test_better_reporting_on_conftest_load_failure(self, testdir, request): + """Show a user-friendly traceback on conftest import failures (#486, #3332)""" testdir.makepyfile("") - testdir.makeconftest("import qwerty") + testdir.makeconftest( + """ + def foo(): + import qwerty + foo() + """ + ) result = testdir.runpytest("--help") result.stdout.fnmatch_lines( """ @@ -144,10 +151,23 @@ def test_issue486_better_reporting_on_conftest_load_failure(self, testdir): """ ) result = testdir.runpytest() + dirname = request.node.name + "0" + exc_name = ( + "ModuleNotFoundError" if sys.version_info >= (3, 6) else "ImportError" + ) result.stderr.fnmatch_lines( - """ - *ERROR*could not load*conftest.py* - """ + [ + "ImportError while loading conftest '*{sep}{dirname}{sep}conftest.py'.".format( + dirname=dirname, sep=os.sep + ), + "conftest.py:3: in ", + " foo()", + "conftest.py:2: in foo", + " import qwerty", + "E {}: No module named {q}qwerty{q}".format( + exc_name, q="'" if six.PY3 else "" + ), + ] ) def test_early_skip(self, testdir): From 2cb35346792af2eb22c719be07dee409c9ef5ba8 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Fri, 12 Oct 2018 10:19:50 -0300 Subject: [PATCH 3/4] Move filter_traceback to _pytest._code --- src/_pytest/_code/__init__.py | 1 + src/_pytest/_code/code.py | 35 ++++++++++++++++++++++++++++++++++ src/_pytest/config/__init__.py | 5 +---- src/_pytest/python.py | 33 +------------------------------- 4 files changed, 38 insertions(+), 36 deletions(-) diff --git a/src/_pytest/_code/__init__.py b/src/_pytest/_code/__init__.py index 815c13b42c2..7885d51dead 100644 --- a/src/_pytest/_code/__init__.py +++ b/src/_pytest/_code/__init__.py @@ -4,6 +4,7 @@ from .code import ExceptionInfo # noqa from .code import Frame # noqa from .code import Traceback # noqa +from .code import filter_traceback # noqa from .code import getrawcode # noqa from .source import Source # noqa from .source import compile_ as compile # noqa diff --git a/src/_pytest/_code/code.py b/src/_pytest/_code/code.py index b2321c02d4b..c0f6d21a2f7 100644 --- a/src/_pytest/_code/code.py +++ b/src/_pytest/_code/code.py @@ -6,8 +6,10 @@ from inspect import CO_VARARGS, CO_VARKEYWORDS import attr +import pluggy import re from weakref import ref +import _pytest from _pytest.compat import _PY2, _PY3, PY35, safe_str from six import text_type import py @@ -1007,3 +1009,36 @@ def is_recursion_error(excinfo): return "maximum recursion depth exceeded" in str(excinfo.value) except UnicodeError: return False + + +# relative paths that we use to filter traceback entries from appearing to the user; +# see filter_traceback +# note: if we need to add more paths than what we have now we should probably use a list +# for better maintenance + +_PLUGGY_DIR = py.path.local(pluggy.__file__.rstrip("oc")) +# pluggy is either a package or a single module depending on the version +if _PLUGGY_DIR.basename == "__init__.py": + _PLUGGY_DIR = _PLUGGY_DIR.dirpath() +_PYTEST_DIR = py.path.local(_pytest.__file__).dirpath() +_PY_DIR = py.path.local(py.__file__).dirpath() + + +def filter_traceback(entry): + """Return True if a TracebackEntry instance should be removed from tracebacks: + * dynamically generated code (no code to show up for it); + * internal traceback from pytest or its internal libraries, py and pluggy. + """ + # entry.path might sometimes return a str object when the entry + # points to dynamically generated code + # see https://bitbucket.org/pytest-dev/py/issues/71 + raw_filename = entry.frame.code.raw.co_filename + is_generated = "<" in raw_filename and ">" in raw_filename + if is_generated: + return False + # entry.path might point to a non-existing file, in which case it will + # also return a str object. see #1133 + p = py.path.local(entry.path) + return ( + not p.relto(_PLUGGY_DIR) and not p.relto(_PYTEST_DIR) and not p.relto(_PY_DIR) + ) diff --git a/src/_pytest/config/__init__.py b/src/_pytest/config/__init__.py index 363412abb67..3a3e064a904 100644 --- a/src/_pytest/config/__init__.py +++ b/src/_pytest/config/__init__.py @@ -19,6 +19,7 @@ import _pytest.hookspec # the extension point definitions import _pytest.assertion from pluggy import PluginManager, HookimplMarker, HookspecMarker +from _pytest._code import ExceptionInfo, filter_traceback from _pytest.compat import safe_str from .exceptions import UsageError, PrintHelp from .findpaths import determine_setup, exists @@ -57,15 +58,11 @@ def main(args=None, plugins=None): try: config = _prepareconfig(args, plugins) except ConftestImportFailure as e: - from _pytest._code import ExceptionInfo - exc_info = ExceptionInfo(e.excinfo) tw = py.io.TerminalWriter(sys.stderr) tw.line( "ImportError while loading conftest '{e.path}'.".format(e=e), red=True ) - from _pytest.python import filter_traceback - exc_info.traceback = exc_info.traceback.filter(filter_traceback) exc_repr = ( exc_info.getrepr(style="short", chain=False) diff --git a/src/_pytest/python.py b/src/_pytest/python.py index e9d05666f23..8337c7469fa 100644 --- a/src/_pytest/python.py +++ b/src/_pytest/python.py @@ -16,7 +16,7 @@ from _pytest.config import hookimpl import _pytest -import pluggy +from _pytest._code import filter_traceback from _pytest import fixtures from _pytest import nodes from _pytest import deprecated @@ -46,37 +46,6 @@ ) from _pytest.warning_types import RemovedInPytest4Warning, PytestWarning -# relative paths that we use to filter traceback entries from appearing to the user; -# see filter_traceback -# note: if we need to add more paths than what we have now we should probably use a list -# for better maintenance -_pluggy_dir = py.path.local(pluggy.__file__.rstrip("oc")) -# pluggy is either a package or a single module depending on the version -if _pluggy_dir.basename == "__init__.py": - _pluggy_dir = _pluggy_dir.dirpath() -_pytest_dir = py.path.local(_pytest.__file__).dirpath() -_py_dir = py.path.local(py.__file__).dirpath() - - -def filter_traceback(entry): - """Return True if a TracebackEntry instance should be removed from tracebacks: - * dynamically generated code (no code to show up for it); - * internal traceback from pytest or its internal libraries, py and pluggy. - """ - # entry.path might sometimes return a str object when the entry - # points to dynamically generated code - # see https://bitbucket.org/pytest-dev/py/issues/71 - raw_filename = entry.frame.code.raw.co_filename - is_generated = "<" in raw_filename and ">" in raw_filename - if is_generated: - return False - # entry.path might point to a non-existing file, in which case it will - # also return a str object. see #1133 - p = py.path.local(entry.path) - return ( - not p.relto(_pluggy_dir) and not p.relto(_pytest_dir) and not p.relto(_py_dir) - ) - def pyobj_property(name): def get(self): From ef97121d4282c516e8e355a3c75cd42169848c19 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Fri, 12 Oct 2018 10:57:13 -0300 Subject: [PATCH 4/4] Removed unused ConftestImportFailure.__str__ method --- src/_pytest/config/__init__.py | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/src/_pytest/config/__init__.py b/src/_pytest/config/__init__.py index 3a3e064a904..88cbf14bab0 100644 --- a/src/_pytest/config/__init__.py +++ b/src/_pytest/config/__init__.py @@ -3,7 +3,6 @@ import argparse import inspect import shlex -import traceback import types import warnings import copy @@ -27,9 +26,6 @@ hookimpl = HookimplMarker("pytest") hookspec = HookspecMarker("pytest") -# pytest startup -# - class ConftestImportFailure(Exception): def __init__(self, path, excinfo): @@ -37,12 +33,6 @@ def __init__(self, path, excinfo): self.path = path self.excinfo = excinfo - def __str__(self): - etype, evalue, etb = self.excinfo - formatted = traceback.format_tb(etb) - # The level of the tracebacks we want to print is hand crafted :( - return repr(evalue) + "\n" + "".join(formatted[2:]) - def main(args=None, plugins=None): """ return exit code, after performing an in-process test run.