Fix collection and simplify tests

We should not collect classes at the module level when the config is set. Doing so is not only the correct thing, as we will not collect the class, but also lets us avoid having the same logic at the Class collector (as it will not be collected at all now).

Author: nicoddemus

src/_pytest/python.py

@@ -405,6 +405,7 @@ def collect(self) -> Iterable[nodes.Item | nodes.Collector]:
         # __dict__ is definition ordered.
         seen: set[str] = set()
         dict_values: list[list[nodes.Item | nodes.Collector]] = []
+        collect_imported_tests = self.session.config.getini("collect_imported_tests")
         ihook = self.ihook
         for dic in dicts:
             values: list[nodes.Item | nodes.Collector] = []
@@ -417,12 +418,10 @@ def collect(self) -> Iterable[nodes.Item | nodes.Collector]:
                     continue
                 seen.add(name)
 
-                if not self.session.config.getini("collect_imported_tests"):
-                    # Do not collect imported functions
-                    if inspect.isfunction(obj) and isinstance(self, Module):
-                        fn_defined_at = obj.__module__
-                        in_module = self._getobj().__name__
-                        if fn_defined_at != in_module:
+                if not collect_imported_tests and isinstance(self, Module):
+                    # Do not collect functions and classes from other modules.
+                    if inspect.isfunction(obj) or inspect.isclass(obj):
+                        if obj.__module__ != self._getobj().__name__:
                             continue
 
                 res = ihook.pytest_pycollect_makeitem(
@@ -750,16 +749,6 @@ def newinstance(self):
         return self.obj()
 
     def collect(self) -> Iterable[nodes.Item | nodes.Collector]:
-        if not self.config.getini("collect_imported_tests"):
-            # This entire branch will discard (not collect) a class
-            # if it is imported (defined in a different module)
-            if isinstance(self, Class) and isinstance(self.parent, Module):
-                if inspect.isclass(self._getobj()):
-                    class_defined_at = self._getobj().__module__
-                    in_module = self.parent._getobj().__name__
-                    if class_defined_at != in_module:
-                        return []
-
         if not safe_getattr(self.obj, "__test__", True):
             return []
         if hasinit(self.obj):

testing/test_collect_imported_tests.py

@@ -0,0 +1,103 @@
+"""Tests for the `collect_imported_tests` configuration value."""
+
+from __future__ import annotations
+
+import textwrap
+
+from _pytest.pytester import Pytester
+import pytest
+
+
+def setup_import_class_test(pytester: Pytester) -> None:
+    src_dir = pytester.mkdir("src")
+    tests_dir = pytester.mkdir("tests")
+    src_file = src_dir / "foo.py"
+
+    src_file.write_text(
+        textwrap.dedent("""\
+            class Testament:
+                def test_collections(self):
+                    pass
+
+            def test_testament(): pass
+        """),
+        encoding="utf-8",
+    )
+
+    test_file = tests_dir / "foo_test.py"
+    test_file.write_text(
+        textwrap.dedent("""\
+            from foo import Testament, test_testament
+
+            class TestDomain:
+                def test(self):
+                    testament = Testament()
+                    assert testament
+        """),
+        encoding="utf-8",
+    )
+
+    pytester.syspathinsert(src_dir)
+
+
+def test_collect_imports_disabled(pytester: Pytester) -> None:
+    """
+    When collect_imported_tests is disabled, only objects in the
+    test modules are collected as tests, so the imported names (`Testament` and `test_testament`)
+    are not collected.
+    """
+    pytester.makeini(
+        """
+        [pytest]
+        testpaths = "tests"
+        collect_imported_tests = false
+        """
+    )
+
+    setup_import_class_test(pytester)
+    result = pytester.runpytest("-v")
+    result.stdout.fnmatch_lines(
+        [
+            "tests/foo_test.py::TestDomain::test PASSED*",
+        ]
+    )
+
+    # Ensure that the hooks were only called for the collected item.
+    reprec = result.reprec  # type:ignore[attr-defined]
+    reports = reprec.getreports("pytest_collectreport")
+    [modified] = reprec.getcalls("pytest_collection_modifyitems")
+    [item_collected] = reprec.getcalls("pytest_itemcollected")
+
+    assert [x.nodeid for x in reports] == [
+        "",
+        "tests/foo_test.py::TestDomain",
+        "tests/foo_test.py",
+        "tests",
+    ]
+    assert [x.nodeid for x in modified.items] == ["tests/foo_test.py::TestDomain::test"]
+    assert item_collected.item.nodeid == "tests/foo_test.py::TestDomain::test"
+
+
+@pytest.mark.parametrize("configure_ini", [False, True])
+def test_collect_imports_enabled(pytester: Pytester, configure_ini: bool) -> None:
+    """
+    When collect_imported_tests is enabled (the default), all names in the
+    test modules are collected as tests.
+    """
+    if configure_ini:
+        pytester.makeini(
+            """
+            [pytest]
+            collect_imported_tests = true
+            """
+        )
+
+    setup_import_class_test(pytester)
+    result = pytester.runpytest("-v")
+    result.stdout.fnmatch_lines(
+        [
+            "tests/foo_test.py::Testament::test_collections PASSED*",
+            "tests/foo_test.py::test_testament PASSED*",
+            "tests/foo_test.py::TestDomain::test PASSED*",
+        ]
+    )