pytest-asyncio 0.23.* breaks most hail asyncio tests #14130
Labels
needs-triage
A brand new issue that needs triaging.
Comments
|
The fix appears to be to pin to <0.23. |
danking
added a commit
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.
danking
added a commit
to danking/hail
that referenced
this issue
Feb 16, 2024
Stacked on hail-is#14316. These are tests for the bug fixed by hail-is#14130.
danking
added a commit
to danking/hail
that referenced
this issue
Feb 22, 2024
Stacked on hail-is#14316. These are tests for the bug fixed by hail-is#14130.
danking
added a commit
to danking/hail
that referenced
this issue
Feb 22, 2024
Stacked on hail-is#14316. These are tests for the bug fixed by hail-is#14130.
danking
added a commit
to danking/hail
that referenced
this issue
Feb 28, 2024
Stacked on hail-is#14316. These are tests for the bug fixed by hail-is#14130.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
What happened?
I'm not sure why but the latest version of asyncio seems to use different event loops for non-default-scoped fixtures from the event loops for the tests. This causes issues with aiohttp.ClientSession which expects the event loop at allocation time to match the event loop at request time.
It seems this was an intentional change in pytest-asyncio 0.23.0. The pytest-asyncio website documents how to ensure every test uses the same event loop (archive) but this does not appear to ensure that a module or session scoped fixtures share the same event loop as their dependent tests (I've created an issue for this).
Version
42bf451
Relevant log output
No response
The text was updated successfully, but these errors were encountered: