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

Warning when defining session scoped event_loop fixture: "pytest-asyncio will close the event loop for you, but future versions of the library will no longer do so." #531

Closed
james-johnston-thumbtack opened this issue Apr 16, 2023 · 4 comments · Fixed by #577

Comments

@james-johnston-thumbtack

This warning was recently added in #510

I am getting this warning with the following test case. Essentially, we have two directories with async tests. One directory defines a session-scoped event_loop, and the other one doesn't do anything with event_loop.

File tests/integration/conftest.py:

import asyncio
from typing import  Iterator

import pytest


# override event_loop fixture from pytest-asyncio so it has session scope.  see:
# https://pytest-asyncio.readthedocs.io/en/latest/reference/fixtures.html
@pytest.fixture(scope="session")
def event_loop() -> Iterator[asyncio.AbstractEventLoop]:
    loop = asyncio.get_event_loop_policy().new_event_loop()
    yield loop
    loop.close()

File tests/integration/test_integration.py:

async def test_integration() -> None:
    pass

File tests/unit/test_unit.py:

async def test_unit() -> None:
    pass

Also set the pytest configuration in pyproject.toml as follows:

[tool.pytest.ini_options]
asyncio_mode = "auto"

Now run pytest from the root directory:

% pytest
====================================================================================================================================== test session starts =======================================================================================================================================
platform darwin -- Python 3.11.1, pytest-7.2.2, pluggy-1.0.0
rootdir: /Users/james.johnston/Thumbtack/postgres-schema/docker/migrate, configfile: pyproject.toml
plugins: asyncio-0.21.0
asyncio: mode=Mode.AUTO
collected 2 items

tests/integration/test_integration.py .                                                                                                                                                                                                                                                    [ 50%]
tests/unit/test_unit.py .                                                                                                                                                                                                                                                                  [100%]

======================================================================================================================================== warnings summary ========================================================================================================================================
tests/unit/test_unit.py::test_unit
  /Users/james.johnston/Library/Caches/pypoetry/virtualenvs/ttmigrate-E386TQsv-py3.11/lib/python3.11/site-packages/pytest_asyncio/plugin.py:444: DeprecationWarning: pytest-asyncio detected an unclosed event loop when tearing down the event_loop
  fixture: <_UnixSelectorEventLoop running=False closed=False debug=False>
  pytest-asyncio will close the event loop for you, but future versions of the
  library will no longer do so. In order to ensure compatibility with future
  versions, please make sure that:
      1. Any custom "event_loop" fixture properly closes the loop after yielding it
      2. Your code does not modify the event loop in async fixtures or tests

    warnings.warn(

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
================================================================================================================================== 2 passed, 1 warning in 0.03s ==================================================================================================================================
(ttmigrate-py3.11) james.johnston@james migrate %

I found that if I move the event_loop fixture to a root tests/conftest.py, then I no longer get the warning. I am not sure if that is an intentional limitation (or feature?), or if it is a bug.

If the limitation is intentional, may I suggest a documentation improvement? The section at https://pytest-asyncio.readthedocs.io/en/latest/reference/fixtures.html#event-loop talks at length about making your own session-scoped event_loop fixture. But it does not talk about this issue, and it was a little time-consuming to track down in my project.

@seifertm
Copy link
Contributor

seifertm commented Apr 18, 2023

Thanks for the report and the great reproducer!

The intention of the warning is to point out flawed event loop handling. With the deprecation of asyncio.get_event_loop() in Python 3.12 onwards [1], pytest-asyncio will no longer be able to clean up event loops for the user.

Unfortunately, the example seems to suffer from exactly this problem. When running the code with pytest --setup-show, we can see that the unit tests are executed in a function scoped event loop, while the session-wide loop is still in scope:

tests/integration/test_integration.py 
SETUP    S event_loop
        tests/integration/test_integration.py::test_integration (fixtures used: event_loop).
tests/unit/test_unit.py 
        SETUP    F event_loop
        tests/unit/test_unit.py::test_unit (fixtures used: event_loop).
        TEARDOWN F event_loop
TEARDOWN S event_loop

Since we can only have one event loop [0], the function-scoped fixture will close the session-scoped event loop prematurely.

If you move the conftest.py to the tests folder, both unit and integration tests will use the same loop:

tests/integration/test_integration.py 
SETUP    S event_loop
        tests/integration/test_integration.py::test_integration (fixtures used: event_loop).
tests/unit/test_unit.py 
        tests/unit/test_unit.py::test_unit (fixtures used: event_loop).
TEARDOWN S event_loop

I think the warning does what it's supposed to. I also agree that it's probably a good idea to adjust the docs accordingly.

In your particular case, you can probably achieve your goal by using a module-scoped fixture for the integration tests:

tests/integration/test_integration.py 
    SETUP    M event_loop
        tests/integration/test_integration.py::test_integration (fixtures used: event_loop).
    TEARDOWN M event_loop
tests/unit/test_unit.py 
        SETUP    F event_loop
        tests/unit/test_unit.py::test_unit (fixtures used: event_loop).
        TEARDOWN F event_loop

[0] technically it's one loop per thread, but we're not running in multiple threads
[1] the function was only deprecated when called outside of a running loop. Inside of a running loop, it's an alias of asyncio.get_running_loop()

@james-johnston-thumbtack
Copy link
Author

@seifertm yeah that all makes sense, and aligns with what I was guessing might be happening. In my case I just moved the fixture to the root like I mentioned in my original post. Although I did test and see that a module scope fixture worked too.

From what it's sounding like, maybe it's as simple as some doc improvements... (and/or improvements to the warning message)

@serpulga

This comment was marked as off-topic.

@seifertm

This comment was marked as off-topic.

seifertm added a commit to seifertm/pytest-asyncio that referenced this issue Jul 12, 2023
The example for overriding event loop scopes now uses a module scope rather than a session scope. Due to the constraint that event_loop fixture scopes should not overlap, a session-scoped fixture is usually not that helpful.

Addresses pytest-dev#531

Signed-off-by: Michael Seifert <[email protected]>
seifertm added a commit to seifertm/pytest-asyncio that referenced this issue Jul 12, 2023
seifertm added a commit that referenced this issue Jul 12, 2023
The example for overriding event loop scopes now uses a module scope rather than a session scope. Due to the constraint that event_loop fixture scopes should not overlap, a session-scoped fixture is usually not that helpful.

Addresses #531

Signed-off-by: Michael Seifert <[email protected]>
seifertm added a commit that referenced this issue Jul 12, 2023
…nal possible cause.

Closes #531

Signed-off-by: Michael Seifert <[email protected]>
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

Successfully merging a pull request may close this issue.

3 participants