Cannot test for correct cancellation handling behaviour inside a fixture

[basak]

A pattern I've been using is handing a class instance a nursery when it is constructed, so that it can then run background tasks. Things that are working well:

• This pattern.
• In one case a background task performing cleanup by handling trio.Cancelled.
• I have a fixture that constructs a specialised instance of such a class for test purposes, and am testing a bunch of class functionality effectively.

However, I'm struggling to write a test that uses the fixture, cancels the nursery given to the class instances, and asserts that the cleanup was performed correctly. The docs say:

So what happens if the yield gets cancelled?

First, pytest-trio assumes that something has gone wrong and there’s no point in continuing the test. If the top-level test function is running, then it cancels it.

...and this is what is biting me here. Here's a reduced example:

import pytest
import pytest_trio
import trio


class Foo:
    def __init__(self, nursery):
        self.nursery = nursery
        self.handled_cancellation = False
        nursery.start_soon(self.foo)

    async def foo(self):
        try:
            await trio.sleep_forever()
        except trio.Cancelled:
            self.handled_cancellation = True


@pytest_trio.trio_fixture
async def foo_fixture():
    async with trio.open_nursery() as nursery:
        yield Foo(nursery)

@pytest.mark.trio
async def test_foo(foo_fixture):
    foo_fixture.nursery.cancel_scope.cancel()

    # This line seems to cause the test to terminate
    await trio.testing.wait_all_tasks_blocked()

    assert foo_fixture.handled_cancellation  # this assert never runs
    assert False  # if the test passes then this line definitely never ran

I notice that in newer releases, the test fails with RuntimeError: <fixture 'foo_fixture'> cancelled the test but didn't raise an error which is nice (in older releases the test silently passes).

The behaviour is therefore as documented, but that leads me to the problem: how do I test cancellation behaviour in an object created by a fixture?

Workaround: I can copy the fixture code into a test. But this seems ugly as it requires duplication, precluding code reuse.

[njsmith]

If you put the nursery into a fixture, then you're scoping the entire test to run inside that nursery. So your test isn't just cancelling your Foo's background worker; you're also cancelling your test itself.

I suppose if you really wanted to, you could have the fixture open a nursery + spawn a child task, then have that child task open a second nursery and put the Foo object into the second nursery. Then cancelling the second nursery wouldn't cancel the test itself. I feel like just putting the nursery inside the test might be easier though :-).

[basak]

On Fri, Apr 09, 2021 at 07:44:15AM -0700, Nathaniel J. Smith wrote: If you put the nursery into a fixture, then you're scoping the entire test to run inside that nursery. So your test isn't just cancelling your `Foo`'s background worker; you're also cancelling your test itself.

Thanks. I didn't realise that's what was happening.

I suppose if you really wanted to, you could have the fixture open a nursery + spawn a child task, then have that child task open a *second* nursery and put the `Foo` object into the second nursery. Then cancelling the second nursery wouldn't cancel the test itself. I feel like just putting the nursery inside the test might be easier though :-).

Yes - I'll just put the ~test inside the nursery~ nursery inside the test [!] :) Wouldn't you consider this surprising behaviour though? Ideally, a fixture would be able to create a nursery that a test _could_ cancel without cancelling the test, no? I wonder if it would be possible to put the logic you describe into a decorator and ship that as part of pytest-trio, or similar.

[njsmith]

You could certainly make a fixture that implemented that nested nursery logic I mentioned. I'm not sure how if it would be very popular in general, though? I've never found it to onerous to write async with open_nursery() as nursery: inside my tests when I wanted a more narrowly scoped nursery... it's only one line :-). And when I'm testing cancellation I personally like to write an explicit cancel scope inside the test, so the test body explicitly shows what's being cancelled and what isn't.

BTW, the advantage of the test being inside the nursery is that if some background task running in the nursery raises an unhandled exception, you want that to cancel the test and give you a test failure. I guess you'd still have that property even with the double-nested-nursery trick, because an unhandled exception will propagate out and cancel both nurseries.

[basak]

I've never found it to onerous to write async with open_nursery() as nursery: inside my tests when I wanted a more narrowly scoped nursery... it's only one line :-)

The reason is that I'm putting complexity into a fixture (setting up fakes, etc). For tests that don't test cancellation, it's then really convenient to use that fixture (this being, AIUI, the entire purpose of fixtures). Because the fixture sets up first, I can't both create a nursery in the test and use the fixture (with that nursery). This is why I'm having my fixture create the nursery - or in the common case, I can just use the built-in nursery fixture, as I'm never cancelling it so it is safe to use that one. But then I have the odd test to test cancellation behaviour, and then I can't use the fixture I already have. I have to duplicate what that fixture does in the test.

I suppose I could factor out most of what the fixture does into a separate function that creates the nursery and sets up the fakes, etc. Then a fixture (that creates the nursery) could call that for use in my common non-cancellation case, and my cancellation behaviour test could avoid using the fixture and use my factored out function directly as an exception, setting up its own nursery. This is probably what I'll end up doing for now.