From c85fce39b60a6cc3537e9da3e7a4f4946cfe4d49 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Sat, 17 Feb 2024 10:39:15 -0300 Subject: [PATCH] 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 --- changelog/11475.improvement.rst | 1 + src/_pytest/pathlib.py | 17 ++++ testing/test_pathlib.py | 143 +++++++++++++++++++++++++++++++- 3 files changed, 160 insertions(+), 1 deletion(-) create mode 100644 changelog/11475.improvement.rst diff --git a/changelog/11475.improvement.rst b/changelog/11475.improvement.rst new file mode 100644 index 00000000000..fc6e8be3a3a --- /dev/null +++ b/changelog/11475.improvement.rst @@ -0,0 +1 @@ +:ref:`--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. diff --git a/src/_pytest/pathlib.py b/src/_pytest/pathlib.py index 8097adb22e1..8c4e2fd87aa 100644 --- a/src/_pytest/pathlib.py +++ b/src/_pytest/pathlib.py @@ -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] diff --git a/testing/test_pathlib.py b/testing/test_pathlib.py index d515cb5bd1a..13c22b8007a 100644 --- a/testing/test_pathlib.py +++ b/testing/test_pathlib.py @@ -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 @@ -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 @@ -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,