-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
How should fixtures scopes work for nested packages and classes? #11205
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
Comments
Hi @bluetech, Thanks for writing this up. I agree option 2 is more natural and makes more sense: I expect that a class-scoped fixture to have a class-scope, regardless if it is nested or not, while also expecting it to match the collection tree. I think this is even more obvious if we expand the example a bit to: import pytest
@pytest.fixture(scope="class")
def fix(request):
print("SETUP", request.node)
yield
print("TEARDOWN", request.node)
class TestTop:
def test_top(self, fix):
print("top")
class TestNested1:
def test_nested1(self, fix):
print("nested1")
class TestNested2:
def test_nested2(self, fix):
print("nested2")
def test_bottom(self, fix):
print("bottom") I would expect this to agree with option 2, with this output:
So 👍 from me for option 2. |
I also like option 2 I do see some pain points wrt fixture definition place vs scopes in practice It may be necessary to help fixture definitions to pick the scope/nesting in more detail I Hope I can find some time later for a example |
Hi, I am interested in taking on this issue, because I have a class where my team is tasked to contribute to an open source project. I wanted to ask if there is any advice on where to look/ how my team can start taking on this issue. Thanks! |
We have two collection nodes (and fixture scopes),
Package
andClass
, which may be nested. I'll use classes for example because it's easier to discuss (single file) but it's the same for packages.Consider the following:
The collection tree is:
What should be printed?
(I'm adding some indentation to the outputs to make them clearer).
Option 1 - Current behavior
The current behavior is the following.
Technical details why it happens
Each fixture definition is pytest becomes a
FixtureDef
instance. In the example we have oneFixtureDef
, forfix
.In the current implementation, the
FixtureDef
itself holds a fixture's value (cached_value
). When a fixture value is requested (SubRequest.getfixturevalue()
), if theFixtureDef
'scached_value
is not already set, it calculates it by running the fixture function. Then it schedules a finalizer on the fixture request's node (request.node
) to clear thecached_value
.In the example:
test_top
requestsfix
for the first time, which runs the fixture and schedules to clearfix
'scached_result
whenTestTop
is torn down.test_nested1
requestsfix
. At this point thefix
FixtureDef
hascached_value
set (sinceTestTop
's finalizer hasn't run yet), so it just uses it without rerunning the fixture. Then it schedules a finalizer to clearfix
whenTestNested1
is torn down.TestNested1
is torn down which clearsfix
.test_nested2
requestsfix
. Thecached_value
is cleared so it reruns the fixture.TestNested2
is torn down, clearsfix
.TestTop
is torn down, wants to clearcached_value
, but it is already cleared, goes 🤷 and continues.Option 2 - Nested scope
Nested classes/packages get their own scope, nested within the parent class/package scope.
Option 3 - Independent scope
Nested classes/packages get their own scope, independent of the parent class/package.
Option 4 - Shared scope
Nested classes do not have a scope of their own; same scope as the top class/package.
My opinion
Option 1 seems broken to me, I don't think there's any case to be made for it.
Option 2 is my preference, I think it's the most coherent with the collection tree, lexical/filesystem layout and is pretty intuitive, for both packages and classes.
Option 3 I think is not intuitive, i.e. I think most would not expect fixtures to behave this way. It imposes some restrictions on tests ordering between the class and nested. Note that currently due to #7777, Packages work like this in practice.
Option 4 would be my second choice, it does make sense logically (that what if there is no
test_top
?), but I think it's less intuitive than Option 2.I can write more but don't want to make this too long.
The text was updated successfully, but these errors were encountered: