From 03f3aad1ef9665a07dcc4d871b0a042d2ed20777 Mon Sep 17 00:00:00 2001 From: Michael Seifert Date: Mon, 30 Dec 2024 20:02:36 +0100 Subject: [PATCH 1/4] fix: Fix broken event loop when a function-scoped test is in between two wider-scoped tests. The event_loop fixture finalizers only close event loops that were not created by pytest-asyncio. This prevents the finalizers from accidentally closing a module-scoped loop, for example. --- docs/reference/changelog.rst | 5 +++ pytest_asyncio/plugin.py | 48 +++++++++++++++------- tests/markers/test_mixed_scope.py | 36 ++++++++++++++++ tests/test_event_loop_fixture_finalizer.py | 3 +- 4 files changed, 76 insertions(+), 16 deletions(-) create mode 100644 tests/markers/test_mixed_scope.py diff --git a/docs/reference/changelog.rst b/docs/reference/changelog.rst index 0b302d53..e219b723 100644 --- a/docs/reference/changelog.rst +++ b/docs/reference/changelog.rst @@ -2,6 +2,11 @@ Changelog ========= +0.25.1 (UNRELEASED) +=================== +- Fixes an issue that caused a broken event loop when a function-scoped test was executed in between two tests with wider loop scope `#950 `_ + + 0.25.0 (2024-12-13) =================== - Deprecated: Added warning when asyncio test requests async ``@pytest.fixture`` in strict mode. This will become an error in a future version of flake8-asyncio. `#979 `_ diff --git a/pytest_asyncio/plugin.py b/pytest_asyncio/plugin.py index 12ead10f..45b4001b 100644 --- a/pytest_asyncio/plugin.py +++ b/pytest_asyncio/plugin.py @@ -10,7 +10,7 @@ import inspect import socket import warnings -from asyncio import AbstractEventLoopPolicy +from asyncio import AbstractEventLoop, AbstractEventLoopPolicy from collections.abc import ( AsyncIterator, Awaitable, @@ -762,6 +762,19 @@ def _temporary_event_loop_policy(policy: AbstractEventLoopPolicy) -> Iterator[No try: yield finally: + # Try detecting user-created event loops that were left unclosed + # at the end of a test. + try: + current_loop: AbstractEventLoop | None = _get_event_loop_no_warn() + except RuntimeError: + current_loop = None + if current_loop is not None and not current_loop.is_closed(): + warnings.warn( + _UNCLOSED_EVENT_LOOP_WARNING % current_loop, + DeprecationWarning, + ) + current_loop.close() + asyncio.set_event_loop_policy(old_loop_policy) # When a test uses both a scoped event loop and the event_loop fixture, # the "_provide_clean_event_loop" finalizer of the event_loop fixture @@ -906,7 +919,7 @@ def _close_event_loop() -> None: loop = policy.get_event_loop() except RuntimeError: loop = None - if loop is not None: + if loop is not None and not getattr(loop, "__pytest_asyncio", False): if not loop.is_closed(): warnings.warn( _UNCLOSED_EVENT_LOOP_WARNING % loop, @@ -923,7 +936,7 @@ def _restore_policy(): loop = _get_event_loop_no_warn(previous_policy) except RuntimeError: loop = None - if loop: + if loop and not getattr(loop, "__pytest_asyncio", False): loop.close() asyncio.set_event_loop_policy(previous_policy) @@ -938,8 +951,13 @@ def _provide_clean_event_loop() -> None: # 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) + try: + old_loop = _get_event_loop_no_warn(policy) + except RuntimeError: + old_loop = None + if old_loop is not None and not getattr(old_loop, "__pytest_asyncio", False): + new_loop = policy.new_event_loop() + policy.set_event_loop(new_loop) def _get_event_loop_no_warn( @@ -1122,16 +1140,16 @@ def _retrieve_scope_root(item: Collector | Item, scope: str) -> Collector: def event_loop(request: FixtureRequest) -> Iterator[asyncio.AbstractEventLoop]: """Create an instance of the default event loop for each test case.""" new_loop_policy = request.getfixturevalue(event_loop_policy.__name__) - asyncio.set_event_loop_policy(new_loop_policy) - loop = asyncio.get_event_loop_policy().new_event_loop() - # Add a magic value to the event loop, so pytest-asyncio can determine if the - # event_loop fixture was overridden. Other implementations of event_loop don't - # set this value. - # The magic value must be set as part of the function definition, because pytest - # seems to have multiple instances of the same FixtureDef or fixture function - loop.__original_fixture_loop = True # type: ignore[attr-defined] - yield loop - loop.close() + with _temporary_event_loop_policy(new_loop_policy): + loop = asyncio.get_event_loop_policy().new_event_loop() + # Add a magic value to the event loop, so pytest-asyncio can determine if the + # event_loop fixture was overridden. Other implementations of event_loop don't + # set this value. + # The magic value must be set as part of the function definition, because pytest + # seems to have multiple instances of the same FixtureDef or fixture function + loop.__original_fixture_loop = True # type: ignore[attr-defined] + yield loop + loop.close() @pytest.fixture(scope="session") diff --git a/tests/markers/test_mixed_scope.py b/tests/markers/test_mixed_scope.py new file mode 100644 index 00000000..40eaaa35 --- /dev/null +++ b/tests/markers/test_mixed_scope.py @@ -0,0 +1,36 @@ +from __future__ import annotations + +from textwrap import dedent + +from pytest import Pytester + + +def test_function_scoped_loop_restores_previous_loop_scope(pytester: Pytester): + pytester.makeini("[pytest]\nasyncio_default_fixture_loop_scope = function") + pytester.makepyfile( + dedent( + """\ + import asyncio + import pytest + + + module_loop: asyncio.AbstractEventLoop + + @pytest.mark.asyncio(loop_scope="module") + async def test_remember_loop(): + global module_loop + module_loop = asyncio.get_running_loop() + + @pytest.mark.asyncio(loop_scope="function") + async def test_with_function_scoped_loop(): + pass + + @pytest.mark.asyncio(loop_scope="module") + async def test_runs_in_same_loop(): + global module_loop + assert asyncio.get_running_loop() is module_loop + """ + ) + ) + result = pytester.runpytest("--asyncio-mode=strict") + result.assert_outcomes(passed=3) diff --git a/tests/test_event_loop_fixture_finalizer.py b/tests/test_event_loop_fixture_finalizer.py index 17cc85b9..1e378643 100644 --- a/tests/test_event_loop_fixture_finalizer.py +++ b/tests/test_event_loop_fixture_finalizer.py @@ -14,7 +14,8 @@ def test_event_loop_fixture_finalizer_returns_fresh_loop_after_test(pytester: Py import pytest - loop = asyncio.get_event_loop_policy().get_event_loop() + loop = asyncio.new_event_loop() + asyncio.set_event_loop(loop) @pytest.mark.asyncio async def test_1(): From a86f08a16ba75100c63763d08516ad474a6a32f6 Mon Sep 17 00:00:00 2001 From: Michael Seifert Date: Mon, 30 Dec 2024 20:16:43 +0100 Subject: [PATCH 2/4] refactor: Extracted function to check if a loop was created by pytest-asyncio. --- pytest_asyncio/plugin.py | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/pytest_asyncio/plugin.py b/pytest_asyncio/plugin.py index 45b4001b..f63cbc03 100644 --- a/pytest_asyncio/plugin.py +++ b/pytest_asyncio/plugin.py @@ -872,8 +872,7 @@ def pytest_fixture_setup( policy = asyncio.get_event_loop_policy() try: old_loop = _get_event_loop_no_warn(policy) - is_pytest_asyncio_loop = getattr(old_loop, "__pytest_asyncio", False) - if old_loop is not loop and not is_pytest_asyncio_loop: + if old_loop is not loop and not _is_pytest_asyncio_loop(old_loop): old_loop.close() except RuntimeError: # Either the current event loop has been set to None @@ -886,6 +885,10 @@ def pytest_fixture_setup( yield +def _is_pytest_asyncio_loop(loop: AbstractEventLoop) -> bool: + return getattr(loop, "__pytest_asyncio", False) + + def _add_finalizers(fixturedef: FixtureDef, *finalizers: Callable[[], object]) -> None: """ Registers the specified fixture finalizers in the fixture. @@ -919,7 +922,7 @@ def _close_event_loop() -> None: loop = policy.get_event_loop() except RuntimeError: loop = None - if loop is not None and not getattr(loop, "__pytest_asyncio", False): + if loop is not None and not _is_pytest_asyncio_loop(loop): if not loop.is_closed(): warnings.warn( _UNCLOSED_EVENT_LOOP_WARNING % loop, @@ -936,7 +939,7 @@ def _restore_policy(): loop = _get_event_loop_no_warn(previous_policy) except RuntimeError: loop = None - if loop and not getattr(loop, "__pytest_asyncio", False): + if loop and not _is_pytest_asyncio_loop(loop): loop.close() asyncio.set_event_loop_policy(previous_policy) @@ -955,7 +958,7 @@ def _provide_clean_event_loop() -> None: old_loop = _get_event_loop_no_warn(policy) except RuntimeError: old_loop = None - if old_loop is not None and not getattr(old_loop, "__pytest_asyncio", False): + if old_loop is not None and not _is_pytest_asyncio_loop(old_loop): new_loop = policy.new_event_loop() policy.set_event_loop(new_loop) From c7c0d370ca5b0028e6d62d4c0db4afcb547de103 Mon Sep 17 00:00:00 2001 From: Michael Seifert Date: Mon, 30 Dec 2024 20:20:39 +0100 Subject: [PATCH 3/4] refactor: Extracted a function to mark an event loop as created by pytest-asyncio. --- pytest_asyncio/plugin.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/pytest_asyncio/plugin.py b/pytest_asyncio/plugin.py index f63cbc03..14ad8e82 100644 --- a/pytest_asyncio/plugin.py +++ b/pytest_asyncio/plugin.py @@ -709,8 +709,7 @@ def scoped_event_loop( ) -> Iterator[asyncio.AbstractEventLoop]: new_loop_policy = event_loop_policy with _temporary_event_loop_policy(new_loop_policy): - loop = asyncio.new_event_loop() - loop.__pytest_asyncio = True # type: ignore[attr-defined] + loop = _make_pytest_asyncio_loop(asyncio.new_event_loop()) asyncio.set_event_loop(loop) yield loop loop.close() @@ -885,6 +884,11 @@ def pytest_fixture_setup( yield +def _make_pytest_asyncio_loop(loop: AbstractEventLoop) -> AbstractEventLoop: + loop.__pytest_asyncio = True # type: ignore[attr-defined] + return loop + + def _is_pytest_asyncio_loop(loop: AbstractEventLoop) -> bool: return getattr(loop, "__pytest_asyncio", False) @@ -1161,8 +1165,7 @@ def _session_event_loop( ) -> Iterator[asyncio.AbstractEventLoop]: new_loop_policy = event_loop_policy with _temporary_event_loop_policy(new_loop_policy): - loop = asyncio.new_event_loop() - loop.__pytest_asyncio = True # type: ignore[attr-defined] + loop = _make_pytest_asyncio_loop(asyncio.new_event_loop()) asyncio.set_event_loop(loop) yield loop loop.close() From 7600594433bada0539742e7fa0bb4840a5499960 Mon Sep 17 00:00:00 2001 From: Michael Seifert Date: Mon, 30 Dec 2024 20:24:13 +0100 Subject: [PATCH 4/4] refactor: Replace the "__original_fixture_loop" magic attribute with the more generic "__pytest_asyncio" magic attribute. --- pytest_asyncio/plugin.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pytest_asyncio/plugin.py b/pytest_asyncio/plugin.py index 14ad8e82..00d52b2c 100644 --- a/pytest_asyncio/plugin.py +++ b/pytest_asyncio/plugin.py @@ -861,7 +861,7 @@ def pytest_fixture_setup( # Weird behavior was observed when checking for an attribute of FixtureDef.func # Instead, we now check for a special attribute of the returned event loop fixture_filename = inspect.getsourcefile(fixturedef.func) - if not getattr(loop, "__original_fixture_loop", False): + if not _is_pytest_asyncio_loop(loop): _, fixture_line_number = inspect.getsourcelines(fixturedef.func) warnings.warn( _REDEFINED_EVENT_LOOP_FIXTURE_WARNING @@ -1154,7 +1154,7 @@ def event_loop(request: FixtureRequest) -> Iterator[asyncio.AbstractEventLoop]: # set this value. # The magic value must be set as part of the function definition, because pytest # seems to have multiple instances of the same FixtureDef or fixture function - loop.__original_fixture_loop = True # type: ignore[attr-defined] + loop = _make_pytest_asyncio_loop(loop) yield loop loop.close()