Confusing `AssertionError` when using `request.getfixturevalue` with new fixture during teardown
With 8.3.3 (as well as the current main, f373974707f57a0b28d12563e4d03c7cd54c70d9), 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:
https://github.com/pytest-dev/pytest/blob/d0f136fe64f9374f18a04562305b178fb380d1ec/src/_pytest/runner.py#L519-L527
This seems similar to what has been reported (and improved) by @petebman here:
- #9984
- #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
fixtrequests it during teardown - That causes pytest to register a new teardown finalizer for
tmp_pathwith 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...
Let's forbid getfixturevalue to trigger setup after the fixture was yielded
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.
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)?
We can start with deprecating and replace it with a exception later