cleaned up, DAMPified, and added tests to ensure fixtures only add ther finalizer to a dependee fixture once

Author: SalmonMode

src/_pytest/fixtures.py

@@ -888,16 +888,13 @@ def finish(self, request):
 
     def execute(self, request):
         for argname in self._dependee_fixture_argnames(request):
+            if argname == "request":
+                continue
             fixturedef = request._get_active_fixturedef(argname)
-            if argname != "request":
-                for fin in fixturedef._finalizers:
-                    if "request" in getattr(fin, "keywords", {}):
-                        if self == fin.keywords["request"]._fixturedef:
-                            break
-                else:
-                    fixturedef.addfinalizer(
-                        functools.partial(self.finish, request=request)
-                    )
+            if not self._will_be_finalized_by_fixture(fixturedef):
+                fixturedef.addfinalizer(
+                    functools.partial(self.finish, request=request)
+                )
 
         my_cache_key = self.cache_key(request)
         cached_result = getattr(self, "cached_result", None)
@@ -917,6 +914,25 @@ def execute(self, request):
         hook = self._fixturemanager.session.gethookproxy(request.node.fspath)
         return hook.pytest_fixture_setup(fixturedef=self, request=request)
 
+    def _will_be_finalized_by_fixture(self, fixturedef):
+        """Whether or not this fixture be finalized by the passed fixture.
+
+        Every ``:class:FixtureDef`` keeps a list of all the finishers (tear downs) of
+        other ``:class:FixtureDef`` instances that it should run before running its own.
+        Finishers are added to this list not by this ``:class:FixtureDef``, but by the
+        other ``:class:FixtureDef`` instances. They tell this instance that it's
+        responsible for tearing them down before it tears itself down.
+
+        This method allows a ``:class:FixtureDef`` to check if it has already told
+        another ``:class:FixtureDef`` that the latter ``:class:FixtureDef`` is
+        responsible for tearing down this ``:class:FixtureDef``.
+        """
+        for finalizer in fixturedef._finalizers:
+            if "request" in getattr(finalizer, "keywords", {}):
+                if self == finalizer.keywords["request"]._fixturedef:
+                    return True
+        return False
+
     def _dependee_fixture_argnames(self, request):
         """A list of argnames for fixtures that this fixture depends on.
 

testing/python/fixtures.py

@@ -4374,3 +4374,23 @@ def test_suffix(fix_combined):
     )
     result = testdir.runpytest("-vv", str(p1))
     assert result.ret == 0
+
+
+class TestFinalizerOnlyAddedOnce:
+
+    @pytest.fixture(scope="class", autouse=True)
+    def a(self):
+        pass
+
+    @pytest.fixture(scope="class", autouse=True)
+    def b(self, a):
+        pass
+
+    def test_a_will_finalize_b(self, request):
+        a = request._get_active_fixturedef("a")
+        b = request._get_active_fixturedef("b")
+        assert b._will_be_finalized_by_fixture(a)
+
+    def test_a_only_finishes_one(self, request):
+        a = request._get_active_fixturedef("a")
+        assert len(a._finalizers)