From ab6dacf1d1e1ff0c5be70a3c5f48e63168168721 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Sat, 13 Jun 2020 11:29:01 -0300 Subject: [PATCH] Introduce --import-mode=importlib (#7246) Fix #5821 Co-authored-by: Ran Benita --- changelog/7245.feature.rst | 14 ++ doc/en/goodpractices.rst | 23 +++- doc/en/pythonpath.rst | 64 ++++++++- src/_pytest/_code/code.py | 5 +- src/_pytest/compat.py | 36 +++++ src/_pytest/config/__init__.py | 40 ++++-- src/_pytest/doctest.py | 7 +- src/_pytest/main.py | 8 ++ src/_pytest/nodes.py | 4 +- src/_pytest/pathlib.py | 138 +++++++++++++++++++ src/_pytest/python.py | 15 +- testing/acceptance_test.py | 5 +- testing/python/collect.py | 10 +- testing/python/fixtures.py | 4 +- testing/test_collection.py | 80 +++++++++++ testing/test_compat.py | 59 ++++++++ testing/test_conftest.py | 42 +++--- testing/test_pathlib.py | 242 ++++++++++++++++++++++++++++++++- testing/test_pluginmanager.py | 8 +- 19 files changed, 734 insertions(+), 70 deletions(-) create mode 100644 changelog/7245.feature.rst diff --git a/changelog/7245.feature.rst b/changelog/7245.feature.rst new file mode 100644 index 000000000..05c3a6c04 --- /dev/null +++ b/changelog/7245.feature.rst @@ -0,0 +1,14 @@ +New ``--import-mode=importlib`` option that uses `importlib `__ to import test modules. + +Traditionally pytest used ``__import__`` while changing ``sys.path`` to import test modules (which +also changes ``sys.modules`` as a side-effect), which works but has a number of drawbacks, like requiring test modules +that don't live in packages to have unique names (as they need to reside under a unique name in ``sys.modules``). + +``--import-mode=importlib`` uses more fine grained import mechanisms from ``importlib`` which don't +require pytest to change ``sys.path`` or ``sys.modules`` at all, eliminating much of the drawbacks +of the previous mode. + +We intend to make ``--import-mode=importlib`` the default in future versions, so users are encouraged +to try the new mode and provide feedback (both positive or negative) in issue `#7245 `__. + +You can read more about this option in `the documentation `__. diff --git a/doc/en/goodpractices.rst b/doc/en/goodpractices.rst index 16b41eda4..ee5674fd6 100644 --- a/doc/en/goodpractices.rst +++ b/doc/en/goodpractices.rst @@ -91,7 +91,8 @@ This has the following benefits: See :ref:`pytest vs python -m pytest` for more information about the difference between calling ``pytest`` and ``python -m pytest``. -Note that using this scheme your test files must have **unique names**, because +Note that this scheme has a drawback if you are using ``prepend`` :ref:`import mode ` +(which is the default): your test files must have **unique names**, because ``pytest`` will import them as *top-level* modules since there are no packages to derive a full package name from. In other words, the test files in the example above will be imported as ``test_app`` and ``test_view`` top-level modules by adding ``tests/`` to @@ -118,9 +119,12 @@ Now pytest will load the modules as ``tests.foo.test_view`` and ``tests.bar.test you to have modules with the same name. But now this introduces a subtle problem: in order to load the test modules from the ``tests`` directory, pytest prepends the root of the repository to ``sys.path``, which adds the side-effect that now ``mypkg`` is also importable. + This is problematic if you are using a tool like `tox`_ to test your package in a virtual environment, because you want to test the *installed* version of your package, not the local code from the repository. +.. _`src-layout`: + In this situation, it is **strongly** suggested to use a ``src`` layout where application root package resides in a sub-directory of your root: @@ -145,6 +149,15 @@ sub-directory of your root: This layout prevents a lot of common pitfalls and has many benefits, which are better explained in this excellent `blog post by Ionel Cristian Mărieș `_. +.. note:: + The new ``--import-mode=importlib`` (see :ref:`import-modes`) doesn't have + any of the drawbacks above because ``sys.path`` and ``sys.modules`` are not changed when importing + test modules, so users that run + into this issue are strongly encouraged to try it and report if the new option works well for them. + + The ``src`` directory layout is still strongly recommended however. + + Tests as part of application code ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -190,8 +203,8 @@ Note that this layout also works in conjunction with the ``src`` layout mentione .. note:: - If ``pytest`` finds an "a/b/test_module.py" test file while - recursing into the filesystem it determines the import name + In ``prepend`` and ``append`` import-modes, if pytest finds a ``"a/b/test_module.py"`` + test file while recursing into the filesystem it determines the import name as follows: * determine ``basedir``: this is the first "upward" (towards the root) @@ -212,6 +225,10 @@ Note that this layout also works in conjunction with the ``src`` layout mentione from each other and thus deriving a canonical import name helps to avoid surprises such as a test module getting imported twice. + With ``--import-mode=importlib`` things are less convoluted because + pytest doesn't need to change ``sys.path`` or ``sys.modules``, making things + much less surprising. + .. _`virtualenv`: https://pypi.org/project/virtualenv/ .. _`buildout`: http://www.buildout.org/ diff --git a/doc/en/pythonpath.rst b/doc/en/pythonpath.rst index f2c86fab9..b8f4de9d9 100644 --- a/doc/en/pythonpath.rst +++ b/doc/en/pythonpath.rst @@ -3,11 +3,65 @@ pytest import mechanisms and ``sys.path``/``PYTHONPATH`` ======================================================== -Here's a list of scenarios where pytest may need to change ``sys.path`` in order -to import test modules or ``conftest.py`` files. +.. _`import-modes`: + +Import modes +------------ + +pytest as a testing framework needs to import test modules and ``conftest.py`` files for execution. + +Importing files in Python (at least until recently) is a non-trivial processes, often requiring +changing `sys.path `__. Some aspects of the +import process can be controlled through the ``--import-mode`` command-line flag, which can assume +these values: + +* ``prepend`` (default): the directory path containing each module will be inserted into the *beginning* + of ``sys.path`` if not already there, and then imported with the `__import__ `__ builtin. + + This requires test module names to be unique when the test directory tree is not arranged in + packages, because the modules will put in ``sys.modules`` after importing. + + This is the classic mechanism, dating back from the time Python 2 was still supported. + +* ``append``: the directory containing each module is appended to the end of ``sys.path`` if not already + there, and imported with ``__import__``. + + This better allows to run test modules against installed versions of a package even if the + package under test has the same import root. For example: + + :: + + testing/__init__.py + testing/test_pkg_under_test.py + pkg_under_test/ + + the tests will run against the installed version + of ``pkg_under_test`` when ``--import-mode=append`` is used whereas + with ``prepend`` they would pick up the local version. This kind of confusion is why + we advocate for using :ref:`src ` layouts. + + Same as ``prepend``, requires test module names to be unique when the test directory tree is + not arranged in packages, because the modules will put in ``sys.modules`` after importing. + +* ``importlib``: new in pytest-6.0, this mode uses `importlib `__ to import test modules. This gives full control over the import process, and doesn't require + changing ``sys.path`` or ``sys.modules`` at all. + + For this reason this doesn't require test module names to be unique at all, but also makes test + modules non-importable by each other. This was made possible in previous modes, for tests not residing + in Python packages, because of the side-effects of changing ``sys.path`` and ``sys.modules`` + mentioned above. Users which require this should turn their tests into proper packages instead. + + We intend to make ``importlib`` the default in future releases. + +``prepend`` and ``append`` import modes scenarios +------------------------------------------------- + +Here's a list of scenarios when using ``prepend`` or ``append`` import modes where pytest needs to +change ``sys.path`` in order to import test modules or ``conftest.py`` files, and the issues users +might encounter because of that. Test modules / ``conftest.py`` files inside packages ----------------------------------------------------- +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Consider this file and directory layout:: @@ -28,8 +82,6 @@ When executing: pytest root/ - - pytest will find ``foo/bar/tests/test_foo.py`` and realize it is part of a package given that there's an ``__init__.py`` file in the same folder. It will then search upwards until it can find the last folder which still contains an ``__init__.py`` file in order to find the package *root* (in @@ -44,7 +96,7 @@ and allow test modules to have duplicated names. This is also discussed in detai :ref:`test discovery`. Standalone test modules / ``conftest.py`` files ------------------------------------------------ +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Consider this file and directory layout:: diff --git a/src/_pytest/_code/code.py b/src/_pytest/_code/code.py index 121ef3a9b..65e5aa6d5 100644 --- a/src/_pytest/_code/code.py +++ b/src/_pytest/_code/code.py @@ -1204,7 +1204,10 @@ _PY_DIR = py.path.local(py.__file__).dirpath() def filter_traceback(entry: TracebackEntry) -> bool: - """Return True if a TracebackEntry instance should be removed from tracebacks: + """Return True if a TracebackEntry instance should be included in tracebacks. + + We hide traceback entries of: + * dynamically generated code (no code to show up for it); * internal traceback from pytest or its internal libraries, py and pluggy. """ diff --git a/src/_pytest/compat.py b/src/_pytest/compat.py index 84f9609a7..cd7dca719 100644 --- a/src/_pytest/compat.py +++ b/src/_pytest/compat.py @@ -33,6 +33,7 @@ else: if TYPE_CHECKING: + from typing import NoReturn from typing import Type from typing_extensions import Final @@ -401,3 +402,38 @@ else: from collections import OrderedDict order_preserving_dict = OrderedDict + + +# Perform exhaustiveness checking. +# +# Consider this example: +# +# MyUnion = Union[int, str] +# +# def handle(x: MyUnion) -> int { +# if isinstance(x, int): +# return 1 +# elif isinstance(x, str): +# return 2 +# else: +# raise Exception('unreachable') +# +# Now suppose we add a new variant: +# +# MyUnion = Union[int, str, bytes] +# +# After doing this, we must remember ourselves to go and update the handle +# function to handle the new variant. +# +# With `assert_never` we can do better: +# +# // throw new Error('unreachable'); +# return assert_never(x) +# +# Now, if we forget to handle the new variant, the type-checker will emit a +# compile-time error, instead of the runtime error we would have gotten +# previously. +# +# This also work for Enums (if you use `is` to compare) and Literals. +def assert_never(value: "NoReturn") -> "NoReturn": + assert False, "Unhandled value: {} ({})".format(value, type(value).__name__) diff --git a/src/_pytest/config/__init__.py b/src/_pytest/config/__init__.py index b77968110..400acb51f 100644 --- a/src/_pytest/config/__init__.py +++ b/src/_pytest/config/__init__.py @@ -41,6 +41,7 @@ from _pytest.compat import importlib_metadata from _pytest.compat import TYPE_CHECKING from _pytest.outcomes import fail from _pytest.outcomes import Skipped +from _pytest.pathlib import import_path from _pytest.pathlib import Path from _pytest.store import Store from _pytest.warning_types import PytestConfigWarning @@ -98,6 +99,15 @@ class ConftestImportFailure(Exception): ) +def filter_traceback_for_conftest_import_failure(entry) -> bool: + """filters tracebacks entries which point to pytest internals or importlib. + + Make a special case for importlib because we use it to import test modules and conftest files + in _pytest.pathlib.import_path. + """ + return filter_traceback(entry) and "importlib" not in str(entry.path).split(os.sep) + + def main(args=None, plugins=None) -> Union[int, ExitCode]: """ return exit code, after performing an in-process test run. @@ -115,7 +125,9 @@ def main(args=None, plugins=None) -> Union[int, ExitCode]: tw.line( "ImportError while loading conftest '{e.path}'.".format(e=e), red=True ) - exc_info.traceback = exc_info.traceback.filter(filter_traceback) + exc_info.traceback = exc_info.traceback.filter( + filter_traceback_for_conftest_import_failure + ) exc_repr = ( exc_info.getrepr(style="short", chain=False) if exc_info.traceback @@ -450,21 +462,21 @@ class PytestPluginManager(PluginManager): path = path[:i] anchor = current.join(path, abs=1) if anchor.exists(): # we found some file object - self._try_load_conftest(anchor) + self._try_load_conftest(anchor, namespace.importmode) foundanchor = True if not foundanchor: - self._try_load_conftest(current) + self._try_load_conftest(current, namespace.importmode) - def _try_load_conftest(self, anchor): - self._getconftestmodules(anchor) + def _try_load_conftest(self, anchor, importmode): + self._getconftestmodules(anchor, importmode) # let's also consider test* subdirs if anchor.check(dir=1): for x in anchor.listdir("test*"): if x.check(dir=1): - self._getconftestmodules(x) + self._getconftestmodules(x, importmode) @lru_cache(maxsize=128) - def _getconftestmodules(self, path): + def _getconftestmodules(self, path, importmode): if self._noconftest: return [] @@ -482,13 +494,13 @@ class PytestPluginManager(PluginManager): continue conftestpath = parent.join("conftest.py") if conftestpath.isfile(): - mod = self._importconftest(conftestpath) + mod = self._importconftest(conftestpath, importmode) clist.append(mod) self._dirpath2confmods[directory] = clist return clist - def _rget_with_confmod(self, name, path): - modules = self._getconftestmodules(path) + def _rget_with_confmod(self, name, path, importmode): + modules = self._getconftestmodules(path, importmode) for mod in reversed(modules): try: return mod, getattr(mod, name) @@ -496,7 +508,7 @@ class PytestPluginManager(PluginManager): continue raise KeyError(name) - def _importconftest(self, conftestpath): + def _importconftest(self, conftestpath, importmode): # Use a resolved Path object as key to avoid loading the same conftest twice # with build systems that create build directories containing # symlinks to actual files. @@ -512,7 +524,7 @@ class PytestPluginManager(PluginManager): _ensure_removed_sysmodule(conftestpath.purebasename) try: - mod = conftestpath.pyimport() + mod = import_path(conftestpath, mode=importmode) except Exception as e: raise ConftestImportFailure(conftestpath, sys.exc_info()) from e @@ -1213,7 +1225,9 @@ class Config: def _getconftest_pathlist(self, name, path): try: - mod, relroots = self.pluginmanager._rget_with_confmod(name, path) + mod, relroots = self.pluginmanager._rget_with_confmod( + name, path, self.getoption("importmode") + ) except KeyError: return None modpath = py.path.local(mod.__file__).dirpath() diff --git a/src/_pytest/doctest.py b/src/_pytest/doctest.py index 7aaacb481..181c66b95 100644 --- a/src/_pytest/doctest.py +++ b/src/_pytest/doctest.py @@ -33,6 +33,7 @@ from _pytest.config import Config from _pytest.config.argparsing import Parser from _pytest.fixtures import FixtureRequest from _pytest.outcomes import OutcomeException +from _pytest.pathlib import import_path from _pytest.python_api import approx from _pytest.warning_types import PytestWarning @@ -530,10 +531,12 @@ class DoctestModule(pytest.Module): ) if self.fspath.basename == "conftest.py": - module = self.config.pluginmanager._importconftest(self.fspath) + module = self.config.pluginmanager._importconftest( + self.fspath, self.config.getoption("importmode") + ) else: try: - module = self.fspath.pyimport() + module = import_path(self.fspath) except ImportError: if self.config.getvalue("doctest_ignore_import_errors"): pytest.skip("unable to import module %r" % self.fspath) diff --git a/src/_pytest/main.py b/src/_pytest/main.py index 2ec9046b0..b7a3a958a 100644 --- a/src/_pytest/main.py +++ b/src/_pytest/main.py @@ -173,6 +173,14 @@ def pytest_addoption(parser: Parser) -> None: default=False, help="Don't ignore tests in a local virtualenv directory", ) + group.addoption( + "--import-mode", + default="prepend", + choices=["prepend", "append", "importlib"], + dest="importmode", + help="prepend/append to sys.path when importing test modules and conftest files, " + "default is to prepend.", + ) group = parser.getgroup("debugconfig", "test session debugging and configuration") group.addoption( diff --git a/src/_pytest/nodes.py b/src/_pytest/nodes.py index c6c77f529..4c7aa1bcd 100644 --- a/src/_pytest/nodes.py +++ b/src/_pytest/nodes.py @@ -547,7 +547,9 @@ class FSCollector(Collector): # check if we have the common case of running # hooks with all conftest.py files pm = self.config.pluginmanager - my_conftestmodules = pm._getconftestmodules(fspath) + my_conftestmodules = pm._getconftestmodules( + fspath, self.config.getoption("importmode") + ) remove_mods = pm._conftest_plugins.difference(my_conftestmodules) if remove_mods: # one or more conftests are not in use at this fspath diff --git a/src/_pytest/pathlib.py b/src/_pytest/pathlib.py index 98ec936a1..66ae9a51d 100644 --- a/src/_pytest/pathlib.py +++ b/src/_pytest/pathlib.py @@ -1,24 +1,31 @@ import atexit import contextlib import fnmatch +import importlib.util import itertools import os import shutil import sys import uuid import warnings +from enum import Enum from functools import partial from os.path import expanduser from os.path import expandvars from os.path import isabs from os.path import sep from posixpath import sep as posix_sep +from types import ModuleType from typing import Iterable from typing import Iterator +from typing import Optional from typing import Set from typing import TypeVar from typing import Union +import py + +from _pytest.compat import assert_never from _pytest.outcomes import skip from _pytest.warning_types import PytestWarning @@ -413,3 +420,134 @@ def symlink_or_skip(src, dst, **kwargs): os.symlink(str(src), str(dst), **kwargs) except OSError as e: skip("symlinks not supported: {}".format(e)) + + +class ImportMode(Enum): + """Possible values for `mode` parameter of `import_path`""" + + prepend = "prepend" + append = "append" + importlib = "importlib" + + +class ImportPathMismatchError(ImportError): + """Raised on import_path() if there is a mismatch of __file__'s. + + This can happen when `import_path` is called multiple times with different filenames that has + the same basename but reside in packages + (for example "/tests1/test_foo.py" and "/tests2/test_foo.py"). + """ + + +def import_path( + p: Union[str, py.path.local, Path], + *, + mode: Union[str, ImportMode] = ImportMode.prepend +) -> ModuleType: + """ + Imports and returns a module from the given path, which can be a file (a module) or + a directory (a package). + + The import mechanism used is controlled by the `mode` parameter: + + * `mode == ImportMode.prepend`: the directory containing the module (or package, taking + `__init__.py` files into account) will be put at the *start* of `sys.path` before + being imported with `__import__. + + * `mode == ImportMode.append`: same as `prepend`, but the directory will be appended + to the end of `sys.path`, if not already in `sys.path`. + + * `mode == ImportMode.importlib`: uses more fine control mechanisms provided by `importlib` + to import the module, which avoids having to use `__import__` and muck with `sys.path` + at all. It effectively allows having same-named test modules in different places. + + :raise ImportPathMismatchError: if after importing the given `path` and the module `__file__` + are different. Only raised in `prepend` and `append` modes. + """ + mode = ImportMode(mode) + + path = Path(p) + + if not path.exists(): + raise ImportError(path) + + if mode is ImportMode.importlib: + module_name = path.stem + + for meta_importer in sys.meta_path: + spec = meta_importer.find_spec(module_name, [str(path.parent)]) + if spec is not None: + break + else: + spec = importlib.util.spec_from_file_location(module_name, str(path)) + + if spec is None: + raise ImportError( + "Can't find module {} at location {}".format(module_name, str(path)) + ) + mod = importlib.util.module_from_spec(spec) + spec.loader.exec_module(mod) # type: ignore[union-attr] + return mod + + pkg_path = resolve_package_path(path) + if pkg_path is not None: + pkg_root = pkg_path.parent + names = list(path.with_suffix("").relative_to(pkg_root).parts) + if names[-1] == "__init__": + names.pop() + module_name = ".".join(names) + else: + pkg_root = path.parent + module_name = path.stem + + # change sys.path permanently: restoring it at the end of this function would cause surprising + # problems because of delayed imports: for example, a conftest.py file imported by this function + # might have local imports, which would fail at runtime if we restored sys.path. + if mode is ImportMode.append: + if str(pkg_root) not in sys.path: + sys.path.append(str(pkg_root)) + elif mode is ImportMode.prepend: + if str(pkg_root) != sys.path[0]: + sys.path.insert(0, str(pkg_root)) + else: + assert_never(mode) + + importlib.import_module(module_name) + + mod = sys.modules[module_name] + if path.name == "__init__.py": + return mod + + ignore = os.environ.get("PY_IGNORE_IMPORTMISMATCH", "") + if ignore != "1": + module_file = mod.__file__ + if module_file.endswith((".pyc", ".pyo")): + module_file = module_file[:-1] + if module_file.endswith(os.path.sep + "__init__.py"): + module_file = module_file[: -(len(os.path.sep + "__init__.py"))] + + try: + is_same = os.path.samefile(str(path), module_file) + except FileNotFoundError: + is_same = False + + if not is_same: + raise ImportPathMismatchError(module_name, module_file, path) + + return mod + + +def resolve_package_path(path: Path) -> Optional[Path]: + """Return the Python package path by looking for the last + directory upwards which still contains an __init__.py. + Return None if it can not be determined. + """ + result = None + for parent in itertools.chain((path,), path.parents): + if parent.is_dir(): + if not parent.joinpath("__init__.py").is_file(): + break + if not parent.name.isidentifier(): + break + result = parent + return result diff --git a/src/_pytest/python.py b/src/_pytest/python.py index c52771057..bf45b8830 100644 --- a/src/_pytest/python.py +++ b/src/_pytest/python.py @@ -59,6 +59,8 @@ from _pytest.mark.structures import MarkDecorator from _pytest.mark.structures import normalize_mark_list from _pytest.outcomes import fail from _pytest.outcomes import skip +from _pytest.pathlib import import_path +from _pytest.pathlib import ImportPathMismatchError from _pytest.pathlib import parts from _pytest.reports import TerminalRepr from _pytest.warning_types import PytestCollectionWarning @@ -115,15 +117,6 @@ def pytest_addoption(parser: Parser) -> None: "side effects(use at your own risk)", ) - group.addoption( - "--import-mode", - default="prepend", - choices=["prepend", "append"], - dest="importmode", - help="prepend/append to sys.path when importing test modules, " - "default is to prepend.", - ) - def pytest_cmdline_main(config: Config) -> Optional[Union[int, ExitCode]]: if config.option.showfixtures: @@ -557,10 +550,10 @@ class Module(nodes.File, PyCollector): # we assume we are only called once per module importmode = self.config.getoption("--import-mode") try: - mod = self.fspath.pyimport(ensuresyspath=importmode) + mod = import_path(self.fspath, mode=importmode) except SyntaxError: raise self.CollectError(ExceptionInfo.from_current().getrepr(style="short")) - except self.fspath.ImportMismatchError as e: + except ImportPathMismatchError as e: raise self.CollectError( "import file mismatch:\n" "imported module %r has this __file__ attribute:\n" diff --git a/testing/acceptance_test.py b/testing/acceptance_test.py index 686fe1b98..d8f7a501a 100644 --- a/testing/acceptance_test.py +++ b/testing/acceptance_test.py @@ -147,7 +147,8 @@ class TestGeneralUsage: else: assert loaded == ["myplugin1", "myplugin2", "mycov"] - def test_assertion_magic(self, testdir): + @pytest.mark.parametrize("import_mode", ["prepend", "append", "importlib"]) + def test_assertion_rewrite(self, testdir, import_mode): p = testdir.makepyfile( """ def test_this(): @@ -155,7 +156,7 @@ class TestGeneralUsage: assert x """ ) - result = testdir.runpytest(p) + result = testdir.runpytest(p, "--import-mode={}".format(import_mode)) result.stdout.fnmatch_lines(["> assert x", "E assert 0"]) assert result.ret == 1 diff --git a/testing/python/collect.py b/testing/python/collect.py index 7824ceff1..e98a21f1c 100644 --- a/testing/python/collect.py +++ b/testing/python/collect.py @@ -1,4 +1,3 @@ -import os import sys import textwrap from typing import Any @@ -109,11 +108,10 @@ class TestModule: assert result.ret == 2 stdout = result.stdout.str() - for name in ("_pytest", os.path.join("py", "_path")): - if verbose == 2: - assert name in stdout - else: - assert name not in stdout + if verbose == 2: + assert "_pytest" in stdout + else: + assert "_pytest" not in stdout def test_show_traceback_import_error_unicode(self, testdir): """Check test modules collected which raise ImportError with unicode messages diff --git a/testing/python/fixtures.py b/testing/python/fixtures.py index 353ce46cd..a34478675 100644 --- a/testing/python/fixtures.py +++ b/testing/python/fixtures.py @@ -1894,7 +1894,9 @@ class TestAutouseManagement: reprec = testdir.inline_run("-v", "-s", confcut) reprec.assertoutcome(passed=8) config = reprec.getcalls("pytest_unconfigure")[0].config - values = config.pluginmanager._getconftestmodules(p)[0].values + values = config.pluginmanager._getconftestmodules(p, importmode="prepend")[ + 0 + ].values assert values == ["fin_a1", "fin_a2", "fin_b1", "fin_b2"] * 2 def test_scope_ordering(self, testdir): diff --git a/testing/test_collection.py b/testing/test_collection.py index 6644881ea..d7a9b0439 100644 --- a/testing/test_collection.py +++ b/testing/test_collection.py @@ -1342,3 +1342,83 @@ def test_fscollector_from_parent(tmpdir, request): parent=request.session, fspath=tmpdir / "foo", x=10 ) assert collector.x == 10 + + +class TestImportModeImportlib: + def test_collect_duplicate_names(self, testdir): + """--import-mode=importlib can import modules with same names that are not in packages.""" + testdir.makepyfile( + **{ + "tests_a/test_foo.py": "def test_foo1(): pass", + "tests_b/test_foo.py": "def test_foo2(): pass", + } + ) + result = testdir.runpytest("-v", "--import-mode=importlib") + result.stdout.fnmatch_lines( + [ + "tests_a/test_foo.py::test_foo1 *", + "tests_b/test_foo.py::test_foo2 *", + "* 2 passed in *", + ] + ) + + def test_conftest(self, testdir): + """Directory containing conftest modules are not put in sys.path as a side-effect of + importing them.""" + tests_dir = testdir.tmpdir.join("tests") + testdir.makepyfile( + **{ + "tests/conftest.py": "", + "tests/test_foo.py": """ + import sys + def test_check(): + assert r"{tests_dir}" not in sys.path + """.format( + tests_dir=tests_dir + ), + } + ) + result = testdir.runpytest("-v", "--import-mode=importlib") + result.stdout.fnmatch_lines(["* 1 passed in *"]) + + def setup_conftest_and_foo(self, testdir): + """Setup a tests folder to be used to test if modules in that folder can be imported + due to side-effects of --import-mode or not.""" + testdir.makepyfile( + **{ + "tests/conftest.py": "", + "tests/foo.py": """ + def foo(): return 42 + """, + "tests/test_foo.py": """ + def test_check(): + from foo import foo + assert foo() == 42 + """, + } + ) + + def test_modules_importable_as_side_effect(self, testdir): + """In import-modes `prepend` and `append`, we are able to import modules from folders + containing conftest.py files due to the side effect of changing sys.path.""" + self.setup_conftest_and_foo(testdir) + result = testdir.runpytest("-v", "--import-mode=prepend") + result.stdout.fnmatch_lines(["* 1 passed in *"]) + + def test_modules_not_importable_as_side_effect(self, testdir): + """In import-mode `importlib`, modules in folders containing conftest.py are not + importable, as don't change sys.path or sys.modules as side effect of importing + the conftest.py file. + """ + self.setup_conftest_and_foo(testdir) + result = testdir.runpytest("-v", "--import-mode=importlib") + exc_name = ( + "ModuleNotFoundError" if sys.version_info[:2] > (3, 5) else "ImportError" + ) + result.stdout.fnmatch_lines( + [ + "*{}: No module named 'foo'".format(exc_name), + "tests?test_foo.py:2: {}".format(exc_name), + "* 1 failed in *", + ] + ) diff --git a/testing/test_compat.py b/testing/test_compat.py index 45468b5f8..5debe87a3 100644 --- a/testing/test_compat.py +++ b/testing/test_compat.py @@ -1,16 +1,23 @@ +import enum import sys from functools import partial from functools import wraps +from typing import Union import pytest from _pytest.compat import _PytestWrapper +from _pytest.compat import assert_never from _pytest.compat import cached_property from _pytest.compat import get_real_func from _pytest.compat import is_generator from _pytest.compat import safe_getattr from _pytest.compat import safe_isclass +from _pytest.compat import TYPE_CHECKING from _pytest.outcomes import OutcomeException +if TYPE_CHECKING: + from typing_extensions import Literal + def test_is_generator(): def zap(): @@ -205,3 +212,55 @@ def test_cached_property() -> None: assert ncalls == 1 assert c2.prop == 2 assert c1.prop == 1 + + +def test_assert_never_union() -> None: + x = 10 # type: Union[int, str] + + if isinstance(x, int): + pass + else: + with pytest.raises(AssertionError): + assert_never(x) # type: ignore[arg-type] + + if isinstance(x, int): + pass + elif isinstance(x, str): + pass + else: + assert_never(x) + + +def test_assert_never_enum() -> None: + E = enum.Enum("E", "a b") + x = E.a # type: E + + if x is E.a: + pass + else: + with pytest.raises(AssertionError): + assert_never(x) # type: ignore[arg-type] + + if x is E.a: + pass + elif x is E.b: + pass + else: + assert_never(x) + + +def test_assert_never_literal() -> None: + x = "a" # type: Literal["a", "b"] + + if x == "a": + pass + else: + with pytest.raises(AssertionError): + assert_never(x) # type: ignore[arg-type] + + if x == "a": + pass + elif x == "b": + pass + else: + assert_never(x) diff --git a/testing/test_conftest.py b/testing/test_conftest.py index 0df303bc7..724e6f464 100644 --- a/testing/test_conftest.py +++ b/testing/test_conftest.py @@ -23,6 +23,7 @@ def conftest_setinitial(conftest, args, confcutdir=None): self.confcutdir = str(confcutdir) self.noconftest = False self.pyargs = False + self.importmode = "prepend" conftest._set_initial_conftests(Namespace()) @@ -43,35 +44,38 @@ class TestConftestValueAccessGlobal: def test_basic_init(self, basedir): conftest = PytestPluginManager() p = basedir.join("adir") - assert conftest._rget_with_confmod("a", p)[1] == 1 + assert conftest._rget_with_confmod("a", p, importmode="prepend")[1] == 1 def test_immediate_initialiation_and_incremental_are_the_same(self, basedir): conftest = PytestPluginManager() assert not len(conftest._dirpath2confmods) - conftest._getconftestmodules(basedir) + conftest._getconftestmodules(basedir, importmode="prepend") snap1 = len(conftest._dirpath2confmods) assert snap1 == 1 - conftest._getconftestmodules(basedir.join("adir")) + conftest._getconftestmodules(basedir.join("adir"), importmode="prepend") assert len(conftest._dirpath2confmods) == snap1 + 1 - conftest._getconftestmodules(basedir.join("b")) + conftest._getconftestmodules(basedir.join("b"), importmode="prepend") assert len(conftest._dirpath2confmods) == snap1 + 2 def test_value_access_not_existing(self, basedir): conftest = ConftestWithSetinitial(basedir) with pytest.raises(KeyError): - conftest._rget_with_confmod("a", basedir) + conftest._rget_with_confmod("a", basedir, importmode="prepend") def test_value_access_by_path(self, basedir): conftest = ConftestWithSetinitial(basedir) adir = basedir.join("adir") - assert conftest._rget_with_confmod("a", adir)[1] == 1 - assert conftest._rget_with_confmod("a", adir.join("b"))[1] == 1.5 + assert conftest._rget_with_confmod("a", adir, importmode="prepend")[1] == 1 + assert ( + conftest._rget_with_confmod("a", adir.join("b"), importmode="prepend")[1] + == 1.5 + ) def test_value_access_with_confmod(self, basedir): startdir = basedir.join("adir", "b") startdir.ensure("xx", dir=True) conftest = ConftestWithSetinitial(startdir) - mod, value = conftest._rget_with_confmod("a", startdir) + mod, value = conftest._rget_with_confmod("a", startdir, importmode="prepend") assert value == 1.5 path = py.path.local(mod.__file__) assert path.dirpath() == basedir.join("adir", "b") @@ -91,7 +95,7 @@ def test_doubledash_considered(testdir): conf.ensure("conftest.py") conftest = PytestPluginManager() conftest_setinitial(conftest, [conf.basename, conf.basename]) - values = conftest._getconftestmodules(conf) + values = conftest._getconftestmodules(conf, importmode="prepend") assert len(values) == 1 @@ -114,13 +118,13 @@ def test_conftest_global_import(testdir): import py, pytest from _pytest.config import PytestPluginManager conf = PytestPluginManager() - mod = conf._importconftest(py.path.local("conftest.py")) + mod = conf._importconftest(py.path.local("conftest.py"), importmode="prepend") assert mod.x == 3 import conftest assert conftest is mod, (conftest, mod) subconf = py.path.local().ensure("sub", "conftest.py") subconf.write("y=4") - mod2 = conf._importconftest(subconf) + mod2 = conf._importconftest(subconf, importmode="prepend") assert mod != mod2 assert mod2.y == 4 import conftest @@ -136,17 +140,17 @@ def test_conftestcutdir(testdir): p = testdir.mkdir("x") conftest = PytestPluginManager() conftest_setinitial(conftest, [testdir.tmpdir], confcutdir=p) - values = conftest._getconftestmodules(p) + values = conftest._getconftestmodules(p, importmode="prepend") assert len(values) == 0 - values = conftest._getconftestmodules(conf.dirpath()) + values = conftest._getconftestmodules(conf.dirpath(), importmode="prepend") assert len(values) == 0 assert conf not in conftest._conftestpath2mod # but we can still import a conftest directly - conftest._importconftest(conf) - values = conftest._getconftestmodules(conf.dirpath()) + conftest._importconftest(conf, importmode="prepend") + values = conftest._getconftestmodules(conf.dirpath(), importmode="prepend") assert values[0].__file__.startswith(str(conf)) # and all sub paths get updated properly - values = conftest._getconftestmodules(p) + values = conftest._getconftestmodules(p, importmode="prepend") assert len(values) == 1 assert values[0].__file__.startswith(str(conf)) @@ -155,7 +159,7 @@ def test_conftestcutdir_inplace_considered(testdir): conf = testdir.makeconftest("") conftest = PytestPluginManager() conftest_setinitial(conftest, [conf.dirpath()], confcutdir=conf.dirpath()) - values = conftest._getconftestmodules(conf.dirpath()) + values = conftest._getconftestmodules(conf.dirpath(), importmode="prepend") assert len(values) == 1 assert values[0].__file__.startswith(str(conf)) @@ -340,13 +344,13 @@ def test_conftest_import_order(testdir, monkeypatch): ct2 = sub.join("conftest.py") ct2.write("") - def impct(p): + def impct(p, importmode): return p conftest = PytestPluginManager() conftest._confcutdir = testdir.tmpdir monkeypatch.setattr(conftest, "_importconftest", impct) - assert conftest._getconftestmodules(sub) == [ct1, ct2] + assert conftest._getconftestmodules(sub, importmode="prepend") == [ct1, ct2] def test_fixture_dependency(testdir): diff --git a/testing/test_pathlib.py b/testing/test_pathlib.py index acc963199..126e1718e 100644 --- a/testing/test_pathlib.py +++ b/testing/test_pathlib.py @@ -1,6 +1,7 @@ import os.path import sys import unittest.mock +from textwrap import dedent import py @@ -9,11 +10,14 @@ from _pytest.pathlib import ensure_deletable from _pytest.pathlib import fnmatch_ex from _pytest.pathlib import get_extended_length_path_str from _pytest.pathlib import get_lock_path +from _pytest.pathlib import import_path +from _pytest.pathlib import ImportPathMismatchError from _pytest.pathlib import maybe_delete_a_numbered_dir from _pytest.pathlib import Path +from _pytest.pathlib import resolve_package_path -class TestPort: +class TestFNMatcherPort: """Test that our port of py.common.FNMatcher (fnmatch_ex) produces the same results as the original py.path.local.fnmatch method. """ @@ -79,6 +83,242 @@ class TestPort: assert not match(pattern, path) +class TestImportPath: + """ + + Most of the tests here were copied from py lib's tests for "py.local.path.pyimport". + + Having our own pyimport-like function is inline with removing py.path dependency in the future. + """ + + @pytest.yield_fixture(scope="session") + def path1(self, tmpdir_factory): + path = tmpdir_factory.mktemp("path") + self.setuptestfs(path) + yield path + assert path.join("samplefile").check() + + def setuptestfs(self, path): + # print "setting up test fs for", repr(path) + samplefile = path.ensure("samplefile") + samplefile.write("samplefile\n") + + execfile = path.ensure("execfile") + execfile.write("x=42") + + execfilepy = path.ensure("execfile.py") + execfilepy.write("x=42") + + d = {1: 2, "hello": "world", "answer": 42} + path.ensure("samplepickle").dump(d) + + sampledir = path.ensure("sampledir", dir=1) + sampledir.ensure("otherfile") + + otherdir = path.ensure("otherdir", dir=1) + otherdir.ensure("__init__.py") + + module_a = otherdir.ensure("a.py") + module_a.write("from .b import stuff as result\n") + module_b = otherdir.ensure("b.py") + module_b.write('stuff="got it"\n') + module_c = otherdir.ensure("c.py") + module_c.write( + dedent( + """ + import py; + import otherdir.a + value = otherdir.a.result + """ + ) + ) + module_d = otherdir.ensure("d.py") + module_d.write( + dedent( + """ + import py; + from otherdir import a + value2 = a.result + """ + ) + ) + + def test_smoke_test(self, path1): + obj = import_path(path1.join("execfile.py")) + assert obj.x == 42 # type: ignore[attr-defined] + assert obj.__name__ == "execfile" + + def test_renamed_dir_creates_mismatch(self, tmpdir, monkeypatch): + p = tmpdir.ensure("a", "test_x123.py") + import_path(p) + tmpdir.join("a").move(tmpdir.join("b")) + with pytest.raises(ImportPathMismatchError): + import_path(tmpdir.join("b", "test_x123.py")) + + # Errors can be ignored. + monkeypatch.setenv("PY_IGNORE_IMPORTMISMATCH", "1") + import_path(tmpdir.join("b", "test_x123.py")) + + # PY_IGNORE_IMPORTMISMATCH=0 does not ignore error. + monkeypatch.setenv("PY_IGNORE_IMPORTMISMATCH", "0") + with pytest.raises(ImportPathMismatchError): + import_path(tmpdir.join("b", "test_x123.py")) + + def test_messy_name(self, tmpdir): + # http://bitbucket.org/hpk42/py-trunk/issue/129 + path = tmpdir.ensure("foo__init__.py") + module = import_path(path) + assert module.__name__ == "foo__init__" + + def test_dir(self, tmpdir): + p = tmpdir.join("hello_123") + p_init = p.ensure("__init__.py") + m = import_path(p) + assert m.__name__ == "hello_123" + m = import_path(p_init) + assert m.__name__ == "hello_123" + + def test_a(self, path1): + otherdir = path1.join("otherdir") + mod = import_path(otherdir.join("a.py")) + assert mod.result == "got it" # type: ignore[attr-defined] + assert mod.__name__ == "otherdir.a" + + def test_b(self, path1): + otherdir = path1.join("otherdir") + mod = import_path(otherdir.join("b.py")) + assert mod.stuff == "got it" # type: ignore[attr-defined] + assert mod.__name__ == "otherdir.b" + + def test_c(self, path1): + otherdir = path1.join("otherdir") + mod = import_path(otherdir.join("c.py")) + assert mod.value == "got it" # type: ignore[attr-defined] + + def test_d(self, path1): + otherdir = path1.join("otherdir") + mod = import_path(otherdir.join("d.py")) + assert mod.value2 == "got it" # type: ignore[attr-defined] + + def test_import_after(self, tmpdir): + tmpdir.ensure("xxxpackage", "__init__.py") + mod1path = tmpdir.ensure("xxxpackage", "module1.py") + mod1 = import_path(mod1path) + assert mod1.__name__ == "xxxpackage.module1" + from xxxpackage import module1 + + assert module1 is mod1 + + def test_check_filepath_consistency(self, monkeypatch, tmpdir): + name = "pointsback123" + ModuleType = type(os) + p = tmpdir.ensure(name + ".py") + for ending in (".pyc", ".pyo"): + mod = ModuleType(name) + pseudopath = tmpdir.ensure(name + ending) + mod.__file__ = str(pseudopath) + monkeypatch.setitem(sys.modules, name, mod) + newmod = import_path(p) + assert mod == newmod + monkeypatch.undo() + mod = ModuleType(name) + pseudopath = tmpdir.ensure(name + "123.py") + mod.__file__ = str(pseudopath) + monkeypatch.setitem(sys.modules, name, mod) + with pytest.raises(ImportPathMismatchError) as excinfo: + import_path(p) + modname, modfile, orig = excinfo.value.args + assert modname == name + assert modfile == pseudopath + assert orig == p + assert issubclass(ImportPathMismatchError, ImportError) + + def test_issue131_on__init__(self, tmpdir): + # __init__.py files may be namespace packages, and thus the + # __file__ of an imported module may not be ourselves + # see issue + p1 = tmpdir.ensure("proja", "__init__.py") + p2 = tmpdir.ensure("sub", "proja", "__init__.py") + m1 = import_path(p1) + m2 = import_path(p2) + assert m1 == m2 + + def test_ensuresyspath_append(self, tmpdir): + root1 = tmpdir.mkdir("root1") + file1 = root1.ensure("x123.py") + assert str(root1) not in sys.path + import_path(file1, mode="append") + assert str(root1) == sys.path[-1] + assert str(root1) not in sys.path[:-1] + + def test_invalid_path(self, tmpdir): + with pytest.raises(ImportError): + import_path(tmpdir.join("invalid.py")) + + @pytest.fixture + def simple_module(self, tmpdir): + fn = tmpdir.join("mymod.py") + fn.write( + dedent( + """ + def foo(x): return 40 + x + """ + ) + ) + return fn + + def test_importmode_importlib(self, simple_module): + """importlib mode does not change sys.path""" + module = import_path(simple_module, mode="importlib") + assert module.foo(2) == 42 # type: ignore[attr-defined] + assert simple_module.dirname not in sys.path + + def test_importmode_twice_is_different_module(self, simple_module): + """importlib mode always returns a new module""" + module1 = import_path(simple_module, mode="importlib") + module2 = import_path(simple_module, mode="importlib") + assert module1 is not module2 + + def test_no_meta_path_found(self, simple_module, monkeypatch): + """Even without any meta_path should still import module""" + monkeypatch.setattr(sys, "meta_path", []) + module = import_path(simple_module, mode="importlib") + assert module.foo(2) == 42 # type: ignore[attr-defined] + + # mode='importlib' fails if no spec is found to load the module + import importlib.util + + monkeypatch.setattr( + importlib.util, "spec_from_file_location", lambda *args: None + ) + with pytest.raises(ImportError): + import_path(simple_module, mode="importlib") + + +def test_resolve_package_path(tmp_path): + pkg = tmp_path / "pkg1" + pkg.mkdir() + (pkg / "__init__.py").touch() + (pkg / "subdir").mkdir() + (pkg / "subdir/__init__.py").touch() + assert resolve_package_path(pkg) == pkg + assert resolve_package_path(pkg.joinpath("subdir", "__init__.py")) == pkg + + +def test_package_unimportable(tmp_path): + pkg = tmp_path / "pkg1-1" + pkg.mkdir() + pkg.joinpath("__init__.py").touch() + subdir = pkg.joinpath("subdir") + subdir.mkdir() + pkg.joinpath("subdir/__init__.py").touch() + assert resolve_package_path(subdir) == subdir + xyz = subdir.joinpath("xyz.py") + xyz.touch() + assert resolve_package_path(xyz) == subdir + assert not resolve_package_path(pkg) + + def test_access_denied_during_cleanup(tmp_path, monkeypatch): """Ensure that deleting a numbered dir does not fail because of OSErrors (#4262).""" path = tmp_path / "temp-1" diff --git a/testing/test_pluginmanager.py b/testing/test_pluginmanager.py index 713687578..fc327e6c0 100644 --- a/testing/test_pluginmanager.py +++ b/testing/test_pluginmanager.py @@ -37,7 +37,7 @@ class TestPytestPluginInteractions: pm.hook.pytest_addhooks.call_historic( kwargs=dict(pluginmanager=config.pluginmanager) ) - config.pluginmanager._importconftest(conf) + config.pluginmanager._importconftest(conf, importmode="prepend") # print(config.pluginmanager.get_plugins()) res = config.hook.pytest_myhook(xyz=10) assert res == [11] @@ -64,7 +64,7 @@ class TestPytestPluginInteractions: default=True) """ ) - config.pluginmanager._importconftest(p) + config.pluginmanager._importconftest(p, importmode="prepend") assert config.option.test123 def test_configure(self, testdir): @@ -129,10 +129,10 @@ class TestPytestPluginInteractions: conftest1 = testdir.tmpdir.join("tests/conftest.py") conftest2 = testdir.tmpdir.join("tests/subdir/conftest.py") - config.pluginmanager._importconftest(conftest1) + config.pluginmanager._importconftest(conftest1, importmode="prepend") ihook_a = session.gethookproxy(testdir.tmpdir.join("tests")) assert ihook_a is not None - config.pluginmanager._importconftest(conftest2) + config.pluginmanager._importconftest(conftest2, importmode="prepend") ihook_b = session.gethookproxy(testdir.tmpdir.join("tests")) assert ihook_a is not ihook_b