From 8a63a652b258448c74d8a157f9afb56b729f56bc Mon Sep 17 00:00:00 2001 From: Askaholic Date: Sat, 12 Mar 2022 12:26:13 -0900 Subject: [PATCH 1/7] Add cleanup code to event_loop that mimics the behavior of asyncio.run --- pytest_asyncio/plugin.py | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/pytest_asyncio/plugin.py b/pytest_asyncio/plugin.py index ce172f5f..7c84f4fd 100644 --- a/pytest_asyncio/plugin.py +++ b/pytest_asyncio/plugin.py @@ -3,6 +3,7 @@ import contextlib import enum import functools +import gc import inspect import socket import sys @@ -488,7 +489,19 @@ def event_loop(request: "pytest.FixtureRequest") -> Iterator[asyncio.AbstractEve """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() + # Cleanup code copied from the implementation of asyncio.run() + try: + asyncio.runners._cancel_all_tasks(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() + # 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() def _unused_port(socket_type: int) -> int: From effc2e5432765eecf70a1d7643bbe54a10129ace Mon Sep 17 00:00:00 2001 From: Askaholic Date: Sat, 19 Mar 2022 10:54:42 -0800 Subject: [PATCH 2/7] Skip cleanup if the loop was closed in the test --- pytest_asyncio/plugin.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/pytest_asyncio/plugin.py b/pytest_asyncio/plugin.py index 7c84f4fd..3518ede9 100644 --- a/pytest_asyncio/plugin.py +++ b/pytest_asyncio/plugin.py @@ -491,10 +491,11 @@ def event_loop(request: "pytest.FixtureRequest") -> Iterator[asyncio.AbstractEve yield loop # Cleanup code copied from the implementation of asyncio.run() try: - asyncio.runners._cancel_all_tasks(loop) - loop.run_until_complete(loop.shutdown_asyncgens()) - if sys.version_info >= (3, 9): - loop.run_until_complete(loop.shutdown_default_executor()) + if not loop.is_closed(): + asyncio.runners._cancel_all_tasks(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() # Call the garbage collector to trigger ResourceWarning's as soon From 046154d05f88071f57d06cfa94c845cd12db7518 Mon Sep 17 00:00:00 2001 From: Askaholic Date: Sat, 19 Mar 2022 10:55:16 -0800 Subject: [PATCH 3/7] Add test for verifying task cancellation --- tests/test_event_loop_cleanup.py | 38 ++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) create mode 100644 tests/test_event_loop_cleanup.py diff --git a/tests/test_event_loop_cleanup.py b/tests/test_event_loop_cleanup.py new file mode 100644 index 00000000..4b46d3d9 --- /dev/null +++ b/tests/test_event_loop_cleanup.py @@ -0,0 +1,38 @@ +from textwrap import dedent + + +def test_task_canceled_on_test_end(testdir): + 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!") From a43a7b55bd10c0cca852793854bd5f3bcecab295 Mon Sep 17 00:00:00 2001 From: Askaholic Date: Fri, 25 Mar 2022 17:39:29 -0800 Subject: [PATCH 4/7] Move cleanup code to pytest_fixture_post_finalizer --- pytest_asyncio/plugin.py | 34 ++++++++++++++++------------------ 1 file changed, 16 insertions(+), 18 deletions(-) diff --git a/pytest_asyncio/plugin.py b/pytest_asyncio/plugin.py index 3518ede9..cdf0e0b7 100644 --- a/pytest_asyncio/plugin.py +++ b/pytest_asyncio/plugin.py @@ -361,8 +361,15 @@ 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(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) @@ -487,22 +494,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 - # Cleanup code copied from the implementation of asyncio.run() - try: - if not loop.is_closed(): - asyncio.runners._cancel_all_tasks(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() - # 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() + return 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() + # Event loop cleanup handled by pytest_fixture_post_finalizer def _unused_port(socket_type: int) -> int: From b7f1a16592dc394b10e9c318cb338ea764dff70a Mon Sep 17 00:00:00 2001 From: Michael Seifert Date: Tue, 13 Sep 2022 21:40:43 +0200 Subject: [PATCH 5/7] test: Close event_loop in test_event_loop_scope/test_4 to avoid side-effects on other tests. Signed-off-by: Michael Seifert --- tests/test_event_loop_scope.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/test_event_loop_scope.py b/tests/test_event_loop_scope.py index 21fd6415..ff57c8f3 100644 --- a/tests/test_event_loop_scope.py +++ b/tests/test_event_loop_scope.py @@ -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) From 03ee1b1e9b66a9873b449db5bfadd94341909606 Mon Sep 17 00:00:00 2001 From: Michael Seifert Date: Tue, 13 Sep 2022 21:42:14 +0200 Subject: [PATCH 6/7] fix: Yield loop from event_loop fixture to avoid violating function signature. This prevents gc.collect() from being unreachable. Signed-off-by: Michael Seifert --- pytest_asyncio/plugin.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pytest_asyncio/plugin.py b/pytest_asyncio/plugin.py index cdf0e0b7..2e32585b 100644 --- a/pytest_asyncio/plugin.py +++ b/pytest_asyncio/plugin.py @@ -494,7 +494,7 @@ 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.""" - return asyncio.get_event_loop_policy().new_event_loop() + 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 From 5024eb8607235dd4993b631cce0b379923b1357e Mon Sep 17 00:00:00 2001 From: Michael Seifert Date: Tue, 13 Sep 2022 21:42:48 +0200 Subject: [PATCH 7/7] refactor: Ignore mypy problem when calling asyncio.runners._cancel_all_tasks. Signed-off-by: Michael Seifert --- pytest_asyncio/plugin.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pytest_asyncio/plugin.py b/pytest_asyncio/plugin.py index 2e32585b..70de2442 100644 --- a/pytest_asyncio/plugin.py +++ b/pytest_asyncio/plugin.py @@ -364,7 +364,9 @@ def pytest_fixture_post_finalizer(fixturedef: FixtureDef, request: SubRequest) - # Cleanup code based on the implementation of asyncio.run() try: if not loop.is_closed(): - asyncio.runners._cancel_all_tasks(loop) + 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())