-
-
Notifications
You must be signed in to change notification settings - Fork 31.9k
gh-108996: add tests for msvcrt #109004
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
gh-108996: add tests for msvcrt #109004
Conversation
aisk
commented
Sep 6, 2023
•
edited by bedevere-bot
Loading
edited by bedevere-bot
- Issue: Add tests for msvcrt module #108996
Pending review, this can be merged since the test_threading failure is not related. (It has failed elsewhere today, including on current main on my machine.) I will look at the patch. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be nice if one of the Windows experts looked at this, especially the tests of file operations (which I have no familiarity with).
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase And if you don't make the requested changes, you will be put in the comfy chair! |
Co-authored-by: Terry Jan Reedy <tjreedy@udel.edu>
Co-authored-by: Terry Jan Reedy <tjreedy@udel.edu>
Co-authored-by: Terry Jan Reedy <tjreedy@udel.edu>
Co-authored-by: Terry Jan Reedy <tjreedy@udel.edu>
Co-authored-by: Terry Jan Reedy <tjreedy@udel.edu>
Co-authored-by: Terry Jan Reedy <tjreedy@udel.edu>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems okay to me. Passing tests are better than no tests at all.
Co-authored-by: Steve Dower <steve.dower@microsoft.com>
Thanks @aisk for the PR, and @terryjreedy for merging it 🌮🎉.. I'm working now to backport this PR to: 3.11, 3.12. |
(cherry picked from commit bcb2ab5) Co-authored-by: AN Long <aisk@users.noreply.github.com> Co-authored-by: Terry Jan Reedy <tjreedy@udel.edu> Co-authored-by: Steve Dower <steve.dower@microsoft.com>
(cherry picked from commit bcb2ab5) Co-authored-by: AN Long <aisk@users.noreply.github.com> Co-authored-by: Terry Jan Reedy <tjreedy@udel.edu> Co-authored-by: Steve Dower <steve.dower@microsoft.com>
|
This change broke "AMD64 Windows10 3.x" buildbot and the Windows x64 CI job on GitHub Action :-( buildbot failure: https://buildbot.python.org/all/#builders/146/builds/6284 GHA failure: https://github.com/python/cpython/actions/runs/6127105160/job/16632346105?pr=109168
|
The GitHub Action job also started to log a new error:
|
"AMD64 Windows10 3.x" buildbot failures:
|
I disabled backport, to avoid backporting a change which breaks the CI. |
Ah, another strange thing in the GHA job:
I don't know why test_devpoll is run on Windows, nor why it does crash :-( The test starts with: if not hasattr(select, 'devpoll') :
raise unittest.SkipTest('test works only on Solaris OS family') But it's a bug in libregrtest which reports a test_msvcrt crash as a test_devpoll crash? |
Funny, it passed above. Feel free to revert: I don't know how. |
I don't know why it passed before!? It's surprising :-( At least, we know a buildbot which should be tested if a fix is proposed. |
I don't think that it would be productive to revert. Instead, I wrote PR #109169 to keep the test but always skip the test (on all platforms) for now, until someone is available to fix it. |
Merged. The strange part is that I re-ran Windows x64 job on my PR #109168 before PR #109169 was merged, and all tests pass Windows x64! It seems like test_msvcrt is not reliable :-( |
Similar failure on "AMD64 Windows11 Non-Debug 3.x" buildbot:
logs: https://buildbot.python.org/all/#/builders/914/builds/2794 |
@terryjreedy: if you want, next week if can have a look at the failing tests. But when I ran the test manually on my Windows VM: oops, it just worked. I don't know which kind of "terminal" the buildbot and the GitHub Action Windows jobs use. These days, I run tests in a SSH connection on a Linux terminal to my Windows VM. In this case, apparently, Python seems to be satisfied with the terminal that it gets. |
A first step would be to only skip tests which are known to fail, and run the other tests which are fine. |
For the |
I am done with this as these errors are out of my league. |
@zooba knows more and wrote some of the changes. |
I figured out what happened: The One should paste the test case in cpython/Lib/test/test_winconsoleio.py Lines 192 to 199 in 4297499
test_msvcrt to reproduce the error result.
I tried to call FlushConsoleInputBuffer via ctypes in #109226 to fix the error. |
Tests still fails😂Looks like there are more issues... |
Is it possible to create a "new terminal" just to run test_msvcrt? |