GeezerVison is showing github.com — bpo-38323: Temporarily skip close_kill_running() for MultiLoopWatcher in test_asyncio by aeros · Pull Request #20013 · python/cpython
Conversation
aeros
commented
May 8, 2020
•
edited
Loading
Uh oh!
There was an error while loading. Please reload this page.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
aeros
commented
May 8, 2020
•
edited
Loading
Uh oh!
There was an error while loading. Please reload this page.
|
Oops, that's an easy fix for the windows failure. I should be able to just use asyncio.MultiLoopWatcher instead of asyncio.unix_events.MultiLoopWatcher.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
======================================================================
ERROR: test_close_kill_running (test.test_asyncio.test_subprocess.SubprocessProactorTests)
----------------------------------------------------------------------
Traceback (most recent call last):
File "d:\a\1\s\lib\test\test_asyncio\test_subprocess.py", line 463, in test_close_kill_running
if isinstance(asyncio.get_child_watcher(),
File "d:\a\1\s\lib\asyncio\events.py", line 766, in get_child_watcher
return get_event_loop_policy().get_child_watcher()
File "d:\a\1\s\lib\asyncio\events.py", line 602, in get_child_watcher
raise NotImplementedError
NotImplementedError
Oh, I hadn't realized that get_child_watcher() wasn't supported on Windows. I'll add a platform check.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
@vstinner Would you mind giving this a quick look-over before I merge it? This PR is fairly straightforward since it's just skipping a test, but I'm not quite comfortable yet with merging my own without an approval (particularly with the earlier CI failures).
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
|
Actually, are you sure that test_close_kill_running() is the one that needs to be skipped? @asvetlov added code to address the hang in that test here: #18457
If you look at this comment of @vstinner, it seems like he might be referring to tests other than test_close_kill_running(): https://bugs.python.org/issue38323#msg368061
(like the one you mentioned test_stdin_stdout()).
Either way, it would be important to know if this test was still hanging even with Andrew's change, as that would be a new failure variant.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
aeros
commented
May 10, 2020
•
edited
Loading
Uh oh!
There was an error while loading. Please reload this page.
|
@cjerdonek Based on the following comment:
we may want to consider skipping test_close_kill_running for MultiLoopWatcher until we can find one
There are more MultiLoopWatcher tests which hang randomly, it's not only test_close_kill_running().
I'm fine with skipping tests until someone can come up with a fix.
I'm fairly certain that test_close_kill_running() was still hanging randomly, even after Andrew's fix. I was unable to reproduce the failure locally though, so I wasn't able to confirm it myself. I made this PR based on @vstinner's above comment, but if/when I find the time, I'll try to look through to buildbot logs to confirm if there were still hangs for test_close_kill_running() after Andrew merged #18457.
|
cjerdonek reacted with thumbs up emoji
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
I'll try to look through to buildbot logs to confirm if there were still hangs for test_close_kill_running()
That would be great. (In all the times I've reproduced the hang locally, Andrew's change would have prevented it.)
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
Closing as the entire SubprocessMultiLoopWatcherTests tests are currently skipped.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Assignees
No one assigned
Milestone
No milestone
Development
Successfully merging this pull request may close these issues.
Uh oh!
There was an error while loading. Please reload this page.
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.
|