Skip to content

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

Open
bluetech opened this issue Jul 13, 2023 · 3 comments
Open

How should fixtures scopes work for nested packages and classes? #11205

bluetech opened this issue Jul 13, 2023 · 3 comments
Labels
topic: fixtures anything involving fixtures directly or indirectly

Comments

@bluetech
Copy link
Member

We have two collection nodes (and fixture scopes), Package and Class, 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:

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")

The collection tree is:

<Module x.py>
  <Class TestTop>
    <Function test_top>
    <Class TestNested1>
      <Function test_nested1>
    <Class TestNested2>
      <Function test_nested2>

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.

SETUP <Class TestTop>
    top
    nested1
TEARDOWN <Class TestTop>
SETUP <Class TestNested2>
    nested2
TEARDOWN <Class TestNested2>
Technical details why it happens

Each fixture definition is pytest becomes a FixtureDef instance. In the example we have one FixtureDef, for fix.

In the current implementation, the FixtureDef itself holds a fixture's value (cached_value). When a fixture value is requested (SubRequest.getfixturevalue()), if the FixtureDef's cached_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 the cached_value.

In the example:

  • test_top requests fix for the first time, which runs the fixture and schedules to clear fix's cached_result when TestTop is torn down.
  • test_nested1 requests fix. At this point the fix FixtureDef has cached_value set (since TestTop's finalizer hasn't run yet), so it just uses it without rerunning the fixture. Then it schedules a finalizer to clear fix when TestNested1 is torn down.
  • TestNested1 is torn down which clears fix.
  • test_nested2 requests fix. The cached_value is cleared so it reruns the fixture.
  • TestNested2 is torn down, clears fix.
  • TestTop is torn down, wants to clear cached_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.

SETUP <Class TestTop>
    top
    SETUP <Class TestNested1>
        nested1
    TEARDOWN <Class TestNested1>
    SETUP <Class TestNested2>
        nested2
    TEARDOWN <Class TestNested2>
TEARDOWN <Class TestTop>

Option 3 - Independent scope

Nested classes/packages get their own scope, independent of the parent class/package.

SETUP <Class TestTop>
    top
TEARDOWN <Class TestTop>
SETUP <Class TestNested1>
    nested1
TEARDOWN <Class TestNested1>
SETUP <Class TestNested2>
    nested2
TEARDOWN <Class TestNested2>

Option 4 - Shared scope

Nested classes do not have a scope of their own; same scope as the top class/package.

SETUP <Class TestTop>
    top
    nested1
    nested2
TEARDOWN <Class TestNested2>

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.

@nicoddemus
Copy link
Member

nicoddemus commented Jul 13, 2023

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:

SETUP <Class TestTop>
    top
    SETUP <Class TestNested1>
        nested1
    TEARDOWN <Class TestNested1>
    SETUP <Class TestNested2>
        nested2
    TEARDOWN <Class TestNested2>
    bottom
TEARDOWN <Class TestTop>

So 👍 from me for option 2.

@RonnyPfannschmidt
Copy link
Member

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

@JamieC2002
Copy link

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!

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