Confusing AssertionError when using request.getfixturevalue with new fixture during teardown

[The-Compiler]

With 8.3.3 (as well as the current main, f373974), something like

import pytest


def test_stuff(fixt):
    pass


@pytest.fixture
def fixt(request):
    yield
    # e.g. store screenshots on failures for GUI tests
    request.getfixturevalue("tmp_path")

results in a rather confusing internal AssertionError from pytest:

============================= test session starts ==============================
collected 1 item

test_cleanup.py .E                                                       [100%]

==================================== ERRORS ====================================
_______________________ ERROR at teardown of test_stuff ________________________

request = <SubRequest 'fixt' for <Function test_stuff>>

    @pytest.fixture
    def fixt(request):
        yield
        # e.g. store screenshots on failures for GUI tests
>       request.getfixturevalue("tmp_path")

test_cleanup.py:12: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
/usr/lib/python3.12/site-packages/_pytest/fixtures.py:532: in getfixturevalue
    fixturedef = self._get_active_fixturedef(argname)
/usr/lib/python3.12/site-packages/_pytest/fixtures.py:617: in _get_active_fixturedef
    fixturedef.execute(request=subrequest)
/usr/lib/python3.12/site-packages/_pytest/fixtures.py:1094: in execute
    request.node.addfinalizer(finalizer)
/usr/lib/python3.12/site-packages/_pytest/nodes.py:391: in addfinalizer
    self.session._setupstate.addfinalizer(fin, self)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <_pytest.runner.SetupState object at 0x7457a8320470>
finalizer = functools.partial(<bound method FixtureDef.finish of <FixtureDef argname='tmp_path' scope='function' baseid=''>>, request=<SubRequest 'tmp_path' for <Function test_stuff>>)
node = <Function test_stuff>

    def addfinalizer(self, finalizer: Callable[[], object], node: Node) -> None:
        """Attach a finalizer to the given node.
    
        The node must be currently active in the stack.
        """
        assert node and not isinstance(node, tuple)
        assert callable(finalizer)
>       assert node in self.stack, (node, self.stack)
E       AssertionError: (<Function test_stuff>, {})

/usr/lib/python3.12/site-packages/_pytest/runner.py:526: AssertionError
=========================== short test summary info ============================
ERROR test_cleanup.py::test_stuff - AssertionError: (<Function test_stuff>, {})
========================== 1 passed, 1 error in 0.04s ==========================

which is here:

pytest/src/_pytest/runner.py

Lines 519 to 527 in d0f136f

	def addfinalizer(self, finalizer: Callable[[], object], node: Node) -> None:
	"""Attach a finalizer to the given node.
	The node must be currently active in the stack.
	"""
	assert node and not isinstance(node, tuple)
	assert callable(finalizer)
	assert node in self.stack, (node, self.stack)
	self.stack[node][0].append(finalizer)

This seems similar to what has been reported (and improved) by @petebman here:

• request.getfixturevalue crashes in certain cases. #9984
• Improve error message in getfixturevalue #9990

and indeed, also requesting tmp_path from the test (after fixt, so it's torn down first) leads to a much better error message:

AssertionError: The fixture value for "tmp_path" is not available. This can happen when the fixture has already been torn down.

From what I can gather, what happens here instead is:

• The test itself does not use tmp_path
• Then fixt requests it during teardown
• That causes pytest to register a new teardown finalizer for tmp_path with the current test node
• But that test node is already gone because it was just torn down
• Thus causing the AssertionError

---

In the actual context we saw this in qutebrowser, this was made even more difficult to debug:

• For some reason I couldn't track down yet, something in pytest (potentially some __tracebackhide__ in my code?!) decided to just show:

>    fixture = self.request.getfixturevalue('take_x11_screenshot')
E    AssertionError: [...]

without a full traceback.

• Due to parametrization and lots of fixtures involved, the displayed structures were quite big:

AssertionError: (<Function test_sandboxing[disable-seccomp-bpf-True-False-True-You are NOT adequately sandboxed.]>, {<Session  exitstatus=<ExitCode.OK: 0> testsfailed=3 testscollected=3>: ([<bound method Node.teardown of <Session  exitstatus=<ExitCode.OK: 0> testsfailed=3 testscollected=3>>, functools.partial(<bound method FixtureDef.finish of <FixtureDef argname='check_display' scope='session' baseid='tests'>>, request=<SubRequest 'check_display' for <Function test_sandboxing[enable-all-True-True-True-You are adequately sandboxed.]>>), functools.partial(<bound method FixtureDef.finish of <FixtureDef argname='check_yaml_c_exts' scope='session' baseid='tests'>>, request=<SubRequest 'check_yaml_c_exts' for <Function test_sandboxing[enable-all-True-True-True-You are adequately sandboxed.]>>), functools.partial(<bound method FixtureDef.finish of <FixtureDef argname='fail_on_logging' scope='session' baseid='tests'>>, request=<SubRequest 'fail_on_logging' for <Function test_sandboxing[enable-all-True-True-True-You are adequately sandboxed.]>>), functools.partial(<bound method FixtureDef.finish of <FixtureDef argname='qapp_args' scope='session' baseid='tests'>>, request=<SubRequest 'qapp_args' for <Function test_sandboxing[enable-all-True-True-True-You are adequately sandboxed.]>>), functools.partial(<bound method FixtureDef.finish of <FixtureDef argname='qapp_cls' scope='session' baseid=''>>, request=<SubRequest 'qapp_cls' for <Function test_sandboxing[enable-all-True-True-True-You are adequately sandboxed.]>>), functools.partial(<bound method FixtureDef.finish of <FixtureDef argname='pytestconfig' scope='session' baseid=''>>, request=<SubRequest 'pytestconfig' for <Function test_sandboxing[enable-all-True-True-True-You are adequately sandboxed.]>>), functools.partial(<bound method FixtureDef.finish of <FixtureDef argname='qapp' scope='session' baseid=''>>, request=<SubRequest 'qapp' for <Function test_sandboxing[enable-all-True-True-True-You are adequately sandboxed.]>>), functools.partial(<bound method FixtureDef.finish of <FixtureDef argname='qapp' scope='session' baseid='tests'>>, request=<SubRequest 'qapp' for <Function test_sandboxing[enable-all-True-True-True-You are adequately sandboxed.]>>), functools.partial(<bound method FixtureDef.finish of <FixtureDef argname='server' scope='session' baseid='tests/end2end'>>, request=<SubRequest 'server' for <Function test_sandboxing[enable-all-True-True-True-You are adequately sandboxed.]>>), functools.partial(<bound method FixtureDef.finish of <FixtureDef argname='tmp_path_factory' scope='session' baseid=''>>, request=<SubRequest 'tmp_path_factory' for <Function test_sandboxing[enable-all-True-True-True-You are adequately sandboxed.]>>)], None), <Dir work>: ([<bound method Node.teardown of <Dir work>>], None), <Dir tests>: ([<bound method Node.teardown of <Dir tests>>], None), <Dir end2end>: ([<bound method Node.teardown of <Dir end2end>>], None), <Module test_invocations.py>: ([<bound method Node.teardown of <Module test_invocations.py>>], None)})

• It only happening sometimes (rarely), perhaps because the teardown order was different or something...

[RonnyPfannschmidt]

Let's forbid getfixturevalue to trigger setup after the fixture was yielded

[The-Compiler]

As in, make it error out explicitly if we're in the teardown phase and request a fixture that has not already been requested before? Makes sense I'd say.

[The-Compiler]

Oh, though I'm not sure how much existing usage it'd break - evidently there are cases where this kind of thing still seems to work out currently? Maybe we should deprecate it first or something (plus improve the error message if it happens to hard-fail)?

[RonnyPfannschmidt]

We can start with deprecating and replace it with a exception later