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

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

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
24 changes: 19 additions & 5 deletions pytest_asyncio/plugin.py
Expand Up @@ -3,6 +3,7 @@
import contextlib
import enum
import functools
import gc
import inspect
import socket
import sys
Expand Down Expand Up @@ -360,8 +361,17 @@ def pytest_fixture_post_finalizer(fixturedef: FixtureDef, request: SubRequest) -
except RuntimeError:
loop = None
if loop is not None:
# Clean up existing loop to avoid ResourceWarnings
loop.close()
# Cleanup code based on the implementation of asyncio.run()
try:
if not loop.is_closed():
asyncio.runners._cancel_all_tasks( # type: ignore[attr-defined]
loop
)
loop.run_until_complete(loop.shutdown_asyncgens())
if sys.version_info >= (3, 9):
loop.run_until_complete(loop.shutdown_default_executor())
finally:
loop.close()
new_loop = policy.new_event_loop() # Replace existing event loop
# Ensure subsequent calls to get_event_loop() succeed
policy.set_event_loop(new_loop)
Expand Down Expand Up @@ -486,9 +496,13 @@ def pytest_runtest_setup(item: pytest.Item) -> None:
@pytest.fixture
def event_loop(request: "pytest.FixtureRequest") -> Iterator[asyncio.AbstractEventLoop]:
"""Create an instance of the default event loop for each test case."""
loop = asyncio.get_event_loop_policy().new_event_loop()
yield loop
loop.close()
yield asyncio.get_event_loop_policy().new_event_loop()
# Call the garbage collector to trigger ResourceWarning's as soon
# as possible (these are triggered in various __del__ methods).
# Without this, resources opened in one test can fail other tests
# when the warning is generated.
gc.collect()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A bunch of people implement their own event_loop fixture, in order to control fixture scope. If we change the fixture definition, we'd have to inform them to adjust as well.

Asking users to remove loop.close() to benefit from the new cleanup code is an easy sell. Adding a call to gc.collect in the event_loop fixture not so much.

Is there away around the gc.collect call in the fixture code?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose the call to gc.collect isn't directly related to task cleanup. I added this in my codebase because there were some tests that opened sockets and never closed them. Having the garbage collector run during the test teardown would make sure that the correct test failed when running pytest with -W error. But this is unrelated to pytest-asyncio so maybe it doesn't belong here.

# Event loop cleanup handled by pytest_fixture_post_finalizer


def _unused_port(socket_type: int) -> int:
Expand Down
38 changes: 38 additions & 0 deletions tests/test_event_loop_cleanup.py
@@ -0,0 +1,38 @@
from textwrap import dedent


def test_task_canceled_on_test_end(testdir):
seifertm marked this conversation as resolved.
Show resolved Hide resolved
testdir.makepyfile(
dedent(
"""\
import asyncio
import pytest

pytest_plugins = 'pytest_asyncio'

@pytest.mark.asyncio
async def test_a():
loop = asyncio.get_event_loop()

async def run_forever():
while True:
await asyncio.sleep(0.1)

loop.create_task(run_forever())
"""
)
)
testdir.makefile(
".ini",
pytest=dedent(
"""\
[pytest]
asyncio_mode = strict
filterwarnings =
error
"""
),
)
result = testdir.runpytest_subprocess()
result.assert_outcomes(passed=1)
result.stderr.no_fnmatch_line("Task was destroyed but it is pending!")
3 changes: 3 additions & 0 deletions tests/test_event_loop_scope.py
Expand Up @@ -34,4 +34,7 @@ def test_3():
def test_4(event_loop):
# If a test sets the loop to None -- pytest_fixture_post_finalizer()
# still should work

# Close to avoid ResourceWarning about unclosed socket as a side effect
event_loop.close()
asyncio.get_event_loop_policy().set_event_loop(None)