Skip to content

De-emphaze addfinalizer in favour of ExitStack #5587

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
graingert opened this issue Jul 10, 2019 · 3 comments · Fixed by #5589
Closed

De-emphaze addfinalizer in favour of ExitStack #5587

graingert opened this issue Jul 10, 2019 · 3 comments · Fixed by #5589
Labels
type: proposal proposal for a new feature, often to gather opinions or design the API around the new feature

Comments

@graingert
Copy link
Member

graingert commented Jul 10, 2019

now that py2 support is dropped we should discourage addfinalizer and suggest people make their own ExitStack in a yield fixture

@pytest.fixture
def equipments():
    with contextlib.ExitStack() as exit_stack:
        r = []
        for port in ('C1', 'C3', 'C28'):
            equip = connect(port)
            exit_stack.callback(equip.disconnect)
            r.append(equip)
        yield r
@graingert
Copy link
Member Author

graingert commented Jul 10, 2019

now that py2 support is dropped a handy little fixture that might be worth being built in:

@pytest.fixture
def exit_stack():
    with contextlib.ExitStack() as exit_stack:
        yield exit_stack

can be used as:

def test_foo(exit_stack):
    v = exit_stack.enter_context(cm)
    ...
    some_service = Service()
    exit_stack.callback(some_service.disconnect)

or maybe https://docs.pytest.org/en/latest/reference.html#_pytest.fixtures.FixtureRequest could expose an .exit_stack to be used instead of addfinalizer:

@pytest.fixture
def equipments(request):
    r = []
    for port in ('C1', 'C3', 'C28'):
        equip = connect(port)
        request.exit_stack.callback(equip.disconnect)
        r.append(equip)
    return r

@graingert
Copy link
Member Author

or maybe we should discourage addfinalizer entirely and suggest people make their own ExitStack in a yield fixture?

@pytest.fixture
def equipments():
    with contextlib.ExitStack() as exit_stack:
        r = []
        for port in ('C1', 'C3', 'C28'):
            equip = connect(port)
            exit_stack.callback(equip.disconnect)
            r.append(equip)
        yield r

@graingert graingert changed the title exit_stack builtin fixture addfinalizer vs ExitStack Jul 10, 2019
@graingert graingert changed the title addfinalizer vs ExitStack Deemphase addfinalizer vs ExitStack Jul 10, 2019
@graingert graingert changed the title Deemphase addfinalizer vs ExitStack Deemphase addfinalizer in favour of ExitStack Jul 10, 2019
@graingert graingert changed the title Deemphase addfinalizer in favour of ExitStack De-emphaze addfinalizer in favour of ExitStack Jul 10, 2019
@nicoddemus
Copy link
Member

Not sure, addfinalizer is definitely not going anywhere, but I'm not against adding an example of ExitStack + yield fixture to the docs. 👍

@Zac-HD Zac-HD added the type: proposal proposal for a new feature, often to gather opinions or design the API around the new feature label Jul 12, 2019
graingert added a commit that referenced this issue Jul 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: proposal proposal for a new feature, often to gather opinions or design the API around the new feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants