Make first import also use specs

Author: nicoddemus

src/_pytest/pathlib.py

@@ -522,22 +522,19 @@ def import_path(
         raise ImportError(path)
 
     if mode is ImportMode.importlib:
-        # Obtain the canonical name of this path if we can resolve it to a python package,
-        # and try to import it without changing sys.path.
-        # If it works, we import it and return the module.
-        _, module_name = resolve_pkg_root_and_module_name(path)
+        # Try to import this module using the standard import mechanisms, but
+        # without touching sys.path.
         try:
-            importlib.import_module(module_name)
-        except (ImportError, ImportWarning):
-            # Cannot be imported normally with the current sys.path, so fallback
-            # to the more complex import-path mechanism.
+            pkg_root, module_name = resolve_pkg_root_and_module_name(
+                path, consider_ns_packages=True
+            )
+        except ValueError:
             pass
         else:
-            # Double check that the imported module is the one we
-            # were given, otherwise it is easy to import modules with common names like "test"
-            # from another location.
-            mod = sys.modules[module_name]
-            if mod.__file__ and Path(mod.__file__) == path:
+            mod = _import_module_using_spec(
+                module_name, path, pkg_root, insert_modules=False
+            )
+            if mod is not None:
                 return mod
 
         # Could not import the module with the current sys.path, so we fall back
@@ -546,22 +543,17 @@ def import_path(
         with contextlib.suppress(KeyError):
             return sys.modules[module_name]
 
-        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:
+        mod = _import_module_using_spec(
+            module_name, path, path.parent, insert_modules=True
+        )
+        if mod is None:
             raise ImportError(f"Can't find module {module_name} at location {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_root, module_name = resolve_pkg_root_and_module_name(path)
+    try:
+        pkg_root, module_name = resolve_pkg_root_and_module_name(path)
+    except ValueError:
+        pkg_root, module_name = path.parent, 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
@@ -603,6 +595,27 @@ def import_path(
     return mod
 
 
+def _import_module_using_spec(
+    module_name: str, module_path: Path, module_location: Path, *, insert_modules: bool
+) -> Optional[ModuleType]:
+    """Tries to import a module by its canonical name, path to the .py file and its location."""
+    for meta_importer in sys.meta_path:
+        spec = meta_importer.find_spec(module_name, [str(module_location)])
+        if spec is not None:
+            break
+    else:
+        spec = importlib.util.spec_from_file_location(module_name, str(module_path))
+    if spec is not None:
+        mod = importlib.util.module_from_spec(spec)
+        sys.modules[module_name] = mod
+        spec.loader.exec_module(mod)  # type: ignore[union-attr]
+        if insert_modules:
+            insert_missing_modules(sys.modules, module_name)
+        return mod
+
+    return None
+
+
 # Implement a special _is_same function on Windows which returns True if the two filenames
 # compare equal, to circumvent os.path.samefile returning False for mounts in UNC (#7678).
 if sys.platform.startswith("win"):
@@ -707,7 +720,9 @@ def resolve_package_path(path: Path) -> Optional[Path]:
     return result
 
 
-def resolve_pkg_root_and_module_name(path: Path) -> Tuple[Path, str]:
+def resolve_pkg_root_and_module_name(
+    path: Path, *, consider_ns_packages: bool = False
+) -> Tuple[Path, str]:
     """
     Return the path to the directory of the root package that contains the
     given Python file, and its module name:
@@ -721,20 +736,30 @@ def resolve_pkg_root_and_module_name(path: Path) -> Tuple[Path, str]:
 
     Passing the full path to `models.py` will yield Path("src") and "app.core.models".
 
-    If the given path does not belong to a package (missing __init__.py files), returns
-    just the parent path and the name of the file as module name.
+    If consider_ns_packages is True, then we additionally check if the top-level directory
+    without __init__.py is reachable from sys.path; if it is, it is then considered a namespace package:
+
+    https://packaging.python.org/en/latest/guides/packaging-namespace-packages
+
+    This is not the default because we need to analyze carefully if it is safe to assume this
+    for all imports.
+
+    Raises ValueError if the given path does not belong to a package (missing any __init__.py files).
     """
     pkg_path = resolve_package_path(path)
     if pkg_path is not None:
         pkg_root = pkg_path.parent
+        # pkg_root.parent does not contain a __init__.py file, as per resolve_package_path,
+        # but if it is reachable from sys.argv, it should be considered a namespace package.
+        # https://packaging.python.org/en/latest/guides/packaging-namespace-packages/
+        if consider_ns_packages and str(pkg_root.parent) in sys.path:
+            pkg_root = pkg_root.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
-    return pkg_root, module_name
+        return pkg_root, module_name
+    raise ValueError(f"Could not resolve for {path}")
 
 
 def scandir(

testing/test_doctest.py

@@ -117,12 +117,12 @@ def test_simple_doctestfile(self, pytester: Pytester):
     def test_importmode(self, pytester: Pytester):
         pytester.makepyfile(
             **{
-                "namespacepkg/innerpkg/__init__.py": "",
-                "namespacepkg/innerpkg/a.py": """
+                "src/namespacepkg/innerpkg/__init__.py": "",
+                "src/namespacepkg/innerpkg/a.py": """
                   def some_func():
                     return 42
                 """,
-                "namespacepkg/innerpkg/b.py": """
+                "src/namespacepkg/innerpkg/b.py": """
                   from namespacepkg.innerpkg.a import some_func
                   def my_func():
                     '''
@@ -133,6 +133,10 @@ def my_func():
                 """,
             }
         )
+        # For 'namespacepkg' to be considered a namespace package, it needs to be reachable
+        # from sys.path:
+        # https://packaging.python.org/en/latest/guides/packaging-namespace-packages
+        pytester.syspathinsert(pytester.path / "src")
         reprec = pytester.inline_run("--doctest-modules", "--import-mode=importlib")
         reprec.assertoutcome(passed=1)
 

testing/test_pathlib.py

@@ -620,15 +620,15 @@ def test_module_name_from_path(self, tmp_path: Path) -> None:
         result = module_name_from_path(tmp_path / ".env/tests/test_foo.py", tmp_path)
         assert result == "_env.tests.test_foo"
 
-    def test_resolve_pkg_root_and_module_name(self, tmp_path: Path) -> None:
+    def test_resolve_pkg_root_and_module_name(
+        self, tmp_path: Path, monkeypatch: MonkeyPatch
+    ) -> None:
         # Create a directory structure first without __init__.py files.
         (tmp_path / "src/app/core").mkdir(parents=True)
         models_py = tmp_path / "src/app/core/models.py"
         models_py.touch()
-        assert resolve_pkg_root_and_module_name(models_py) == (
-            models_py.parent,
-            "models",
-        )
+        with pytest.raises(ValueError):
+            _ = resolve_pkg_root_and_module_name(models_py)
 
         # Create the __init__.py files, it should now resolve to a proper module name.
         (tmp_path / "src/app/__init__.py").touch()
@@ -638,6 +638,15 @@ def test_resolve_pkg_root_and_module_name(self, tmp_path: Path) -> None:
             "app.core.models",
         )
 
+        # If we add tmp_path to sys.path, src becomes a namespace package.
+        monkeypatch.syspath_prepend(tmp_path)
+        assert resolve_pkg_root_and_module_name(
+            models_py, consider_ns_packages=True
+        ) == (
+            tmp_path,
+            "src.app.core.models",
+        )
+
     def test_insert_missing_modules(
         self, monkeypatch: MonkeyPatch, tmp_path: Path
     ) -> None:
@@ -893,6 +902,32 @@ def test_import_using_normal_mechanism_first_integration(
             ]
         )
 
+    def test_import_path_imports_correct_file(self, pytester: Pytester) -> None:
+        """
+        Import the module by the given path, even if other module with the same name
+        is reachable from sys.path.
+        """
+        pytester.syspathinsert()
+        # Create a 'x.py' module reachable from sys.path that raises AssertionError
+        # if imported.
+        x_at_root = pytester.path / "x.py"
+        x_at_root.write_text("assert False", encoding="ascii")
+
+        # Create another x.py module, but in some subdirectories to ensure it is not
+        # accessible from sys.path.
+        x_in_sub_folder = pytester.path / "a/b/x.py"
+        x_in_sub_folder.parent.mkdir(parents=True)
+        x_in_sub_folder.write_text("X = 'a/b/x'", encoding="ascii")
+
+        # Import our x.py module from the subdirectories.
+        # The 'x.py' module from sys.path was not imported for sure because
+        # it did not raise an error.
+        mod = import_path(
+            x_in_sub_folder, mode=ImportMode.importlib, root=pytester.path
+        )
+        assert mod.__file__ and Path(mod.__file__) == x_in_sub_folder
+        assert mod.X == "a/b/x"
+
 
 def test_safe_exists(tmp_path: Path) -> None:
     d = tmp_path.joinpath("some_dir")