remove some comments, remove now unused _schedule_finalizers(), move tests to a pytester test in testing/python/fixtures.py

src/_pytest/fixtures.py

@@ -633,13 +633,6 @@ class FixtureRequest(abc.ABC):
         # Make sure the fixture value is cached, running it if it isn't
         fixturedef.execute(request=subrequest)
 
-    def _schedule_finalizers(
-        self, fixturedef: "FixtureDef[object]", subrequest: "SubRequest"
-    ) -> None:
-        # If fixture function failed it might have registered finalizers.
-        finalizer = functools.partial(fixturedef.finish, request=subrequest)
-        subrequest.node.addfinalizer(finalizer)
-
 
 @final
 class TopRequest(FixtureRequest):
@@ -766,21 +759,6 @@ class SubRequest(FixtureRequest):
     def addfinalizer(self, finalizer: Callable[[], object]) -> None:
         self._fixturedef.addfinalizer(finalizer)
 
-    def _schedule_finalizers(
-        self, fixturedef: "FixtureDef[object]", subrequest: "SubRequest"
-    ) -> None:
-        # If the executing fixturedef was not explicitly requested in the argument list (via
-        # getfixturevalue inside the fixture call) then ensure this fixture def will be finished
-        # first.
-        if (
-            fixturedef.argname not in self._fixture_defs
-            and fixturedef.argname not in self._pyfuncitem.fixturenames
-        ):
-            fixturedef.addfinalizer(
-                functools.partial(self._fixturedef.finish, request=self)
-            )
-        super()._schedule_finalizers(fixturedef, subrequest)
-
 
 @final
 class FixtureLookupError(LookupError):
@@ -1037,7 +1015,6 @@ class FixtureDef(Generic[FixtureValue]):
             self.cached_result = None
             self._finalizers.clear()
 
-    # Note: the return value is entirely unused, no tests depend on it
     def execute(self, request: SubRequest) -> FixtureValue:
         finalizer = functools.partial(self.finish, request=request)
         # Get required arguments and register our own finish()
@@ -1076,7 +1053,6 @@ class FixtureDef(Generic[FixtureValue]):
             # schedule our finalizer, even if the setup failed
             request.node.addfinalizer(finalizer)
 
-        # note: unused
         return result
 
     def cache_key(self, request: SubRequest) -> object:

testing/python/fixtures.py

@@ -4559,3 +4559,80 @@ def test_deduplicate_names() -> None:
     assert items == ("a", "b", "c", "d")
     items = deduplicate_names((*items, "g", "f", "g", "e", "b"))
     assert items == ("a", "b", "c", "d", "g", "f", "e")
+
+
+def test_scope_fixture_caching_1(pytester: Pytester) -> None:
+    """
+    Make sure setup and finalization is only run once when using fixture
+    multiple times. This might be a duplicate of another test."""
+    pytester.makepyfile(
+        """
+        from __future__ import annotations
+
+        from typing import Generator
+
+        import pytest
+        executed: list[str] = []
+        @pytest.fixture(scope="class")
+        def fixture_1() -> Generator[None, None, None]:
+            executed.append("fix setup")
+            yield
+            executed.append("fix teardown")
+
+
+        class TestFixtureCaching:
+            def test_1(self, fixture_1: None) -> None:
+                assert executed == ["fix setup"]
+
+            def test_2(self, fixture_1: None) -> None:
+                assert executed == ["fix setup"]
+
+
+        def test_expected_setup_and_teardown() -> None:
+            assert executed == ["fix setup", "fix teardown"]
+        """
+    )
+    result = pytester.runpytest()
+    assert result.ret == 0
+
+
+def test_scope_fixture_caching_2(pytester: Pytester) -> None:
+    """Make sure setup & finalization is only run once, with a cached exception."""
+    pytester.makepyfile(
+        """
+        from __future__ import annotations
+
+        from typing import Generator
+
+        import pytest
+        executed_crash: list[str] = []
+
+
+        @pytest.fixture(scope="class")
+        def fixture_crash(request: pytest.FixtureRequest) -> None:
+            executed_crash.append("fix_crash setup")
+
+            def my_finalizer() -> None:
+                executed_crash.append("fix_crash teardown")
+
+            request.addfinalizer(my_finalizer)
+
+            raise Exception("foo")
+
+
+        class TestFixtureCachingException:
+            @pytest.mark.xfail
+            def test_crash_1(self, fixture_crash: None) -> None:
+                ...
+
+            @pytest.mark.xfail
+            def test_crash_2(self, fixture_crash: None) -> None:
+                ...
+
+
+        def test_crash_expected_setup_and_teardown() -> None:
+            assert executed_crash == ["fix_crash setup", "fix_crash teardown"]
+        """
+    )
+    result = pytester.runpytest()
+    assert result.ret == 0

testing/python/test_scope_fixture_caching.py

@@ -1,65 +0,0 @@
-from __future__ import annotations
-
-from typing import Generator
-
-import pytest
-
-
-# These tests will fail if run out of order, or selectively ... so they should probably be defined in a different way
-
-executed: list[str] = []
-
-
-#########
-# Make sure setup and finalization is only run once when using fixture multiple times
-# I'm pretty sure there's other tests that checks this already though, though idr fully where
-#########
-@pytest.fixture(scope="class")
-def fixture_1() -> Generator[None, None, None]:
-    executed.append("fix setup")
-    yield
-    executed.append("fix teardown")
-
-
-class TestFixtureCaching:
-    def test_1(self, fixture_1: None) -> None:
-        assert executed == ["fix setup"]
-
-    def test_2(self, fixture_1: None) -> None:
-        assert executed == ["fix setup"]
-
-
-def test_expected_setup_and_teardown() -> None:
-    assert executed == ["fix setup", "fix teardown"]
-
-
-######
-# Make sure setup & finalization is only run once, with a cached exception
-######
-executed_crash: list[str] = []
-
-
-@pytest.fixture(scope="class")
-def fixture_crash(request: pytest.FixtureRequest) -> None:
-    executed_crash.append("fix_crash setup")
-
-    def my_finalizer() -> None:
-        executed_crash.append("fix_crash teardown")
-
-    request.addfinalizer(my_finalizer)
-
-    raise Exception("foo")
-
-
-class TestFixtureCachingException:
-    @pytest.mark.xfail
-    def test_crash_1(self, fixture_crash: None) -> None:
-        assert False
-
-    @pytest.mark.xfail
-    def test_crash_2(self, fixture_crash: None) -> None:
-        assert False
-
-
-def test_crash_expected_setup_and_teardown() -> None:
-    assert executed_crash == ["fix_crash setup", "fix_crash teardown"]