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

Release 0.14 yields ScopeMismatch for event_loop fixture #171

Closed
Askaholic opened this issue Jun 28, 2020 · 12 comments
Closed

Release 0.14 yields ScopeMismatch for event_loop fixture #171

Askaholic opened this issue Jun 28, 2020 · 12 comments

Comments

@Askaholic
Copy link

When upgrading to pytest-asyncio 0.14 all of my tests error out with the following error:

ScopeMismatch: You tried to access the 'function' scoped fixture 'event_loop' with a 'session' scoped request object, involved factories
.venv/lib/python3.7/site-packages/pytest_asyncio/plugin.py:136:  def wrapper(*args, **kwargs)

This does not occur on pytest-asyncio 0.12.

System information:

platform linux -- Python 3.7.6, pytest-5.4.3, py-1.9.0, pluggy-0.13.1
plugins: asyncio-0.14.0, cov-2.10.0, hypothesis-5.18.3, mock-3.1.1 

Additionally here is a link to the travis build which also has this issue.
https://travis-ci.org/github/FAForever/server/builds/702782646

@alblasco
Copy link
Contributor

alblasco commented Jun 28, 2020

Thanks for your feedback.

I have forked https://github.com/FAForever/server to reproduce it. And I get same result. The problem is that you have async fixtures that are session scope. E.g: test_data
https://github.com/FAForever/server/blob/v1.4.3/tests/conftest.py#L52-L57

As copied from pytest-asyncio documentation:

All scopes are supported, but if you use a non-function scope you will need to redefine the event_loop fixture to have the same or broader scope. Async fixtures need the event loop, and so must have the same or narrower scope than the event_loop fixture.

So you just have to include next copy in your conftest.py:

@pytest.fixture(scope='session')
def event_loop():
    """Create an instance of the default event loop for each test case."""
    loop = asyncio.get_event_loop_policy().new_event_loop()
    yield loop
    loop.close()

Note: About why pytest doesn´t raise this error with pytest-asyncio V0.12 is something that I have no answer, and It's beyond pytest-asyncio. It's something that depends on pytest, but IMHO it should raise this error too with pytest-asyncio V0.12. See also #169, that also needs to increase event_loop scope, but it was not clear initially from pytest error messages.

@Tinche
Copy link
Member

Tinche commented Jun 28, 2020

@Askaholic I agree with @alblasco , the plugin is stricter for certain classes of issues now. If you have a session-scoped async fixture, you should redefine the event loop to be session-scoped too. Is this doable?

The problem with larger-scoped pytest-asyncio fixtures is: we don't know if your fixture will need the event loop when it does tear-down. I can see a use-case for session-scoped fixtures that just use the event loop to load some data and never use it again; those fixtures could technically work but it's complicated. If your fixture does something more complex and has a broader scope than the event loop fixture, on tear down it's going to have to use a totally different event loop and it will cause issues with connections/asyncio tasks etc.

@Askaholic
Copy link
Author

Thanks for the suggestion! Since it works on v0.12 I assumed it was some problem with pytest-asyncio internals.

Will redefining the event_loop fixture like that cause all tests to be run in the same event loop? Will that cause other problems? I'm assuming there is a good reason why each test gets it's own event loop by default.

@alblasco
Copy link
Contributor

Will redefining the event_loop fixture like that cause all tests to be run in the same event loop? Will that cause other problems?
This is the standard solution, and it works fine.
Try it.

@Tinche
Copy link
Member

Tinche commented Jun 29, 2020

Another option is to rework whatever fixture was using the event loop to be function scoped.

It's not particularly dangerous to reuse the event loop. It's a tradeoff between efficiency (reusing the same loop) and having a cleaner test environment (recreating the loop, this making sure all event loop state is reset).

@Askaholic
Copy link
Author

I tried to redefine the event loop as you suggested, but now I'm getting a RuntimeError from aiomysql when trying to connect:

RuntimeError: Task <Task pending coro=<pytest_fixture_setup.<locals>.wrapper.<locals>.setup() running at server/.venv/lib/python3.7/site-packages/pytest_asyncio/plugin.py:110> cb=[_run_until_complete_cb() at /usr/local/lib/python3.7/asyncio/base_events.py:157]> got Future <Future pending cb=[BaseSelectorEventLoop._sock_connect_done(16)()]> attached to a different loop

Even though I pass the event_loop fixture to aiomysql via the loop parameter. (this is with pytest-asyncio 0.12). I'm not sure where multiple event loops would be coming from because I've removed all other calls to get_event_loop or new_event_loop.

@Tinche
Copy link
Member

Tinche commented Jun 29, 2020

@Askaholic Interesting, could you prepare a minimal repro example somewhere so we can debug this?

@Askaholic
Copy link
Author

Askaholic commented Jul 1, 2020

This does it for me:

import asyncio

import pytest


@pytest.fixture(scope="session")
def event_loop():
    loop = asyncio.get_event_loop_policy().new_event_loop()
    yield loop
    loop.close()


@pytest.fixture(scope="session", autouse=True)
async def test_data(event_loop):
    await asyncio.open_connection(
        host="127.0.0.1",
        port=3306,
        loop=event_loop
    )


@pytest.mark.asyncio
async def test_something():
    assert True

Yields a test error:

RuntimeError: Task <Task pending coro=<pytest_fixture_setup.<locals>.wrapper.<locals>.setup() running at .venv/lib/python3.7/site-packages/pytest_asyncio/plugin.py:110> cb=[_run_until_complete_cb() at /usr/local/lib/python3.7/asyncio/base_events.py:157]> got Future <Future pending cb=[BaseSelectorEventLoop._sock_connect_done(16)()]> attached to a different loop

It seems that the problem comes from passing the event loop via the loop argument. If I remove loop=event_loop then I do not get the error.

@dimaqq
Copy link

dimaqq commented Jul 10, 2020

My 2c: since py3.8 (?) event loop should not be passed as an argument.

Instead, the scaffolding should ensure that the current event loop is set correctly. And it will be used by asyncio.* functions. Perhaps the "custom" event loop is created but not set as current?

@Askaholic
Copy link
Author

Askaholic commented Oct 5, 2020

New developments on this. I have managed to reproduce the ScopeMismatch error. It seems to be caused by defining an async fixture with scope other than function.

Minimal example:

import pytest

@pytest.fixture(scope="module")
async def async_fixture():
    pass

def test_foo(async_fixture):
    pass

# To demonstrate that it happens for both types of test functions
@pytest.mark.asyncio
async def test_bar(async_fixture):
    pass

Test output:

============================================================= test session starts =============================================================
platform linux -- Python 3.7.6, pytest-6.1.1, py-1.9.0, pluggy-0.13.1
rootdir: /home/my_user/Documents/fa-server
plugins: cov-2.10.1, mock-3.3.1, asyncio-0.14.0, hypothesis-5.37.0
collected 2 items                                                                                                                             

test_pytest.py EE                                                                                                                       [100%]

=================================================================== ERRORS ====================================================================
_________________________________________________________ ERROR at setup of test_foo __________________________________________________________
ScopeMismatch: You tried to access the 'function' scoped fixture 'event_loop' with a 'module' scoped request object, involved factories
.venv/lib/python3.7/site-packages/pytest_asyncio/plugin.py:136:  def wrapper(*args, **kwargs)
.venv/lib/python3.7/site-packages/pytest_asyncio/plugin.py:205:  def event_loop(request)
_________________________________________________________ ERROR at setup of test_bar __________________________________________________________
ScopeMismatch: You tried to access the 'function' scoped fixture 'event_loop' with a 'module' scoped request object, involved factories
.venv/lib/python3.7/site-packages/pytest_asyncio/plugin.py:136:  def wrapper(*args, **kwargs)
=========================================================== short test summary info ===========================================================
ERROR test_pytest.py::test_foo
ERROR test_pytest.py::test_bar

EDIT: Ha, I just read over the earlier comments which state exactly this :P

@Askaholic
Copy link
Author

Revisiting this again. So the reason why redefining the event_loop fixture didn't work for me before was that my project uses aiomysql which until recently was passing around the event loop through the deprecated arguments. However, this has finally been fixed in this PR aio-libs/aiomysql#631. I tried the proposed solution of redefining the event_loop fixture, and it worked!

However... It made my tests take about twice as long to run (10 minutes rather than 5) which is kindof a big deal for me. I think this is mostly because I have a lot of background tasks that are created in function scoped fixtures, so the event loop just gets bogged down over the course of the test run.

Since I just need my test_data fixture to run once at the start of my tests, I came up with the the solution of just making the fixture synchronous and creating a local event loop inside of that fixture (using asyncio.run):

@pytest.fixture(scope="session", autouse=True)
def test_data():
    async def _test_data():
        # Contents of my old async fixture here

    asyncio.run(_test_data())

This way I don't need to change the scope of the event loop for my tests.

@seifertm
Copy link
Contributor

Thanks for keeping this thread updated!

It sounds like the issue has been solved (apart from the performance problem). If anything else comes up, feel free to open another issue.

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

No branches or pull requests

5 participants