Add cleanup code to event_loop that mimics the behavior of asyncio.run

[Askaholic]

This is something I found useful and implemented in one of my own projects so I thought I'd make a PR in case the maintainers think this would be a good addition to the library itself. I'd be happy to modify it if only a subset of the changes is desired.

Some issues that I ran into with the current implementation:

• Errors that occur in a task spawned during one test can leak into subsequent tests since simply calling loop.close() is not sufficient to shutdown the event loop. This can make it extremely difficult to track down which test is causing the errors, since the test that fails is not actually the one that has the problem.
• When running with -W error under python 3.9 a similar problem happens with ResourceWarnings, where it becomes extremely difficult to figure out which test is actually opening the resources that are generating the warnings since the GC doesn't actually clean them up until some later test is already running.

Closes #200

[seifertm]

Thanks for the contribution!

The linked issue is not the only request of that feature. There is also #222. We closed #222 in favour of #235, which intends to used aioloop-proxy to have a "virtualized" event loop.

The aioloop-proxy issue has not been started, as far as I know. I think it's a good idea to add clean up code for outstanding tasks independently of #235.

Could you add some tests that assert the correct behaviour of your code?

[Askaholic]

Great! I've read through the issues you linked and I think I understand the current state of this feature. Sounds like eventually a more sophisticated solution will be added, but for now this PR will be useful. I'll get the development environment set up and work on writing a few tests!

[asvetlov]

Regarding https://github.com/pytest-dev/pytest-asyncio/issues/235 (aioloop-proxy based solution). I have a work-in-progress draft: https://github.com/pytest-dev/pytest-asyncio/pull/312 It is not finished yet, sorry. Now I'm working on improving asyncio itself, Python 3.11 feature freeze is coming.

[seifertm]

Thanks for uploading the latest version. I had some time to look into the issue that causes ResourceWarnings in the tests.

The culprit seems to be test_event_loop_scope.py::test_4. The test case requests the event_loop fixture and sets the current global loop to None. Requesting event_loop initializes a new loop. The code in pytest_fixture_post_finalizer tries to retrieve the current loop via asyncio.get_event_loop_policy().get_event_loop(), but that reference was set to None in the test case. The result is a stale event loop that triggers a ResourceWarning about an unclosed socket.

I was trying to work around the issue by using fixturedef.cached_result in pytest_fixture_post_finalizer to retrieve the loop provided by the event_loop fixture. Until now that wasn't successful, though. I'll keep digging…

[codecov-commenter]

Codecov Report

Base: 93.81% // Head: 93.23% // Decreases project coverage by -0.57% :warning:

Coverage data is based on head (5024eb8) compared to base (5697df2). Patch has no changes to coverable lines.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #309      +/-   ##
==========================================
- Coverage   93.81%   93.23%   -0.58%     
==========================================
  Files           3        3              
  Lines         275      281       +6     
  Branches       41       43       +2     
==========================================
+ Hits          258      262       +4     
- Misses         11       12       +1     
- Partials        6        7       +1     

Impacted Files	Coverage Δ
plugin.py	93.11% <0.00%> (-0.59%)	:arrow_down:

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

[seifertm]

I modified test_event_loop_scope.py::test_4 accordingly to avoid side-effects in other tests. The PR has been rebased and two typing issues have been addressed.

[elupus]

I've run into the issue of tasks leaking over test bounderings when i tried to get home assistant updated to later pytest-asyncio: So far i have these fixes/hacks: I've not managed to get it to run reliable as of yet thou.

Cleaning up of lingering tasks: https://github.com/home-assistant/core/pull/82475/commits/6e9130dd90ce7c5ebdd580f047d29ae1636b81ee

Cleaning up of lingering timers: https://github.com/home-assistant/core/pull/82475/commits/4d1335f75636475e45f423c1e3a0423230105528

[elupus]

I also just noticed that aiohttp loop fixture did trigger a GC call after test finishes: https://github.com/aio-libs/aiohttp/blob/905e0625777c20e4cf2d57fcfd4355827312f797/aiohttp/test_utils.py#L525

That likely explains some of my failures.

[matzipan]

So are the open threads the only things left for this to go forward? Or is there something else missing? I'll try to find some time for this.