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

Author: nicoddemus

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.

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

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."""