Skip to content

Commit

Permalink
Make first import also use specs
Browse files Browse the repository at this point in the history
  • Loading branch information
nicoddemus committed Feb 18, 2024
1 parent b95d86b commit 8a0f1a9
Show file tree
Hide file tree
Showing 3 changed files with 105 additions and 41 deletions.
91 changes: 58 additions & 33 deletions src/_pytest/pathlib.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(

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

View check run for this annotation

Codecov / codecov/patch

src/_pytest/pathlib.py#L527-L528

Added lines #L527 - L528 were not covered by tests
path, consider_ns_packages=True
)
except ValueError:
pass

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

View check run for this annotation

Codecov / codecov/patch

src/_pytest/pathlib.py#L531-L532

Added lines #L531 - L532 were not covered by tests
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(

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

View check run for this annotation

Codecov / codecov/patch

src/_pytest/pathlib.py#L534

Added line #L534 was not covered by tests
module_name, path, pkg_root, insert_modules=False
)
if mod is not None:
return mod

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

View check run for this annotation

Codecov / codecov/patch

src/_pytest/pathlib.py#L538

Added line #L538 was not covered by tests

# Could not import the module with the current sys.path, so we fall back
Expand All @@ -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(

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

View check run for this annotation

Codecov / codecov/patch

src/_pytest/pathlib.py#L546

Added line #L546 was not covered by tests
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
Expand Down Expand Up @@ -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)])

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

View check run for this annotation

Codecov / codecov/patch

src/_pytest/pathlib.py#L603

Added line #L603 was not covered by tests
if spec is not None:
break

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

View check run for this annotation

Codecov / codecov/patch

src/_pytest/pathlib.py#L605

Added line #L605 was not covered by tests
else:
spec = importlib.util.spec_from_file_location(module_name, str(module_path))

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

View check run for this annotation

Codecov / codecov/patch

src/_pytest/pathlib.py#L607

Added line #L607 was not covered by tests
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]

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

View check run for this annotation

Codecov / codecov/patch

src/_pytest/pathlib.py#L609-L611

Added lines #L609 - L611 were not covered by tests
if insert_modules:
insert_missing_modules(sys.modules, module_name)
return mod

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

View check run for this annotation

Codecov / codecov/patch

src/_pytest/pathlib.py#L613-L614

Added lines #L613 - L614 were not covered by tests

return None

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

View check run for this annotation

Codecov / codecov/patch

src/_pytest/pathlib.py#L616

Added line #L616 was not covered by tests


# 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"):
Expand Down Expand Up @@ -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:
Expand All @@ -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

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

View check run for this annotation

Codecov / codecov/patch

src/_pytest/pathlib.py#L756

Added line #L756 was not covered by tests
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(
Expand Down
10 changes: 7 additions & 3 deletions testing/test_doctest.py
Original file line number Diff line number Diff line change
Expand Up @@ -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():
'''
Expand All @@ -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)

Expand Down
45 changes: 40 additions & 5 deletions testing/test_pathlib.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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:
Expand Down Expand Up @@ -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")
Expand Down

0 comments on commit 8a0f1a9

Please sign in to comment.