Skip to content

Commit

Permalink
Introduce --import-mode=importlib
Browse files Browse the repository at this point in the history
This introduces --import-mode=importlib, which uses fine-grained facilities
from importlib to import test modules and conftest files, bypassing
the need to change sys.path and sys.modules as side-effect of that.

I've also opened pytest-dev#7245 to gather feedback on the new import mode.
  • Loading branch information
nicoddemus committed May 23, 2020
1 parent b38edec commit e6e912c
Show file tree
Hide file tree
Showing 8 changed files with 148 additions and 25 deletions.
14 changes: 14 additions & 0 deletions changelog/7245.feature.rst
@@ -0,0 +1,14 @@
New ``--import-mode=importlib`` option that uses `importlib <https://docs.python.org/3/library/importlib.html>`__ 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 <https://github.com/pytest-dev/pytest/issues/7245>`__.

You can read more about this option in `the documentation <https://docs.pytest.org/en/latest/pythonpath.html#import-modes>`__.
23 changes: 20 additions & 3 deletions doc/en/goodpractices.rst
Expand Up @@ -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 <import-modes>`
(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
Expand All @@ -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:

Expand All @@ -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ș <https://blog.ionelmc.ro/2014/05/25/python-packaging/#the-structure>`_.

.. 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
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Expand Down Expand Up @@ -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)
Expand All @@ -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/
Expand Down
61 changes: 55 additions & 6 deletions doc/en/pythonpath.rst
Expand Up @@ -3,11 +3,62 @@
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 moduels and ``conftest.py`` files for execution.

Importing files in Python (at least until recently) is a non-trivial processes, often requiring
changing `sys.path <https://docs.python.org/3/library/sys.html#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__ <https://docs.python.org/3/library/functions.html#__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 <src-layout>` 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 <https://docs.python.org/3/library/importlib.html>`__ 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.

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::

Expand All @@ -28,8 +79,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
Expand All @@ -44,7 +93,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::

Expand Down
26 changes: 14 additions & 12 deletions src/_pytest/config/__init__.py
Expand Up @@ -450,21 +450,21 @@ def _set_initial_conftests(self, namespace):
path = path[:i]
anchor = current.join(path, abs=1)
if exists(anchor): # 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 []

Expand All @@ -482,21 +482,21 @@ def _getconftestmodules(self, path):
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)
except AttributeError:
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.
Expand All @@ -512,7 +512,7 @@ def _importconftest(self, conftestpath):
_ensure_removed_sysmodule(conftestpath.purebasename)

try:
mod = conftestpath.pyimport()
mod = conftestpath.pyimport(ensuresyspath=importmode)
except Exception as e:
raise ConftestImportFailure(conftestpath, sys.exc_info()) from e

Expand Down Expand Up @@ -1149,7 +1149,9 @@ def _getini(self, name: str) -> Any:

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()
Expand Down
4 changes: 3 additions & 1 deletion src/_pytest/nodes.py
Expand Up @@ -503,7 +503,9 @@ def _gethookproxy(self, fspath: py.path.local):
# 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
Expand Down
2 changes: 1 addition & 1 deletion src/_pytest/python.py
Expand Up @@ -105,7 +105,7 @@ def pytest_addoption(parser):
group.addoption(
"--import-mode",
default="prepend",
choices=["prepend", "append"],
choices=["prepend", "append", "importlib"],
dest="importmode",
help="prepend/append to sys.path when importing test modules, "
"default is to prepend.",
Expand Down
5 changes: 3 additions & 2 deletions testing/acceptance_test.py
Expand Up @@ -149,15 +149,16 @@ def my_dists():
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():
x = 0
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

Expand Down
38 changes: 38 additions & 0 deletions testing/test_collection.py
Expand Up @@ -1353,3 +1353,41 @@ def from_parent(cls, parent, *, fspath, x):
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 *"])

0 comments on commit e6e912c

Please sign in to comment.