From 5ef42ddccd0b5ac4a508513c5c80185f1e4e6805 Mon Sep 17 00:00:00 2001 From: Michael Seifert Date: Sat, 4 Feb 2023 13:58:27 +0100 Subject: [PATCH 1/9] [tests] Run tests for event loop policy inside Pytester to avoid pollution of other tests with a custom event loop. Signed-off-by: Michael Seifert --- tests/respect_event_loop_policy/conftest.py | 16 ------ .../test_respects_event_loop_policy.py | 17 ------ tests/test_event_loop_fixture.py | 53 +++++++++++++++++++ 3 files changed, 53 insertions(+), 33 deletions(-) delete mode 100644 tests/respect_event_loop_policy/conftest.py delete mode 100644 tests/respect_event_loop_policy/test_respects_event_loop_policy.py create mode 100644 tests/test_event_loop_fixture.py diff --git a/tests/respect_event_loop_policy/conftest.py b/tests/respect_event_loop_policy/conftest.py deleted file mode 100644 index 2c5cef24..00000000 --- a/tests/respect_event_loop_policy/conftest.py +++ /dev/null @@ -1,16 +0,0 @@ -"""Defines and sets a custom event loop policy""" -import asyncio -from asyncio import DefaultEventLoopPolicy, SelectorEventLoop - - -class TestEventLoop(SelectorEventLoop): - pass - - -class TestEventLoopPolicy(DefaultEventLoopPolicy): - def new_event_loop(self): - return TestEventLoop() - - -# This statement represents a code which sets a custom event loop policy -asyncio.set_event_loop_policy(TestEventLoopPolicy()) diff --git a/tests/respect_event_loop_policy/test_respects_event_loop_policy.py b/tests/respect_event_loop_policy/test_respects_event_loop_policy.py deleted file mode 100644 index 610b3388..00000000 --- a/tests/respect_event_loop_policy/test_respects_event_loop_policy.py +++ /dev/null @@ -1,17 +0,0 @@ -"""Tests that any externally provided event loop policy remains unaltered.""" -import asyncio - -import pytest - - -@pytest.mark.asyncio -async def test_uses_loop_provided_by_custom_policy(): - """Asserts that test cases use the event loop - provided by the custom event loop policy""" - assert type(asyncio.get_event_loop()).__name__ == "TestEventLoop" - - -@pytest.mark.asyncio -async def test_custom_policy_is_not_overwritten(): - """Asserts that any custom event loop policy stays the same across test cases""" - assert type(asyncio.get_event_loop()).__name__ == "TestEventLoop" diff --git a/tests/test_event_loop_fixture.py b/tests/test_event_loop_fixture.py new file mode 100644 index 00000000..aaf591c9 --- /dev/null +++ b/tests/test_event_loop_fixture.py @@ -0,0 +1,53 @@ +from textwrap import dedent + +from pytest import Pytester + + +def test_event_loop_fixture_respects_event_loop_policy(pytester: Pytester): + pytester.makeconftest( + dedent( + """\ + '''Defines and sets a custom event loop policy''' + import asyncio + from asyncio import DefaultEventLoopPolicy, SelectorEventLoop + + class TestEventLoop(SelectorEventLoop): + pass + + class TestEventLoopPolicy(DefaultEventLoopPolicy): + def new_event_loop(self): + return TestEventLoop() + + # This statement represents a code which sets a custom event loop policy + asyncio.set_event_loop_policy(TestEventLoopPolicy()) + """ + ) + ) + pytester.makepyfile( + dedent( + """\ + '''Tests that any externally provided event loop policy remains unaltered''' + import asyncio + + import pytest + + + @pytest.mark.asyncio + async def test_uses_loop_provided_by_custom_policy(): + '''Asserts that test cases use the event loop + provided by the custom event loop policy''' + assert type(asyncio.get_event_loop()).__name__ == "TestEventLoop" + + + @pytest.mark.asyncio + async def test_custom_policy_is_not_overwritten(): + ''' + Asserts that any custom event loop policy stays the same + across test cases. + ''' + assert type(asyncio.get_event_loop()).__name__ == "TestEventLoop" + """ + ) + ) + result = pytester.runpytest_subprocess("--asyncio-mode=strict") + result.assert_outcomes(passed=2) From 31d09f998a95a10ab86335671675efd2b6163b55 Mon Sep 17 00:00:00 2001 From: Michael Seifert Date: Sat, 4 Feb 2023 13:59:32 +0100 Subject: [PATCH 2/9] [tests] test_async_fixtures_scope closes the event loop on fixture teardown Signed-off-by: Michael Seifert --- tests/async_fixtures/test_async_fixtures_scope.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/async_fixtures/test_async_fixtures_scope.py b/tests/async_fixtures/test_async_fixtures_scope.py index b150f8a8..079a981a 100644 --- a/tests/async_fixtures/test_async_fixtures_scope.py +++ b/tests/async_fixtures/test_async_fixtures_scope.py @@ -10,7 +10,9 @@ @pytest.fixture(scope="module") def event_loop(): """A module-scoped event loop.""" - return asyncio.new_event_loop() + loop = asyncio.new_event_loop() + yield loop + loop.close() @pytest.fixture(scope="module") From 55586fd796bb7daa94d5498b45e02a5c3100cc52 Mon Sep 17 00:00:00 2001 From: Michael Seifert Date: Sat, 4 Feb 2023 14:32:51 +0100 Subject: [PATCH 3/9] [refactor] Substituted use of pytest_fixture_post_finalizer hook with fixture finalizer. The fixture finalizer is invoked once for each fixture, whereas the hook may be invoked multiple times for any specific fixture. Signed-off-by: Michael Seifert --- pytest_asyncio/plugin.py | 52 +++++++++++++++++++--------------------- 1 file changed, 25 insertions(+), 27 deletions(-) diff --git a/pytest_asyncio/plugin.py b/pytest_asyncio/plugin.py index 21809b6d..5dd25535 100644 --- a/pytest_asyncio/plugin.py +++ b/pytest_asyncio/plugin.py @@ -370,39 +370,18 @@ def _hypothesis_test_wraps_coroutine(function: Any) -> bool: return _is_coroutine(function.hypothesis.inner_test) -@pytest.hookimpl(trylast=True) -def pytest_fixture_post_finalizer(fixturedef: FixtureDef, request: SubRequest) -> None: - """ - Called after fixture teardown. - - Note that this function may be called multiple times for any specific fixture. - see https://github.com/pytest-dev/pytest/issues/5848 - """ - if fixturedef.argname == "event_loop": - policy = asyncio.get_event_loop_policy() - try: - loop = policy.get_event_loop() - except RuntimeError: - loop = None - if loop is not None: - # Clean up existing loop to avoid ResourceWarnings - loop.close() - # At this point, the event loop for the current thread is closed. - # When a user calls asyncio.get_event_loop(), they will get a closed loop. - # In order to avoid this side effect from pytest-asyncio, we need to replace - # the current loop with a fresh one. - # Note that we cannot set the loop to None, because get_event_loop only creates - # a new loop, when set_event_loop has not been called. - new_loop = policy.new_event_loop() - policy.set_event_loop(new_loop) - - @pytest.hookimpl(hookwrapper=True) def pytest_fixture_setup( fixturedef: FixtureDef, request: SubRequest ) -> Optional[object]: """Adjust the event loop policy when an event loop is produced.""" if fixturedef.argname == "event_loop": + # The use of a fixture finalizer is preferred over the + # pytest_fixture_post_finalizer hook. The fixture finalizer is invoked once + # for each fixture, whereas the hook may be invoked multiple times for + # any specific fixture. + # see https://github.com/pytest-dev/pytest/issues/5848 + fixturedef.addfinalizer(_close_event_loop) outcome = yield loop = outcome.get_result() policy = asyncio.get_event_loop_policy() @@ -421,6 +400,25 @@ def pytest_fixture_setup( yield +def _close_event_loop() -> None: + policy = asyncio.get_event_loop_policy() + try: + loop = policy.get_event_loop() + except RuntimeError: + loop = None + if loop is not None: + # Clean up existing loop to avoid ResourceWarnings + loop.close() + # At this point, the event loop for the current thread is closed. + # When a user calls asyncio.get_event_loop(), they will get a closed loop. + # In order to avoid this side effect from pytest-asyncio, we need to replace + # the current loop with a fresh one. + # Note that we cannot set the loop to None, because get_event_loop only creates + # a new loop, when set_event_loop has not been called. + new_loop = policy.new_event_loop() + policy.set_event_loop(new_loop) + + @pytest.hookimpl(tryfirst=True, hookwrapper=True) def pytest_pyfunc_call(pyfuncitem: pytest.Function) -> Optional[object]: """ From be29c642d67e2f27472cfe4e8144b902603801f0 Mon Sep 17 00:00:00 2001 From: Michael Seifert Date: Sat, 4 Feb 2023 14:47:12 +0100 Subject: [PATCH 4/9] [refactor] Replaced use of the testdir fixture with pytester in hypothesis/test_base Signed-off-by: Michael Seifert --- tests/hypothesis/test_base.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/tests/hypothesis/test_base.py b/tests/hypothesis/test_base.py index e6da3427..43fc6084 100644 --- a/tests/hypothesis/test_base.py +++ b/tests/hypothesis/test_base.py @@ -6,6 +6,7 @@ import pytest from hypothesis import given, strategies as st +from pytest import Pytester @pytest.fixture(scope="module") @@ -43,8 +44,8 @@ async def test_can_use_fixture_provided_event_loop(event_loop, n): await semaphore.acquire() -def test_async_auto_marked(testdir): - testdir.makepyfile( +def test_async_auto_marked(pytester: Pytester): + pytester.makepyfile( dedent( """\ import asyncio @@ -60,13 +61,13 @@ async def test_hypothesis(n: int): """ ) ) - result = testdir.runpytest("--asyncio-mode=auto") + result = pytester.runpytest("--asyncio-mode=auto") result.assert_outcomes(passed=1) -def test_sync_not_auto_marked(testdir): +def test_sync_not_auto_marked(pytester: Pytester): """Assert that synchronous Hypothesis functions are not marked with asyncio""" - testdir.makepyfile( + pytester.makepyfile( dedent( """\ import asyncio @@ -84,5 +85,5 @@ def test_hypothesis(request, n: int): """ ) ) - result = testdir.runpytest("--asyncio-mode=auto") + result = pytester.runpytest("--asyncio-mode=auto") result.assert_outcomes(passed=1) From ce5017cc32a8fec30455dee548d3943dc8080b3e Mon Sep 17 00:00:00 2001 From: Michael Seifert Date: Sat, 4 Feb 2023 14:51:01 +0100 Subject: [PATCH 5/9] [refactor] Isolate module-scoped event loop fixture in hypothesis/test_base using pytester. Signed-off-by: Michael Seifert --- tests/hypothesis/test_base.py | 42 +++++++++++++++++++++++------------ 1 file changed, 28 insertions(+), 14 deletions(-) diff --git a/tests/hypothesis/test_base.py b/tests/hypothesis/test_base.py index 43fc6084..aef20d79 100644 --- a/tests/hypothesis/test_base.py +++ b/tests/hypothesis/test_base.py @@ -1,7 +1,6 @@ """Tests for the Hypothesis integration, which wraps async functions in a sync shim for Hypothesis. """ -import asyncio from textwrap import dedent import pytest @@ -9,13 +8,6 @@ from pytest import Pytester -@pytest.fixture(scope="module") -def event_loop(): - loop = asyncio.get_event_loop_policy().new_event_loop() - yield loop - loop.close() - - @given(st.integers()) @pytest.mark.asyncio async def test_mark_inner(n): @@ -36,12 +28,34 @@ async def test_mark_and_parametrize(x, y): assert y in (1, 2) -@given(st.integers()) -@pytest.mark.asyncio -async def test_can_use_fixture_provided_event_loop(event_loop, n): - semaphore = asyncio.Semaphore(value=0) - event_loop.call_soon(semaphore.release) - await semaphore.acquire() +def test_can_use_explicit_event_loop_fixture(pytester: Pytester): + pytester.makepyfile( + dedent( + """\ + import asyncio + import pytest + from hypothesis import given + import hypothesis.strategies as st + + pytest_plugins = 'pytest_asyncio' + + @pytest.fixture(scope="module") + def event_loop(): + loop = asyncio.get_event_loop_policy().new_event_loop() + yield loop + loop.close() + + @given(st.integers()) + @pytest.mark.asyncio + async def test_explicit_fixture_request(event_loop, n): + semaphore = asyncio.Semaphore(value=0) + event_loop.call_soon(semaphore.release) + await semaphore.acquire() + """ + ) + ) + result = pytester.runpytest("--asyncio-mode=strict") + result.assert_outcomes(passed=1) def test_async_auto_marked(pytester: Pytester): From 47c91dd494ac9340955f0ae1a58106c033bd6432 Mon Sep 17 00:00:00 2001 From: Michael Seifert Date: Sat, 4 Feb 2023 15:22:43 +0100 Subject: [PATCH 6/9] [feat] Emit ResourceWarning when event_loop is torn down and the current event loop has not been closed. Signed-off-by: Michael Seifert --- docs/source/reference/changelog.rst | 1 + pytest_asyncio/plugin.py | 22 ++++++++++++++++- tests/test_event_loop_fixture_finalizer.py | 28 ++++++++++++++++++++++ 3 files changed, 50 insertions(+), 1 deletion(-) diff --git a/docs/source/reference/changelog.rst b/docs/source/reference/changelog.rst index af4eb7bf..851bbea0 100644 --- a/docs/source/reference/changelog.rst +++ b/docs/source/reference/changelog.rst @@ -5,6 +5,7 @@ Changelog UNRELEASED ================= - Drop compatibility with pytest 6.1. Pytest-asyncio now depends on pytest 7.0 or newer. +- event_loop fixture teardown emits a ResourceWarning when the current event loop has not been closed. 0.20.3 (22-12-08) ================= diff --git a/pytest_asyncio/plugin.py b/pytest_asyncio/plugin.py index 5dd25535..a9ced52a 100644 --- a/pytest_asyncio/plugin.py +++ b/pytest_asyncio/plugin.py @@ -7,6 +7,7 @@ import socket import sys import warnings +from textwrap import dedent from typing import ( Any, AsyncIterator, @@ -400,6 +401,17 @@ def pytest_fixture_setup( yield +_UNCLOSED_EVENT_LOOP_WARNING = dedent( + """\ + unclosed event loop %r. + Possible causes are: + 1. A custom "event_loop" fixture is used which doesn't close the loop + 2. Your code or one of your dependencies created a new event loop during + the test run + """ +) + + def _close_event_loop() -> None: policy = asyncio.get_event_loop_policy() try: @@ -407,7 +419,15 @@ def _close_event_loop() -> None: except RuntimeError: loop = None if loop is not None: - # Clean up existing loop to avoid ResourceWarnings + # Emit ResourceWarnings in the context of the fixture/test case + # rather than waiting for the interpreter to trigger the warning when + # garbage collecting the event loop. + if not loop.is_closed(): + warnings.warn( + _UNCLOSED_EVENT_LOOP_WARNING % loop, + ResourceWarning, + source=loop, + ) loop.close() # At this point, the event loop for the current thread is closed. # When a user calls asyncio.get_event_loop(), they will get a closed loop. diff --git a/tests/test_event_loop_fixture_finalizer.py b/tests/test_event_loop_fixture_finalizer.py index d3622a08..2d12f7f4 100644 --- a/tests/test_event_loop_fixture_finalizer.py +++ b/tests/test_event_loop_fixture_finalizer.py @@ -86,3 +86,31 @@ async def test_async_with_explicit_fixture_request(event_loop): ) result = pytester.runpytest("--asyncio-mode=strict") result.assert_outcomes(passed=1) + + +def test_event_loop_fixture_finalizer_raises_warning_when_loop_is_unclosed( + pytester: Pytester, +): + pytester.makepyfile( + dedent( + """\ + import asyncio + import pytest + import pytest_asyncio + + pytest_plugins = 'pytest_asyncio' + + @pytest.fixture + def event_loop(): + loop = asyncio.get_event_loop_policy().new_event_loop() + yield loop + + @pytest.mark.asyncio + async def test_ends_with_unclosed_loop(): + pass + """ + ) + ) + result = pytester.runpytest("--asyncio-mode=strict", "-W", "default") + result.assert_outcomes(passed=1, warnings=1) + result.stdout.fnmatch_lines("*unclosed event loop*") From de7e71f9252ee4413ffc32b7f395ca14a225df2d Mon Sep 17 00:00:00 2001 From: Michael Seifert Date: Sat, 4 Feb 2023 15:28:53 +0100 Subject: [PATCH 7/9] [refactor] Split up event_loop fixture finalizer into a finalizer for closing the loop and a finalizer for providing a new loop. Signed-off-by: Michael Seifert --- pytest_asyncio/plugin.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/pytest_asyncio/plugin.py b/pytest_asyncio/plugin.py index a9ced52a..539a8d13 100644 --- a/pytest_asyncio/plugin.py +++ b/pytest_asyncio/plugin.py @@ -382,6 +382,7 @@ def pytest_fixture_setup( # for each fixture, whereas the hook may be invoked multiple times for # any specific fixture. # see https://github.com/pytest-dev/pytest/issues/5848 + fixturedef.addfinalizer(_provide_clean_event_loop) fixturedef.addfinalizer(_close_event_loop) outcome = yield loop = outcome.get_result() @@ -429,12 +430,16 @@ def _close_event_loop() -> None: source=loop, ) loop.close() + + +def _provide_clean_event_loop() -> None: # At this point, the event loop for the current thread is closed. # When a user calls asyncio.get_event_loop(), they will get a closed loop. # In order to avoid this side effect from pytest-asyncio, we need to replace # the current loop with a fresh one. # Note that we cannot set the loop to None, because get_event_loop only creates # a new loop, when set_event_loop has not been called. + policy = asyncio.get_event_loop_policy() new_loop = policy.new_event_loop() policy.set_event_loop(new_loop) From d1c817be94f38a1faa6abb1cbb567e4de04060c0 Mon Sep 17 00:00:00 2001 From: Michael Seifert Date: Sat, 4 Feb 2023 15:32:21 +0100 Subject: [PATCH 8/9] [refactor] Created convenience function to add multiple finalizers to a single fixture. Signed-off-by: Michael Seifert --- pytest_asyncio/plugin.py | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/pytest_asyncio/plugin.py b/pytest_asyncio/plugin.py index 539a8d13..2b4c2712 100644 --- a/pytest_asyncio/plugin.py +++ b/pytest_asyncio/plugin.py @@ -382,8 +382,11 @@ def pytest_fixture_setup( # for each fixture, whereas the hook may be invoked multiple times for # any specific fixture. # see https://github.com/pytest-dev/pytest/issues/5848 - fixturedef.addfinalizer(_provide_clean_event_loop) - fixturedef.addfinalizer(_close_event_loop) + _add_finalizers( + fixturedef, + _close_event_loop, + _provide_clean_event_loop, + ) outcome = yield loop = outcome.get_result() policy = asyncio.get_event_loop_policy() @@ -402,6 +405,16 @@ def pytest_fixture_setup( yield +def _add_finalizers(fixturedef: FixtureDef, *finalizers: Callable[[], object]) -> None: + """ + Regsiters the specified fixture finalizers in the fixture. + :param fixturedef: Fixture definition which finalizers should be added to + :param finalizers: Finalizers to be added + """ + for finalizer in reversed(finalizers): + fixturedef.addfinalizer(finalizer) + + _UNCLOSED_EVENT_LOOP_WARNING = dedent( """\ unclosed event loop %r. From c66691cb7135145f8dcaf2b772a017df77de1433 Mon Sep 17 00:00:00 2001 From: Michael Seifert Date: Sat, 4 Feb 2023 15:32:46 +0100 Subject: [PATCH 9/9] [refactor] Created convenience function to add multiple finalizers to a single fixture. Signed-off-by: Michael Seifert --- pytest_asyncio/plugin.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pytest_asyncio/plugin.py b/pytest_asyncio/plugin.py index 2b4c2712..0b6fa9db 100644 --- a/pytest_asyncio/plugin.py +++ b/pytest_asyncio/plugin.py @@ -408,6 +408,9 @@ def pytest_fixture_setup( def _add_finalizers(fixturedef: FixtureDef, *finalizers: Callable[[], object]) -> None: """ Regsiters the specified fixture finalizers in the fixture. + + Finalizers need to specified in the exact order in which they should be invoked. + :param fixturedef: Fixture definition which finalizers should be added to :param finalizers: Finalizers to be added """