From b96c042da24a17c437b83cf10ac3c7a082f149ba Mon Sep 17 00:00:00 2001 From: alblasco Date: Sun, 24 May 2020 19:54:23 +0200 Subject: [PATCH 1/7] 1) A new test case that fails with 0.12.0, and pass with this commit. test_async_fixtures_with_finalizer_scope.py: 2) Main problem is due to side effects of asyncio.get_event_loop(). See: https://github.com/python/cpython/blob/3.8/Lib/asyncio/events.py#L636 This method could either return an existing loop or create a new one. This commit replaces all asyncio.get_event_loop() with previous pytest-asyncio code (0.10.0), so plugin uses the loop provided by event_loop fixture (instead of calling asyncio.get_event_loop()) Except following block, that has not been modified in this commit (as it behaves similar to 0.10.0) https://github.com/pytest-dev/pytest-asyncio/blob/v0.12.0/pytest_asyncio/plugin.py#L54-L66 Changes are for using always the new loop provided by event_loop fixture. Instead of calling get_event_loop() that: - either returns global recorded loop (with set_event_loop()) in case there is one, - or otherwise creates and record a new one --- pytest_asyncio/plugin.py | 32 ++++++++++------ ...est_async_fixtures_with_finalizer_scope.py | 38 +++++++++++++++++++ 2 files changed, 59 insertions(+), 11 deletions(-) create mode 100644 tests/async_fixtures/test_async_fixtures_with_finalizer_scope.py diff --git a/pytest_asyncio/plugin.py b/pytest_asyncio/plugin.py index c0b65da2..3749b496 100644 --- a/pytest_asyncio/plugin.py +++ b/pytest_asyncio/plugin.py @@ -69,13 +69,20 @@ def pytest_fixture_setup(fixturedef, request): # This is an async generator function. Wrap it accordingly. generator = fixturedef.func + strip_event_loop = False + if 'event_loop' not in fixturedef.argnames: + fixturedef.argnames += ('event_loop', ) + strip_event_loop = True strip_request = False if 'request' not in fixturedef.argnames: fixturedef.argnames += ('request', ) strip_request = True def wrapper(*args, **kwargs): + loop = kwargs['event_loop'] request = kwargs['request'] + if strip_event_loop: + del kwargs['event_loop'] if strip_request: del kwargs['request'] @@ -96,21 +103,30 @@ async def async_finalizer(): msg = "Async generator fixture didn't stop." msg += "Yield only once." raise ValueError(msg) - asyncio.get_event_loop().run_until_complete(async_finalizer()) + loop.run_until_complete(async_finalizer()) request.addfinalizer(finalizer) - return asyncio.get_event_loop().run_until_complete(setup()) + return loop.run_until_complete(setup()) fixturedef.func = wrapper elif inspect.iscoroutinefunction(fixturedef.func): coro = fixturedef.func + strip_event_loop = False + if 'event_loop' not in fixturedef.argnames: + fixturedef.argnames += ('event_loop', ) + strip_event_loop = True + def wrapper(*args, **kwargs): + loop = kwargs['event_loop'] + if strip_event_loop: + del kwargs['event_loop'] + async def setup(): res = await coro(*args, **kwargs) return res - return asyncio.get_event_loop().run_until_complete(setup()) + return loop.run_until_complete(setup()) fixturedef.func = wrapper yield @@ -144,15 +160,9 @@ def wrap_in_sync(func, _loop): def inner(**kwargs): coro = func(**kwargs) if coro is not None: + task = asyncio.ensure_future(coro, loop=_loop) try: - loop = asyncio.get_event_loop() - except RuntimeError as exc: - if 'no current event loop' not in str(exc): - raise - loop = _loop - task = asyncio.ensure_future(coro, loop=loop) - try: - loop.run_until_complete(task) + _loop.run_until_complete(task) except BaseException: # run_until_complete doesn't get the result from exceptions # that are not subclasses of `Exception`. Consume all diff --git a/tests/async_fixtures/test_async_fixtures_with_finalizer_scope.py b/tests/async_fixtures/test_async_fixtures_with_finalizer_scope.py new file mode 100644 index 00000000..113a4d28 --- /dev/null +++ b/tests/async_fixtures/test_async_fixtures_with_finalizer_scope.py @@ -0,0 +1,38 @@ +import asyncio +import contextlib +import functools +import pytest + + +@pytest.mark.asyncio +async def test_module_scope(port): + await asyncio.sleep(0.01) + assert port + +@pytest.fixture(scope="module") +def event_loop(): + """Change event_loop fixture to module level.""" + policy = asyncio.get_event_loop_policy() + loop = policy.new_event_loop() + yield loop + loop.close() + + +@pytest.fixture(scope="module") +async def port(request, event_loop): + def port_finalizer(finalizer): + async def port_afinalizer(): + await finalizer(None, None, None) + event_loop.run_until_complete(port_afinalizer()) + + context_manager = port_map() + port = await context_manager.__aenter__() + request.addfinalizer(functools.partial(port_finalizer, context_manager.__aexit__)) + return True + + +@contextlib.asynccontextmanager +async def port_map(): + worker = asyncio.create_task(asyncio.sleep(0.2)) + yield + await worker From bf4538ccf2dbaf29491eb47e402f3a8165330ca9 Mon Sep 17 00:00:00 2001 From: alblasco Date: Mon, 25 May 2020 13:05:46 +0200 Subject: [PATCH 2/7] 1) Test case (test_async_fixtures_with_finalizer) refactoring to pass on python 3.6 & 3.5 2) An additional test case (test_module_with_get_event_loop_finalizer). Refactoring previous test case I got to another potential issue: Finalizers using get_event_loop() instead of event_loop [on a module scope fixture] To be able to pass this new test case I needed some additional changes on next event_loop fixture block( https://github.com/pytest-dev/pytest-asyncio/blob/v0.12.0/pytest_asyncio/plugin.py#L54-L66): - This block needs to apply on all event_loop fixture scopes (so I remove 'asyncio' in request.keywords, that only apply to function scope) - The get_event_loop() method inside this block has following side effects (See https://github.com/python/cpython/blob/3.8/Lib/asyncio/events.py#L636): - As no loop is still set, then L636 is executed, and so a new event_loop (let's call it L2) is first created and then set. - And then finalizer will restore L2 at the end of test. What has no sense, to restore L2 that wasn't before the test. And additionally this L2 is never closed. So it seeems that get_event_loop() should be removed, and so I see no reasons for any finalizer. 3) DRY: New FixtureStripper to avoid repeating code --- pytest_asyncio/plugin.py | 64 ++++++++++--------- .../test_async_fixtures_with_finalizer.py | 51 +++++++++++++++ ...est_async_fixtures_with_finalizer_scope.py | 38 ----------- 3 files changed, 86 insertions(+), 67 deletions(-) create mode 100644 tests/async_fixtures/test_async_fixtures_with_finalizer.py delete mode 100644 tests/async_fixtures/test_async_fixtures_with_finalizer_scope.py diff --git a/pytest_asyncio/plugin.py b/pytest_asyncio/plugin.py index 3749b496..e7fd6565 100644 --- a/pytest_asyncio/plugin.py +++ b/pytest_asyncio/plugin.py @@ -48,43 +48,53 @@ def pytest_pycollect_makeitem(collector, name, obj): return list(collector._genfunctions(name, obj)) +class FixtureStripper: + """Include additional Fixture, and then strip them""" + REQUEST = "request" + EVENT_LOOP = "event_loop" + + def __init__(self, fixturedef): + self.fixturedef = fixturedef + self.to_strip = set() + + def add(self, name): + """Add fixture name to fixturedef + and record in to_strip list (If not previously included)""" + if name in self.fixturedef.argnames: + return + self.fixturedef.argnames += (name, ) + self.to_strip.add(name) + + def get_and_strip_from(self, name, data_dict): + """Strip name from data, and return value""" + result = data_dict[name] + if name in self.to_strip: + del data_dict[name] + return result + + @pytest.hookimpl(hookwrapper=True) def pytest_fixture_setup(fixturedef, request): """Adjust the event loop policy when an event loop is produced.""" - if fixturedef.argname == "event_loop" and 'asyncio' in request.keywords: + if fixturedef.argname == "event_loop": outcome = yield loop = outcome.get_result() policy = asyncio.get_event_loop_policy() - try: - old_loop = policy.get_event_loop() - except RuntimeError as exc: - if 'no current event loop' not in str(exc): - raise - old_loop = None policy.set_event_loop(loop) - fixturedef.addfinalizer(lambda: policy.set_event_loop(old_loop)) return if isasyncgenfunction(fixturedef.func): # This is an async generator function. Wrap it accordingly. generator = fixturedef.func - strip_event_loop = False - if 'event_loop' not in fixturedef.argnames: - fixturedef.argnames += ('event_loop', ) - strip_event_loop = True - strip_request = False - if 'request' not in fixturedef.argnames: - fixturedef.argnames += ('request', ) - strip_request = True + fixture_stripper = FixtureStripper(fixturedef) + fixture_stripper.add(FixtureStripper.EVENT_LOOP) + fixture_stripper.add(FixtureStripper.REQUEST) + def wrapper(*args, **kwargs): - loop = kwargs['event_loop'] - request = kwargs['request'] - if strip_event_loop: - del kwargs['event_loop'] - if strip_request: - del kwargs['request'] + loop = fixture_stripper.get_and_strip_from(FixtureStripper.EVENT_LOOP, kwargs) + request = fixture_stripper.get_and_strip_from(FixtureStripper.REQUEST, kwargs) gen_obj = generator(*args, **kwargs) @@ -112,15 +122,11 @@ async def async_finalizer(): elif inspect.iscoroutinefunction(fixturedef.func): coro = fixturedef.func - strip_event_loop = False - if 'event_loop' not in fixturedef.argnames: - fixturedef.argnames += ('event_loop', ) - strip_event_loop = True + fixture_stripper = FixtureStripper(fixturedef) + fixture_stripper.add(FixtureStripper.EVENT_LOOP) def wrapper(*args, **kwargs): - loop = kwargs['event_loop'] - if strip_event_loop: - del kwargs['event_loop'] + loop = fixture_stripper.get_and_strip_from(FixtureStripper.EVENT_LOOP, kwargs) async def setup(): res = await coro(*args, **kwargs) diff --git a/tests/async_fixtures/test_async_fixtures_with_finalizer.py b/tests/async_fixtures/test_async_fixtures_with_finalizer.py new file mode 100644 index 00000000..cc6033b8 --- /dev/null +++ b/tests/async_fixtures/test_async_fixtures_with_finalizer.py @@ -0,0 +1,51 @@ +import asyncio +import contextlib +import functools +import pytest + + +@pytest.mark.asyncio +async def test_module_with_event_loop_finalizer(port1): + await asyncio.sleep(0.01) + assert port1 + +@pytest.mark.asyncio +async def test_module_with_get_event_loop_finalizer(port2): + await asyncio.sleep(0.01) + assert port2 + +@pytest.fixture(scope="module") +def event_loop(): + """Change event_loop fixture to module level.""" + policy = asyncio.get_event_loop_policy() + loop = policy.new_event_loop() + yield loop + loop.close() + + +@pytest.fixture(scope="module") +async def port1(request, event_loop): + def port_finalizer(finalizer): + async def port_afinalizer(): + # await task inside get_event_loop() + # RantimeError is raised if task is created on a different loop + await finalizer + event_loop.run_until_complete(port_afinalizer()) + + worker = asyncio.create_task(asyncio.sleep(0.2)) + request.addfinalizer(functools.partial(port_finalizer, worker)) + return True + + +@pytest.fixture(scope="module") +async def port2(request, event_loop): + def port_finalizer(finalizer): + async def port_afinalizer(): + # await task inside get_event_loop() + # if loop is different a RuntimeError is raised + await finalizer + asyncio.get_event_loop().run_until_complete(port_afinalizer()) + + worker = asyncio.create_task(asyncio.sleep(0.2)) + request.addfinalizer(functools.partial(port_finalizer, worker)) + return True diff --git a/tests/async_fixtures/test_async_fixtures_with_finalizer_scope.py b/tests/async_fixtures/test_async_fixtures_with_finalizer_scope.py deleted file mode 100644 index 113a4d28..00000000 --- a/tests/async_fixtures/test_async_fixtures_with_finalizer_scope.py +++ /dev/null @@ -1,38 +0,0 @@ -import asyncio -import contextlib -import functools -import pytest - - -@pytest.mark.asyncio -async def test_module_scope(port): - await asyncio.sleep(0.01) - assert port - -@pytest.fixture(scope="module") -def event_loop(): - """Change event_loop fixture to module level.""" - policy = asyncio.get_event_loop_policy() - loop = policy.new_event_loop() - yield loop - loop.close() - - -@pytest.fixture(scope="module") -async def port(request, event_loop): - def port_finalizer(finalizer): - async def port_afinalizer(): - await finalizer(None, None, None) - event_loop.run_until_complete(port_afinalizer()) - - context_manager = port_map() - port = await context_manager.__aenter__() - request.addfinalizer(functools.partial(port_finalizer, context_manager.__aexit__)) - return True - - -@contextlib.asynccontextmanager -async def port_map(): - worker = asyncio.create_task(asyncio.sleep(0.2)) - yield - await worker From a151a606c60afdcd31de447cdab1eef1fbf78c7c Mon Sep 17 00:00:00 2001 From: alblasco Date: Mon, 25 May 2020 15:55:54 +0200 Subject: [PATCH 3/7] To solve test cases that fail: MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Even_loop() fixture properly closes the loop (but as explained below this is not enough to reset behaviour): ``` @pytest.fixture def event_loop(request): """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() ``` Subsequent calls to asyncio.get_event_loop() returns the previous closed loop, instead of providing a new loop. This is due to (https://github.com/python/cpython/blob/3.8/Lib/asyncio/events.py#L634) variable _set_called is True, and so even when loop is None, then no new_event_loop is created [I really don´t know the reason of this _set_called behaviour]. The best solution I have found is to set_event_loop_policy(None). This completely resets global variable and so any subsequent call to asyncio.get_event_loop() provides a new event_loop. I have included this in the pytest hook (pytest_fixture_post_finalizer), that is called after fixture teardowns. Note: Same behaviour can be done modifying event_loop() fixture, but I have discarded it as this would force users that have overwritten even_loop fixture to modify its implementation: ``` @pytest.fixture def event_loop(request): """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() asyncio.set_event_loop_policy(None) ``` --- pytest_asyncio/plugin.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/pytest_asyncio/plugin.py b/pytest_asyncio/plugin.py index e7fd6565..2fdc5f4e 100644 --- a/pytest_asyncio/plugin.py +++ b/pytest_asyncio/plugin.py @@ -72,6 +72,14 @@ def get_and_strip_from(self, name, data_dict): del data_dict[name] return result +@pytest.hookimpl(trylast=True) +def pytest_fixture_post_finalizer(fixturedef, request): + """Called after fixture teardown""" + if fixturedef.argname == "event_loop": + # Set empty loop policy, so that subsequent get_event_loop() provides a new loop + asyncio.set_event_loop_policy(None) + + @pytest.hookimpl(hookwrapper=True) def pytest_fixture_setup(fixturedef, request): From 341c3698132cd1a992e09dc1023cbf8a40af2cbc Mon Sep 17 00:00:00 2001 From: alblasco Date: Mon, 25 May 2020 16:24:34 +0200 Subject: [PATCH 4/7] FIX new test_cases on python 3.5 & 3.6 asyncio.create_task has been added in Python 3.7. The low-level asyncio.ensure_future() function can be used instead (works in all Python versions but is less readable). https://docs.python.org/3/library/asyncio-task.html#asyncio.create_task --- tests/async_fixtures/test_async_fixtures_with_finalizer.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/async_fixtures/test_async_fixtures_with_finalizer.py b/tests/async_fixtures/test_async_fixtures_with_finalizer.py index cc6033b8..22e5b3a1 100644 --- a/tests/async_fixtures/test_async_fixtures_with_finalizer.py +++ b/tests/async_fixtures/test_async_fixtures_with_finalizer.py @@ -32,7 +32,7 @@ async def port_afinalizer(): await finalizer event_loop.run_until_complete(port_afinalizer()) - worker = asyncio.create_task(asyncio.sleep(0.2)) + worker = asyncio.ensure_future(asyncio.sleep(0.2)) request.addfinalizer(functools.partial(port_finalizer, worker)) return True @@ -46,6 +46,6 @@ async def port_afinalizer(): await finalizer asyncio.get_event_loop().run_until_complete(port_afinalizer()) - worker = asyncio.create_task(asyncio.sleep(0.2)) + worker = asyncio.ensure_future(asyncio.sleep(0.2)) request.addfinalizer(functools.partial(port_finalizer, worker)) return True From f246a8c827c839b8efb39dac93247ee5df8801aa Mon Sep 17 00:00:00 2001 From: Angel Luis Blasco Date: Tue, 26 May 2020 09:15:37 +0200 Subject: [PATCH 5/7] Clarify names and comments, according to yanlend comments 26 May --- .../test_async_fixtures_with_finalizer.py | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/tests/async_fixtures/test_async_fixtures_with_finalizer.py b/tests/async_fixtures/test_async_fixtures_with_finalizer.py index 22e5b3a1..fb276cdc 100644 --- a/tests/async_fixtures/test_async_fixtures_with_finalizer.py +++ b/tests/async_fixtures/test_async_fixtures_with_finalizer.py @@ -5,14 +5,14 @@ @pytest.mark.asyncio -async def test_module_with_event_loop_finalizer(port1): +async def test_module_with_event_loop_finalizer(port_with_event_loop_finalizer): await asyncio.sleep(0.01) - assert port1 + assert port_with_event_loop_finalizer @pytest.mark.asyncio -async def test_module_with_get_event_loop_finalizer(port2): +async def test_module_with_get_event_loop_finalizer(port_with_get_event_loop_finalizer): await asyncio.sleep(0.01) - assert port2 + assert port_with_get_event_loop_finalizer @pytest.fixture(scope="module") def event_loop(): @@ -24,11 +24,11 @@ def event_loop(): @pytest.fixture(scope="module") -async def port1(request, event_loop): +async def port_with_event_loop_finalizer(request, event_loop): def port_finalizer(finalizer): async def port_afinalizer(): - # await task inside get_event_loop() - # RantimeError is raised if task is created on a different loop + # await task using loop provided by event_loop fixture + # RuntimeError is raised if task is created on a different loop await finalizer event_loop.run_until_complete(port_afinalizer()) @@ -38,11 +38,11 @@ async def port_afinalizer(): @pytest.fixture(scope="module") -async def port2(request, event_loop): +async def port_with_get_event_loop_finalizer(request, event_loop): def port_finalizer(finalizer): async def port_afinalizer(): - # await task inside get_event_loop() - # if loop is different a RuntimeError is raised + # await task using loop provided by asyncio.get_event_loop() + # RuntimeError is raised if task is created on a different loop await finalizer asyncio.get_event_loop().run_until_complete(port_afinalizer()) From 882c11e564bcc4a27b43c785460d90d72e6c1818 Mon Sep 17 00:00:00 2001 From: Angel Luis Blasco Date: Tue, 26 May 2020 09:23:45 +0200 Subject: [PATCH 6/7] One import is not needed --- tests/async_fixtures/test_async_fixtures_with_finalizer.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/async_fixtures/test_async_fixtures_with_finalizer.py b/tests/async_fixtures/test_async_fixtures_with_finalizer.py index fb276cdc..44b5bbe4 100644 --- a/tests/async_fixtures/test_async_fixtures_with_finalizer.py +++ b/tests/async_fixtures/test_async_fixtures_with_finalizer.py @@ -1,5 +1,4 @@ import asyncio -import contextlib import functools import pytest From 81af28995d06da5aa06239c37e6c7f35400a2bb1 Mon Sep 17 00:00:00 2001 From: alblasco Date: Mon, 1 Jun 2020 20:27:07 +0200 Subject: [PATCH 7/7] A line is added to the changelog. Please review release date --- README.rst | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/README.rst b/README.rst index 92ae8f40..77e735b7 100644 --- a/README.rst +++ b/README.rst @@ -175,28 +175,32 @@ Only test coroutines will be affected (by default, coroutines prefixed by Changelog --------- +0.13.0 (2020-XX-XX) +~~~~~~~~~~~~~~~~~~~ +- Fix `#162 `_, and ``event_loop`` fixture behavior now is coherent on all scopes. + `#164 `_ 0.12.0 (2020-05-04) ~~~~~~~~~~~~~~~~~~~ - Run the event loop fixture as soon as possible. This helps with fixtures that have an implicit dependency on the event loop. - `#156` + `#156 `_ 0.11.0 (2020-04-20) ~~~~~~~~~~~~~~~~~~~ - Test on 3.8, drop 3.3 and 3.4. Stick to 0.10 for these versions. - `#152` + `#152 `_ - Use the new Pytest 5.4.0 Function API. We therefore depend on pytest >= 5.4.0. - `#142` + `#142 `_ - Better ``pytest.skip`` support. - `#126` + `#126 `_ 0.10.0 (2019-01-08) ~~~~~~~~~~~~~~~~~~~~ - ``pytest-asyncio`` integrates with `Hypothesis `_ to support ``@given`` on async test functions using ``asyncio``. - `#102` + `#102 `_ - Pytest 4.1 support. - `#105` + `#105 `_ 0.9.0 (2018-07-28) ~~~~~~~~~~~~~~~~~~ @@ -208,7 +212,7 @@ Changelog 0.8.0 (2017-09-23) ~~~~~~~~~~~~~~~~~~ - Improve integration with other packages (like aiohttp) with more careful event loop handling. - `#64` + `#64 `_ 0.7.0 (2017-09-08) ~~~~~~~~~~~~~~~~~~