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

documentation on how to force all tests in one session to use the same event loop appears to not force fixtures into the same event loop #744

Closed
danking opened this issue Jan 8, 2024 · 5 comments
Labels
Milestone

Comments

@danking
Copy link

danking commented Jan 8, 2024

I checked in all the files referenced below into this repo: https://github.com/danking/pytest-asyncio-confusion

pytest==7.4.3
pytest-asyncio==0.23.3

Simple test file:

import pytest
import asyncio


@pytest.fixture(scope='session')
async def foo():
    yield asyncio.get_running_loop()


async def test_foobar(foo):
    assert foo == asyncio.get_running_loop()

conftest.py taken directly from the docs:

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)

pytest.ini:

[pytest]
asyncio_mode = auto

I expected this to pass but instead it fails:

# pytest foo.py
========================================================================================= test session starts =========================================================================================
platform darwin -- Python 3.10.9, pytest-7.4.3, pluggy-1.3.0
rootdir: /private/tmp/bar
configfile: pytest.ini
plugins: xdist-2.5.0, timeout-2.2.0, instafail-0.5.0, asyncio-0.23.3, devtools-0.12.2, timestamper-0.0.9, metadata-3.0.0, anyio-4.1.0, html-1.22.1, forked-1.6.0, accept-0.1.9, image-diff-0.0.11
asyncio: mode=auto
collected 1 item                                                                                                                                                                                      

foo.py F                                                                                                                                                                                        [100%]

============================================================================================== FAILURES ===============================================================================================
_____________________________________________________________________________________________ test_foobar _____________________________________________________________________________________________

foo = <_UnixSelectorEventLoop running=False closed=False debug=False>

    async def test_foobar(foo):
>       assert foo == asyncio.get_running_loop()
E       assert <_UnixSelectorEventLoop running=False closed=False debug=False> == <_UnixSelectorEventLoop running=True closed=False debug=False>
E        +  where <_UnixSelectorEventLoop running=True closed=False debug=False> = <built-in function get_running_loop>()
E        +    where <built-in function get_running_loop> = asyncio.get_running_loop

foo.py:14: AssertionError
======================================================================================= short test summary info =======================================================================================
FAILED foo.py::test_foobar - assert <_UnixSelectorEventLoop running=False closed=False debug=False> == <_UnixSelectorEventLoop running=True closed=False debug=False>
========================================================================================== 1 failed in 0.05s ==========================================================================================

Am I being dense? I feel like I must misunderstand something very simple. If I explicitly request session scope it works:

# g diff
diff --git a/foo.py b/foo.py
index 2f7efc0..608da8a 100644
--- a/foo.py
+++ b/foo.py
@@ -8,6 +8,7 @@ async def foo():
     yield asyncio.get_running_loop()
 
 
+@pytest.mark.asyncio(scope='session')
 async def test_foobar(foo):
    assert foo == asyncio.get_running_loop()
# pytest foo.py
========================================================================================= test session starts =========================================================================================
platform darwin -- Python 3.10.9, pytest-7.4.3, pluggy-1.3.0
rootdir: /private/tmp/bar
configfile: pytest.ini
plugins: xdist-2.5.0, timeout-2.2.0, instafail-0.5.0, asyncio-0.23.3, devtools-0.12.2, timestamper-0.0.9, metadata-3.0.0, anyio-4.1.0, html-1.22.1, forked-1.6.0, accept-0.1.9, image-diff-0.0.11
asyncio: mode=auto
collected 1 item                                                                                                                                                                                      

foo.py .                                                                                                                                                                                        [100%]

========================================================================================== 1 passed in 0.01s ==========================================================================================
@danking
Copy link
Author

danking commented Jan 8, 2024

This is definitely related to #706, #705, and #718, but I think my essential ask here is for documentation on how to ensure a particular test (or scope of tests, or group of tests) uses the same event loop as a particular fixture (or scope of fixtures, or group of fixtures).

The example I gave above may seem contrived, but it's essential to the correct functioning of aiohttp. The aiohttp.ClientSession saves a copy of the event loop at allocation time and assumes the same event loop is in use at HTTP request time.

If I have any non-default scoped fixture that creates a client session (possibly multiple layers down from the object I'm actually allocating), that fixture will yield a (nondeterministically) broken client session.

@danking danking changed the title documentation on how to force all tests in one session to use the same event loop appears to not work documentation on how to force all tests and fixtures in one session to use the same event loop appears to not work Jan 8, 2024
@danking danking changed the title documentation on how to force all tests and fixtures in one session to use the same event loop appears to not work documentation on how to force all tests in one session to use the same event loop appears to not force fixtures into the same event loop Jan 8, 2024
danking added a commit to hail-is/hail that referenced this issue Jan 9, 2024
…op (#14097)

Fixes #14130.

We pervasively assume:
1. That our entire system is used within a single Python thread.
2. That once an event loop is created that's the only event loop that
will exist forever.

Pytest (and newer version of IPython, afaict) violate this pretty
liberally.

~~pytest_asyncio has [explicit instructions on how to run every test in
the same event
loop](https://pytest-asyncio.readthedocs.io/en/latest/how-to-guides/run_session_tests_in_same_loop.html).
I've implemented those here.~~ [These instructions don't
work](pytest-dev/pytest-asyncio#744). It seems
that the reliable way to ensure we're using one event loop everywhere is
to use pytest-asyncio < 0.23 and to define an event_loop fixture with
scope `'session'`.

I also switched test_batch.py into pytest-only style. This allows me to
use session-scoped fixtures so that they exist exactly once for the
entire test suite execution.

Also:
- `RouterAsyncFS` methods must either be a static method or an async
method. We must not create an FS in a sync method. Both `parse_url` and
`copy_part_size` now both do not allocate an FS.
- `httpx.py` now eagerly errors if the running event loop in `request`
differs from that at allocation time. Annoying but much better error
message than this nonsense about timeout context managers.
- `hail_event_loop` either gets the current thread's event loop (running
or not, doesn't matter to us) or creates a fresh event loop and sets it
as the current thread's event loop. The previous code didn't guarantee
we'd get an event loop b/c `get_event_loop` fails if `set_event_loop`
was previously called.
- `conftest.py` is inherited downward, so I lifted fixtures out of
test_copy.py and friends and into a common `hailtop/conftest.py`
- I added `make -C hail pytest-inter-cloud` for testing the inter cloud
directory. You still need appropriate permissions and authn.
- I removed extraneous pytest.mark.asyncio since we use auto mode
everywhere.
- `FailureInjectingClientSession` creates an `aiohttp.ClientSession` and
therefore must be used while an event loop is running. Easiest fix was
to make the test async.
@rumbarum
Copy link

#706 (comment)

You'd better move to prior version. -> 0.21.1.
I also wasted a day, figuring out why contextvar got spawend on another event_loop.

@seifertm
Copy link
Contributor

You'd better move to prior version. -> 0.21.1. I also wasted a day, figuring out why contextvar got spawend on another event_loop.

I agree. If pytest-asyncio v0.23 is currently causing issues for you, I suggest to continue using v0.21 until the issues are resolved.

@danking The v0.23 release allows tests to be run in different event loops. There's essentially one loop for each hierarchy level of the test suite (session, package, module, class, function). Unfortunately, pytest-asyncio also assumes that the scope of a fixture is coupled with the scope of the event loop. This means that a session-scoped fixture always runs in the session-scope loop. There's currently no way to have a session-scoped fixture running in the same loop as a function-scoped test. This is the core issue of #706.

@rumbarum I'm sorry that this caused you spending so much time on this issue.

@seifertm seifertm added the bug label Jan 28, 2024
@seifertm seifertm added this to the v0.23 milestone Jan 28, 2024
@rumbarum
Copy link

@seifertm
I appreciate your works.
And thanks for your words.

@seifertm
Copy link
Contributor

I can reproduce the error of the OP. However, when changing the line async_test.add_marker(session_scope_marker) in conftest.py to async_test.add_marker(session_scope_marker, append=False) the test passes.

The append=False kwarg was missing in the documentation at the time of the original post. It has since been updated (see #705).

@danking I hope this addresses your issue, otherwise feel free to comment and we can revisit this.

@seifertm seifertm closed this as not planned Won't fix, can't repro, duplicate, stale Jul 15, 2024
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

3 participants