From ff806b239e6fb542ef1c9a5feb7cfad229a433df Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Sat, 20 Apr 2024 08:31:33 -0300 Subject: [PATCH] importlib: set children as attribute of parent modules (#12208) Now `importlib` mode will correctly set the imported modules as an attribute of their parent modules. As helpfully posted on #12194, that's how the Python import module works so we should follow suit. In addition, we also try to import the parent modules as part of the process of importing a child module, again mirroring how Python importing works. Fix #12194 --- changelog/12194.bugfix.rst | 1 + src/_pytest/pathlib.py | 36 ++++++++-- testing/test_pathlib.py | 133 +++++++++++++++++++++++++++++++++++++ 3 files changed, 166 insertions(+), 4 deletions(-) create mode 100644 changelog/12194.bugfix.rst diff --git a/changelog/12194.bugfix.rst b/changelog/12194.bugfix.rst new file mode 100644 index 000000000..6983ba35a --- /dev/null +++ b/changelog/12194.bugfix.rst @@ -0,0 +1 @@ +Fixed a bug with ``--importmode=importlib`` and ``--doctest-modules`` where child modules did not appear as attributes in parent modules. diff --git a/src/_pytest/pathlib.py b/src/_pytest/pathlib.py index 254d9d946..7f01c011b 100644 --- a/src/_pytest/pathlib.py +++ b/src/_pytest/pathlib.py @@ -620,10 +620,6 @@ def _import_module_using_spec( :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). - Note: we can probably drop insert_missing_modules altogether: instead of - generating module names such as "src.tests.test_foo", which require intermediate - empty modules, we might just as well generate unique module names like - "src_tests_test_foo". """ # Checking with sys.meta_path first in case one of its hooks can import this module, # such as our own assertion-rewrite hook. @@ -636,9 +632,41 @@ def _import_module_using_spec( 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: Optional[ModuleType] = 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 spec.loader.exec_module(mod) # type: ignore[union-attr] + + # Set this module as an attribute of the parent module (#12194). + if parent_module is not None: + setattr(parent_module, name, mod) + if insert_modules: insert_missing_modules(sys.modules, module_name) return mod diff --git a/testing/test_pathlib.py b/testing/test_pathlib.py index f96151bdd..688d13f2f 100644 --- a/testing/test_pathlib.py +++ b/testing/test_pathlib.py @@ -1126,6 +1126,139 @@ def test_safe_exists(tmp_path: Path) -> None: assert safe_exists(p) is False +def test_import_sets_module_as_attribute(pytester: Pytester) -> None: + """Unittest test for #12194.""" + pytester.path.joinpath("foo/bar/baz").mkdir(parents=True) + pytester.path.joinpath("foo/__init__.py").touch() + pytester.path.joinpath("foo/bar/__init__.py").touch() + pytester.path.joinpath("foo/bar/baz/__init__.py").touch() + pytester.syspathinsert() + + # Import foo.bar.baz and ensure parent modules also ended up imported. + baz = import_path( + pytester.path.joinpath("foo/bar/baz/__init__.py"), + mode=ImportMode.importlib, + root=pytester.path, + consider_namespace_packages=False, + ) + assert baz.__name__ == "foo.bar.baz" + foo = sys.modules["foo"] + assert foo.__name__ == "foo" + bar = sys.modules["foo.bar"] + assert bar.__name__ == "foo.bar" + + # Check parent modules have an attribute pointing to their children. + assert bar.baz is baz + assert foo.bar is bar + + # Ensure we returned the "foo.bar" module cached in sys.modules. + bar_2 = import_path( + pytester.path.joinpath("foo/bar/__init__.py"), + mode=ImportMode.importlib, + root=pytester.path, + consider_namespace_packages=False, + ) + assert bar_2 is bar + + +def test_import_sets_module_as_attribute_without_init_files(pytester: Pytester) -> None: + """Similar to test_import_sets_module_as_attribute, but without __init__.py files.""" + pytester.path.joinpath("foo/bar").mkdir(parents=True) + pytester.path.joinpath("foo/bar/baz.py").touch() + pytester.syspathinsert() + + # Import foo.bar.baz and ensure parent modules also ended up imported. + baz = import_path( + pytester.path.joinpath("foo/bar/baz.py"), + mode=ImportMode.importlib, + root=pytester.path, + consider_namespace_packages=False, + ) + assert baz.__name__ == "foo.bar.baz" + foo = sys.modules["foo"] + assert foo.__name__ == "foo" + bar = sys.modules["foo.bar"] + assert bar.__name__ == "foo.bar" + + # Check parent modules have an attribute pointing to their children. + assert bar.baz is baz + assert foo.bar is bar + + # Ensure we returned the "foo.bar.baz" module cached in sys.modules. + baz_2 = import_path( + pytester.path.joinpath("foo/bar/baz.py"), + mode=ImportMode.importlib, + root=pytester.path, + consider_namespace_packages=False, + ) + assert baz_2 is baz + + +def test_import_sets_module_as_attribute_regression(pytester: Pytester) -> None: + """Regression test for #12194.""" + pytester.path.joinpath("foo/bar/baz").mkdir(parents=True) + pytester.path.joinpath("foo/__init__.py").touch() + pytester.path.joinpath("foo/bar/__init__.py").touch() + pytester.path.joinpath("foo/bar/baz/__init__.py").touch() + f = pytester.makepyfile( + """ + import foo + from foo.bar import baz + foo.bar.baz + + def test_foo() -> None: + pass + """ + ) + + pytester.syspathinsert() + result = pytester.runpython(f) + assert result.ret == 0 + + result = pytester.runpytest("--import-mode=importlib", "--doctest-modules") + assert result.ret == 0 + + +def test_import_submodule_not_namespace(pytester: Pytester) -> None: + """ + Regression test for importing a submodule 'foo.bar' while there is a 'bar' directory + reachable from sys.path -- ensuring the top-level module does not end up imported as a namespace + package. + + #12194 + https://github.com/pytest-dev/pytest/pull/12208#issuecomment-2056458432 + """ + pytester.syspathinsert() + # Create package 'foo' with a submodule 'bar'. + pytester.path.joinpath("foo").mkdir() + foo_path = pytester.path.joinpath("foo/__init__.py") + foo_path.touch() + bar_path = pytester.path.joinpath("foo/bar.py") + bar_path.touch() + # Create top-level directory in `sys.path` with the same name as that submodule. + pytester.path.joinpath("bar").mkdir() + + # Import `foo`, then `foo.bar`, and check they were imported from the correct location. + foo = import_path( + foo_path, + mode=ImportMode.importlib, + root=pytester.path, + consider_namespace_packages=False, + ) + bar = import_path( + bar_path, + mode=ImportMode.importlib, + root=pytester.path, + consider_namespace_packages=False, + ) + assert foo.__name__ == "foo" + assert bar.__name__ == "foo.bar" + assert foo.__file__ is not None + assert bar.__file__ is not None + assert Path(foo.__file__) == foo_path + assert Path(bar.__file__) == bar_path + + class TestNamespacePackages: """Test import_path support when importing from properly namespace packages."""