Fix event_loop is run after other fixtures by simonfagerholm · Pull Request #156 · pytest-dev/pytest-asyncio

simonfagerholm · 2020-04-22T20:38:50Z

Resolves #154, resolves #157, resolves #158
Also adds unittests for the use case.

Not ready for merge, breaks test_asyncio_marker_without_loop, which was added in #64.

Lines 116 to 128 in a7e5795

    
           class TestUnexistingLoop: 
        
               @pytest.fixture 
        
               def remove_loop(self): 
        
                   old_loop = asyncio.get_event_loop() 
        
                   asyncio.set_event_loop(None) 
        
                   yield 
        
                   asyncio.set_event_loop(old_loop) 
        
               @pytest.mark.asyncio 
        
               async def test_asyncio_marker_without_loop(self, remove_loop): 
        
                   """Test the asyncio pytest marker in a Test class.""" 
        
                   ret = await async_coro() 
        
                   assert ret == 'ok'

Discussion for possible fix of broken test below

simonfagerholm · 2020-04-22T20:45:06Z

I did something that seems to fix the test, however I'm really not sure the behaviour is wanted:

diff --git a/pytest_asyncio/plugin.py b/pytest_asyncio/plugin.py
index 9860e04..c0b65da 100644
--- a/pytest_asyncio/plugin.py
+++ b/pytest_asyncio/plugin.py
@@ -125,14 +125,18 @@ def pytest_pyfunc_call(pyfuncitem):
     if 'asyncio' in pyfuncitem.keywords:
         if getattr(pyfuncitem.obj, 'is_hypothesis_test', False):
             pyfuncitem.obj.hypothesis.inner_test = wrap_in_sync(
-                pyfuncitem.obj.hypothesis.inner_test
+                pyfuncitem.obj.hypothesis.inner_test,
+                _loop=pyfuncitem.funcargs['event_loop']
             )
         else:
-            pyfuncitem.obj = wrap_in_sync(pyfuncitem.obj)
+            pyfuncitem.obj = wrap_in_sync(
+                pyfuncitem.obj,
+                _loop=pyfuncitem.funcargs['event_loop']
+            )
     yield


-def wrap_in_sync(func):
+def wrap_in_sync(func, _loop):
     """Return a sync wrapper around an async function executing it in the
     current event loop."""

@@ -140,9 +144,15 @@ def wrap_in_sync(func):
     def inner(**kwargs):
         coro = func(**kwargs)
         if coro is not None:
-            task = asyncio.ensure_future(coro)
             try:
-                asyncio.get_event_loop().run_until_complete(task)
+                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)
             except BaseException:
                 # run_until_complete doesn't get the result from exceptions
                 # that are not subclasses of `Exception`. Consume all

Basically the wrapper will attach the loop from the event_loop if get_event_loop fails because no current event loop is available.

rsebille · 2020-04-23T11:51:12Z

Hi,

I think the behavior is fine (second time that I look to the source of pytest-asyncio s don't take my word for it :D), but I would take it up a notch to "An asynchronous test function should run on the event loop that was used for its fixtures".

So I tried the following changes, and tests continue to pass but can't keep thinking that is probably too easy :).

diff --git a/pytest_asyncio/plugin.py b/pytest_asyncio/plugin.py
index 9860e04..08cf9d2 100644
--- a/pytest_asyncio/plugin.py
+++ b/pytest_asyncio/plugin.py
@@ -125,14 +125,18 @@ def pytest_pyfunc_call(pyfuncitem):
     if 'asyncio' in pyfuncitem.keywords:
         if getattr(pyfuncitem.obj, 'is_hypothesis_test', False):
             pyfuncitem.obj.hypothesis.inner_test = wrap_in_sync(
-                pyfuncitem.obj.hypothesis.inner_test
+                pyfuncitem.obj.hypothesis.inner_test,
+                _loop=pyfuncitem.funcargs['event_loop'],
             )
         else:
-            pyfuncitem.obj = wrap_in_sync(pyfuncitem.obj)
+            pyfuncitem.obj = wrap_in_sync(
+                pyfuncitem.obj,
+                _loop=pyfuncitem.funcargs['event_loop'],
+            )
     yield
 
 
-def wrap_in_sync(func):
+def wrap_in_sync(func, _loop):
     """Return a sync wrapper around an async function executing it in the
     current event loop."""
 
@@ -140,9 +144,9 @@ def wrap_in_sync(func):
     def inner(**kwargs):
         coro = func(**kwargs)
         if coro is not None:
-            task = asyncio.ensure_future(coro)
+            task = asyncio.ensure_future(coro, loop=_loop)
             try:
-                asyncio.get_event_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

simonfagerholm · 2020-04-23T11:57:33Z

@rsebille The behavior and your comment differ I think:
In your change you always attach the loop from event_loop, which may or may not bethe same loop the fixtures run in.
I try to get the current loop first, enabling fixtures to change the loop if needed. As a fallback I use the event_loop, in case there is no loop (which should mean a fixture has set it to None as we know we started the event_loop-fixture before running fixtures).
Thus I think my original suggestion matches your comment more than the code you submitted.

simonfagerholm · 2020-04-23T12:26:31Z

@rsebille I forgot to add that I appreciate that you took time to look into this and any future support :)

rsebille · 2020-04-23T12:41:38Z

I totally assume (slaps ensues) that asynchronous fixtures were explicitly run in the event_loop event loop to avoid "different loop" error and that the plugin would not allow changing the event loop without overriding event_loop.

@simonfagerholm No problem, thank to you for taking the time on this.

Tinche · 2020-04-24T15:04:13Z

Thanks for this, I will look at this over the weekend!

simonfagerholm · 2020-04-25T06:12:18Z

@Tinche Thats great! In my digging I found out that the test is supposed to test for problems with pytest-aiohttp.
So I used the following as a test bench and discovered that it:

passes on pytest-asyncio==0.10
fails on pytest-asyncio==0.11
passes on my current branch
passes on my suggested addition to the branch fixing the unittest

As it fails on pytest-asyncio==0.11 and passes on this PR, it seems the unittest did not protect against that problem.

import pytest
from aiohttp import web


async def hello(request):
    return web.Response(body=b'Hello, world')


def create_app(loop):
    app = web.Application(loop=loop)
    app.router.add_route('GET', '/', hello)
    return app


async def test_hello(aiohttp_client):
    client = await aiohttp_client(create_app)
    resp = await client.get('/')
    assert resp.status == 200
    text = await resp.text()
    assert 'Hello, world' in text


@pytest.fixture
async def async_fixture():
    yield 'Hi from async_fixture()!'


@pytest.mark.asyncio
async def test_async_fixture_fixed(async_fixture):
    assert async_fixture == 'Hi from async_fixture()!'

============================= test session starts =============================
platform win32 -- Python 3.7.7, pytest-5.4.1, py-1.8.1, pluggy-0.13.1 -- python.exe
cachedir: .pytest_cache
rootdir: aiohttp_test
plugins: aiohttp-0.3.0, asyncio-0.11.0
collecting ... collected 2 items

test_aiohttp_error.py::test_hello[pyloop] 
test_aiohttp_error.py::test_async_fixture_fixed PASSED                         [ 50%]ERROR                    [100%]
test setup failed
args = (), kwargs = {}
request = <SubRequest 'async_fixture' for <Function test_async_fixture_fixed>>
setup = <function pytest_fixture_setup.<locals>.wrapper.<locals>.setup at 0x0000019B771C0678>
finalizer = <function pytest_fixture_setup.<locals>.wrapper.<locals>.finalizer at 0x0000019B771C0F78>

    def wrapper(*args, **kwargs):
        request = kwargs['request']
        if strip_request:
            del kwargs['request']
    
        gen_obj = generator(*args, **kwargs)
    
        async def setup():
            res = await gen_obj.__anext__()
            return res
    
        def finalizer():
            """Yield again, to finalize."""
            async def async_finalizer():
                try:
                    await gen_obj.__anext__()
                except StopAsyncIteration:
                    pass
                else:
                    msg = "Async generator fixture didn't stop."
                    msg += "Yield only once."
                    raise ValueError(msg)
            asyncio.get_event_loop().run_until_complete(async_finalizer())
    
        request.addfinalizer(finalizer)
>       return asyncio.get_event_loop().run_until_complete(setup())

venv\lib\site-packages\pytest_asyncio\plugin.py:102: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <asyncio.windows_events.WindowsSelectorEventLoopPolicy object at 0x0000019B771580C8>

    def get_event_loop(self):
        """Get the event loop for the current context.
    
        Returns an instance of EventLoop or raises an exception.
        """
        if (self._local._loop is None and
                not self._local._set_called and
                isinstance(threading.current_thread(), threading._MainThread)):
            self.set_event_loop(self.new_event_loop())
    
        if self._local._loop is None:
            raise RuntimeError('There is no current event loop in thread %r.'
>                              % threading.current_thread().name)
E           RuntimeError: There is no current event loop in thread 'MainThread'.

C:\Program Files\Python\Python37\lib\asyncio\events.py:644: RuntimeError

test_aiohttp_error.py::test_async_fixture_fixed ERROR                    [100%]
test_aiohttp_error.py:27 (test_async_fixture_fixed)
def finalizer():
        """Yield again, to finalize."""
        async def async_finalizer():
            try:
                await gen_obj.__anext__()
            except StopAsyncIteration:
                pass
            else:
                msg = "Async generator fixture didn't stop."
                msg += "Yield only once."
                raise ValueError(msg)
>       asyncio.get_event_loop().run_until_complete(async_finalizer())

venv\lib\site-packages\pytest_asyncio\plugin.py:99: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <asyncio.windows_events.WindowsSelectorEventLoopPolicy object at 0x0000019B771580C8>

    def get_event_loop(self):
        """Get the event loop for the current context.
    
        Returns an instance of EventLoop or raises an exception.
        """
        if (self._local._loop is None and
                not self._local._set_called and
                isinstance(threading.current_thread(), threading._MainThread)):
            self.set_event_loop(self.new_event_loop())
    
        if self._local._loop is None:
            raise RuntimeError('There is no current event loop in thread %r.'
>                              % threading.current_thread().name)
E           RuntimeError: There is no current event loop in thread 'MainThread'.

C:\Program Files\Python\Python37\lib\asyncio\events.py:644: RuntimeError


=================================== ERRORS ====================================

simonfagerholm · 2020-04-25T06:32:04Z

I also tested the examples in #157 and verified that this PR solves #157.

Tinche · 2020-04-26T22:52:41Z

Yeah, I guess that unittest from #64 was useless.

On @simonfagerholm's branch, and applied the linked diff (which I'm ok with). So I tried installing pytest-aiohttp to run the provided example, and having pytest-aiohttp installed breaks a bunch of our own tests. I guess that can't be avoided? It was this way back on 0.10.0 too.

If you could add a changelog entry, I'll release this, with thanks :)

simonfagerholm · 2020-04-27T05:59:39Z

@Tinche Pushed a commit of my diff above. Thanks for your help in the investigation, do you have any ideas on how to test the compatibility with aiohttp? I somewhat dislike the inclusion of aiohttp in the tests, but it might not be possible in other ways.
Maybe a "test suite" of compatibility with other packages including aiohttp, that can be separated from the other tests.

Tinche · 2020-04-27T09:53:06Z

Actually I don't really have a good idea. It would be ideal if they started depending on us, since I feel they are a higher level component...

Awaited issue: pytest-dev/pytest-asyncio#154 Awaited fix: pytest-dev/pytest-asyncio#156

Tinche · 2020-05-03T22:43:33Z

Thanks a lot @simonfagerholm !

simonfagerholm added 2 commits April 22, 2020 19:18

Added unittests for issue #154.

c0c3e0c

Put event_loop first among the fixtures of asyncio tests, fixes #154.

fa65ca9

simonfagerholm mentioned this pull request Apr 23, 2020

Error of "attached to a different loop" appears in 0.11.0 but not in 0.10.0 #154

Closed

Fixed failing test case, 'test_asyncio_marker_without_loop'.

bfad9db

simonfagerholm mentioned this pull request Apr 25, 2020

RuntimeError: Event loop is closed as of 0.11.0 in particular test setup #157

Closed

simonfagerholm changed the title ~~Fix "attached to a different loop", issue #154~~ Fix "attached to a different loop" and "event loop is closed" Apr 25, 2020

nolar added a commit to nolar/kopf-fork-from-zalando-incubator that referenced this pull request Apr 27, 2020

Pin pytest-asyncio to 0.10.0 until event_loop priority issue is fixed

2ff9731

Awaited issue: pytest-dev/pytest-asyncio#154 Awaited fix: pytest-dev/pytest-asyncio#156

nolar mentioned this pull request Apr 27, 2020

Fix async tests with no explicit event_loop usage/declaration zalando-incubator/kopf#352

Merged

5 tasks done

simonfagerholm mentioned this pull request Apr 28, 2020

RuntimeError in aiohttp's TimerContext with 0.11.0 #158

Closed

simonfagerholm changed the title ~~Fix "attached to a different loop" and "event loop is closed"~~ Fix event_loop is run after other fixtures Apr 28, 2020

matham mentioned this pull request Apr 29, 2020

Tests: Fix failing tests kivy/kivy#6845

Merged

Tinche merged commit b45de23 into pytest-dev:master May 3, 2020

sphuber mentioned this pull request Aug 13, 2020

Tests incompatible with pytest-asyncio = 0.11.0 aiidateam/kiwipy#47

Closed

kopf-archiver Bot mentioned this pull request Aug 19, 2020

[PR] Fix async tests with no explicit event_loop usage/declaration nolar/kopf#352

Closed

5 tasks done

Gallaecio mentioned this pull request Oct 8, 2021

[tests] add Python 3.10 env, update pytest dependency scrapy-plugins/scrapy-playwright#30

Merged

coderabbitai Bot mentioned this pull request Apr 22, 2026

chore(nodes): full node test improvements and mock cleanup rocketride-org/rocketride-server#687

Merged

7 tasks

Navigation Menu

Fix event_loop is run after other fixtures - #156

Fix event_loop is run after other fixtures#156
Tinche merged 3 commits into
pytest-dev:masterfrom
simonfagerholm:fix_issue_154

simonfagerholm commented Apr 22, 2020 •

edited

Loading

Uh oh!

simonfagerholm commented Apr 22, 2020 •

edited

Loading

Uh oh!

rsebille commented Apr 23, 2020

Uh oh!

simonfagerholm commented Apr 23, 2020 •

edited

Loading

Uh oh!

simonfagerholm commented Apr 23, 2020

Uh oh!

rsebille commented Apr 23, 2020

Uh oh!

Tinche commented Apr 24, 2020

Uh oh!

simonfagerholm commented Apr 25, 2020 •

edited

Loading

Uh oh!

simonfagerholm commented Apr 25, 2020

Uh oh!

Tinche commented Apr 26, 2020

Uh oh!

simonfagerholm commented Apr 27, 2020

Uh oh!

Tinche commented Apr 27, 2020

Uh oh!

Tinche commented May 3, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	class TestUnexistingLoop:
	@pytest.fixture
	def remove_loop(self):
	old_loop = asyncio.get_event_loop()
	asyncio.set_event_loop(None)
	yield
	asyncio.set_event_loop(old_loop)

	@pytest.mark.asyncio
	async def test_asyncio_marker_without_loop(self, remove_loop):
	"""Test the asyncio pytest marker in a Test class."""
	ret = await async_coro()
	assert ret == 'ok'

Conversation

simonfagerholm commented Apr 22, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

simonfagerholm commented Apr 22, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rsebille commented Apr 23, 2020

Uh oh!

simonfagerholm commented Apr 23, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

simonfagerholm commented Apr 23, 2020

Uh oh!

rsebille commented Apr 23, 2020

Uh oh!

Tinche commented Apr 24, 2020

Uh oh!

simonfagerholm commented Apr 25, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

simonfagerholm commented Apr 25, 2020

Uh oh!

Tinche commented Apr 26, 2020

Uh oh!

simonfagerholm commented Apr 27, 2020

Uh oh!

Tinche commented Apr 27, 2020

Uh oh!

Tinche commented May 3, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

simonfagerholm commented Apr 22, 2020 •

edited

Loading

simonfagerholm commented Apr 22, 2020 •

edited

Loading

simonfagerholm commented Apr 23, 2020 •

edited

Loading

simonfagerholm commented Apr 25, 2020 •

edited

Loading