gh-108996: fix and enable test_msvcrt#109226
Uh oh!
There was an error while loading. Please reload this page.
Conversation
vstinner left a comment
vstinner left a comment There was a problem hiding this comment.
I would prefer to start easy: reenable the whole file, but still skip tests known to fail, and then in a following PR, fix these tests. So we can spend more time to test in length the second PR.
Lib/test/test_msvcrt.py Outdated
| @@ -1,9 +1,8 @@ | |||
| import ctypes | |||
There was a problem hiding this comment.
ctypes is not always available. Please use try: import ctypes except ImportError: ctypes = None, and then skip tests using ctypes if ctypes is None.
Lib/test/test_msvcrt.py Outdated
| sys.stdin = old_stdin | ||
| with open('CONIN$', 'rb', buffering=0) as stdin: | ||
| h = msvcrt.get_osfhandle(stdin.fileno()) | ||
| ctypes.windll.kernel32.FlushConsoleInputBuffer(h) |
There was a problem hiding this comment.
Maybe it should be exposed in msvcrt, maybe as a private function just for testing.
aisk commented Sep 10, 2023
@vstinner Current implementation doesn't failes in latest two runs, maybe it's been fixed. But the freebsd tests failed: I think it's weird, the tests should not run on freebsd. |
vstinner commented Sep 10, 2023
The test failed because ctypes is not available on this FreeBSD CI:
I told you, in tests, you should tolerate that ctypes is not available. Or write a wrapper to FlushConsoleInputBuffer() in |
aisk commented Sep 11, 2023
@vstinner Ah sorry, just understood what you mean above... |
aisk commented Sep 11, 2023
This change passed the tests in the last three times, I think it's ready to review. |
aisk commented Sep 15, 2023
Latest run failed because of freebsd test instance out of limit, not related to this change. |
Uh oh!
There was an error while loading. Please reload this page.
zooba commented Sep 21, 2023
I don't have any further feedback. @vstinner are you happy with this? |
vstinner commented Sep 21, 2023
!buildbot Windows |
bedevere-bot commented Sep 21, 2023
vstinner commented Sep 21, 2023
!buildbot AMD64 Windows |
bedevere-bot commented Sep 21, 2023
vstinner commented Sep 21, 2023
Let's see if buildbots are happy :-) |
vstinner commented Sep 22, 2023
Tests passed on these 4 Windows buildbots: good.
Example on ARM64 Windows Non-Debug PR: |
vstinner commented Sep 22, 2023
Merged, thanks @aisk for your contribution. Let's see how it goes this time :-) I'm not surprised that these tests failed. It's hard to write reliable tests on a terminal :-( Calling FlushConsoleInputBuffer() seems to make tests more reliable: good! I enhanced/completed the commit message. |
aisk commented Sep 22, 2023
Thanks for the review! |
* Add _testconsole.flush_console_input_buffer() function. * test_kbhit(), test_getwch() and test_getwche() now call flush_console_input_buffer(). * Don't override sys.stdin anymore (not needed).
* Add _testconsole.flush_console_input_buffer() function. * test_kbhit(), test_getwch() and test_getwche() now call flush_console_input_buffer(). * Don't override sys.stdin anymore (not needed).
4 participants
More details: #109004