Skip to content

Conversation

@aizpurua23a
Copy link
Contributor

@aizpurua23a aizpurua23a commented Jul 26, 2022

Fixes #10023
Add note to clarify the difference in execution order between after-yield finalizers and request.addfinalizer finalizers.

aizpurua23a and others added 2 commits July 26, 2022 21:22
Add note to clarify the difference in execution order between after-yield finalizers and `request.addfinalizer` finalizers.
Copy link
Member

@nicoddemus nicoddemus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot @aizpurua23a! Left a few comments, please take a look.

@pytest.fixture
def fix_w_yield():
yield
print("after_yield_1")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not a good example I think, because of course both prints will be executed sequentially (pytest cannot mess with how Python executes code within a function).

I suggest we use two fixtures:

    @pytest.fixture
    def fix_w_yield1():
        yield
        print("after_yield_1")

    @pytest.fixture
    def fix_w_yield2():
        yield
        print("after_yield_2")

And then make test_bar request both:

    def test_bar(fix_w_yield1, fix_w_yield2):
        print("test_bar")

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I see.
The execution order with the addfinalizer method is evident, so the execution order only needs clarification in this case.
Lovely! How about now?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can keep the examples using addfinalizer as they are useful for comparision, so could you please bring them back? Thanks

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure thing, re-added.

@pytest.fixture
def fix_w_yield():
yield
print("after_yield_1")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can keep the examples using addfinalizer as they are useful for comparision, so could you please bring them back? Thanks

Note on finalizer order
""""""""""""""""""""""""

Finalizers are executed in a first-in-last-out order, while operations after yield are executed sequentially.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

while operations after yield are executed sequentially.

This sentence needs tweaking if we look at the output. 👍

Finalizers are executed in FILO order, that is correct, because we can see that adding fin2 and fin1 we get back fn1, fn2 in the output.

But the same can be said for the yield fixtures: we request them in fix_w_yield1, fix_w_yield2 order, and we get fix_w_yield2 and fix_w_yield1 in the output.

That's because yield fixtures use addfinalizer behind the scenes: when the fixture executes, we register a function with addfinalizer that resumes the generator (which in turn calls the teardown code).

Not sure how to "fix" this sentence, or perhaps we can explain something along the lines of what I wrote above, if that makes things clearer.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that makes sense.
I tried to explain how both examples show the similar FILO behavior, plus added the hidden usage of addfinalizer after the examples.
What do you think?

@nicoddemus
Copy link
Member

Thanks @aizpurua23a!

cc @fish-face

@nicoddemus nicoddemus changed the title Update fixtures.rst w/ finalizer order - closes #10023 Update fixtures.rst w/ finalizer order [SQUASH] Jul 27, 2022
@nicoddemus nicoddemus changed the title Update fixtures.rst w/ finalizer order [SQUASH] Update fixtures.rst w/ finalizer order Aug 15, 2022
@nicoddemus nicoddemus merged commit 7378f35 into pytest-dev:main Aug 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

request.addfinalizer order of execution is undocumented

2 participants