Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 34k
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
Uh oh!
There was an error while loading. Please reload this page.
Conversation
terryjreedy commented Sep 6, 2023
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. |
terryjreedy left a comment
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).
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
bedevere-bot commented Sep 6, 2023
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>
zooba left a comment
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.
Uh oh!
There was an error while loading. Please reload this page.
Co-authored-by: Steve Dower <steve.dower@microsoft.com>
miss-islington commented Sep 8, 2023
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>
bedevere-bot commented Sep 8, 2023
|
vstinner commented Sep 8, 2023
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 |
vstinner commented Sep 8, 2023
The GitHub Action job also started to log a new error: |
vstinner commented Sep 8, 2023
"AMD64 Windows10 3.x" buildbot failures: |
vstinner commented Sep 8, 2023
I disabled backport, to avoid backporting a change which breaks the CI. |
vstinner commented Sep 8, 2023
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: ifnothasattr(select, 'devpoll') : raiseunittest.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? |
terryjreedy commented Sep 8, 2023
vstinner commented Sep 8, 2023
terryjreedy commented Sep 8, 2023
Funny, it passed above. Feel free to revert: I don't know how. |
vstinner commented Sep 8, 2023
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. |
vstinner commented Sep 8, 2023
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. |
vstinner commented Sep 9, 2023
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 :-( |
vstinner commented Sep 9, 2023
Similar failure on "AMD64 Windows11 Non-Debug 3.x" buildbot: logs: https://buildbot.python.org/all/#/builders/914/builds/2794 |
vstinner commented Sep 9, 2023
@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. |
vstinner commented Sep 9, 2023
A first step would be to only skip tests which are known to fail, and run the other tests which are fine. |
aisk commented Sep 9, 2023
For the |
terryjreedy commented Sep 10, 2023
I am done with this as these errors are out of my league. |
terryjreedy commented Sep 10, 2023
@zooba knows more and wrote some of the changes. |
aisk commented Sep 10, 2023 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
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. |
aisk commented Sep 10, 2023
Tests still fails😂Looks like there are more issues... |
vstinner commented Sep 10, 2023
Is it possible to create a "new terminal" just to run test_msvcrt? |
Uh oh!
There was an error while loading. Please reload this page.