◐ Shell
clean mode source ↗

bpo-38323: Temporarily skip close_kill_running() for MultiLoopWatcher in test_asyncio by aeros · Pull Request #20013 · python/cpython

pppery

@aeros

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.

Co-authored-by: pppery <mapreader@olum.org>

@aeros

======================================================================
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.

@aeros

@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).

cjerdonek

Co-authored-by: Chris Jerdonek <chris.jerdonek@gmail.com>

@cjerdonek

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.

@aeros

@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

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.)

@kumaraditya303

Closing as the entire SubprocessMultiLoopWatcherTests tests are currently skipped.