Fixture with dependencies executes `pytest_fixture_post_finalizer` multiple times

[mbra]

The example below shows that the pytest_fixture_post_finalizer hook is executed n+1 times, where n is the number of fixtures depended upon. From a quick look this seems to be caused by the fact that the FixtureDef.finish method is both registered for every sub-fixture in FixtureDef.execute (

pytest/src/_pytest/fixtures.py

Line 860 in 702acdb

fixturedef.addfinalizer(functools.partial(self.finish, request=request))

) as well as by FixtureRequest._schedule_finalizers (

pytest/src/_pytest/fixtures.py

Line 577 in 702acdb

functools.partial(fixturedef.finish, request=subrequest), subrequest.node

).

Since FixtureDef.finish clears self._finalizers at the end, this does not lead to finalizers being executed multiple times, the hook however is executed in the finally clause every time.

I did not dig far enough into the code to tell what effect registering finish with the fixtures' dependency has. I assume this is probably for a reason and cannot be helped, but it did not feel right to just create a pull request that protects the hook by checking self._finalizers first. Even though I do not know what to do about it, the code looks like someone with a little more in depth knowledge might be able to not only fix the bug but cleanup the way the finalizers are registered a little.

On the other hand, the hook is currently executed even when no finalizer is registered (I used yield in the example but return will produce the exact same result). That might be considered something between a bug and an inconsistency, but perhaps it would be a good idea to check self._finalizers before executing the hook no matter what happens to the rest of the code.

test.py:

import pytest


@pytest.fixture
def lower1():
    yield True


@pytest.fixture
def lower2():
    yield True


@pytest.fixture
def upper(lower1, lower2):
    yield lower1 and lower2


def test(upper):
    assert upper

conftest.py:

import pytest


@pytest.hookimpl(hookwrapper=True)
def pytest_fixture_post_finalizer(fixturedef):
    yield
    print("\n", fixturedef, end="")

$ py.test test.py  -sv
================ test session starts ================  
platform linux -- Python 3.6.8, pytest-5.0.2.dev94+gfd2fb36ea, py-1.8.0, pluggy-0.12.0 -- /home/mvb/.virtualenvs/python3/bin/python3.6
cachedir: .pytest_cache
rootdir: /home/mvb/tmp/tmp.4BCgz52tAn
plugins: cov-2.7.1, httpbin-0.3.0
collected 1 item                                                                                                                                             

test.py::test PASSED
 <FixtureDef argname='upper' scope='function' baseid='test.py'>
 <FixtureDef argname='upper' scope='function' baseid='test.py'>
 <FixtureDef argname='lower2' scope='function' baseid='test.py'>
 <FixtureDef argname='upper' scope='function' baseid='test.py'>
 <FixtureDef argname='lower1' scope='function' baseid='test.py'>
================ 1 passed in 0.01 seconds ================

[mbra]

Mhh, actually just guarding against self._finalizers being empty won't prevent the hook from being executed completely. Changing the fixtures to use return will still execute the hook for lower1 and lower2 because upper registers itself as a finalizer on both, making the check for self._finalizers pass. :-(