Skip to content

Fixture teardown order incorrect when using getfixturevalue #9151

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Hawk777 opened this issue Oct 1, 2021 · 5 comments
Closed

Fixture teardown order incorrect when using getfixturevalue #9151

Hawk777 opened this issue Oct 1, 2021 · 5 comments
Labels
topic: fixtures anything involving fixtures directly or indirectly

Comments

@Hawk777
Copy link
Contributor

Hawk777 commented Oct 1, 2021

I am running Pytest 6.2.5.

Consider the following test file:

import logging

import pytest


@pytest.fixture(scope="session")
def a():
    logging.info("A: setup")
    yield None
    logging.info("A: cleanup")


@pytest.fixture(scope="session")
def b(request):
    logging.info("B: setup")
    request.getfixturevalue("a")
    yield None
    logging.info("B: cleanup")


@pytest.fixture(scope="session")
def c(request, b):
    logging.info("C: setup")


def test_foo(c, a):
    logging.info("In test_foo")

def test_bar(c, a):
    logging.info("In test_bar")

When run with py.test -s -v -v --log-level=info --log-cli-level=info it produces the following output:

==================================================================================== test session starts =====================================================================================
platform linux -- Python 3.9.6, pytest-6.2.5, py-1.10.0, pluggy-0.13.1 -- /usr/bin/python3.9
cachedir: .pytest_cache
rootdir: /tmp/foo
plugins: pkgcore-0.12.7
collected 2 items                                                                                                                                                                            

test_stuff.py::test_foo 
--------------------------------------------------------------------------------------- live log setup ---------------------------------------------------------------------------------------
INFO     root:test_stuff.py:15 B: setup
INFO     root:test_stuff.py:8 A: setup
INFO     root:test_stuff.py:23 C: setup
--------------------------------------------------------------------------------------- live log call ----------------------------------------------------------------------------------------
INFO     root:test_stuff.py:27 In test_foo
PASSED
test_stuff.py::test_bar 
--------------------------------------------------------------------------------------- live log call ----------------------------------------------------------------------------------------
INFO     root:test_stuff.py:30 In test_bar
PASSED
------------------------------------------------------------------------------------- live log teardown --------------------------------------------------------------------------------------
INFO     root:test_stuff.py:10 A: cleanup
INFO     root:test_stuff.py:18 B: cleanup


===================================================================================== 2 passed in 0.03s ======================================================================================

This is wrong: fixture B depends on fixture A via request.getfixturevalue, and therefore fixture B should be cleaned up before fixture A.

This can’t be a duplicate of GH-1895 because that ticket was fixed in 4.4.0, and this still happens in 6.2.5. I don’t think it’s a duplicate of GH-6512 either, because that ticket says that 5.3.2 was OK and 5.3.3 was broken; however, my issue posted here shows up in both 5.3.2 and also 5.3.3.

$ pip list
Package        Version
-------------- -------
attrs          21.2.0
iniconfig      1.1.1
more-itertools 8.10.0
packaging      21.0
pip            21.1.3
pluggy         0.13.1
py             1.10.0
pyparsing      2.4.7
pytest         6.2.5
setuptools     56.0.0
toml           0.10.2
wcwidth        0.2.5
@Hawk777
Copy link
Contributor Author

Hawk777 commented Oct 1, 2021

Observations:

  • It is necessary to have two test functions. If either test_foo or test_bar is deleted, the cleanup order changes to the proper BA.
  • If the order of parameters to test_foo is swapped, the order of fixture setups changes from BAC to ABC, but the order of cleanups remains wrong.
  • If the order of parameters to test_bar is swapped, the order of fixture setups remains the same at BAC, but the order of cleanups is swapped (and therefore correct).

@Zac-HD Zac-HD added the topic: fixtures anything involving fixtures directly or indirectly label Oct 3, 2021
@h3ndrk
Copy link

h3ndrk commented Feb 22, 2023

I'm not sure but maybe the request.getfixturevalue() belongs to the "pre"-setup of your fixture instead of the setup-"body" of your fixture? In your example, you would need to delay the logging:

 @pytest.fixture(scope="session")
 def b(request):
-    logging.info("B: setup")
     request.getfixturevalue("a")
+    logging.info("B: setup")
     yield None
     logging.info("B: cleanup")

@RonnyPfannschmidt
Copy link
Member

closing as behaving as intended even if non-intuitive

your quirky roundabout setup triggers the setup of B before a, then A gets requested by b dynamically

this in the end enforces that A is added to the setup state after b, and thus has to be taken down before b

@Hawk777
Copy link
Contributor Author

Hawk777 commented Feb 24, 2023

OK, thanks for clarifying. Do you think this is something that should be called out explicitly in the documentation?

Here, is the text:

Once pytest figures out a linear order for the fixtures, it will run each one up until it returns or yields, and then move on to the next fixture in the list to do the same thing.

Once the test is finished, pytest will go back down the list of fixtures, but in the reverse order, taking each one that yielded, and running the code inside it that was after the yield statement.

Then here is the text:

When a fixture requests another fixture, the other fixture is executed first. So if fixture a requests fixture b, fixture b will execute first, because a depends on b and can’t operate without it. Even if a doesn’t need the result of b, it can still request b if it needs to make sure it is executed after b.

Neither of these mention request.getfixturevalue explicitly. I interpreted the situation as that request.getfixturevalue adds a dependency edge from the caller to the callee; therefore since one requests another (via getfixturevalue) then that means the interpretation of which “executed first” is according to the second quoted block of text (the caller depends on the callee, therefore the callee is considered to have executed first, although in reality the execution is nested when using request.getfixturevalue); then, according to the first quoted block, since the callee executed first, it should be torn down last.

It sounds like that is not, and is not intended to be, the case. Which piece of logic is incorrect? Is it that request.getfixturevalue should not be considered to add a dependency edge, and should not be considered to “request” the value of the callee in the linear ordering sense?

I see there is this text on the reference for request.getfixturevalue itself:

This method can be used during the test setup phase or the test run phase, but during the test teardown phase a fixture’s value may not be available.

However I interpreted that to mean that you couldn’t call request.getfixturevalue during teardown, not that a previously obtained value might have been invalidated by teardown time.

I’m happy to put together a PR against the documentation if you think it’s worthwhile, once I understand what meaning was intended (I don’t want to overreach and write documentation that promises more or less than was intended).

@RonnyPfannschmidt
Copy link
Member

I'll have to get back at his later, I'll have to make time to dig into exact terminology

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: fixtures anything involving fixtures directly or indirectly
Projects
None yet
Development

No branches or pull requests

4 participants