pytest-dev / pytest-asyncio

Asyncio support for pytest
https://pytest-asyncio.readthedocs.io
Apache License 2.0
1.4k stars 145 forks source link

Task finalizers run outside of event loop #878

Open CGamesPlay opened 1 month ago

CGamesPlay commented 1 month ago

Something about how pytest-asyncio is handling finalizers isn't working right. When a task is abandoned by a test, the task's finalizers get run outside of the event loop, meaning any async cleanup will not be run. The following test fails with pytest-asyncio, but works fine if you switch to using asyncio.run.

import asyncio
import pytest

async def func():
    try:
        await asyncio.sleep(0.1)
    finally:
        assert asyncio.current_task() is not None

@pytest.mark.asyncio
async def test_it():
    asyncio.create_task(func())

I encountered this issue because my cleanup code was calling ContextVar.reset, and it would emit a warning (shown below), and I believe this is the root cause.

ValueError: <Token var=<ContextVar name='var' at 0x106a40a40> at 0x106a74240> was created in a different Context

Python 3.11.9 Pytest 8.2.2 pytest-asyncio 0.23.7

seifertm commented 1 month ago

Thanks for the report and the reproducer!

asyncio.run has dedicated logic to get rid of all remaining tasks when the asyncio.Runner is closed (see https://github.com/python/cpython/blob/3.12/Lib/asyncio/runners.py). Although we plan to do this in the future, pytest-asyncio doesn't use asyncio.Runner or asyncio.run at the moment. The logic to clean up the remaining tasks is not accessible via public API, either (see #309).

I don't think we can address this issue anytime soon.

CGamesPlay commented 1 month ago

Thanks for the response!

Honestly, up until I discovered this bug, I just assumed that pytest-asyncio was a simple wrapper around asyncio.run. Now I understand the internals a bit better I see that it isn't, but my question is: why isn't it? What does a user gain by the additional complexity, and in what circumstances would a user be wiser to just wrap their tests in asyncio.run? It might be helpful to add the answers to this question in the documentation somewhere as well.

seifertm commented 1 month ago

Thanks for the response!

Honestly, up until I discovered this bug, I just assumed that pytest-asyncio was a simple wrapper around asyncio.run. Now I understand the internals a bit better I see that it isn't, but my question is: why isn't it? What does a user gain by the additional complexity, and in what circumstances would a user be wiser to just wrap their tests in asyncio.run? It might be helpful to add the answers to this question in the documentation somewhere as well.

I think the straightforward reason is legacy. Pytest-asyncio has been around for some time and the asyncio API is evolving. A lot of the low-level asyncio calls used by pytest-asyncio have been (partially) deprecated.

The current plan for pytest-asyncio is to move to using asyncio.Runner internally. This should replace the deprecated calls and open up new possibilities, such as correct handling of contextvars.