bpo-34187: _WindowsConsoleIO rises correct exception#9922
bpo-34187: _WindowsConsoleIO rises correct exception#9922augustogoulart wants to merge 6 commits intopython:mainfrom augustogoulart:bpo34187
Conversation
_WindowsConsoleIO implementation of fileno should raise a ValueError instead of io.UnsupportedOperation if the internal handle value is INVALID_HANDLE_VALUE.
| /*[clinic end generated code: output=006fa74ce3b5cfbf input=079adc330ddaabe6]*/ | ||
| { | ||
| if (self->fd < 0 && self->handle != INVALID_HANDLE_VALUE) { | ||
| if (self->handle == INVALID_HANDLE_VALUE) |
There was a problem hiding this comment.
This if needs curly braces.
There was a problem hiding this comment.
Thanks for making this PR!
ISTM that the potential issue in internal_close is separate from the main issue regarding fileno. I recommend removing the internal_close change from this PR, and possibly creating a separate PR for it.
Regarding the fileno issue, please add a test that reproduces the relevant bug.
|
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 poked with soft cushions! |
|
I have made the requested changes; please review again. |
|
Thanks for making the requested changes! @taleinat: please review the changes made to this pull request. |
taleinat
left a comment
taleinat
left a comment
There was a problem hiding this comment.
ISTM this change does more than just raise a different exception, since it also makes sure to close the file handle in a certain case. I'd like to see a test for that too.
This will need a NEWS entry; install blurb and run blurb add to create one.
Also a minor inline comment.
| with ConIO('CONOUT$', 'w') as f: | ||
| self.assertEqual(f.write(b''), 0) | ||
|
|
||
| def test_fileno_raises_correct_exception(self): |
There was a problem hiding this comment.
I suggest replacing "correct_exception" with something about the file being closed.
|
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 |
4 participants
_WindowsConsoleIOimplementation of fileno should raise aValueErrorinstead of
io.UnsupportedOperationif the internal handle value isINVALID_HANDLE_VALUE.
Previous issue that originated issue34187:
https://bugs.python.org/issue34115
https://bugs.python.org/issue34187