Fix an outdated test in asyncio #24477
Conversation
The test was added in f3e2e092 before `_StopError` was removed in 41f69f4, after which this test will not fail without the fix it covers. This PR fixes the test to resume its coverage.
|
This PR is stale because it has been open for 30 days with no activity. |
MaxwellDupre
left a comment
MaxwellDupre
left a comment
There was a problem hiding this comment.
Ran 2287 tests in 83.131s
OK (skipped=18) All windows tests
Also hit base_events.
Looks good.
github-actions
bot
removed
the
stale
✅ Deploy Preview for python-cpython-preview canceled.
|
| self.loop.run_forever() | ||
| except KeyboardInterrupt: | ||
| pass | ||
| self.loop.call_later(0.01, func) |
There was a problem hiding this comment.
A bit unfortunate that we had to add a 10 msec delay here -- in most cases that will waste 10 msec and occasionally it will not wait long enough and the test will flake-fail. Why won't call_soon work?
There was a problem hiding this comment.
Background: this test covers a bug that base exceptions like KeyboardInterrupt will cause the loop to be closed twice, leading to an early exit in the next loop run.
This test was added before _StopError was dropped, at that time the loop will stop as soon as _StopError is raised, even if the _ready queue still has a handle (which is the next run - the func() here), therefore this was a good test. But now the loop will exhaust the _ready queue before stopping, so this test couldn't cover the bug anymore without this PR. That's also why we cannot simply add func() into the _ready queue by call_soon().
You're right about the flakyness of using call_later() tho. Now I think a better fix would be to use chained call_soon() to skip the first iteration where the buggy stop occured:
| self.loop.call_later(0.01, func) | |
| # If the issue persists, the loop will exit early after the first iteration, | |
| # in that case our func() in the second iteration won't be called. | |
| self.loop.call_soon(self.loop.call_soon, func) |
I verified and this error fails if the fix in f3e2e09 is reverted.
There was a problem hiding this comment.
Clever solution, go ahead and make it a PR, assign to me or @kumaraditya303 for review.
8 participants
The test was added in f3e2e092 before
_StopErrorwas removed in 41f69f4, after which this test will not fail without the fix it covers. This PR fixes the test to resume its coverage.Refs: MagicStack/uvloop#337