Async fixtures request wrong event loop with 0.23.0a0

[2e0byo]

Interesting things seem to happen with the current logic in 0.23.0a when collecting async fixtures.

Running a venv with almost nothing installed:

# requirements.txt
pytest
pytest-asyncio==0.23.0a0

# pyproject.toml
[tool.pytest.ini_options]
asyncio_mode = "auto"

A single dir tests contains an __init__.py and the test_async_fixtures.py file; pytest is invoked as pytest tests/

This fails:

# tests/test_async_fixture.py
import asyncio

import pytest

@pytest.fixture(scope="package")
def event_loop_policy() -> asyncio.AbstractEventLoopPolicy:
    return asyncio.DefaultEventLoopPolicy()


@pytest.fixture(scope="package")
async def async_fixture():
    yield 7


async def test_async_fixture(async_fixture):
    assert async_fixture == 7

Complaining:

ScopeMismatch: You tried to access the function scoped fixture event_loop with a package scoped request object, involved factories:
tests/test_fixture.py:13:  def async_fixture()

Hmm. As a guess, is async_fixture implicitly requesting the wrong event loop? What happens if I mark it?

@pytest.fixture(scope="package")
@pytest.mark.asyncio(scope="package")
async def async_fixture():
    yield 7

No, the same as before (I have no idea if one is supposed to be able to mark fixtures like this).

What about marking the whole module explicitly?

import asyncio

import pytest

pytestmark = pytest.mark.asyncio(scope="package")
...

Ah! now we get a more interesting error:

file /tmp/t/tests/test_fixture.py, line 20
  async def test_async_fixture(async_fixture):
      assert async_fixture == 7
E       fixture 'tests/__init__.py::<event_loop>' not found
>       available fixtures: _session_event_loop, async_fixture, cache, capfd, capfdbinary, caplog, capsys, capsysbinary, doctest_namespace, event_loop, event_loop_policy, monkeypatch, pytestconfig, record_property, record_testsuite_property, record_xml_attribute, recwarn, tests/test_fixture.py::<event_loop>, tmp_path, tmp_path_factory, tmpdir, tmpdir_factory, unused_tcp_port, unused_tcp_port_factory, unused_udp_port, unused_udp_port_factory
>       use 'pytest --fixtures [testpath]' for help on them.

What about marking the test function with an explicit scope?

@pytest.mark.asyncio(scope="package")
async def test_async_fixture(async_fixture):
    assert async_fixture == 7

This raises the new 'Multiple asyncio event loops with different scopes' error.

Traceback

.venv/lib/python3.11/site-packages/_pytest/runner.py:341: in from_call
    result: Optional[TResult] = func()
.venv/lib/python3.11/site-packages/_pytest/runner.py:372: in <lambda>
    call = CallInfo.from_call(lambda: list(collector.collect()), "collect")
.venv/lib/python3.11/site-packages/_pytest/python.py:534: in collect
    return super().collect()
.venv/lib/python3.11/site-packages/_pytest/python.py:455: in collect
    res = ihook.pytest_pycollect_makeitem(
.venv/lib/python3.11/site-packages/pluggy/_hooks.py:493: in __call__
    return self._hookexec(self.name, self._hookimpls, kwargs, firstresult)
.venv/lib/python3.11/site-packages/pluggy/_manager.py:115: in _hookexec
    return self._inner_hookexec(hook_name, methods, kwargs, firstresult)
.venv/lib/python3.11/site-packages/pytest_asyncio/plugin.py:525: in pytest_pycollect_makeitem_convert_async_functions_to_subclass
    node_or_list_of_nodes = hook_result.get_result()
.venv/lib/python3.11/site-packages/_pytest/python.py:271: in pytest_pycollect_makeitem
    return list(collector._genfunctions(name, obj))
.venv/lib/python3.11/site-packages/_pytest/python.py:498: in _genfunctions
    self.ihook.pytest_generate_tests.call_extra(methods, dict(metafunc=metafunc))
.venv/lib/python3.11/site-packages/pluggy/_hooks.py:552: in call_extra
    return self._hookexec(self.name, hookimpls, kwargs, firstresult)
.venv/lib/python3.11/site-packages/pluggy/_manager.py:115: in _hookexec
    return self._inner_hookexec(hook_name, methods, kwargs, firstresult)
.venv/lib/python3.11/site-packages/pytest_asyncio/plugin.py:674: in pytest_generate_tests
    raise MultipleEventLoopsRequestedError(
E   pytest_asyncio.plugin.MultipleEventLoopsRequestedError: Multiple asyncio event loops with different scopes have been requested
E   by tests/test_fixture.py::test_async_fixture. The test explicitly requests the event_loop fixture, while
E   another event loop with package scope is provided by tests/__init__.py.
E   Remove "event_loop" from the requested fixture in your test to run the test
E   in a package-scoped event loop or remove the scope argument from the "asyncio"
E   mark to run the test in a function-scoped event loop.

Assuming I'm using the new code correctly, it looks like there's a problem with async fixtures and policies. At any rate here's a failing test case.

[seifertm]

Thanks for giving the alpha release a try!

The v0.22 release relied on marks applied to pytest Collectors (e.g. classes and modules). The logic hasn't been adjusted accordingly in the alpha release and there were no tests with mixed scopes between fixtures and tests. As a result, this problem wasn't caught.

In my opinion, the expected behaviour of your first example is that async_fixture is run in a package-scoped loop and test_async_fixture is run in a function-scoped loop.

When test_async_fixture is marked with asyncio(scope="package"), both should run in the same loop.

[2e0byo]

Shouldn't test_async_fixture run in a package scoped event loop because of the event_loop_policy fixture?

[seifertm]

Shouldn't test_async_fixture run in a package scoped event loop because of the event_loop_policy fixture?

The idea was that the loop scope for each test is determined by the scope argument to the asyncio mark of the respective test.

The event_loop_policy was meant to be independent from that. It returns an event loop policy (=a factory for event loops) and applies to all tests and fixtures in its scope. Those tests and fixtures use the policy to instantiate event loops with different scopes. The policy fixture just happens to have package scope in your initial example, because pytest would not allow it to be requested by async_fixture, otherwise.

In general, a user shouldn't have to touch the policy fixture at all, unless they specifically want to test with a non-default event loop or with multiple event loops.

Does this idea make sense? Or do you think it would be confusing?

[2e0byo]

Ah. I can see the logic behind that, but it wasn't actually my motivation for proposing the event_loop_policy fixture. I intended the policy to be an alternative to the event_loop fixture for scopes (package, session) which would otherwise require a lot of boilerplate marking, because of the inability to apply marks over scopes larger than module. On the other hand, I can see the logic here: the policy is as you say a factory, and relying on fixture scope to tell us only to call that factory once is a tad non-obvious.

I think there are really three distinct problems here, and we need three solutions to them:

1. sometimes users want to customise the event loop itself (e.g. to parametrise across different event loop implementations). The event_loop_policy is the right fixture for this, and it doesn't really matter what scope it has (except internally as you note), since it returns a factory.

2. sometimes users want to run tests in different scoped loops. The solution for this is the scope argument.

3. sometimes users want to mark a whole package (or all tests) as running in a different scope. I think the solution for this has to be a fixture for ease of use. Thus I propose a configuration fixture called something like asyncio_mark, which would be used like this:

from pytest_asyncio import Mark # hypothetical

@pytest.fixture # scope could be set here if required
def asyncio_mark():
    return Mark(scope="package")

The fixture could be auto-requested, like event_loop, and would be an alternative to the mark, examined at the same point. If both were set an error could be raised, or one could take preference. As a matter of style it should be used only for package and session scopes.

The current setup requires something like putting

pytestmark = pytest.mark.asyncio(scope="session")

at the top of every test file. This isn't the end of the world, but I think in general adding boilerplate is best avoided, and it's definitely a problem with large test cases for session-scoped fixtures (e.g. tests for a database bound app).

If I'm right this would still be preferable to the event_loop fixture, since I gather the problem with that is that it makes it hard to change how pytest-asyncio handles event loops, as the event loop generating code ends up wrapped up with a lot of custom logic and is hard to change. (In general dependencies in pytest have this problem: it took me the best part of two days to extricate some internal dependencies from our database mocking framework from pytest fixtures, because custom logic had grown up around them.) But it would still provide a way to run code right before and after an event loop scope is set up, which is something users might legitimately want to do. Only, by not having that fixture provide the event loop, there is no way for custom logic to interfere with it, and pytest-asyncio's life is made simpler.

I'd be interested to know if I've got the right end of the stick about the motivation for deprecating event_loop as well.

Thanks for your time!

[seifertm]

the policy is as you say a factory, and relying on fixture scope to tell us only to call that factory once is a tad non-obvious.

I agree. It's not intuitive that the policy needs to be scoped at least as large as the largest fixture scope or test scope affected by the policy. I hope we can reduce errors related to this issue with good documentation. I deliberately used session scope for the policy fixtures in the how-tos. I think we should add a paragraph about this issue specifically, though. On the upside, pytest will give an appropriate error message when the policy fixture scope doesn't match. I think we can get away with that for now.

I think there are really three distinct problems here, and we need three solutions to them:

1. sometimes users want to customise the event loop itself (e.g. to parametrise across different event loop implementations).  The `event_loop_policy` is the right fixture for this, and it doesn't really matter what scope it has (except internally as you note), since it returns a factory.

2. sometimes users want to run tests in different scoped loops.  The solution for this is the scope argument.

3. sometimes users want to mark a whole package (or all tests) as running in a different scope.  I think the solution for this has to be a fixture for ease of use.  Thus I propose a configuration fixture called something like `asyncio_mark`, which would be used like this:

The current setup requires something like putting

pytestmark = pytest.mark.asyncio(scope="session")

at the top of every test file. This isn't the end of the world, but I think in general adding boilerplate is best avoided, and it's definitely a problem with large test cases for session-scoped fixtures (e.g. tests for a database bound app).

That's a really good breakdown. I wish I had it earlier :)
If I understand correctly, the first and second points are covered. What's missing is a convenient way to mark all tests in a package or in session. Otherwise, users with large test suites will have to mark each module individually. I haven't thought of that.

from pytest_asyncio import Mark # hypothetical

@pytest.fixture # scope could be set here if required
def asyncio_mark():
    return Mark(scope="package")

The fixture could be auto-requested, like event_loop, and would be an alternative to the mark, examined at the same point. If both were set an error could be raised, or one could take preference. As a matter of style it should be used only for package and session scopes.

That's an interesting proposal. From the top of my head, I think we'd need some special code make that happen. Markers are available during collection time, whereas fixtures are only evaluated when tests are run. Pytest-asyncio currently performs some sort of "fixture preprocessing" during collection time, but it requires reaching rather deep into pytest internals. I suppose the proposed fixture would work, but it wouldn't be particularly easy to implement.

I scoured the pytest bug tracker and docs if there's a way to mark packages or sessions. Apparently, it is possible to mark packages using a pytestmark inside the __init__.py of a package. (see pytest-dev/pytest#9245)
This doesn't seem to be documented. I'll reach out to the pytest devs and see if this is something that we can use. That would alleviate the issue a bit, since there are generally much fewer packages than modules in a test suite. It would also use a standard pytest feature rather than custom logic.

event_loop deprecation reasoning

If I'm right this would still be preferable to the event_loop fixture, since I gather the problem with that is that it makes it hard to change how pytest-asyncio handles event loops, as the event loop generating code ends up wrapped up with a lot of custom logic and is hard to change.

Exactly, this is the biggest problem. The event_loop fixture became a magnet for all kinds of functionality that is completely unrelated. Here's one of many examples (not my code, but posting it without reference):

@pytest.fixture(scope='session')
def event_loop(
    use_uvloop: bool,
) -> Generator[asyncio.AbstractEventLoop, None, None]:
    """Get event loop.

    Share event loop between all tests. Necessary for session scoped asyncio
    fixtures.

    Source: https://github.com/pytest-dev/pytest-asyncio#event_loop
    """
    context: ContextManager[Any]
    # Note: both of these are excluded from coverage because only one will
    # execute depending on the value of --use-uvloop
    if use_uvloop:  # pragma: no cover
        uvloop.install()
        context = contextlib.nullcontext()
    else:  # pragma: no cover
        context = mock.patch(
            'uvloop.install',
            side_effect=RuntimeError(
                'uvloop.install() was called when --use-uvloop=False. uvloop '
                'should only be used when --use-uvloop is passed to pytest.',
            ),
        )

    policy = asyncio.get_event_loop_policy()
    loop = policy.new_event_loop()
    with context:
        yield loop
    loop.close()

The freedom for users to add to the implementation of the fixture leads to a lot if different failure modes, each of which requiring dedicated cleanup code in pytest-asyncio to avoid messing up subsequent tests. For example, pytest-asyncio closes the old event loop before using installing the loop provided by event_loop fixture, in order to avoid ResourceWarnings:

pytest-asyncio/pytest_asyncio/plugin.py

Lines 399 to 410 in a10cbde

	try:
	with warnings.catch_warnings():
	warnings.simplefilter("ignore", DeprecationWarning)
	old_loop = policy.get_event_loop()
	if old_loop is not loop:
	old_loop.close()
	except RuntimeError:
	# Either the current event loop has been set to None
	# or the loop policy doesn't specify to create new loops
	# or we're not in the main thread
	pass
	policy.set_event_loop(loop)

It also installs a fixture finalizer to catch cases where a user forgot to call loop.close() in the event_loop fixture:

pytest-asyncio/pytest_asyncio/plugin.py

Lines 443 to 455 in a10cbde

	def _close_event_loop() -> None:
	policy = asyncio.get_event_loop_policy()
	try:
	loop = policy.get_event_loop()
	except RuntimeError:
	loop = None
	if loop is not None:
	if not loop.is_closed():
	warnings.warn(
	_UNCLOSED_EVENT_LOOP_WARNING % loop,
	DeprecationWarning,
	)
	loop.close()

Counting the event_loop fixture itself, there are already three different cases where pytest-asyncio tries to close the loop. All these special cases make it extremely hard to implement meaningful changes, especially for new contributors. It would mean that users have to change all their custom event_loop implementations every time pytest-asyncio makes a change to the fixture. Changes to the fixture are required to support #127, for example.

But it would still provide a way to run code right before and after an event loop scope is set up, which is something users might legitimately want to do. Only, by not having that fixture provide the event loop, there is no way for custom logic to interfere with it, and pytest-asyncio's life is made simpler.

The problem with custom code is that the problem often isn't immediately visible in the current test, especially when considering ResourceWarnings emitted when something is garbage collected.

The purpose of pytest-asyncio is simply to make it more convenient to test asyncio code. This comes with the restriction that pytest-asyncio is in control of the asyncio event loop. If users require low-level control over the loop, I'm happy to provide those, but they should come as defined interfaces.

[seifertm]

Cross-referencing the question about package-level pytestmark statements in the pytest project:
pytest-dev/pytest#11612

[2e0byo]

The problem with custom code is that the problem often isn't immediately visible in the current test, especially when considering ResourceWarnings emitted when something is garbage collected.

I've spent a reasonable amount of time dealing with this; I agree that anything which makes it easier to see exactly where a loop is created and torn down is a good idea.

The purpose of pytest-asyncio is simply to make it more convenient to test asyncio code. This comes with the restriction that pytest-asyncio is in control of the asyncio event loop. If users require low-level control over the loop, I'm happy to provide those, but they should come as defined interfaces.

That makes sense. I think the policy (later factory) interface is probably all one could ever require. I've been trying to think of something I couldn't do with it, but even things like running tasks on the loop or deliberately closing it (both of which probably belong in the test or a dedicated fixture if they were somehow required) could be done by overriding the factory. The case above, for instance, would be better handled IMHO just by being able to hand the right policy out to use uvloop.

(Since I've inadvertently closed the issue and posted this too early I'll continue in a separate message.)

[2e0byo]

That's an interesting proposal. From the top of my head, I think we'd need some special code make that happen. Markers are available during collection time, whereas fixtures are only evaluated when tests are run. Pytest-asyncio currently performs some sort of "fixture preprocessing" during collection time, but it requires reaching rather deep into pytest internals. I suppose the proposed fixture would work, but it wouldn't be particularly easy to implement.

Ah, somehow I thought the setup happened when the fixture was requested. That would be a solution, but I don't want to propose serious architecture changes.

I scoured the pytest bug tracker and docs if there's a way to mark packages or sessions. Apparently, it is possible to mark packages using a pytestmark inside the init.py of a package. (see pytest-dev/pytest#9245)
This doesn't seem to be documented. I'll reach out to the pytest devs and see if this is something that we can use. That would alleviate the issue a bit, since there are generally much fewer packages than modules in a test suite. It would also use a standard pytest feature rather than custom logic.

Personally I think this would be fine. However, it appears from some testing here not to work in sub-packages. There does need to be some way of running a single event loop for a package and all its subpackages, although I think just repeating the mark would likely work fine (the 'package' scope already takes subpackages into account, it just appears that the __init__.py is only being evaluated for the outer package.

For session scope I think a hook setting the mark dynamically might be the way to go: crudely one can do something like:

# tests/conftest.py
import pytest


def pytest_collection_modifyitems(items):
    marker = pytest.mark.asyncio(scope="session")
    for item in (x for x in items if "Coroutine" in repr(x)):
        item.add_marker(marker)

(with a proper test for coroutines: I couldn't find anything quickly in inspect which seemed to work)

And then the following passes:

# tests/pkg/test_marks.py
import pytest
from pytest import FixtureRequest

marker = pytest.mark.asyncio(scope="session").mark


def test_sync_marks(request: FixtureRequest):
    marks = list(request.node.iter_markers())

    assert marker not in marks


async def test_async_marks(request: FixtureRequest):
    marks = list(request.node.iter_markers())

    assert marker in marks


@pytest.mark.asyncio(scope="function")
async def test_custom_mark(request: FixtureRequest):
    marks = list(request.node.iter_markers())

    assert marker in marks
    assert pytest.mark.asyncio(scope="function").mark in marks

I don't know what the behaviour should be in the third case.

In any case I doubt many test suites use both session scope loops and second fixture scoped loops (I thought that wasn't supported, although I don't know): if a session scoped fixture is used it's probably the only one. So a snippet in the docs for (effectively) a session scoped hook to set the mark is just as easy as a fixture to use, if a little less intuitive.
It would be a good idea to warn that per the docs the hook will be called for all tests regardless of which conftest.py it is in, so it really is for session scoped configuration, unless multiple marks are supported.

[my vim fingers keep closing this issue by mistake, sorry: not sure what I'm pressing]