Skip to content
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

Fixture tear-down order broken in 5.3.3 #6512

Open
mschmitzer opened this issue Jan 20, 2020 · 12 comments
Open

Fixture tear-down order broken in 5.3.3 #6512

mschmitzer opened this issue Jan 20, 2020 · 12 comments
Labels
topic: fixtures anything involving fixtures directly or indirectly

Comments

@mschmitzer
Copy link

A change in pytest 5.3.3 (9918093, to be precise) broke fixture teardown for us. What happens is that fixture A depends on fixture B, but B gets torn down before A. This means that B is already torn down when A's tear-down code executes. Both fixtures have the same scope.
Reverting 9918093 on top of 5.3.3 restores the old behavior.

The test suite in question is quite involved. Sadly I haven't been able to create a minimal reproducer so far.

The contents of the virtualenv are here: pip-list.txt.

Pytest Version is 5.3.3, on Ubuntu 18.04.3, Python 3.6.8

@The-Compiler
Copy link
Member

Duplicate of #6492

@The-Compiler The-Compiler marked this as a duplicate of #6492 Jan 20, 2020
@nicoddemus
Copy link
Member

Thanks @MarcSchmitzer, while it does seem like a duplicate, meanwhile if you can get a minimal reproducer it would be great; #6492 is under investigation still, so a reproducer would help a lot.

@blueyed
Copy link
Contributor

blueyed commented Jan 20, 2020

/cc @SalmonMode this also seems not like a real duplicate of #6492. Let's verify/triage this separately for now.

@blueyed blueyed reopened this Jan 20, 2020
@blueyed blueyed added the topic: fixtures anything involving fixtures directly or indirectly label Jan 20, 2020
@SalmonMode
Copy link
Contributor

@MarcSchmitzer if possible, are you able to run pytest --setup-plan with pytest 5.3.2 and pytest 5.3.3 so we can see the difference?

@mschmitzer
Copy link
Author

mschmitzer commented Jan 21, 2020

Plans are here: plan-5.3.2.txt plan-5.3.3.txt. There's a lot of custom fixtures in there, I'm afraid. The relevant change is the second one: The module_conn_pool_factory fixture depends on the connection_service fixture. In the 5.3.3 plan, the service gets torn down first, though.

Edited to add: The plan produced by 5.3.4 is identical to the 5.3.2 plan, by the way.

@mschmitzer
Copy link
Author

I've been trying to pare down the test suite to isolate the problem, and it's really weird. I can make 5.3.3 produce a correct plan by removing test functions even though doing that doesn't change the set of used fixtures. This is so arbitrary, it almost seems like a dictionary order issue. But I'm on Python 3.6 here, so dictionary iteration should be in insertion order, right?

@blueyed
Copy link
Contributor

blueyed commented Jan 21, 2020

@MarcSchmitzer

it almost seems like a dictionary order issue.

That matches what I've seen in pytest-django where the fixture order used in args is not deterministic anymore (flaky) it seems.

But I'm on Python 3.6 here, so dictionary iteration should be in insertion order, right?

Yes.

@mschmitzer
Copy link
Author

mschmitzer commented Jan 21, 2020

dictionary iteration should be in insertion order

But set iteration still appears to be independent of insertion order (but dependent on the hash seed). So maybe that's it..?

@SalmonMode
Copy link
Contributor

@MarcSchmitzer a very interesting example. Thank you! Are you by any chance doing anything with pytest hooks, request.addfinalizer, request.getfixturevalue, or anything out of the ordinary?

@mschmitzer
Copy link
Author

@SalmonMode

Are you by any chance doing anything with pytest hooks, request.addfinalizer, request.getfixturevalue, or anything out of the ordinary?

I have removed the few instances of request.addfinalizer that were left from before we started using yield fixtures, without effect. Apart from that, I'd say we're not doing anything unusual.

@blueyed
Copy link
Contributor

blueyed commented Jan 24, 2020

@MarcSchmitzer can you try #6551, please?

@mschmitzer
Copy link
Author

mschmitzer commented Jan 28, 2020

@blueyed

@MarcSchmitzer can you try #6551, please?

Sorry for the late response. I tried it and it does fix the problem. The fixtures are torn down in the correct order and the test suite passes without problems.

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

5 participants