WIP: don't collect instead of filtering out

Author: FreerGit

doc/en/reference/reference.rst

@@ -1301,6 +1301,20 @@ passed multiple times. The expected format is ``name=value``. For example::
    variables, that will be expanded. For more information about cache plugin
    please refer to :ref:`cache_provider`.
 
+.. confval:: collect_imported_tests
+
+   .. versionadded:: 8.4
+
+   Setting this to ``false`` will make pytest collect classes/functions from test
+   files only if they are defined in that file (as opposed to imported there).
+
+   .. code-block:: ini
+
+        [pytest]
+        collect_imported_tests = false
+
+   Default: ``true``
+
 .. confval:: consider_namespace_packages
 
    Controls if pytest should attempt to identify `namespace packages <https://packaging.python.org/en/latest/guides/packaging-namespace-packages>`__
@@ -1838,17 +1852,6 @@ passed multiple times. The expected format is ``name=value``. For example::
 
        pytest testing doc
 
-
-.. confval:: collect_imported_tests
-
-   Setting this to `false` will make pytest collect classes/functions from test
-   files only if they are defined in that file (as opposed to imported there).
-
-    .. code-block:: ini
-
-        [pytest]
-        collect_imported_tests = false
-
 .. confval:: tmp_path_retention_count
 
    How many sessions should we keep the `tmp_path` directories,

src/_pytest/main.py

@@ -964,41 +964,16 @@ def collect(self) -> Iterator[nodes.Item | nodes.Collector]:
             self.trace.root.indent -= 1
 
     def genitems(self, node: nodes.Item | nodes.Collector) -> Iterator[nodes.Item]:
-        import inspect
-
-        from _pytest.python import Class
-        from _pytest.python import Function
-        from _pytest.python import Module
-
         self.trace("genitems", node)
         if isinstance(node, nodes.Item):
             node.ihook.pytest_itemcollected(item=node)
-            if not self.config.getini("collect_imported_tests"):
-                if isinstance(node.parent, Module) and isinstance(node, Function):
-                    if inspect.isfunction(node._getobj()):
-                        fn_defined_at = node._getobj().__module__
-                        in_module = node.parent._getobj().__name__
-                        if fn_defined_at != in_module:
-                            return
             yield node
         else:
             assert isinstance(node, nodes.Collector)
             keepduplicates = self.config.getoption("keepduplicates")
             # For backward compat, dedup only applies to files.
             handle_dupes = not (keepduplicates and isinstance(node, nodes.File))
             rep, duplicate = self._collect_one_node(node, handle_dupes)
-
-            if not self.config.getini("collect_imported_tests"):
-                for subnode in rep.result:
-                    if isinstance(subnode, Class) and isinstance(
-                        subnode.parent, Module
-                    ):
-                        if inspect.isclass(subnode._getobj()):
-                            class_defined_at = subnode._getobj().__module__
-                            in_module = subnode.parent._getobj().__name__
-                            if class_defined_at != in_module:
-                                rep.result.remove(subnode)
-
             if duplicate and not keepduplicates:
                 return
             if rep.passed:

src/_pytest/python.py

@@ -416,6 +416,15 @@ def collect(self) -> Iterable[nodes.Item | nodes.Collector]:
                 if name in seen:
                     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:
+                            continue
+
                 res = ihook.pytest_pycollect_makeitem(
                     collector=self, name=name, obj=obj
                 )
@@ -741,6 +750,16 @@ 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_imports.py

@@ -5,6 +5,9 @@
 from _pytest.pytester import Pytester
 
 
+# Start of tests for classes
+
+
 def run_import_class_test(pytester: Pytester, passed: int = 0, errors: int = 0) -> None:
     src_dir = pytester.mkdir("src")
     tests_dir = pytester.mkdir("tests")
@@ -57,26 +60,37 @@ def test_collect_imports_disabled(pytester: Pytester) -> None:
 
     run_import_class_test(pytester, passed=1)
 
+    # Verify that the state of hooks
+    reprec = pytester.inline_run()
+    items_collected = reprec.getcalls("pytest_itemcollected")
+    assert len(items_collected) == 1
+    for x in items_collected:
+        assert x.item._getobj().__name__ == "test_testament"
 
-def test_collect_imports_default(pytester: Pytester) -> None:
-    pytester.makeini("""
-        [pytest]
-        testpaths = "tests"
-    """)
 
+def test_collect_imports_default(pytester: Pytester) -> None:
     run_import_class_test(pytester, errors=1)
 
+    # TODO, hooks
+
 
 def test_collect_imports_enabled(pytester: Pytester) -> None:
     pytester.makeini("""
         [pytest]
-        testpaths = "tests"
         collect_imported_tests = true
     """)
 
     run_import_class_test(pytester, errors=1)
 
 
+#     # TODO, hooks
+
+
+# End of tests for classes
+#################################
+# Start of tests for functions
+
+
 def run_import_functions_test(
     pytester: Pytester, passed: int, errors: int, failed: int
 ) -> None:
@@ -85,8 +99,8 @@ def run_import_functions_test(
 
     src_file = src_dir / "foo.py"
 
-    # Note that these "tests" are should _not_ be treated as tests.
-    # They are normal functions that happens to have test_* or *_test in the name.
+    # Note that these "tests" should _not_ be treated as tests if `collect_imported_tests = false`
+    # They are normal functions in that case, that happens to have test_* or *_test in the name.
     # Thus should _not_ be collected!
     src_file.write_text(
         textwrap.dedent("""\
@@ -121,7 +135,6 @@ def test_important(self):
             res = test_function()
             if res == 5:
                 pass
-
     """),
         encoding="utf-8",
     )
@@ -138,31 +151,83 @@ def test_collect_function_imports_enabled(pytester: Pytester) -> None:
     """)
 
     run_import_functions_test(pytester, passed=2, errors=0, failed=1)
+    reprec = pytester.inline_run()
+    items_collected = reprec.getcalls("pytest_itemcollected")
+    # Recall that the default is `collect_imported_tests = true`.
+    # Which means that the normal functions are now interpreted as
+    # valid tests and `test_function()` will fail.
+    assert len(items_collected) == 3
+    for x in items_collected:
+        assert x.item._getobj().__name__ in [
+            "test_important",
+            "test_bar",
+            "test_function",
+        ]
 
 
-def test_collect_function_imports_disabled(pytester: Pytester) -> None:
+def test_behaviour_without_testpaths_set_and_false(pytester: Pytester) -> None:
+    # Make sure `collect_imported_tests` has no dependence on `testpaths`
     pytester.makeini("""
         [pytest]
-        # testpaths = "tests"
         collect_imported_tests = false
     """)
 
     run_import_functions_test(pytester, passed=1, errors=0, failed=0)
+    reprec = pytester.inline_run()
+    items_collected = reprec.getcalls("pytest_itemcollected")
+    assert len(items_collected) == 1
+    for x in items_collected:
+        assert x.item._getobj().__name__ == "test_important"
 
 
-def test_behaviour_without_testpaths_set_and_false(pytester: Pytester) -> None:
+def test_behaviour_without_testpaths_set_and_true(pytester: Pytester) -> None:
+    # Make sure `collect_imported_tests` has no dependence on `testpaths`
     pytester.makeini("""
         [pytest]
-        collect_imported_tests = false
+        collect_imported_tests = true
     """)
 
-    run_import_functions_test(pytester, passed=1, errors=0, failed=0)
+    run_import_functions_test(pytester, passed=2, errors=0, failed=1)
+    reprec = pytester.inline_run()
+    items_collected = reprec.getcalls("pytest_itemcollected")
+    assert len(items_collected) == 3
 
 
-def test_behaviour_without_testpaths_set_and_true(pytester: Pytester) -> None:
+def test_hook_behaviour_when_collect_off(pytester: Pytester) -> None:
     pytester.makeini("""
         [pytest]
-        collect_imported_tests = true
+        collect_imported_tests = false
     """)
 
-    run_import_functions_test(pytester, passed=2, errors=0, failed=1)
+    run_import_functions_test(pytester, passed=1, errors=0, failed=0)
+    reprec = pytester.inline_run()
+
+    # reports = reprec.getreports("pytest_collectreport")
+    items_collected = reprec.getcalls("pytest_itemcollected")
+    modified = reprec.getcalls("pytest_collection_modifyitems")
+
+    # print("Reports: ----------------")
+    # print(reports)
+    # for r in reports:
+    #     print(r)
+
+    # TODO this is want I want, I think....
+    # <CollectReport '' lenresult=1 outcome='passed'>
+    # <CollectReport 'tests/foo_test.py::TestDomain' lenresult=1 outcome='passed'>
+    # <CollectReport 'tests/foo_test.py' lenresult=1 outcome='passed'>
+    # <CollectReport 'tests' lenresult=1 outcome='passed'>
+    # <CollectReport '.' lenresult=1 outcome='passed'>
+
+    # TODO
+    # assert(reports.outcome == "passed")
+    # assert(len(reports.result) == 1)
+
+    # print("Items collected: ----------------")
+    # print(items_collected)
+    # print("Modified : ----------------")
+
+    assert len(items_collected) == 1
+    for x in items_collected:
+        assert x.item._getobj().__name__ == "test_important"
+
+    assert len(modified) == 1