Skip to content

Commit

Permalink
1) Test case (test_async_fixtures_with_finalizer) refactoring to pass…
Browse files Browse the repository at this point in the history
… 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
  • Loading branch information
alblasco authored and Tinche committed Jun 2, 2020
1 parent c1131f8 commit f97e900
Show file tree
Hide file tree
Showing 3 changed files with 86 additions and 67 deletions.
64 changes: 35 additions & 29 deletions pytest_asyncio/plugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -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)
Expand Down
51 changes: 51 additions & 0 deletions tests/async_fixtures/test_async_fixtures_with_finalizer.py
Original file line number Diff line number Diff line change
@@ -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
38 changes: 0 additions & 38 deletions tests/async_fixtures/test_async_fixtures_with_finalizer_scope.py

This file was deleted.

0 comments on commit f97e900

Please sign in to comment.