diff --git a/changelog/12592.bugfix.rst b/changelog/12592.bugfix.rst new file mode 100644 index 00000000000..605783bcab4 --- /dev/null +++ b/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. diff --git a/src/_pytest/pathlib.py b/src/_pytest/pathlib.py index 81e52ea729d..dd36559ce1b 100644 --- a/src/_pytest/pathlib.py +++ b/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 diff --git a/testing/test_pathlib.py b/testing/test_pathlib.py index 7c0a0e03d76..62359303f3b 100644 --- a/testing/test_pathlib.py +++ b/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()