Track exceptions when setting up and finalizing fixtures

[ShurikMen]

There is a task to track exceptions when setting up and finalizing fixtures.
When setting up, an exception can be obtained in the pytest_fixture_setup hook:

@pytest.hookimpl(hookwrapper=True)
def pytest_fixture_setup(fixturedef):
    result = yield
    if result.exception:
        print('SETUP EXCEPTION in {0}: {1}'.format(fixturedef.argname, result.exception))

But at finalizing, you can't just get a list of exceptions of all finalizers that have been executed. Because there is no analog of the result as in the pytest_fixture_setup hook.
I propose to finalize the pytest_fixture_post_finalizer specification and an additional argument that will contain the exclusion(s) of all fixture finalizers.

src/_pytest/hookspec.py:

def pytest_fixture_post_finalizer(
    fixturedef: "FixtureDef[Any]", request: "SubRequest", exception: "BaseException | None"
) -> None:

src/_pytest/fixtures.py::FixtureDef::finish

        node = request.node
        if len(exceptions) == 1:
            final_exception = exceptions[0]
        elif len(exceptions) > 1:
            msg = f'errors while tearing down fixture "{self.argname}" of {node}'
            final_exception = BaseExceptionGroup(msg, exceptions[::-1])
        else:
            final_exception = None
        node.ihook.pytest_fixture_post_finalizer(fixturedef=self, request=request, exception=final_exception)
        # Even if finalization fails, we invalidate the cached fixture
        # value and remove all finalizers because they may be bound methods
        # which will keep instances alive.
        self.cached_result = None
        self._finalizers.clear()
        if final_exception:
            raise final_exception

[ShurikMen]

Probably a more correct solution would be to transfer the completion of the fixtures to an independent hook pytest_fixture_teardown.

[bluetech]

I agree that the problem described in the issue is worth solving.

To me the original idea of passing the exception(s) to pytest_fixture_post_finalizer seems better than adding a new pytest_fixture_teardown hook:

• Adding a hook adds complexity and so should be done judiciously when there is an alternative

• This hook is in a hot path so adds some performance overhead

• The pytest_fixture_teardown implementation accesses a bunch of private attributes of FixtureDef, such that a plugin would never be able to sanely outright replace the implementation (at least in a stable, sanctioned way). So the hook would only be useful as a hookwrapper or as a lifecycle hook. But pytest_fixture_post_finalizer already fulfills that need.

• Finally, moving the code over to pytest_fixture_teardown breaks the implementation symmetry between execute and finish which feels wrong to me.

Therefore my preference would be to add the exception/ExceptionGroup to pytest_fixture_post_finalizer, unless there is another use case for pytest_fixture_teardown.

[ShurikMen]

I agree that the problem described in the issue is worth solving.

To me the original idea of passing the exception(s) to pytest_fixture_post_finalizer seems better than adding a new pytest_fixture_teardown hook:

• Adding a hook adds complexity and so should be done judiciously when there is an alternative
• This hook is in a hot path so adds some performance overhead
• The pytest_fixture_teardown implementation accesses a bunch of private attributes of FixtureDef, such that a plugin would never be able to sanely outright replace the implementation (at least in a stable, sanctioned way). So the hook would only be useful as a hookwrapper or as a lifecycle hook. But pytest_fixture_post_finalizer already fulfills that need.
• Finally, moving the code over to pytest_fixture_teardown breaks the implementation symmetry between execute and finish which feels wrong to me.

Therefore my preference would be to add the exception/ExceptionGroup to pytest_fixture_post_finalizer, unless there is another use case for pytest_fixture_teardown.

pytest_fixture_post_finalizer does not track full lifecycle of teardown fixtures. It only notifies of the fact that all finalizers in teardown functions have been completed.
For this reason, there is currently no symmetry between execute and finish, because the execution of the fixture setup code itself is not performed in the execute itself, the setup function code itself is executed in the pytest_fixture_setup hook. execute calls this hook if it has not found the result in the cache.

Therefore, I moved the execution of the finalizers function code to the pytest_fixture_teardown hook, similar to pytest_fixture_setup. It seemed like a better solution to me.

Now I use this hook to track the status of the test object before executing the teardown function and to verify that nothing happened to this object during code execution. Because in case of some errors, further execution of other fixtures and tests is impossible.

    @pytest.hookimpl(hookwrapper=True)
    def pytest_fixture_setup(self, fixturedef: FixtureDef, request: pytest.FixtureRequest) -> Generator[None]:
        # if fixture is not param
        if hasattr(request, 'module') and fixturedef.func.__name__ != 'get_direct_param_fixture_func':
            # check full stack state, before run setup function
            self._check_stack(request.module)
            # add markers to stdout output
            self._mark_state_in_dev_stdout(fixturedef.argname, 'start fixture setup', request.module)
        result = yield
        # if we have ProtocolException after fixture setup, then check and restore stand state
        if result.exception and isinstance(result.exception, ProtocolException):
            self._check_stand_state(request.module)

    @pytest.hookimpl(hookwrapper=True)
    def pytest_fixture_teardown(self, fixturedef: FixtureDef, request: pytest.FixtureRequest) -> Generator[None]:
        # if fixture is not param and have finilizers
        if fixturedef._finalizers and fixturedef.func.__name__ != 'get_direct_param_fixture_func' and hasattr(request, 'module'):
            # check full stack state, before run teardown functions
            self._check_stack(request.module)
            # add markers to stdout output
            self._mark_state_in_dev_stdout(fixturedef.argname, 'start fixture teardown', request.module)
        result = yield
        # if we have exeption after fixture teardown, then check and restore stand state
        if result.exception:
            try:
                raise result.exception
            except *ProtocolException:
                self._check_stand_state(request.module)
            except *BaseException:
                pass

[rplevka]

pytest_fixture_post_finalizer does not track full lifecycle of teardown fixtures. It only notifies of the fact that all finalizers in teardown functions have been completed.
For this reason, there is currently no symmetry between execute and finish, because the execution of the fixture setup code itself is not performed in the execute itself, the setup function code itself is executed in the pytest_fixture_setup hook. execute calls this hook if it has not found the result in the cache.

I agree with this. Our use case is quite simple - we'd like to act upon start of the tear-down part of the fixture (set appropriate logging context for the external logging solution). post-finalizer hook runs code after the tear down is finished (too late for us)

[rplevka]

@bluetech have you had any chance to revisit this, please?