Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FIX #162 "attached to a different loop", and add a new test case #164

Merged
merged 8 commits into from
Jun 2, 2020
32 changes: 21 additions & 11 deletions pytest_asyncio/plugin.py
Expand Up @@ -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']

Expand All @@ -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
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code is copy pasted from line 72. Can you refactor it such that it is needed only once? E.g. pull it into a small function.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed. Included a new class FixtureStripper.

Please review again as I have included a new test case, and this has required additional changes
Thanks in advance.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Had some minor comments, I am happy overall.

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
Expand Down Expand Up @@ -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
Expand Down
38 changes: 38 additions & 0 deletions 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
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to fail the build on older Python versions. I don't think it is an essential part of the test. So you might get it to run on all required versions by changing the test a bit.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

async def port_map():
worker = asyncio.create_task(asyncio.sleep(0.2))
yield
await worker