Fix KeyError with importlib mode (directories with same name) (#12752) (#12843)

Directories inside a namespace package with the same name as the namespace package would cause a `KeyError` with `--import-mode=importlib`.

Fixes #12592

Co-authored-by: Bruno Oliveira <bruno@pytest.org>
(cherry picked from commit 6486c3f)

Co-authored-by: dongfangtianyu <7629022+dongfangtianyu@users.noreply.github.com>

Author: patchback[bot]

changelog/12592.bugfix.rst

@@ -0,0 +1 @@
+Fixed :class:`KeyError` crash when using ``--import-mode=importlib`` in a directory layout where a directory contains a child directory with the same name.

src/_pytest/pathlib.py

@@ -10,6 +10,7 @@
 import fnmatch
 from functools import partial
 from importlib.machinery import ModuleSpec
+from importlib.machinery import PathFinder
 import importlib.util
 import itertools
 import os
@@ -37,8 +38,12 @@
 from _pytest.warning_types import PytestWarning
 
 
-LOCK_TIMEOUT = 60 * 60 * 24 * 3
+if sys.version_info < (3, 11):
+    from importlib._bootstrap_external import _NamespaceLoader as NamespaceLoader
+else:
+    from importlib.machinery import NamespaceLoader
 
+LOCK_TIMEOUT = 60 * 60 * 24 * 3
 
 _AnyPurePath = TypeVar("_AnyPurePath", bound=PurePath)
 
@@ -611,13 +616,78 @@ def _import_module_using_spec(
     module_name: str, module_path: Path, module_location: Path, *, insert_modules: bool
 ) -> ModuleType | None:
     """
-    Tries to import a module by its canonical name, path to the .py file, and its
-    parent location.
+    Tries to import a module by its canonical name, path, and its parent location.
+
+    :param module_name:
+        The expected module name, will become the key of `sys.modules`.
+
+    :param module_path:
+        The file path of the module, for example `/foo/bar/test_demo.py`.
+        If module is a package, pass the path to the  `__init__.py` of the package.
+        If module is a namespace package, pass directory path.
+
+    :param module_location:
+        The parent location of the module.
+        If module is a package, pass the directory containing the `__init__.py` file.
 
     :param insert_modules:
-        If True, will call insert_missing_modules to create empty intermediate modules
-        for made-up module names (when importing test files not reachable from sys.path).
+        If True, will call `insert_missing_modules` to create empty intermediate modules
+        with made-up module names (when importing test files not reachable from `sys.path`).
+
+    Example 1 of parent_module_*:
+
+        module_name:        "a.b.c.demo"
+        module_path:        Path("a/b/c/demo.py")
+        module_location:    Path("a/b/c/")
+        if "a.b.c" is package ("a/b/c/__init__.py" exists), then
+            parent_module_name:         "a.b.c"
+            parent_module_path:         Path("a/b/c/__init__.py")
+            parent_module_location:     Path("a/b/c/")
+        else:
+            parent_module_name:         "a.b.c"
+            parent_module_path:         Path("a/b/c")
+            parent_module_location:     Path("a/b/")
+
+    Example 2 of parent_module_*:
+
+        module_name:        "a.b.c"
+        module_path:        Path("a/b/c/__init__.py")
+        module_location:    Path("a/b/c/")
+        if  "a.b" is package ("a/b/__init__.py" exists), then
+            parent_module_name:         "a.b"
+            parent_module_path:         Path("a/b/__init__.py")
+            parent_module_location:     Path("a/b/")
+        else:
+            parent_module_name:         "a.b"
+            parent_module_path:         Path("a/b/")
+            parent_module_location:     Path("a/")
     """
+    # Attempt to import the parent module, seems is our responsibility:
+    # https://github.com/python/cpython/blob/73906d5c908c1e0b73c5436faeff7d93698fc074/Lib/importlib/_bootstrap.py#L1308-L1311
+    parent_module_name, _, name = module_name.rpartition(".")
+    parent_module: ModuleType | None = None
+    if parent_module_name:
+        parent_module = sys.modules.get(parent_module_name)
+        if parent_module is None:
+            # Get parent_location based on location, get parent_path based on path.
+            if module_path.name == "__init__.py":
+                # If the current module is in a package,
+                # need to leave the package first and then enter the parent module.
+                parent_module_path = module_path.parent.parent
+            else:
+                parent_module_path = module_path.parent
+
+            if (parent_module_path / "__init__.py").is_file():
+                # If the parent module is a package, loading by  __init__.py file.
+                parent_module_path = parent_module_path / "__init__.py"
+
+            parent_module = _import_module_using_spec(
+                parent_module_name,
+                parent_module_path,
+                parent_module_path.parent,
+                insert_modules=insert_modules,
+            )
+
     # Checking with sys.meta_path first in case one of its hooks can import this module,
     # such as our own assertion-rewrite hook.
     for meta_importer in sys.meta_path:
@@ -627,36 +697,18 @@ def _import_module_using_spec(
         if spec_matches_module_path(spec, module_path):
             break
     else:
-        spec = importlib.util.spec_from_file_location(module_name, str(module_path))
+        loader = None
+        if module_path.is_dir():
+            # The `spec_from_file_location` matches a loader based on the file extension by default.
+            # For a namespace package, need to manually specify a loader.
+            loader = NamespaceLoader(name, module_path, PathFinder())
+
+        spec = importlib.util.spec_from_file_location(
+            module_name, str(module_path), loader=loader
+        )
 
     if spec_matches_module_path(spec, module_path):
         assert spec is not None
-        # Attempt to import the parent module, seems is our responsibility:
-        # https://github.com/python/cpython/blob/73906d5c908c1e0b73c5436faeff7d93698fc074/Lib/importlib/_bootstrap.py#L1308-L1311
-        parent_module_name, _, name = module_name.rpartition(".")
-        parent_module: ModuleType | None = None
-        if parent_module_name:
-            parent_module = sys.modules.get(parent_module_name)
-            if parent_module is None:
-                # Find the directory of this module's parent.
-                parent_dir = (
-                    module_path.parent.parent
-                    if module_path.name == "__init__.py"
-                    else module_path.parent
-                )
-                # Consider the parent module path as its __init__.py file, if it has one.
-                parent_module_path = (
-                    parent_dir / "__init__.py"
-                    if (parent_dir / "__init__.py").is_file()
-                    else parent_dir
-                )
-                parent_module = _import_module_using_spec(
-                    parent_module_name,
-                    parent_module_path,
-                    parent_dir,
-                    insert_modules=insert_modules,
-                )
-
         # Find spec and import this module.
         mod = importlib.util.module_from_spec(spec)
         sys.modules[module_name] = mod
@@ -675,10 +727,21 @@ def _import_module_using_spec(
 
 def spec_matches_module_path(module_spec: ModuleSpec | None, module_path: Path) -> bool:
     """Return true if the given ModuleSpec can be used to import the given module path."""
-    if module_spec is None or module_spec.origin is None:
+    if module_spec is None:
         return False
 
-    return Path(module_spec.origin) == module_path
+    if module_spec.origin:
+        return Path(module_spec.origin) == module_path
+
+    # Compare the path with the `module_spec.submodule_Search_Locations` in case
+    # the module is part of a namespace package.
+    # https://docs.python.org/3/library/importlib.html#importlib.machinery.ModuleSpec.submodule_search_locations
+    if module_spec.submodule_search_locations:  # can be None.
+        for path in module_spec.submodule_search_locations:
+            if Path(path) == module_path:
+                return True
+
+    return False
 
 
 # Implement a special _is_same function on Windows which returns True if the two filenames

testing/test_pathlib.py

@@ -17,7 +17,9 @@
 from typing import Sequence
 import unittest.mock
 
+from _pytest.config import ExitCode
 from _pytest.monkeypatch import MonkeyPatch
+from _pytest.pathlib import _import_module_using_spec
 from _pytest.pathlib import bestrelpath
 from _pytest.pathlib import commonpath
 from _pytest.pathlib import compute_module_name
@@ -36,6 +38,7 @@
 from _pytest.pathlib import resolve_package_path
 from _pytest.pathlib import resolve_pkg_root_and_module_name
 from _pytest.pathlib import safe_exists
+from _pytest.pathlib import spec_matches_module_path
 from _pytest.pathlib import symlink_or_skip
 from _pytest.pathlib import visit
 from _pytest.pytester import Pytester
@@ -416,7 +419,7 @@ def test_no_meta_path_found(
         del sys.modules[module.__name__]
 
         monkeypatch.setattr(
-            importlib.util, "spec_from_file_location", lambda *args: None
+            importlib.util, "spec_from_file_location", lambda *args, **kwargs: None
         )
         with pytest.raises(ImportError):
             import_path(
@@ -780,6 +783,62 @@ def test_insert_missing_modules(
         insert_missing_modules(modules, "")
         assert modules == {}
 
+    @pytest.mark.parametrize("b_is_package", [True, False])
+    @pytest.mark.parametrize("insert_modules", [True, False])
+    def test_import_module_using_spec(
+        self, b_is_package, insert_modules, tmp_path: Path
+    ):
+        """
+        Verify that `_import_module_using_spec` can obtain a spec based on the path, thereby enabling the import.
+        When importing, not only the target module is imported, but also the parent modules are recursively imported.
+        """
+        file_path = tmp_path / "a/b/c/demo.py"
+        file_path.parent.mkdir(parents=True)
+        file_path.write_text("my_name='demo'", encoding="utf-8")
+
+        if b_is_package:
+            (tmp_path / "a/b/__init__.py").write_text(
+                "my_name='b.__init__'", encoding="utf-8"
+            )
+
+        mod = _import_module_using_spec(
+            "a.b.c.demo",
+            file_path,
+            file_path.parent,
+            insert_modules=insert_modules,
+        )
+
+        # target module is imported
+        assert mod is not None
+        assert spec_matches_module_path(mod.__spec__, file_path) is True
+
+        mod_demo = sys.modules["a.b.c.demo"]
+        assert "demo.py" in str(mod_demo)
+        assert mod_demo.my_name == "demo"  # Imported and available for use
+
+        # parent modules are recursively imported.
+        mod_a = sys.modules["a"]
+        mod_b = sys.modules["a.b"]
+        mod_c = sys.modules["a.b.c"]
+
+        assert mod_a.b is mod_b
+        assert mod_a.b.c is mod_c
+        assert mod_a.b.c.demo is mod_demo
+
+        assert "namespace" in str(mod_a).lower()
+        assert "namespace" in str(mod_c).lower()
+
+        # Compatibility package and namespace package.
+        if b_is_package:
+            assert "namespace" not in str(mod_b).lower()
+            assert "__init__.py" in str(mod_b).lower()  # Imported __init__.py
+            assert mod_b.my_name == "b.__init__"  # Imported and available for use
+
+        else:
+            assert "namespace" in str(mod_b).lower()
+            with pytest.raises(AttributeError):  # Not imported __init__.py
+                assert mod_b.my_name
+
     def test_parent_contains_child_module_attribute(
         self, monkeypatch: MonkeyPatch, tmp_path: Path
     ):
@@ -1542,6 +1601,19 @@ def test_full_ns_packages_without_init_files(
         ) == (tmp_path / "src/dist2", "ns.a.core.foo.m")
 
 
+def test_ns_import_same_name_directory_12592(
+    tmp_path: Path, pytester: Pytester
+) -> None:
+    """Regression for `--import-mode=importlib` with directory parent and child with same name (#12592)."""
+    y_dir = tmp_path / "x/y/y"
+    y_dir.mkdir(parents=True)
+    test_y = tmp_path / "x/y/test_y.py"
+    test_y.write_text("def test(): pass", encoding="UTF-8")
+
+    result = pytester.runpytest("--import-mode=importlib", test_y)
+    assert result.ret == ExitCode.OK
+
+
 def test_is_importable(pytester: Pytester) -> None:
     pytester.syspathinsert()