From 5eaebc1900e2c5c0fb911ff9c325efa80441d030 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Sat, 16 May 2020 15:46:06 -0300 Subject: [PATCH 1/3] Remove one of the tracebacks from conftest import failures This removes the KeyError from the traceback chain when an conftest fails to import: return self._conftestpath2mod[key] E KeyError: WindowsPath('D:/projects/pytest/.tmp/root/foo/conftest.py') During handling of the above exception, another exception occurred: ... raise RuntimeError("some error") E RuntimeError: some error During handling of the above exception, another exception occurred: ... E _pytest.config.ConftestImportFailure: (...) By slightly changing the code, we can remove the first chain, which is often very confusing to users and doesn't help with anything. Fix #7223 --- src/_pytest/config/__init__.py | 57 ++++++++++++++++++---------------- 1 file changed, 30 insertions(+), 27 deletions(-) diff --git a/src/_pytest/config/__init__.py b/src/_pytest/config/__init__.py index 8a2c16d5d..d45feb624 100644 --- a/src/_pytest/config/__init__.py +++ b/src/_pytest/config/__init__.py @@ -1,5 +1,6 @@ """ command line options, ini-file and conftest.py processing. """ import argparse +import contextlib import copy import enum import inspect @@ -511,34 +512,36 @@ class PytestPluginManager(PluginManager): # Using Path().resolve() is better than py.path.realpath because # it resolves to the correct path/drive in case-insensitive file systems (#5792) key = Path(str(conftestpath)).resolve() - try: - return self._conftestpath2mod[key] - except KeyError: - pkgpath = conftestpath.pypkgpath() - if pkgpath is None: - _ensure_removed_sysmodule(conftestpath.purebasename) - try: - mod = conftestpath.pyimport() - if ( - hasattr(mod, "pytest_plugins") - and self._configured - and not self._using_pyargs - ): - _fail_on_non_top_pytest_plugins(conftestpath, self._confcutdir) - except Exception: - raise ConftestImportFailure(conftestpath, sys.exc_info()) - self._conftest_plugins.add(mod) - self._conftestpath2mod[key] = mod - dirpath = conftestpath.dirpath() - if dirpath in self._dirpath2confmods: - for path, mods in self._dirpath2confmods.items(): - if path and path.relto(dirpath) or path == dirpath: - assert mod not in mods - mods.append(mod) - self.trace("loading conftestmodule {!r}".format(mod)) - self.consider_conftest(mod) - return mod + with contextlib.suppress(KeyError): + return self._conftestpath2mod[key] + + pkgpath = conftestpath.pypkgpath() + if pkgpath is None: + _ensure_removed_sysmodule(conftestpath.purebasename) + + try: + mod = conftestpath.pyimport() + if ( + hasattr(mod, "pytest_plugins") + and self._configured + and not self._using_pyargs + ): + _fail_on_non_top_pytest_plugins(conftestpath, self._confcutdir) + except Exception as e: + raise ConftestImportFailure(conftestpath, sys.exc_info()) from e + + self._conftest_plugins.add(mod) + self._conftestpath2mod[key] = mod + dirpath = conftestpath.dirpath() + if dirpath in self._dirpath2confmods: + for path, mods in self._dirpath2confmods.items(): + if path and path.relto(dirpath) or path == dirpath: + assert mod not in mods + mods.append(mod) + self.trace("loading conftestmodule {!r}".format(mod)) + self.consider_conftest(mod) + return mod # # API for bootstrapping plugin loading From 9e1e7fcabe4ebcdbcdadbd1f3eb8866f3c1821a7 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Sat, 16 May 2020 16:05:12 -0300 Subject: [PATCH 2/3] Use a nice string repr for ConftestImportFailure The default message is often hard to read: E _pytest.config.ConftestImportFailure: (local('D:\\projects\\pytest\\.tmp\\root\\foo\\conftest.py'), (, RuntimeError('some error',), )) Using a shorter message is better: E _pytest.config.ConftestImportFailure: RuntimeError: some error (from D:\projects\pytest\.tmp\root\foo\conftest.py) And we don't really lose any information due to exception chaining. --- src/_pytest/config/__init__.py | 7 ++++++- testing/test_config.py | 17 +++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/src/_pytest/config/__init__.py b/src/_pytest/config/__init__.py index d45feb624..a4fc6e7c1 100644 --- a/src/_pytest/config/__init__.py +++ b/src/_pytest/config/__init__.py @@ -88,10 +88,15 @@ class ExitCode(enum.IntEnum): class ConftestImportFailure(Exception): def __init__(self, path, excinfo): - Exception.__init__(self, path, excinfo) + super().__init__(path, excinfo) self.path = path self.excinfo = excinfo # type: Tuple[Type[Exception], Exception, TracebackType] + def __str__(self): + return "{}: {} (from {})".format( + self.excinfo[0].__name__, self.excinfo[1], self.path + ) + def main(args=None, plugins=None) -> Union[int, ExitCode]: """ return exit code, after performing an in-process test run. diff --git a/testing/test_config.py b/testing/test_config.py index f6bf0499f..7d553e63b 100644 --- a/testing/test_config.py +++ b/testing/test_config.py @@ -10,6 +10,7 @@ import pytest from _pytest.compat import importlib_metadata from _pytest.config import _iter_rewritable_modules from _pytest.config import Config +from _pytest.config import ConftestImportFailure from _pytest.config import ExitCode from _pytest.config.exceptions import UsageError from _pytest.config.findpaths import determine_setup @@ -1471,3 +1472,19 @@ class TestPytestPluginsVariable: assert res.ret == 0 msg = "Defining 'pytest_plugins' in a non-top-level conftest is no longer supported" assert msg not in res.stdout.str() + + +def test_conftest_import_error_repr(tmpdir): + """ + ConftestImportFailure should use a short error message and readable path to the failed + conftest.py file + """ + path = tmpdir.join("foo/conftest.py") + with pytest.raises( + ConftestImportFailure, + match=re.escape("RuntimeError: some error (from {})".format(path)), + ): + try: + raise RuntimeError("some error") + except Exception: + raise ConftestImportFailure(path, sys.exc_info()) From c26f389c09479bc34a0ddb8a531089fa58f9bd80 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Sun, 17 May 2020 11:20:56 -0300 Subject: [PATCH 3/3] Refactor handling of non-top-level pytest_plugins handling Decided to move the 'if' logic together with the error message, as this leaves the _importconftest method cleaner. --- src/_pytest/config/__init__.py | 38 +++++++++++++++++----------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/src/_pytest/config/__init__.py b/src/_pytest/config/__init__.py index a4fc6e7c1..68c3822d0 100644 --- a/src/_pytest/config/__init__.py +++ b/src/_pytest/config/__init__.py @@ -286,19 +286,6 @@ def _prepareconfig( raise -def _fail_on_non_top_pytest_plugins(conftestpath, confcutdir): - msg = ( - "Defining 'pytest_plugins' in a non-top-level conftest is no longer supported:\n" - "It affects the entire test suite instead of just below the conftest as expected.\n" - " {}\n" - "Please move it to a top level conftest file at the rootdir:\n" - " {}\n" - "For more information, visit:\n" - " https://docs.pytest.org/en/latest/deprecations.html#pytest-plugins-in-non-top-level-conftest-files" - ) - fail(msg.format(conftestpath, confcutdir), pytrace=False) - - class PytestPluginManager(PluginManager): """ Overwrites :py:class:`pluggy.PluginManager ` to add pytest-specific @@ -527,15 +514,11 @@ class PytestPluginManager(PluginManager): try: mod = conftestpath.pyimport() - if ( - hasattr(mod, "pytest_plugins") - and self._configured - and not self._using_pyargs - ): - _fail_on_non_top_pytest_plugins(conftestpath, self._confcutdir) except Exception as e: raise ConftestImportFailure(conftestpath, sys.exc_info()) from e + self._check_non_top_pytest_plugins(mod, conftestpath) + self._conftest_plugins.add(mod) self._conftestpath2mod[key] = mod dirpath = conftestpath.dirpath() @@ -548,6 +531,23 @@ class PytestPluginManager(PluginManager): self.consider_conftest(mod) return mod + def _check_non_top_pytest_plugins(self, mod, conftestpath): + if ( + hasattr(mod, "pytest_plugins") + and self._configured + and not self._using_pyargs + ): + msg = ( + "Defining 'pytest_plugins' in a non-top-level conftest is no longer supported:\n" + "It affects the entire test suite instead of just below the conftest as expected.\n" + " {}\n" + "Please move it to a top level conftest file at the rootdir:\n" + " {}\n" + "For more information, visit:\n" + " https://docs.pytest.org/en/latest/deprecations.html#pytest-plugins-in-non-top-level-conftest-files" + ) + fail(msg.format(conftestpath, self._confcutdir), pytrace=False) + # # API for bootstrapping plugin loading #