Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog/12592.bugfix.rst
Original file line number Diff line number Diff line change
@@ -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.
131 changes: 97 additions & 34 deletions src/_pytest/pathlib.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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

Check warning on line 44 in src/_pytest/pathlib.py

View check run for this annotation

Codecov / codecov/patch

src/_pytest/pathlib.py#L44

Added line #L44 was not covered by tests

LOCK_TIMEOUT = 60 * 60 * 24 * 3

_AnyPurePath = TypeVar("_AnyPurePath", bound=PurePath)

Expand Down Expand Up @@ -611,13 +616,78 @@
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:
Expand All @@ -627,36 +697,18 @@
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
Expand All @@ -675,10 +727,21 @@

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
Expand Down
74 changes: 73 additions & 1 deletion testing/test_pathlib.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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
):
Expand Down Expand Up @@ -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()

Expand Down
Loading