From 30dfbc8fe379ece79824874d54eff5aed678cd69 Mon Sep 17 00:00:00 2001 From: Michael Seifert Date: Sun, 19 Nov 2023 11:16:53 +0100 Subject: [PATCH 1/3] [refactor] Moved tests related to tcp and udp port fixtures to a separate module Signed-off-by: Michael Seifert --- tests/test_port_factories.py | 197 +++++++++++++++++++++++++++++++++++ tests/test_simple.py | 160 ---------------------------- 2 files changed, 197 insertions(+), 160 deletions(-) create mode 100644 tests/test_port_factories.py diff --git a/tests/test_port_factories.py b/tests/test_port_factories.py new file mode 100644 index 00000000..cbbd47b4 --- /dev/null +++ b/tests/test_port_factories.py @@ -0,0 +1,197 @@ +from textwrap import dedent + +from pytest import Pytester + +import pytest_asyncio.plugin + + +def test_unused_tcp_port_selects_unused_port(pytester: Pytester): + pytester.makepyfile( + dedent( + """\ + import asyncio + + import pytest + + @pytest.mark.asyncio + async def test_unused_port_fixture(unused_tcp_port): + async def closer(_, writer): + writer.close() + + server1 = await asyncio.start_server( + closer, host="localhost", port=unused_tcp_port + ) + + with pytest.raises(IOError): + await asyncio.start_server( + closer, host="localhost", port=unused_tcp_port + ) + + server1.close() + await server1.wait_closed() + """ + ) + ) + + +def test_unused_udp_port_selects_unused_port(pytester: Pytester): + pytester.makepyfile( + dedent( + """\ + @pytest.mark.asyncio + async def test_unused_udp_port_fixture(unused_udp_port): + class Closer: + def connection_made(self, transport): + pass + + def connection_lost(self, *arg, **kwd): + pass + + event_loop = asyncio.get_running_loop() + transport1, _ = await event_loop.create_datagram_endpoint( + Closer, + local_addr=("127.0.0.1", unused_udp_port), + reuse_port=False, + ) + + with pytest.raises(IOError): + await event_loop.create_datagram_endpoint( + Closer, + local_addr=("127.0.0.1", unused_udp_port), + reuse_port=False, + ) + + transport1.abort() + """ + ) + ) + + +def test_unused_tcp_port_factory_selects_unused_port(pytester: Pytester): + pytester.makepyfile( + dedent( + """\ + @pytest.mark.asyncio + async def test_unused_port_factory_fixture(unused_tcp_port_factory): + async def closer(_, writer): + writer.close() + + port1, port2, port3 = ( + unused_tcp_port_factory(), + unused_tcp_port_factory(), + unused_tcp_port_factory(), + ) + + server1 = await asyncio.start_server( + closer, host="localhost", port=port1 + ) + server2 = await asyncio.start_server( + closer, host="localhost", port=port2 + ) + server3 = await asyncio.start_server( + closer, host="localhost", port=port3 + ) + + for port in port1, port2, port3: + with pytest.raises(IOError): + await asyncio.start_server(closer, host="localhost", port=port) + + server1.close() + await server1.wait_closed() + server2.close() + await server2.wait_closed() + server3.close() + await server3.wait_closed() + """ + ) + ) + + +def test_unused_udp_port_factory_selects_unused_port(pytester: Pytester): + pytester.makepyfile( + dedent( + """\ + @pytest.mark.asyncio + async def test_unused_udp_port_factory_fixture(unused_udp_port_factory): + class Closer: + def connection_made(self, transport): + pass + + def connection_lost(self, *arg, **kwd): + pass + + port1, port2, port3 = ( + unused_udp_port_factory(), + unused_udp_port_factory(), + unused_udp_port_factory(), + ) + + event_loop = asyncio.get_running_loop() + transport1, _ = await event_loop.create_datagram_endpoint( + Closer, + local_addr=("127.0.0.1", port1), + reuse_port=False, + ) + transport2, _ = await event_loop.create_datagram_endpoint( + Closer, + local_addr=("127.0.0.1", port2), + reuse_port=False, + ) + transport3, _ = await event_loop.create_datagram_endpoint( + Closer, + local_addr=("127.0.0.1", port3), + reuse_port=False, + ) + + for port in port1, port2, port3: + with pytest.raises(IOError): + await event_loop.create_datagram_endpoint( + Closer, + local_addr=("127.0.0.1", port), + reuse_port=False, + ) + + transport1.abort() + transport2.abort() + transport3.abort() + """ + ) + ) + + +def test_unused_port_factory_duplicate(unused_tcp_port_factory, monkeypatch): + """Test correct avoidance of duplicate ports.""" + counter = 0 + + def mock_unused_tcp_port(_ignored): + """Force some duplicate ports.""" + nonlocal counter + counter += 1 + if counter < 5: + return 10000 + else: + return 10000 + counter + + monkeypatch.setattr(pytest_asyncio.plugin, "_unused_port", mock_unused_tcp_port) + + assert unused_tcp_port_factory() == 10000 + assert unused_tcp_port_factory() > 10000 + + +def test_unused_udp_port_factory_duplicate(unused_udp_port_factory, monkeypatch): + """Test correct avoidance of duplicate UDP ports.""" + counter = 0 + + def mock_unused_udp_port(_ignored): + """Force some duplicate ports.""" + nonlocal counter + counter += 1 + if counter < 5: + return 10000 + else: + return 10000 + counter + + monkeypatch.setattr(pytest_asyncio.plugin, "_unused_port", mock_unused_udp_port) + + assert unused_udp_port_factory() == 10000 + assert unused_udp_port_factory() > 10000 diff --git a/tests/test_simple.py b/tests/test_simple.py index b6020c69..c448de92 100644 --- a/tests/test_simple.py +++ b/tests/test_simple.py @@ -5,8 +5,6 @@ import pytest from pytest import Pytester -import pytest_asyncio.plugin - async def async_coro(): await asyncio.sleep(0) @@ -69,164 +67,6 @@ async def test_asyncio_marker_with_default_param(a_param=None): await asyncio.sleep(0) -@pytest.mark.asyncio -async def test_unused_port_fixture(unused_tcp_port): - """Test the unused TCP port fixture.""" - - async def closer(_, writer): - writer.close() - - server1 = await asyncio.start_server(closer, host="localhost", port=unused_tcp_port) - - with pytest.raises(IOError): - await asyncio.start_server(closer, host="localhost", port=unused_tcp_port) - - server1.close() - await server1.wait_closed() - - -@pytest.mark.asyncio -async def test_unused_udp_port_fixture(unused_udp_port): - """Test the unused TCP port fixture.""" - - class Closer: - def connection_made(self, transport): - pass - - def connection_lost(self, *arg, **kwd): - pass - - event_loop = asyncio.get_running_loop() - transport1, _ = await event_loop.create_datagram_endpoint( - Closer, - local_addr=("127.0.0.1", unused_udp_port), - reuse_port=False, - ) - - with pytest.raises(IOError): - await event_loop.create_datagram_endpoint( - Closer, - local_addr=("127.0.0.1", unused_udp_port), - reuse_port=False, - ) - - transport1.abort() - - -@pytest.mark.asyncio -async def test_unused_port_factory_fixture(unused_tcp_port_factory): - """Test the unused TCP port factory fixture.""" - - async def closer(_, writer): - writer.close() - - port1, port2, port3 = ( - unused_tcp_port_factory(), - unused_tcp_port_factory(), - unused_tcp_port_factory(), - ) - - server1 = await asyncio.start_server(closer, host="localhost", port=port1) - server2 = await asyncio.start_server(closer, host="localhost", port=port2) - server3 = await asyncio.start_server(closer, host="localhost", port=port3) - - for port in port1, port2, port3: - with pytest.raises(IOError): - await asyncio.start_server(closer, host="localhost", port=port) - - server1.close() - await server1.wait_closed() - server2.close() - await server2.wait_closed() - server3.close() - await server3.wait_closed() - - -@pytest.mark.asyncio -async def test_unused_udp_port_factory_fixture(unused_udp_port_factory): - """Test the unused UDP port factory fixture.""" - - class Closer: - def connection_made(self, transport): - pass - - def connection_lost(self, *arg, **kwd): - pass - - port1, port2, port3 = ( - unused_udp_port_factory(), - unused_udp_port_factory(), - unused_udp_port_factory(), - ) - - event_loop = asyncio.get_running_loop() - transport1, _ = await event_loop.create_datagram_endpoint( - Closer, - local_addr=("127.0.0.1", port1), - reuse_port=False, - ) - transport2, _ = await event_loop.create_datagram_endpoint( - Closer, - local_addr=("127.0.0.1", port2), - reuse_port=False, - ) - transport3, _ = await event_loop.create_datagram_endpoint( - Closer, - local_addr=("127.0.0.1", port3), - reuse_port=False, - ) - - for port in port1, port2, port3: - with pytest.raises(IOError): - await event_loop.create_datagram_endpoint( - Closer, - local_addr=("127.0.0.1", port), - reuse_port=False, - ) - - transport1.abort() - transport2.abort() - transport3.abort() - - -def test_unused_port_factory_duplicate(unused_tcp_port_factory, monkeypatch): - """Test correct avoidance of duplicate ports.""" - counter = 0 - - def mock_unused_tcp_port(_ignored): - """Force some duplicate ports.""" - nonlocal counter - counter += 1 - if counter < 5: - return 10000 - else: - return 10000 + counter - - monkeypatch.setattr(pytest_asyncio.plugin, "_unused_port", mock_unused_tcp_port) - - assert unused_tcp_port_factory() == 10000 - assert unused_tcp_port_factory() > 10000 - - -def test_unused_udp_port_factory_duplicate(unused_udp_port_factory, monkeypatch): - """Test correct avoidance of duplicate UDP ports.""" - counter = 0 - - def mock_unused_udp_port(_ignored): - """Force some duplicate ports.""" - nonlocal counter - counter += 1 - if counter < 5: - return 10000 - else: - return 10000 + counter - - monkeypatch.setattr(pytest_asyncio.plugin, "_unused_port", mock_unused_udp_port) - - assert unused_udp_port_factory() == 10000 - assert unused_udp_port_factory() > 10000 - - class TestMarkerInClassBasedTests: """Test that asyncio marked functions work for methods of test classes.""" From a231faf3fb10a77df8ee087f861ea115146a5805 Mon Sep 17 00:00:00 2001 From: Michael Seifert Date: Wed, 22 Nov 2023 10:22:51 +0100 Subject: [PATCH 2/3] [refactor] Extracted a context manager that sets a temporary event loop policy for the scope of the context. Signed-off-by: Michael Seifert --- pytest_asyncio/plugin.py | 58 +++++++++++++++++++--------------------- 1 file changed, 28 insertions(+), 30 deletions(-) diff --git a/pytest_asyncio/plugin.py b/pytest_asyncio/plugin.py index dfbd9958..8b3cdbf2 100644 --- a/pytest_asyncio/plugin.py +++ b/pytest_asyncio/plugin.py @@ -582,21 +582,11 @@ def scoped_event_loop( event_loop_policy, ) -> Iterator[asyncio.AbstractEventLoop]: new_loop_policy = event_loop_policy - old_loop_policy = asyncio.get_event_loop_policy() - old_loop = asyncio.get_event_loop() - asyncio.set_event_loop_policy(new_loop_policy) - loop = asyncio.new_event_loop() - asyncio.set_event_loop(loop) - yield loop - 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 - # 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() - asyncio.set_event_loop(old_loop) + with _temporary_event_loop_policy(new_loop_policy): + loop = asyncio.new_event_loop() + asyncio.set_event_loop(loop) + yield loop + loop.close() # @pytest.fixture does not register the fixture anywhere, so pytest doesn't # know it exists. We work around this by attaching the fixture function to the @@ -622,6 +612,24 @@ def _removesuffix(s: str, suffix: str) -> str: return s.removesuffix(suffix) +@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() + asyncio.set_event_loop_policy(policy) + try: + yield + finally: + 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 + # 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() + asyncio.set_event_loop(old_loop) + + def pytest_collection_modifyitems( session: Session, config: Config, items: List[Item] ) -> None: @@ -950,21 +958,11 @@ def _session_event_loop( request: FixtureRequest, event_loop_policy: AbstractEventLoopPolicy ) -> Iterator[asyncio.AbstractEventLoop]: new_loop_policy = event_loop_policy - old_loop_policy = asyncio.get_event_loop_policy() - old_loop = asyncio.get_event_loop() - asyncio.set_event_loop_policy(new_loop_policy) - loop = asyncio.new_event_loop() - asyncio.set_event_loop(loop) - yield loop - 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 - # 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() - asyncio.set_event_loop(old_loop) + with _temporary_event_loop_policy(new_loop_policy): + loop = asyncio.new_event_loop() + asyncio.set_event_loop(loop) + yield loop + loop.close() @pytest.fixture(scope="session", autouse=True) From b55b963ef80373e6e080b0ce425232042ebafd01 Mon Sep 17 00:00:00 2001 From: Michael Seifert Date: Wed, 22 Nov 2023 10:54:42 +0100 Subject: [PATCH 3/3] [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 8b3cdbf2..96f73722 100644 --- a/pytest_asyncio/plugin.py +++ b/pytest_asyncio/plugin.py @@ -615,7 +615,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 @@ -626,7 +629,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) @@ -900,15 +906,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)