Fix event_loop is run after other fixtures

[simonfagerholm]

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.

pytest-asyncio/tests/test_simple.py

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]

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]

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]

@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]

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

[rsebille]

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]

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

[simonfagerholm]

@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]

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

[Tinche]

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]

@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]

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...

[Tinche]

Thanks a lot @simonfagerholm !