Skip to content

Commit

Permalink
Change importlib to first try to import modules using the standard me…
Browse files Browse the repository at this point in the history
…chanism

As detailed in
pytest-dev#11475 (comment),
currently with `--import-mode=importlib` pytest will try to import every
file by using a unique module name, regardless if that module could be
imported using the normal import mechanism without touching `sys.path`.

This has the consequence that non-test modules available in `sys.path`
(via other mechanism, such as being installed into a virtualenv,
PYTHONPATH, etc) would end up being imported as standalone modules,
instead of imported with their expected module names.

To illustrate:

```
.env/
  lib/
    site-packages/
      anndata/
        core.py
```

Given `anndata` is installed into the virtual environment, `python -c
"import anndata.core"` works, but pytest with `importlib` mode would
import that module as a standalone module named
`".env.lib.site-packages.anndata.core"`, because importlib module was
designed to import test files which are not reachable from `sys.path`,
but now it is clear that normal modules should be imported using the
standard mechanisms if possible.

Now `imporlib` mode will first try to import the module normally,
without changing `sys.path`, and if that fails it falls back to
importing the module as a standalone module.

This also makes `importlib` respect namespace packages.

This supersedes pytest-dev#11931.

Fix pytest-dev#11475
Close pytest-dev#11931
  • Loading branch information
nicoddemus committed Mar 2, 2024
1 parent 067daf9 commit c85fce3
Show file tree
Hide file tree
Showing 3 changed files with 160 additions and 1 deletion.
1 change: 1 addition & 0 deletions changelog/11475.improvement.rst
@@ -0,0 +1 @@
:ref:`--import-mode=importlib <import-mode-importlib>` now tries to import modules using the standard import mechanism (but still without changing :py:data:`sys.path`), falling back to importing modules directly only if that fails.
17 changes: 17 additions & 0 deletions src/_pytest/pathlib.py
Expand Up @@ -522,6 +522,23 @@ def import_path(
raise ImportError(path)

if mode is ImportMode.importlib:
# Try to import this module using the standard import mechanisms, but
# without touching sys.path.
try:
pkg_root, module_name = resolve_pkg_root_and_module_name(
path, consider_ns_packages=True
)
except CouldNotResolvePathError:
pass
else:
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
# to importing the file as a single module, not being a part of a package.
module_name = module_name_from_path(path, root)
with contextlib.suppress(KeyError):
return sys.modules[module_name]
Expand Down
143 changes: 142 additions & 1 deletion testing/test_pathlib.py
Expand Up @@ -3,6 +3,7 @@
import os.path
from pathlib import Path
import pickle
import shutil
import sys
from textwrap import dedent
from types import ModuleType
Expand Down Expand Up @@ -727,6 +728,146 @@ def test_my_test():
result = pytester.runpytest("--import-mode=importlib")
result.stdout.fnmatch_lines("* 1 passed *")

def create_installed_doctests_and_tests_dir(
self, path: Path, monkeypatch: MonkeyPatch
) -> Tuple[Path, Path, Path]:
"""
Create a directory structure where the application code is installed in a virtual environment,
and the tests are in an outside ".tests" directory.
Return the paths to the core module (installed in the virtualenv), and the test modules.
"""
app = path / "src/app"
app.mkdir(parents=True)
(app / "__init__.py").touch()
core_py = app / "core.py"
core_py.write_text(
dedent(
"""
def foo():
'''
>>> 1 + 1
2
'''
"""
),
encoding="ascii",
)

# Install it into a site-packages directory, and add it to sys.path, mimicking what
# happens when installing into a virtualenv.
site_packages = path / ".env/lib/site-packages"
site_packages.mkdir(parents=True)
shutil.copytree(app, site_packages / "app")
assert (site_packages / "app/core.py").is_file()

monkeypatch.syspath_prepend(site_packages)

# Create the tests files, outside 'src' and the virtualenv.
# We use the same test name on purpose, but in different directories, to ensure
# this works as advertised.
conftest_path1 = path / ".tests/a/conftest.py"
conftest_path1.parent.mkdir(parents=True)
conftest_path1.write_text(
dedent(
"""
import pytest
@pytest.fixture
def a_fix(): return "a"
"""
),
encoding="ascii",
)
test_path1 = path / ".tests/a/test_core.py"
test_path1.write_text(
dedent(
"""
import app.core
def test(a_fix):
assert a_fix == "a"
""",
),
encoding="ascii",
)

conftest_path2 = path / ".tests/b/conftest.py"
conftest_path2.parent.mkdir(parents=True)
conftest_path2.write_text(
dedent(
"""
import pytest
@pytest.fixture
def b_fix(): return "b"
"""
),
encoding="ascii",
)

test_path2 = path / ".tests/b/test_core.py"
test_path2.write_text(
dedent(
"""
import app.core
def test(b_fix):
assert b_fix == "b"
""",
),
encoding="ascii",
)
return (site_packages / "app/core.py"), test_path1, test_path2

def test_import_using_normal_mechanism_first(
self, monkeypatch: MonkeyPatch, pytester: Pytester
) -> None:
"""
Test import_path imports from the canonical location when possible first, only
falling back to its normal flow when the module being imported is not reachable via sys.path (#11475).
"""
core_py, test_path1, test_path2 = self.create_installed_doctests_and_tests_dir(
pytester.path, monkeypatch
)

# core_py is reached from sys.path, so should be imported normally.
mod = import_path(core_py, mode="importlib", root=pytester.path)
assert mod.__name__ == "app.core"
assert mod.__file__ and Path(mod.__file__) == core_py

# tests are not reachable from sys.path, so they are imported as a standalone modules.
# Instead of '.tests.a.test_core', we import as "_tests.a.test_core" because
# importlib considers module names starting with '.' to be local imports.
mod = import_path(test_path1, mode="importlib", root=pytester.path)
assert mod.__name__ == "_tests.a.test_core"
mod = import_path(test_path2, mode="importlib", root=pytester.path)
assert mod.__name__ == "_tests.b.test_core"

def test_import_using_normal_mechanism_first_integration(
self, monkeypatch: MonkeyPatch, pytester: Pytester
) -> None:
"""
Same test as above, but verify the behavior calling pytest.
We should not make this call in the same test as above, as the modules have already
been imported by separate import_path() calls.
"""
core_py, test_path1, test_path2 = self.create_installed_doctests_and_tests_dir(
pytester.path, monkeypatch
)
result = pytester.runpytest(
"--import-mode=importlib",
"--doctest-modules",
"--pyargs",
"app",
"./.tests",
)
result.stdout.fnmatch_lines(
[
f"{core_py.relative_to(pytester.path)} . *",
f"{test_path1.relative_to(pytester.path)} . *",
f"{test_path2.relative_to(pytester.path)} . *",
"* 3 passed*",
]
)

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
Expand Down Expand Up @@ -825,7 +966,7 @@ def setup_directories(
monkeypatch.syspath_prepend(tmp_path / "src/dist2")
return models_py, algorithms_py

@pytest.mark.parametrize("import_mode", ["prepend", "append"])
@pytest.mark.parametrize("import_mode", ["prepend", "append", "importlib"])
def test_resolve_pkg_root_and_module_name_ns_multiple_levels(
self,
tmp_path: Path,
Expand Down

0 comments on commit c85fce3

Please sign in to comment.