-
Notifications
You must be signed in to change notification settings - Fork 161
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
Changes from 1 commit
b96c042
bf4538c
a151a60
341c369
f246a8c
882c11e
81af289
1ee279e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.