bpo-39529: Deprecate creating new event loop in asyncio.get_event_loop() by serhiy-storchaka · Pull Request #23554 · python/cpython
It is a draft. Tests which use asyncio.gather(), asyncio.as_completed(), etc in synchronous code should be rewritten to call them in coroutines, and purposed tests for warnings and errors should be added.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good so far.
| loop = events.get_event_loop() | ||
| task = loop.create_task(coro_or_future) | ||
| if task._source_traceback: | ||
| del task._source_traceback[-1] |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was removed because this code did not work as intented long time ago (if worked at all).
| res = loop.run_until_complete(asyncio.wait_for(coro(), timeout=None)) | ||
| self.assertEqual(res, 'done') | ||
|
|
||
| def test_wait_for_with_global_loop(self): |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is virtually a duplicate of the above test. Initially they were different by passing the loop argument or not, now it is removed
| self.assertAlmostEqual(0.15, loop.time()) | ||
| self.assertEqual(res, 42) | ||
|
|
||
| def test_wait_with_global_loop(self): |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is virtually a duplicate of the above test. Initially they were different by passing the loop argument or not, now it is removed
| self.assertEqual(y, 'b') | ||
| self.assertAlmostEqual(0.10, loop.time()) | ||
|
|
||
| x = loop.run_until_complete(futs[1]) |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently two parts of the test was virtually duplicates. Added new tests (test_as_completed_coroutine_without_loop and below) which are essentially different.
Please give attention to tests. This is the main part of the PR.
This PR is stale because it has been open for 30 days with no activity.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, please go ahead.
Sorry for the very long review.