From fac9092af9fab5625cff0dbecee7865b099f5309 Mon Sep 17 00:00:00 2001 From: Michael Seifert Date: Wed, 22 Nov 2023 10:54:42 +0100 Subject: [PATCH] [fix] Fixes a bug related to the ordering of the "event_loop" fixture in connection with parametrized tests. The fixture evaluation order changed for parametrizations of the same test. The reason is probably the fact that `event_loop` was inserted at position 0 in the pytest fixture closure for the current test. Since the synchronization wrapper for async tests uses the currently installed event loop rather than an explicit reference as of commit 36b226936e17232535e88ca34f9707cdf211776b, we can drop the insertion of the event_loop fixture as the first fixture to be evaluated. This patch also addresses an issue that caused RuntimeErrors when the event loop was set to None in a fixture that is requested by an async test. This can occur due to the use of asyncio.run, for example. Signed-off-by: Michael Seifert --- pytest_asyncio/plugin.py | 21 ++++++++--------- tests/markers/test_class_scope.py | 35 ++++++++++++++++++++++++++++ tests/markers/test_function_scope.py | 34 +++++++++++++++++++++++++++ tests/markers/test_module_scope.py | 34 +++++++++++++++++++++++++++ tests/markers/test_package_scope.py | 35 ++++++++++++++++++++++++++++ tests/markers/test_session_scope.py | 34 +++++++++++++++++++++++++++ 6 files changed, 182 insertions(+), 11 deletions(-) diff --git a/pytest_asyncio/plugin.py b/pytest_asyncio/plugin.py index 9dee882e..4f9ed217 100644 --- a/pytest_asyncio/plugin.py +++ b/pytest_asyncio/plugin.py @@ -623,7 +623,10 @@ def _removesuffix(s: str, suffix: str) -> str: @contextlib.contextmanager def _temporary_event_loop_policy(policy: AbstractEventLoopPolicy) -> Iterator[None]: old_loop_policy = asyncio.get_event_loop_policy() - old_loop = asyncio.get_event_loop() + try: + old_loop = asyncio.get_event_loop() + except RuntimeError: + old_loop = None asyncio.set_event_loop_policy(policy) try: yield @@ -634,7 +637,10 @@ def _temporary_event_loop_policy(policy: AbstractEventLoopPolicy) -> Iterator[No # will already have installed a fresh event loop, in order to shield # subsequent tests from side-effects. We close this loop before restoring # the old loop to avoid ResourceWarnings. - asyncio.get_event_loop().close() + try: + asyncio.get_event_loop().close() + except RuntimeError: + pass asyncio.set_event_loop(old_loop) @@ -908,15 +914,8 @@ def pytest_runtest_setup(item: pytest.Item) -> None: else: event_loop_fixture_id = "event_loop" fixturenames = item.fixturenames # type: ignore[attr-defined] - # inject an event loop fixture for all async tests - if "event_loop" in fixturenames: - # Move the "event_loop" fixture to the beginning of the fixture evaluation - # closure for backwards compatibility - fixturenames.remove("event_loop") - fixturenames.insert(0, "event_loop") - else: - if event_loop_fixture_id not in fixturenames: - fixturenames.append(event_loop_fixture_id) + if event_loop_fixture_id not in fixturenames: + fixturenames.append(event_loop_fixture_id) obj = getattr(item, "obj", None) if not getattr(obj, "hypothesis", False) and getattr( obj, "is_hypothesis_test", False diff --git a/tests/markers/test_class_scope.py b/tests/markers/test_class_scope.py index 1f664774..fa2fe81e 100644 --- a/tests/markers/test_class_scope.py +++ b/tests/markers/test_class_scope.py @@ -251,3 +251,38 @@ async def test_runs_in_different_loop_as_fixture(self, async_fixture): ) result = pytester.runpytest("--asyncio-mode=strict") result.assert_outcomes(passed=1) + + +def test_asyncio_mark_handles_missing_event_loop_triggered_by_fixture( + pytester: pytest.Pytester, +): + pytester.makepyfile( + dedent( + """\ + import pytest + import asyncio + + class TestClass: + @pytest.fixture(scope="class") + def sets_event_loop_to_none(self): + # asyncio.run() creates a new event loop without closing the + # existing one. For any test, but the first one, this leads to + # a ResourceWarning when the discarded loop is destroyed by the + # garbage collector. We close the current loop to avoid this. + try: + asyncio.get_event_loop().close() + except RuntimeError: + pass + return asyncio.run(asyncio.sleep(0)) + # asyncio.run() sets the current event loop to None when finished + + @pytest.mark.asyncio(scope="class") + # parametrization may impact fixture ordering + @pytest.mark.parametrize("n", (0, 1)) + async def test_does_not_fail(self, sets_event_loop_to_none, n): + pass + """ + ) + ) + result = pytester.runpytest("--asyncio-mode=strict") + result.assert_outcomes(passed=2) diff --git a/tests/markers/test_function_scope.py b/tests/markers/test_function_scope.py index df2c3e47..25ff609f 100644 --- a/tests/markers/test_function_scope.py +++ b/tests/markers/test_function_scope.py @@ -145,3 +145,37 @@ async def test_runs_is_same_loop_as_fixture(my_fixture): ) result = pytester.runpytest_subprocess("--asyncio-mode=strict") result.assert_outcomes(passed=1) + + +def test_asyncio_mark_handles_missing_event_loop_triggered_by_fixture( + pytester: Pytester, +): + pytester.makepyfile( + dedent( + """\ + import pytest + import asyncio + + @pytest.fixture + def sets_event_loop_to_none(): + # asyncio.run() creates a new event loop without closing the existing + # one. For any test, but the first one, this leads to a ResourceWarning + # when the discarded loop is destroyed by the garbage collector. + # We close the current loop to avoid this + try: + asyncio.get_event_loop().close() + except RuntimeError: + pass + return asyncio.run(asyncio.sleep(0)) + # asyncio.run() sets the current event loop to None when finished + + @pytest.mark.asyncio + # parametrization may impact fixture ordering + @pytest.mark.parametrize("n", (0, 1)) + async def test_does_not_fail(sets_event_loop_to_none, n): + pass + """ + ) + ) + result = pytester.runpytest("--asyncio-mode=strict") + result.assert_outcomes(passed=2) diff --git a/tests/markers/test_module_scope.py b/tests/markers/test_module_scope.py index b778c9a9..94547e40 100644 --- a/tests/markers/test_module_scope.py +++ b/tests/markers/test_module_scope.py @@ -280,3 +280,37 @@ async def test_runs_in_different_loop_as_fixture(async_fixture): ) result = pytester.runpytest("--asyncio-mode=strict") result.assert_outcomes(passed=1) + + +def test_asyncio_mark_handles_missing_event_loop_triggered_by_fixture( + pytester: Pytester, +): + pytester.makepyfile( + dedent( + """\ + import pytest + import asyncio + + @pytest.fixture(scope="module") + def sets_event_loop_to_none(): + # asyncio.run() creates a new event loop without closing the existing + # one. For any test, but the first one, this leads to a ResourceWarning + # when the discarded loop is destroyed by the garbage collector. + # We close the current loop to avoid this + try: + asyncio.get_event_loop().close() + except RuntimeError: + pass + return asyncio.run(asyncio.sleep(0)) + # asyncio.run() sets the current event loop to None when finished + + @pytest.mark.asyncio(scope="module") + # parametrization may impact fixture ordering + @pytest.mark.parametrize("n", (0, 1)) + async def test_does_not_fail(sets_event_loop_to_none, n): + pass + """ + ) + ) + result = pytester.runpytest("--asyncio-mode=strict") + result.assert_outcomes(passed=2) diff --git a/tests/markers/test_package_scope.py b/tests/markers/test_package_scope.py index 3d898c8d..1dc8a5c9 100644 --- a/tests/markers/test_package_scope.py +++ b/tests/markers/test_package_scope.py @@ -314,3 +314,38 @@ async def test_runs_in_different_loop_as_fixture(async_fixture): ) result = pytester.runpytest("--asyncio-mode=strict") result.assert_outcomes(passed=1) + + +def test_asyncio_mark_handles_missing_event_loop_triggered_by_fixture( + pytester: Pytester, +): + pytester.makepyfile( + __init__="", + test_loop_is_none=dedent( + """\ + import pytest + import asyncio + + @pytest.fixture(scope="package") + def sets_event_loop_to_none(): + # asyncio.run() creates a new event loop without closing the existing + # one. For any test, but the first one, this leads to a ResourceWarning + # when the discarded loop is destroyed by the garbage collector. + # We close the current loop to avoid this + try: + asyncio.get_event_loop().close() + except RuntimeError: + pass + return asyncio.run(asyncio.sleep(0)) + # asyncio.run() sets the current event loop to None when finished + + @pytest.mark.asyncio(scope="package") + # parametrization may impact fixture ordering + @pytest.mark.parametrize("n", (0, 1)) + async def test_does_not_fail(sets_event_loop_to_none, n): + pass + """ + ), + ) + result = pytester.runpytest("--asyncio-mode=strict") + result.assert_outcomes(passed=2) diff --git a/tests/markers/test_session_scope.py b/tests/markers/test_session_scope.py index a9a8b7a8..ac70d01d 100644 --- a/tests/markers/test_session_scope.py +++ b/tests/markers/test_session_scope.py @@ -348,3 +348,37 @@ async def test_runs_in_different_loop_as_fixture(async_fixture): ) result = pytester.runpytest("--asyncio-mode=strict") result.assert_outcomes(passed=1) + + +def test_asyncio_mark_handles_missing_event_loop_triggered_by_fixture( + pytester: Pytester, +): + pytester.makepyfile( + dedent( + """\ + import pytest + import asyncio + + @pytest.fixture(scope="session") + def sets_event_loop_to_none(): + # asyncio.run() creates a new event loop without closing the existing + # one. For any test, but the first one, this leads to a ResourceWarning + # when the discarded loop is destroyed by the garbage collector. + # We close the current loop to avoid this + try: + asyncio.get_event_loop().close() + except RuntimeError: + pass + return asyncio.run(asyncio.sleep(0)) + # asyncio.run() sets the current event loop to None when finished + + @pytest.mark.asyncio(scope="session") + # parametrization may impact fixture ordering + @pytest.mark.parametrize("n", (0, 1)) + async def test_does_not_fail(sets_event_loop_to_none, n): + pass + """ + ) + ) + result = pytester.runpytest("--asyncio-mode=strict") + result.assert_outcomes(passed=2)