Change importlib to first try to import modules using the standard mechanism
As detailed in https://github.com/pytest-dev/pytest/issues/11475#issuecomment-1937043670, 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 #11931. Fix #11475 Close #11931
This commit is contained in:
parent
067daf9f7d
commit
c85fce39b6
|
@ -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.
|
|
@ -522,6 +522,23 @@ def import_path(
|
||||||
raise ImportError(path)
|
raise ImportError(path)
|
||||||
|
|
||||||
if mode is ImportMode.importlib:
|
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)
|
module_name = module_name_from_path(path, root)
|
||||||
with contextlib.suppress(KeyError):
|
with contextlib.suppress(KeyError):
|
||||||
return sys.modules[module_name]
|
return sys.modules[module_name]
|
||||||
|
|
|
@ -3,6 +3,7 @@ import errno
|
||||||
import os.path
|
import os.path
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
import pickle
|
import pickle
|
||||||
|
import shutil
|
||||||
import sys
|
import sys
|
||||||
from textwrap import dedent
|
from textwrap import dedent
|
||||||
from types import ModuleType
|
from types import ModuleType
|
||||||
|
@ -727,6 +728,146 @@ class TestImportLibMode:
|
||||||
result = pytester.runpytest("--import-mode=importlib")
|
result = pytester.runpytest("--import-mode=importlib")
|
||||||
result.stdout.fnmatch_lines("* 1 passed *")
|
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:
|
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
|
Import the module by the given path, even if other module with the same name
|
||||||
|
@ -825,7 +966,7 @@ class TestNamespacePackages:
|
||||||
monkeypatch.syspath_prepend(tmp_path / "src/dist2")
|
monkeypatch.syspath_prepend(tmp_path / "src/dist2")
|
||||||
return models_py, algorithms_py
|
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(
|
def test_resolve_pkg_root_and_module_name_ns_multiple_levels(
|
||||||
self,
|
self,
|
||||||
tmp_path: Path,
|
tmp_path: Path,
|
||||||
|
|
Loading…
Reference in New Issue