python-trio / pytest-trio

Pytest plugin for trio
Other
53 stars 24 forks source link

Design discussion: support higher-scoped (class/module/session level) fixtures? #89

Open oremanj opened 4 years ago

oremanj commented 4 years ago

I'm pretty sure it's possible if we're willing to use greenlet.

Here's a sketch that's missing important details like exception propagation.

Hook pytest_runtest_loop to keep a Trio event loop running in another greenlet for the whole pytest invocation:

async def _manage_fixture(teardown_evt, done_evt, coroutine_or_agen, *, task_status):
    teardown = False
    if not hasattr(coroutine_or_agen, "asend"):
        # fixture with a return statement
        result = await coroutine_or_agen
    else:
        # fixture with a yield statement
        result = await coroutine_or_agen.asend(None)
        teardown = True
    task_status.started(result)
    if teardown:
        try:
            await coroutine_or_agen.asend(None)
        except StopAsyncIteration:
            pass
        else:
            raise RuntimeError("too many yields in fixture")
    done_evt.set()

async def _pytest_trio_main(pytest_greenlet):
    async with trio.open_nursery() as nursery:
        fixtures = {}
        next_msg = "ready"
        while True:
            request = pytest_greenlet.switch(next_msg)
            if request[0] == "setup":
                fixtures[request[1]] = (teardown_evt, done_evt) = (trio.Event(), trio.Event())
                next_switch = await nursery.start(_manage_fixture, teardown_evt, done_evt, request[1])
            elif request[0] == "teardown":
                teardown_evt, done_evt = fixtures.pop(request[1])
                teardown_evt.set()
                await done_evt.wait()
                next_switch = None
            elif request[0] == "run":
                next_switch = await request[1]
            else:
                assert request[0] == "exit"
                return "done"

@pytest.hookimpl(hookwrapper=True)
def pytest_runtestloop(session):
    pytest_greenlet = greenlet.getcurrent()
    session._pytest_trio_trio_greenlet = pytest_greenlet  # or somewhere better
    trio_greenlet = greenlet.greenlet(trio.run)
    first_send = trio_greenlet.switch(_pytest_trio_main, pytest_greenlet)
    assert first_send == "ready"
    yield
    del session._pytest_trio_trio_greenlet
    assert trio_greenlet.switch("exit") == "done"

Hook fixture setup to run async fixtures in the Trio greenlet:

@pytest.hookimpl(hookwrapper=True)
def pytest_fixture_setup(fixturedef, request):
    wrapped_result = yield
    result = wrapped_result.get_result()
    if not hasattr(result, "__await__") and not hasattr(result, "__aiter__"):
        return
    trio_greenlet = request.session._pytest_trio_trio_greenlet
    true_result = trio_greenlet.switch("setup", result)
    if hasattr(result, "__aiter__"):
        request.addfinalizer(partial(trio_greenlet.switch, "teardown", result))
    wrapped_result.force_result(true_result)
    my_cache_key = fixturedef.cache_key(request)
    fixturedef.cached_result[my_cache_key] = (true_result, my_cache_key, None)

Hook running-the-test to run async tests in the Trio greenlet (not shown).

The details are decidedly nontrivial, but I don't see any showstoppers... thoughts? Maybe this could totally replace pytest-trio's current strategy, but it's not a win across the board, so I'm imagining it more as an alternative mode.

Benefits:

Drawbacks:

touilleMan commented 3 years ago

Using the same trio loop with different tests is a very slippy slope (typically with the current nursery fixture we can endup with a test spawning coroutines that will outlive it)

Adding greenlet to the mix is a whole new layer of complexity, pytest-trio is already far from trivial so this seems like another big red flag.

I think the best way to implement higher level fixture is to use them to start a separate trio loop within a thread. The two isolated trio loops (one for the class/module/session fixture, one for the test) is a double edged sword: you have to do extra work if you want to synchronize the two loop, BUT it also mean you cannot break stuff due to subtle interactions between coroutines. So I guess we shouldn't even add this feature to the pytest-trio library given the user has to understand the limitations of the class/module/session fixture. The best we could do would be to add a recipe in the documentation.

tomprince commented 3 years ago

I was investigating using pytest-trio to write some integration tests, where I have some expensive services to setup, so want to have session scoped fixtures for the services. I also want to collect logs from the services as they run, and sometimes restart those services from the tests. It is definitely less appealing if I need to manage my own loop for anything session scoped, or that interacts with code from the session scoped things.

In terms of implementation, it looks like how trio is structured to support guest mode could be used to support suspending and resuming the loop. With the current public API, you could have one long-running trio loop, which is what is exposed to users; then, when running a async test or fixture, iterate the host-loop calls (perhaps using a separate trio loop). This would require at least a little glue to capture the guest calls to run_sync_soon_threadsafe and pass them to the current host loop, or batch them until it starts. Or, with access to internals, or a new API, the loop could be suspended after each fixture or callback is completed.

It is definitely true that you can't use the current nursery fixture from non-function scoped fixtures; the same is true of pytest's temp_path fixture, which is why there is the temp_path_factory fixture for use from those.

svermeulen commented 3 years ago

I'm unclear on what the danger is in using the same trio loop for both session and module fixtures in addition to just function fixtures. I don't understand why we couldn't have a session-scope nursery, a module-scope nursery under that, then the function-level nursery under that, then automatically cancel the nurseries when the module is complete and at the end of the session, in the same way the function level nursery is already cancelled. Is it somehow possible for an async method to outlive the nursery it was spawned in?

jmehnle commented 1 month ago

I wanted to chime in here to emphasize that the lack of support for session/module/class-scoped async fixtures is a serious limitation for us. We're writing unit tests to exercise code accessing a PostgreSQL database using the asyncpg package, and we're trying to spin up a Docker container with PostgreSQL that the tests can run against. Obviously spinning up a Docker container and tearing it down isn't something you can afford to do for every single database test. In a previous life I used the synchronous psycopg package, and defining a session-scoped fixture to manage a database container worked beautifully, but now I've spent the last two days trying to find a way to work around this pytest-trio limitation, to no avail.

I know that pytest-trio advertises its ability to run async fixtures concurrently as a feature, but I would argue that if that feature had to be weighed against support for session-scoped fixtures, the benefit of session-scoped fixtures would almost certainly weigh a lot more heavily from any practical point of view given how much test time can be saved with setting up expensive resources only once vs. many times.

I might be willing to donate some effort to make session/module/class-scoped async fixtures in pytest-trio a reality, but despite digging rather deeply and reading the above discussion I still don't have a good understanding of what the fundamental blocker is (though I do agree that introducing a dependency on greenlet seems wrong). My vague notion is that pytest-trio effectively runs a fresh loop for every async test it handles, so there's no long-lived loop that session-scoped fixtures could setup under, be suspended (yielded) under, and ultimately tear down under. Can you help me understand this better?

oremanj commented 1 month ago

@jmehnle The basic difficulty is that all of pytest's internal test runner logic is synchronous code, so we can't easily open an event loop "around" the test runner and run tests inside of it, because synchronous functions can't do a blocking call into an enclosing async context. However, we now have two technologies that we didn't have when this issue was first opened, either of which might be able to bridge that gap:

Of these I think guest mode is probably a better fit for pytest, and it also avoids the greenlet dependency.

Higher-scoped fixtures require a single Trio run that wraps all tests in their scope. This is incompatible with current popular use of fixtures like autojump_clock, which require that each test have its own Trio run so some of them can use the autojump clock while others don't (the clock must be known when the run starts and can't be swapped out partway). So I think this might actually want to be a separate package from pytest-trio (maybe pytest-xtrio?). Or pytest-trio could support both approaches, but it would need to maintain almost completely independent code for each approach.

To solve your immediate problem, it is probably going to be much much easier to use a synchronous session-level fixture that starts a thread and runs the container from the thread.

jmehnle commented 1 month ago

Thanks for the great overview. I will ponder this in the coming weeks.

FWIW, and for anyone having a similar problem, I have worked around the limitation by using a synchronous session-scoped fixture that spins up the database container and talks to the database using a synchronous database driver to set up the DB, in deviation from the asynchronous driver we use in application code. This is pretty ugly, and I'd still very much love to get rid of this pile of dirt.