Clarify docs about requesting the event_loop fixture

[tuukkamustonen]

https://pytest-asyncio.readthedocs.io/en/latest/reference/fixtures/index.html#event-loop suggests that it's fine to grab event_loop fixture in a non-async function.

0.21+ migration guide in https://pytest-asyncio.readthedocs.io/en/latest/how-to-guides/migrate_from_0_21.html suggests to convert all sync functions requesting the event_loop fixture into async functions, and then acquire the loop via asyncio.get_running_loop().

Which one is it, and what is the rationale of not depending on the event_loop fixture directly?

[seifertm]

The use of event_loop is deprecated. We should adjust the docs to reflect that.
The rationale is summarized here:
#706 (comment)

[tuukkamustonen]

Slowly getting back here.

The thing why I'm asking is that currently we do something like:

def some_fixture(event_loop):
    # do some sync stuff etc...
    # then trigger async actions:
    event_loop.run_until_complete(...)

Similar pattern is not possible with asyncio.get_running_loop() because:

This function can only be called from a coroutine or a callback.

Being able to run invocations in the event loop in a sync function is convenient, as we don't have 100% async codebase, and tests are mostly written in sync.

Do you see the use case here @seifertm? Is the advice to convert the usages to async and then asyncio.to_thread() to sync parts, instead?

[seifertm]

The migration guide suggests converting such tests to async tests and use get_running_loop inside the async test. Following your example:

@pytest_asyncio.fixture()
async def some_fixture():
    # do some sync stuff etc...
    # then trigger async actions:
    event_loop = asyncio.get_running_loop()
    event_loop.run_until_complete(...)

You could possibly even skip asyncio.get_running_loop() in favour of:

@pytest_asyncio.fixture()
async def some_fixture():
    # do some sync stuff etc...
    # then trigger async actions:
    await ...

@tuukkamustonen Do any of those approaches work for your code base?

[tuukkamustonen]

@seifertm Hey hey, sorry I totally missed your response (in December!)

The problem was that earlier it was possible to inject the loop into a sync test, e.g.:

def test_something(event_loop):
    event_loop.run_until_complete(...)

But with the fixture deprecated, you're no longer supposed to do that.

Unless I define my own fixture:

@pytest.fixture
async def event_loop():
    return asyncio.get_running_loop()

def test_something(event_loop):
    event_loop.run_until_complete(...)

Didn't try that, but I guess it would work?

Or, do something like:

class TestSomeClass:
    loop: asyncio.AbstractEventLoop

    @pytext.fixture(autouse=True)
    def init(self):
        self.loop = asyncio.get_running_loop()

    def test_something(self):
        self.loop.run_until_complete(...)

How do you see those? Bad? Naughty?

It's convenient to access the loop from sync test method - even with async codebases, tests may often by written as sync (and FastAPI et al perfectly allow that).

[seifertm]

@tuukkamustonen would it be possible for you to replace

def test_something(event_loop):
event_loop.run_until_complete(...)

with

async def test_something():
    event_loop = asyncio.get_running_loop( )
    event_loop.run_until_complete(...)

or

async def test_something():
    await …

?

My understanding is that these are functionally equivalent and they would allow pytest-asyncio to proceed with the event_loop fixture deprecation.

[tuukkamustonen]

No, that converts the whole test as async, we want to keep them as sync.

The tests might be doing a lot of things, ie. calling DB, reading network, file system, etc and we'd need to convert everything to async. Don't want to take that effort / see the benefit.

Of course, sync IO works in async context, it just blocks the loop. On a usual application that would be disastrous, I don't know what the effect in this case would be? Are async tests still run one-by-one, ie. not concurrently, so that blocking the loop would actually do no harm?

[seifertm]

No, that converts the whole test as async, we want to keep them as sync.

The tests might be doing a lot of things, ie. calling DB, reading network, file system, etc and we'd need to convert everything to async. Don't want to take that effort / see the benefit.

Your benefit is only indirect. Getting rid of event_loop allows pytest-asyncio to adjust to recent asyncio development and simplify its code base. This will hopefully result in more contributions and/or faster development.

Of course, sync IO works in async context, it just blocks the loop. On a usual application that would be disastrous, I don't know what the effect in this case would be? Are async tests still run one-by-one, ie. not concurrently, so that blocking the loop would actually do no harm?

Pytest-asyncio still runs its tests one-by-one. This is a constraint by pytest itself. The asyncio plugin merely tries to make it easier to test async code without having to use asyncio.Runner or asyncio.get_event_loop directly.
I think we're on the same page here: Doing a sync call in an async test will simply block test execution in the same way as in a sync test. There should be no functional change from the conversion I suggested above.

There was an idea to provide automatic code rewriting for such changes using libCST. It's definitely an option to include such a rewriter in pytest-asyncio, but someone would have to contribute it. It's only worth it for large test suites, I guess.

[tuukkamustonen]

Hey, just for clarity's sake, I'm not arguing pytest-asyncio should do something differently 😃 Merely expressing the use case, and that the change might bring some users some troubles.

As discussed, there are two workarounds:

1. Grab the event loop and store it as property somewhere the sync test method can acquire it
2. Just convert to async test methods, and keep calling sync IO - the blocking shouldn't harm anything, and tests would run just the same / just as fast 🤷‍♂

These could have been explained/suggested in the migration docs, though both are a bit hacky, so I understand you don't probably want to give such suggestions 😄 Well, at least they're now documented here, in this ticket.

There was an idea to provide automatic code rewriting for such changes using libCST. It's definitely an option to include such a rewriter in pytest-asyncio, but someone would have to contribute it. It's only worth it for large test suites, I guess.

Honestly, that (complexity of it) sounds like a trap. Could be a separate (and more generic) tool, if someone needs that. Surely not part of pytest-asyncio, is how I'd feel!

(I think we can conclude this discussion - thanks for the help and your work on the lib in general 🙏🏻)