Introduce --import-mode=importlib

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.

Author: nicoddemus

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>`__.

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 <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
@@ -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ș <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
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
@@ -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/

doc/en/pythonpath.rst

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

src/_pytest/config/__init__.py

@@ -450,21 +450,21 @@ def _set_initial_conftests(self, namespace):
                 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,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.
@@ -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
 
@@ -1177,7 +1177,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()

src/_pytest/nodes.py

@@ -547,7 +547,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

src/_pytest/python.py

@@ -118,7 +118,7 @@ def pytest_addoption(parser: Parser) -> None:
     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.",

testing/acceptance_test.py

@@ -147,15 +147,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
 

testing/test_collection.py

@@ -1342,3 +1342,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 *"])