-
-
Notifications
You must be signed in to change notification settings - Fork 32.1k
bpo-32972: Async test case #13386
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
Merged
Merged
bpo-32972: Async test case #13386
Changes from all commits
Commits
Show all changes
19 commits
Select commit
Hold shift + click to select a range
b77e62d
Test on
asvetlov de89fda
Work on
asvetlov 96fcbd9
Merge branch 'master' into async-test-case
asvetlov 9eef723
Fix test method call
asvetlov 18cb7d4
Merge branch 'master' into async-test-case
asvetlov 43091a0
Code cleanup
asvetlov d8ad4d7
Fix test by using positional-arguments only
asvetlov 85df707
Make basic test passing
asvetlov 78d885f
Test addCleanup
asvetlov 58cb1c8
Make test_support happy
asvetlov 0853043
More tests
asvetlov 03480c4
Prefix async setup/teardown methods
asvetlov 8b99f79
Check for setUp/tearDown calls explicitly
asvetlov 80e1898
Add NEWS
asvetlov 0d581af
Merge branch 'master' into async-test-case2
asvetlov 7cedbf0
Rename
asvetlov 30c78b5
Merge branch 'master' into async-test-case2
asvetlov 4cd1a86
Merge branch 'master' into async-test-case2
asvetlov c1ac3b5
Support addAsyncCleanup()
asvetlov File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,158 @@ | ||
import asyncio | ||
import inspect | ||
|
||
from .case import TestCase | ||
|
||
|
||
|
||
class IsolatedAsyncioTestCase(TestCase): | ||
# Names intentionally have a long prefix | ||
# to reduce a chance of clashing with user-defined attributes | ||
# from inherited test case | ||
# | ||
# The class doesn't call loop.run_until_complete(self.setUp()) and family | ||
# but uses a different approach: | ||
# 1. create a long-running task that reads self.setUp() | ||
# awaitable from queue along with a future | ||
# 2. await the awaitable object passing in and set the result | ||
# into the future object | ||
# 3. Outer code puts the awaitable and the future object into a queue | ||
# with waiting for the future | ||
# The trick is necessary because every run_until_complete() call | ||
# creates a new task with embedded ContextVar context. | ||
# To share contextvars between setUp(), test and tearDown() we need to execute | ||
# them inside the same task. | ||
|
||
# Note: the test case modifies event loop policy if the policy was not instantiated | ||
# yet. | ||
# asyncio.get_event_loop_policy() creates a default policy on demand but never | ||
# returns None | ||
# I believe this is not an issue in user level tests but python itself for testing | ||
# should reset a policy in every test module | ||
# by calling asyncio.set_event_loop_policy(None) in tearDownModule() | ||
|
||
def __init__(self, methodName='runTest'): | ||
super().__init__(methodName) | ||
self._asyncioTestLoop = None | ||
self._asyncioCallsQueue = None | ||
|
||
async def asyncSetUp(self): | ||
pass | ||
|
||
async def asyncTearDown(self): | ||
pass | ||
|
||
def addAsyncCleanup(self, func, /, *args, **kwargs): | ||
# A trivial trampoline to addCleanup() | ||
# the function exists because it has a different semantics | ||
# and signature: | ||
# addCleanup() accepts regular functions | ||
# but addAsyncCleanup() accepts coroutines | ||
# | ||
# We intentionally don't add inspect.iscoroutinefunction() check | ||
# for func argument because there is no way | ||
# to check for async function reliably: | ||
# 1. It can be "async def func()" iself | ||
# 2. Class can implement "async def __call__()" method | ||
# 3. Regular "def func()" that returns awaitable object | ||
self.addCleanup(*(func, *args), **kwargs) | ||
|
||
def _callSetUp(self): | ||
self.setUp() | ||
self._callAsync(self.asyncSetUp) | ||
|
||
def _callTestMethod(self, method): | ||
self._callMaybeAsync(method) | ||
|
||
def _callTearDown(self): | ||
self._callAsync(self.asyncTearDown) | ||
self.tearDown() | ||
|
||
def _callCleanup(self, function, *args, **kwargs): | ||
self._callMaybeAsync(function, *args, **kwargs) | ||
|
||
def _callAsync(self, func, /, *args, **kwargs): | ||
assert self._asyncioTestLoop is not None | ||
ret = func(*args, **kwargs) | ||
assert inspect.isawaitable(ret) | ||
fut = self._asyncioTestLoop.create_future() | ||
self._asyncioCallsQueue.put_nowait((fut, ret)) | ||
return self._asyncioTestLoop.run_until_complete(fut) | ||
|
||
def _callMaybeAsync(self, func, /, *args, **kwargs): | ||
assert self._asyncioTestLoop is not None | ||
ret = func(*args, **kwargs) | ||
if inspect.isawaitable(ret): | ||
fut = self._asyncioTestLoop.create_future() | ||
self._asyncioCallsQueue.put_nowait((fut, ret)) | ||
return self._asyncioTestLoop.run_until_complete(fut) | ||
else: | ||
return ret | ||
|
||
async def _asyncioLoopRunner(self): | ||
queue = self._asyncioCallsQueue | ||
while True: | ||
query = await queue.get() | ||
queue.task_done() | ||
if query is None: | ||
return | ||
fut, awaitable = query | ||
try: | ||
ret = await awaitable | ||
if not fut.cancelled(): | ||
fut.set_result(ret) | ||
except asyncio.CancelledError: | ||
raise | ||
except Exception as ex: | ||
if not fut.cancelled(): | ||
fut.set_exception(ex) | ||
|
||
def _setupAsyncioLoop(self): | ||
assert self._asyncioTestLoop is None | ||
loop = asyncio.new_event_loop() | ||
asyncio.set_event_loop(loop) | ||
loop.set_debug(True) | ||
self._asyncioTestLoop = loop | ||
self._asyncioCallsQueue = asyncio.Queue(loop=loop) | ||
self._asyncioCallsTask = loop.create_task(self._asyncioLoopRunner()) | ||
|
||
def _tearDownAsyncioLoop(self): | ||
assert self._asyncioTestLoop is not None | ||
loop = self._asyncioTestLoop | ||
self._asyncioTestLoop = None | ||
self._asyncioCallsQueue.put_nowait(None) | ||
loop.run_until_complete(self._asyncioCallsQueue.join()) | ||
|
||
try: | ||
# cancel all tasks | ||
to_cancel = asyncio.all_tasks(loop) | ||
if not to_cancel: | ||
return | ||
|
||
for task in to_cancel: | ||
task.cancel() | ||
|
||
loop.run_until_complete( | ||
asyncio.gather(*to_cancel, loop=loop, return_exceptions=True)) | ||
|
||
for task in to_cancel: | ||
if task.cancelled(): | ||
continue | ||
if task.exception() is not None: | ||
loop.call_exception_handler({ | ||
'message': 'unhandled exception during test shutdown', | ||
'exception': task.exception(), | ||
'task': task, | ||
}) | ||
# shutdown asyncgens | ||
loop.run_until_complete(loop.shutdown_asyncgens()) | ||
finally: | ||
asyncio.set_event_loop(None) | ||
loop.close() | ||
|
||
def run(self, result=None): | ||
self._setupAsyncioLoop() | ||
try: | ||
return super().run(result) | ||
finally: | ||
self._tearDownAsyncioLoop() |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Why don't we have an explicit
addAsyncCleanup
?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.
For example, we have an explicit async method in AsyncExitStack: https://docs.python.org/3/library/contextlib.html#contextlib.AsyncExitStack.push_async_callback
Even though we toyed with the idea of just reusing the push_callback method and checking if the returned value is an awaitable.
I'm more in favour of
addAsyncCleanup
in this case.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.
I can add
addAsyncCleanup()
method, it makes sense.The implementation should push a callback into the same cleanup queue as used for
addCleanup()
though, to keep cleanups order.I think running all sync cleanups before asyncs and vice versa is a bad idea.
Cleanups should be executed exactly in the reverse adding order.
Also,
self.doCleanups()
should execute all sync and async cleanups altogether.Do you agree?
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.
Yes.