Fix error with --import-mode=importlib and modules containing dataclasses or pickle (#7870)

Co-authored-by: Bruno Oliveira <[email protected]>

Fixes #7856, fixes #7859

Author: tadeu

AUTHORS

@@ -98,6 +98,7 @@ Dominic Mortlock
 Duncan Betts
 Edison Gustavo Muenz
 Edoardo Batini
+Edson Tadeu M. Manoel
 Eduardo Schettino
 Eli Boyarski
 Elizaveta Shashkova

changelog/7856.feature.rst

@@ -0,0 +1,2 @@
+:ref:`--import-mode=importlib <import-modes>` now works with features that
+depend on modules being on :py:data:`sys.modules`, such as :mod:`pickle` and :mod:`dataclasses`.

doc/en/explanation/goodpractices.rst

@@ -151,7 +151,7 @@ This layout prevents a lot of common pitfalls and has many benefits, which are b
 
 .. 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
+    any of the drawbacks above because ``sys.path`` is 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.
 

doc/en/explanation/pythonpath.rst

@@ -16,14 +16,14 @@ import process can be controlled through the ``--import-mode`` command-line flag
 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.
+  of :py:data:`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.
+  packages, because the modules will put in :py:data:`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
+* ``append``: the directory containing each module is appended to the end of :py:data:`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
@@ -41,17 +41,14 @@ these values:
   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.
+  not arranged in packages, because the modules will put in :py:data:`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.
+* ``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 :py:data:`sys.path`.
 
-  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.
+  For this reason this doesn't require test module names to be unique, but also makes test
+  modules non-importable by each other.
 
-  We intend to make ``importlib`` the default in future releases.
+  We intend to make ``importlib`` the default in future releases, depending on feedback.
 
 ``prepend`` and ``append`` import modes scenarios
 -------------------------------------------------

src/_pytest/config/__init__.py

@@ -481,7 +481,9 @@ def pytest_configure(self, config: "Config") -> None:
     #
     # Internal API for local conftest plugin handling.
     #
-    def _set_initial_conftests(self, namespace: argparse.Namespace) -> None:
+    def _set_initial_conftests(
+        self, namespace: argparse.Namespace, rootpath: Path
+    ) -> None:
         """Load initial conftest files given a preparsed "namespace".
 
         As conftest files may add their own command line options which have
@@ -507,26 +509,24 @@ def _set_initial_conftests(self, namespace: argparse.Namespace) -> None:
                 path = path[:i]
             anchor = absolutepath(current / path)
             if anchor.exists():  # we found some file object
-                self._try_load_conftest(anchor, namespace.importmode)
+                self._try_load_conftest(anchor, namespace.importmode, rootpath)
                 foundanchor = True
         if not foundanchor:
-            self._try_load_conftest(current, namespace.importmode)
+            self._try_load_conftest(current, namespace.importmode, rootpath)
 
     def _try_load_conftest(
-        self, anchor: Path, importmode: Union[str, ImportMode]
+        self, anchor: Path, importmode: Union[str, ImportMode], rootpath: Path
     ) -> None:
-        self._getconftestmodules(anchor, importmode)
+        self._getconftestmodules(anchor, importmode, rootpath)
         # let's also consider test* subdirs
         if anchor.is_dir():
             for x in anchor.glob("test*"):
                 if x.is_dir():
-                    self._getconftestmodules(x, importmode)
+                    self._getconftestmodules(x, importmode, rootpath)
 
     @lru_cache(maxsize=128)
     def _getconftestmodules(
-        self,
-        path: Path,
-        importmode: Union[str, ImportMode],
+        self, path: Path, importmode: Union[str, ImportMode], rootpath: Path
     ) -> List[types.ModuleType]:
         if self._noconftest:
             return []
@@ -545,7 +545,7 @@ def _getconftestmodules(
                 continue
             conftestpath = parent / "conftest.py"
             if conftestpath.is_file():
-                mod = self._importconftest(conftestpath, importmode)
+                mod = self._importconftest(conftestpath, importmode, rootpath)
                 clist.append(mod)
         self._dirpath2confmods[directory] = clist
         return clist
@@ -555,8 +555,9 @@ def _rget_with_confmod(
         name: str,
         path: Path,
         importmode: Union[str, ImportMode],
+        rootpath: Path,
     ) -> Tuple[types.ModuleType, Any]:
-        modules = self._getconftestmodules(path, importmode)
+        modules = self._getconftestmodules(path, importmode, rootpath=rootpath)
         for mod in reversed(modules):
             try:
                 return mod, getattr(mod, name)
@@ -565,9 +566,7 @@ def _rget_with_confmod(
         raise KeyError(name)
 
     def _importconftest(
-        self,
-        conftestpath: Path,
-        importmode: Union[str, ImportMode],
+        self, conftestpath: Path, importmode: Union[str, ImportMode], rootpath: Path
     ) -> types.ModuleType:
         # Use a resolved Path object as key to avoid loading the same conftest
         # twice with build systems that create build directories containing
@@ -584,7 +583,7 @@ def _importconftest(
             _ensure_removed_sysmodule(conftestpath.stem)
 
         try:
-            mod = import_path(conftestpath, mode=importmode)
+            mod = import_path(conftestpath, mode=importmode, root=rootpath)
         except Exception as e:
             assert e.__traceback__ is not None
             exc_info = (type(e), e, e.__traceback__)
@@ -1086,7 +1085,9 @@ def _processopt(self, opt: "Argument") -> None:
 
     @hookimpl(trylast=True)
     def pytest_load_initial_conftests(self, early_config: "Config") -> None:
-        self.pluginmanager._set_initial_conftests(early_config.known_args_namespace)
+        self.pluginmanager._set_initial_conftests(
+            early_config.known_args_namespace, rootpath=early_config.rootpath
+        )
 
     def _initini(self, args: Sequence[str]) -> None:
         ns, unknown_args = self._parser.parse_known_and_unknown_args(
@@ -1437,10 +1438,12 @@ def _getini(self, name: str):
             assert type in [None, "string"]
             return value
 
-    def _getconftest_pathlist(self, name: str, path: Path) -> Optional[List[Path]]:
+    def _getconftest_pathlist(
+        self, name: str, path: Path, rootpath: Path
+    ) -> Optional[List[Path]]:
         try:
             mod, relroots = self.pluginmanager._rget_with_confmod(
-                name, path, self.getoption("importmode")
+                name, path, self.getoption("importmode"), rootpath
             )
         except KeyError:
             return None

src/_pytest/doctest.py

@@ -534,11 +534,13 @@ def _find(
 
         if self.path.name == "conftest.py":
             module = self.config.pluginmanager._importconftest(
-                self.path, self.config.getoption("importmode")
+                self.path,
+                self.config.getoption("importmode"),
+                rootpath=self.config.rootpath,
             )
         else:
             try:
-                module = import_path(self.path)
+                module = import_path(self.path, root=self.config.rootpath)
             except ImportError:
                 if self.config.getvalue("doctest_ignore_import_errors"):
                     pytest.skip("unable to import module %r" % self.path)

src/_pytest/main.py

@@ -378,7 +378,9 @@ def _in_venv(path: Path) -> bool:
 
 
 def pytest_ignore_collect(fspath: Path, config: Config) -> Optional[bool]:
-    ignore_paths = config._getconftest_pathlist("collect_ignore", path=fspath.parent)
+    ignore_paths = config._getconftest_pathlist(
+        "collect_ignore", path=fspath.parent, rootpath=config.rootpath
+    )
     ignore_paths = ignore_paths or []
     excludeopt = config.getoption("ignore")
     if excludeopt:
@@ -388,7 +390,7 @@ def pytest_ignore_collect(fspath: Path, config: Config) -> Optional[bool]:
         return True
 
     ignore_globs = config._getconftest_pathlist(
-        "collect_ignore_glob", path=fspath.parent
+        "collect_ignore_glob", path=fspath.parent, rootpath=config.rootpath
     )
     ignore_globs = ignore_globs or []
     excludeglobopt = config.getoption("ignore_glob")
@@ -546,7 +548,9 @@ def gethookproxy(self, fspath: "os.PathLike[str]"):
         # hooks with all conftest.py files.
         pm = self.config.pluginmanager
         my_conftestmodules = pm._getconftestmodules(
-            Path(fspath), self.config.getoption("importmode")
+            Path(fspath),
+            self.config.getoption("importmode"),
+            rootpath=self.config.rootpath,
         )
         remove_mods = pm._conftest_plugins.difference(my_conftestmodules)
         if remove_mods:

src/_pytest/pathlib.py

@@ -23,6 +23,7 @@
 from posixpath import sep as posix_sep
 from types import ModuleType
 from typing import Callable
+from typing import Dict
 from typing import Iterable
 from typing import Iterator
 from typing import Optional
@@ -454,6 +455,7 @@ def import_path(
     p: Union[str, "os.PathLike[str]"],
     *,
     mode: Union[str, ImportMode] = ImportMode.prepend,
+    root: Path,
 ) -> ModuleType:
     """Import and return a module from the given path, which can be a file (a module) or
     a directory (a package).
@@ -471,6 +473,11 @@ def import_path(
       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.
 
+    :param root:
+        Used as an anchor when mode == ImportMode.importlib to obtain
+        a unique name for the module being imported so it can safely be stored
+        into ``sys.modules``.
+
     :raises ImportPathMismatchError:
         If after importing the given `path` and the module `__file__`
         are different. Only raised in `prepend` and `append` modes.
@@ -483,7 +490,7 @@ def import_path(
         raise ImportError(path)
 
     if mode is ImportMode.importlib:
-        module_name = path.stem
+        module_name = module_name_from_path(path, root)
 
         for meta_importer in sys.meta_path:
             spec = meta_importer.find_spec(module_name, [str(path.parent)])
@@ -497,7 +504,9 @@ def import_path(
                 "Can't find module {} at location {}".format(module_name, str(path))
             )
         mod = importlib.util.module_from_spec(spec)
+        sys.modules[module_name] = mod
         spec.loader.exec_module(mod)  # type: ignore[union-attr]
+        insert_missing_modules(sys.modules, module_name)
         return mod
 
     pkg_path = resolve_package_path(path)
@@ -562,6 +571,47 @@ def _is_same(f1: str, f2: str) -> bool:
         return os.path.samefile(f1, f2)
 
 
+def module_name_from_path(path: Path, root: Path) -> str:
+    """
+    Return a dotted module name based on the given path, anchored on root.
+
+    For example: path="projects/src/tests/test_foo.py" and root="/projects", the
+    resulting module name will be "src.tests.test_foo".
+    """
+    path = path.with_suffix("")
+    try:
+        relative_path = path.relative_to(root)
+    except ValueError:
+        # If we can't get a relative path to root, use the full path, except
+        # for the first part ("d:\\" or "/" depending on the platform, for example).
+        path_parts = path.parts[1:]
+    else:
+        # Use the parts for the relative path to the root path.
+        path_parts = relative_path.parts
+
+    return ".".join(path_parts)
+
+
+def insert_missing_modules(modules: Dict[str, ModuleType], module_name: str) -> None:
+    """
+    Used by ``import_path`` to create intermediate modules when using mode=importlib.
+
+    When we want to import a module as "src.tests.test_foo" for example, we need
+    to create empty modules "src" and "src.tests" after inserting "src.tests.test_foo",
+    otherwise "src.tests.test_foo" is not importable by ``__import__``.
+    """
+    module_parts = module_name.split(".")
+    while module_name:
+        if module_name not in modules:
+            module = ModuleType(
+                module_name,
+                doc="Empty module created by pytest's importmode=importlib.",
+            )
+            modules[module_name] = module
+        module_parts.pop(-1)
+        module_name = ".".join(module_parts)
+
+
 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.

src/_pytest/python.py

@@ -577,7 +577,7 @@ def _importtestmodule(self):
         # We assume we are only called once per module.
         importmode = self.config.getoption("--import-mode")
         try:
-            mod = import_path(self.path, mode=importmode)
+            mod = import_path(self.path, mode=importmode, root=self.config.rootpath)
         except SyntaxError as e:
             raise self.CollectError(
                 ExceptionInfo.from_current().getrepr(style="short")

testing/code/test_excinfo.py

@@ -162,7 +162,7 @@ def test_traceback_cut(self) -> None:
     def test_traceback_cut_excludepath(self, pytester: Pytester) -> None:
         p = pytester.makepyfile("def f(): raise ValueError")
         with pytest.raises(ValueError) as excinfo:
-            import_path(p).f()  # type: ignore[attr-defined]
+            import_path(p, root=pytester.path).f()  # type: ignore[attr-defined]
         basedir = Path(pytest.__file__).parent
         newtraceback = excinfo.traceback.cut(excludepath=basedir)
         for x in newtraceback:
@@ -443,7 +443,7 @@ def importasmod(source):
             tmp_path.joinpath("__init__.py").touch()
             modpath.write_text(source)
             importlib.invalidate_caches()
-            return import_path(modpath)
+            return import_path(modpath, root=tmp_path)
 
         return importasmod
 

testing/code/test_source.py

@@ -298,7 +298,7 @@ def method(self):
     )
     path = tmp_path.joinpath("a.py")
     path.write_text(str(source))
-    mod: Any = import_path(path)
+    mod: Any = import_path(path, root=tmp_path)
     s2 = Source(mod.A)
     assert str(source).strip() == str(s2).strip()
 

testing/python/fixtures.py

@@ -2069,9 +2069,9 @@ def test_2(self):
         reprec = pytester.inline_run("-v", "-s", "--confcutdir", pytester.path)
         reprec.assertoutcome(passed=8)
         config = reprec.getcalls("pytest_unconfigure")[0].config
-        values = config.pluginmanager._getconftestmodules(p, importmode="prepend")[
-            0
-        ].values
+        values = config.pluginmanager._getconftestmodules(
+            p, importmode="prepend", rootpath=pytester.path
+        )[0].values
         assert values == ["fin_a1", "fin_a2", "fin_b1", "fin_b2"] * 2
 
     def test_scope_ordering(self, pytester: Pytester) -> None:

testing/test_config.py

@@ -597,8 +597,14 @@ def test_getconftest_pathlist(self, pytester: Pytester, tmp_path: Path) -> None:
         p = tmp_path.joinpath("conftest.py")
         p.write_text(f"pathlist = ['.', {str(somepath)!r}]")
         config = pytester.parseconfigure(p)
-        assert config._getconftest_pathlist("notexist", path=tmp_path) is None
-        pl = config._getconftest_pathlist("pathlist", path=tmp_path) or []
+        assert (
+            config._getconftest_pathlist("notexist", path=tmp_path, rootpath=tmp_path)
+            is None
+        )
+        pl = (
+            config._getconftest_pathlist("pathlist", path=tmp_path, rootpath=tmp_path)
+            or []
+        )
         print(pl)
         assert len(pl) == 2
         assert pl[0] == tmp_path