Refactor fixture finalization

[nicoddemus]

Currently for each fixture which depends on another fixture, a "finalizer" is added to the list of the dependent fixture.

For example:

def test(tmpdir):
   pass

tmpdir, as we know, depends on tmpdir_path to create the temporary directory. Each tmpdir invocation ends up adding its finalization to the list of finalizers of tmpdir_path. This is the mechanism used to finalize fixtures in the correct order (as thought we still have bugs in this area, as #1895 shows for example), and ensures that every tmpdir will be destroyed before the requested tmpdir_path fixture.

This then means that every high-scoped fixture might contain dozens, hundreds or thousands of "finalizers" attached to them. Fixture finalizers can be called multiple times without problems, but this consumes memory: each finalizer keeps its SubRequest object alive, containing a number of small variables:

pytest/src/_pytest/fixtures.py

Lines 341 to 359 in ed68fcf

	class FixtureRequest(FuncargnamesCompatAttr):
	""" A request for a fixture from a test or fixture function.
	A request object gives access to the requesting test context
	and has an optional ``param`` attribute in case
	the fixture is parametrized indirectly.
	"""
	def __init__(self, pyfuncitem):
	self._pyfuncitem = pyfuncitem
	#: fixture for which this request is being performed
	self.fixturename = None
	#: Scope string, one of "function", "class", "module", "session"
	self.scope = "function"
	self._fixture_defs = {} # argname -> FixtureDef
	fixtureinfo = pyfuncitem._fixtureinfo
	self._arg2fixturedefs = fixtureinfo.name2fixturedefs.copy()
	self._arg2index = {}
	self._fixturemanager = pyfuncitem.session._fixturemanager

This can easily be demonstrated by applying this patch:

diff --git a/src/_pytest/runner.py b/src/_pytest/runner.py
index 55dcd805..b3a94bc6 100644
--- a/src/_pytest/runner.py
+++ b/src/_pytest/runner.py
@@ -393,6 +393,8 @@ class SetupState(object):
         for col in self.stack:
             if hasattr(col, "_prepare_exc"):
                 six.reraise(*col._prepare_exc)
+        if self.stack:
+            print(len(self._finalizers.get(self.stack[0])))
         for col in needed_collectors[len(self.stack) :]:
             self.stack.append(col)
             try:

(this prints the finalizers attached to the "Session" node, where the session fixtures attach their finalization to)

And running this test:

import pytest

@pytest.mark.parametrize('i', range(10))
def test(i, tmpdir):
    pass

λ pytest .tmp\test-foo.py -qs
.1
.2
.3
.4
.5
.6
.7
.8
.9
.
10 passed in 0.16 seconds

I believe we can think of ways to refactor the fixture teardown mechanism to avoid this accumulation of finalizers on the objects. Ideally we should build a proper DAG of fixture dependencies which should be destroyed in the proper order. This would also make things more explicit and easier to follow IMHO.

[RonnyPfannschmidt]

we should set up a full topology to manage setup/teardown

[Sup3rGeo]

I was reading the comments on #4055 where it seemed that no changes would be considered in the short term because of the risk of breaking tests in the wild.

Does opening this issue mean that this is now ripe enough for design and eventual implementation?

[nicoddemus]

Does opening this issue mean that this is now ripe enough for design and eventual implementation?

Yep, just yesterday @RonnyPfannschmidt and I were chatting about this. It will be a gradual change but will eventually make the entire fixture execution model much more explicit and easier to follow than what we have today.

We plan to tackle this when he gets back from his hiatus.

[Sup3rGeo]

Awesome, looking forward to it!

I guess it might also help #5460 (#3551 I believe was also using a directed graph approach for fixture dependency/execution).

[afrobbins805]

Hi All, I made a PR #4055 for this issue over a year ago. One of the primary concerns that was made about the PR was that it might break plugins. Most of the major external plugins already have their own test suites. Would integrating the test suites of external plugins into the pytest test suite alleviate the current impasse? It seems like this might be a worthy task anyway to give confidence in any change to core code.

[nicoddemus]

Would integrating the test suites of external plugins into the pytest test suite alleviate the current impasse?

We have thought about this, but it is kind of tricky: external test suites are outside of our control, so they might break for unrelated reasons, which would then break pytest. We could of course devise a way for those breakages to not block PRs and such, though.

[nicoddemus]

#7511 touches on one of the problems highlighted here.