Skip to content
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

Pytest hook to mark all async tests with scope=session not working when using async fixtures #705

Closed
albertferras-vrf opened this issue Dec 4, 2023 · 12 comments
Labels
Milestone

Comments

@albertferras-vrf
Copy link

albertferras-vrf commented Dec 4, 2023

I have followed instructions from https://pytest-asyncio.readthedocs.io/en/latest/how-to-guides/run_session_tests_in_same_loop.html to add the pytest.mark.asyncio(scope="session") to all my async tests.

The problem I'm having is that, when these tests contain an async session-scoped fixture, the tests fail with RunTimeError: Event loop is closed.
If I manually add the @pytest.mark.asyncio(scope="session") mark on my tests (one by one, without using the pytest hook) then it works fine.

This reproduces the issue. Using pytest-asyncio==0.23.2 and pytest==7.4.3 on python 3.11.16. I also have asyncio_mode=auto in case it's relevant

conftest.py

import pytest
from pytest_asyncio import is_async_test


def pytest_collection_modifyitems(items):
    pytest_asyncio_tests = (item for item in items if is_async_test(item))
    session_scope_marker = pytest.mark.asyncio(scope="session")
    for async_test in pytest_asyncio_tests:
        async_test.add_marker(session_scope_marker)

test_hello.py

import pytest
import asyncio


@pytest.fixture
def get_marks(request):
    marks = [repr(m) for m in request.node.iter_markers()]
    if request.node.parent:
        marks += [repr(m) for m in request.node.parent.iter_markers()]
    yield marks


@pytest.fixture(scope="session")
async def myfixture():
    await asyncio.sleep(0.1)
    yield 5


async def test_one(get_marks, myfixture):
    print(get_marks)
    await asyncio.sleep(0.1)
    assert True


async def test_two(get_marks, myfixture):
    print(get_marks)
    await asyncio.sleep(0.1)
    assert True
@seifertm
Copy link
Contributor

seifertm commented Dec 4, 2023

Thanks for the report and the great reproducer!
There seem to be two independent issues:

  1. Some asyncio markers have incorrect scope
  2. The fixture finalizer for myfixture is not run properly

When I change myfixture to return 5, rather than yield 5, the tests pass.

I have to investigate with regards to both problems.

@seifertm seifertm added the bug label Dec 4, 2023
@seifertm seifertm added this to the v0.23.3 milestone Dec 4, 2023
@seifertm
Copy link
Contributor

seifertm commented Dec 4, 2023

Regarding your workaround: Rather than marking each test individually, you could consider marking modules or even packages using pytestmark, until this is fixed.

See also:
How to run all tests in a module in the same event loop
How to run all tests in a package in the same event loop

@albertferras-vrf
Copy link
Author

albertferras-vrf commented Dec 4, 2023

Thanks for the quick response.

I have tried marking it by package (pytestmark = pytest.mark.asyncio(scope="package") on the __init__.py of every directory with tests). but it does not work. I'm getting

E   pytest_asyncio.plugin.MultipleEventLoopsRequestedError: Multiple asyncio event loops with different scopes have been requested
E   by tests/integration/test_xxxx.py::test_xxxxxxx. The test explicitly requests the event_loop fixture, while
E   another event loop with module scope is provided by tests/integration/test_xxxx.py.
E   Remove "event_loop" from the requested fixture in your test to run the test
E   in a module-scoped event loop or remove the scope argument from the "asyncio"
E   mark to run the test in a function-scoped event loop.```

I believe it's because I have some fixtures defined at `tests/conftest.py` that are used inside `tests/integrations/` package

@SavchenkoValeriy
Copy link

First of all, thank for this package and for maintaining it! ❤️

When I change myfixture to return 5, rather than yield 5, the tests pass.

Here I just want to add that it's not always possible to switch from yield to return, e.g. if the fixture uses context manager (the actual problem that I have now).

@seifertm
Copy link
Contributor

seifertm commented Dec 4, 2023

@albertferras-vrf The MultipleEventLoopsRequestedError is raised when a test is marked with @pytest.mark.asyncio(scope=…) and the test simultaneously requests the event_loop fixture in its function arguments.
In this case, pytest-asyncio cannot decide whether the test should run in the event loop associated with the scope kwarg or in the loop provided by the event_loop fixture.

@albertferras-vrf
Copy link
Author

@seifertm the thing is that I have no test explicitly requesting the event_loop fixture in any of my tests.
Might be because I have this in the root conftest.py and it's trying to use this eventloop, which is what I want, because I have fixtures on root conftest.py that I need on this directory. Pretty similar to #706 (comment)

@pytest.fixture(scope="session")
def event_loop_policy(request):
    return CustomEventLoopPolicy()

@WolfgangFellger
Copy link

Chiming in to say that I too have gotten MultipleEventLoopsRequestedError, when no test in the codebase (and certainly not the one reported in the message) requests event_loop explicitly. Only fixtures do that. Sadly it refuted attempts to reproduce so far – if I remove the code and copy the involved fixture tree into a single file it works :-/

@5j9
Copy link

5j9 commented Jan 22, 2024

After spending some time trying to fix this issue, I finally gave up and decided to switch to pytest-aio. No need for overriding event_loop fixture or manually adding scope=session there, the only change I had to make was to manually add aiolib parameter (fixture) to my sync tests (which I only had a few of them, so no big deal for me).

Sorry for posting this here, thought this might save someone else.

@markdoliner
Copy link

I think I experienced the same problem reported by albertferras-vrf in the first post, and I think I fixed it by adding append=False to the call to item.add_marker() from the example. So the last line of the example should be:

        async_test.add_marker(session_scope_marker, append=False)

If anyone tries this, please confirm whether it works or if I'm just imagining it.

It makes sense to me that this would fix it. asyncio_mode is auto so pytest-asyncio will have added the asyncio mark with scope set to "function." And the pytest_collection_modifyitems function doesn't modify that scope, right? It adds the mark again with a different scope, so now the asyncio mark has been added twice. And maybe the "first" one gets used. Setting append=False causes this mark (the one with the session scope) to get prepending so it's the one that's used by pytest-asyncio.

I would love to have the ability to specify the event loop scope via configuration. e.g. If I could set asyncio_default_eventloop_scope = "session" in my pyproject.toml file. That feels appropriate to me for when pytest-asyncio automatically adds the mark.

@markdoliner
Copy link

Just want to mention one other observation: If I have an autouse fixture with scope="function", that appears to cause my tests to run in separate event loops even when following the "How to run all tests in the session in the same event loop" with the append=False fix from my previous comment. Maybe the ability to control the event loop used by a fixture independently from the fixture scope (as discussed in this comment other other follow-up comments) would fix this? It's not clear to me.

@seifertm
Copy link
Contributor

@markdoliner Thanks for the investigation. I can confirm that adding append=False fixes the problem.

This seems to be an issue with the order of the asyncio marks. Each pytest node can have the same mark multiple times.
The code snippet in the how-to uses add_marker to append a marker to each test item. By default, this marker appended to the end of the list. However, all items already have an asyncio mark by the time pytest_collection_modifyitems runs (or we're running in strict mode an non-marker coroutines are ignored). The existing logic in pytest-asyncio checks the first mark to determine the event loop scope in which a test is run, rendering the marker applied in pytest_collection_modifyitems useless.

Changing the behavior of add_marker to prepend the marker via append=False gives the desired result.

This also means it's not possible to override the loop scope with an explicit @pytest.asyncio.mark. Therefore, I support your suggestion for a default loop scope as a config option. We should follow up on this in a separate issue.

I'm planning to do a .post release of pytest-asyncio with updated documentation and close this issue.

@Nikolai-Dimitrov-Vidual
Copy link

After spending some time trying to fix this issue, I finally gave up and decided to switch to pytest-aio. No need for overriding event_loop fixture or manually adding scope=session there, the only change I had to make was to manually add aiolib parameter (fixture) to my sync tests (which I only had a few of them, so no big deal for me).

Sorry for posting this here, thought this might save someone else.

Well it saved me. I've been battling a similar issue for days, loop related errors for seemingly no reason, saw your comment and tried it and after about 5 minutes of cleaning up fixtures everything just worked. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

7 participants