bpo-41818: Make test_openpty() avoid unexpected success due to number of rows and/or number of columns being == 0.#23526
Uh oh!
There was an error while loading. Please reload this page.
Conversation
8vasu commented Nov 26, 2020
@zware thank you for suggesting that helpful command. @asvetlov@ethanfurman here is my proposed proper fix. |
asvetlov commented Nov 27, 2020
Let me check on the buildbot fleet. Just in case. |
asvetlov added the 🔨 test-with-buildbots 
bedevere-bot commented Nov 27, 2020
🤖 New build scheduled with the buildbot fleet by @asvetlov for commit 1178d94873b7a72bcc1426cd4db7d525d7db1cec 🤖 If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again. |
bedevere-bot removed the 🔨 test-with-buildbots 
asvetlov left a comment
asvetlov left a comment There was a problem hiding this comment.
Looks good, a tiny polishing is needed
Lib/test/test_pty.py Outdated
There was a problem hiding this comment.
Drop this comment please, it is not required anymore
Lib/test/test_pty.py Outdated
There was a problem hiding this comment.
Please add a few words to describe the resize trick and why it is necessary.
There was a problem hiding this comment.
I have added some comments. Please let me know if this looks good.
bedevere-bot commented Nov 27, 2020
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! |
…d/or number of columns being == 0. Signed-off-by: Soumendra Ganguly <soumendraganguly@gmail.com>
8vasu commented Nov 27, 2020
I have made the requested changes; please review again. Also, I have rebased so that there is only one clean commit. |
bedevere-bot commented Nov 27, 2020
Thanks for making the requested changes! @asvetlov: please review the changes made to this pull request. |
asvetlov commented Nov 27, 2020
Cool, thanks! |
8vasu commented Nov 27, 2020
You're welcome! |
8vasu commented Nov 27, 2020
The next pull request will make some additions to the tty module as preparation for making changes to pty. |
asvetlov commented Nov 27, 2020
@8vasu please also take a look at We need to suppress the error for the problematic platform at least if there is no idea why SunOS behaves differently. |
8vasu commented Nov 27, 2020
@asvetlov I am working on it now :) I have posted a comment in the bpo issue about the details. |
… of rows and/or number of columns being == 0. (pythonGH-23526)
4 participants
Proposed proper fix for buildbot failures introduced after #22962 merging.
To check if
pty.openpty()properly sets pty slave size based on the size of current stdin,PtyTest.test_openpty()was modifying the size of current stdin first. Let 'x' be the number of rows of stdin; then, we set the number of rows of stdin to some f(x) such that the function f does not have any fixed points; that is, f(x) != x for all x. In #22962, f(x) := x//5; unfortunately, in that case, f does have 0 as a fixed point [ 0//5 = 0 ]. Upon runningunexpected success was reported on Debian; this indeed was due to the # of (rows, cols) being (0,0). The Gentoo buildbot was probably also running with stdin being a tty of size (0,0). The new choice of f(x) := x+1 solves the problem, since this f has no fixed points [ x+1=x has no solution. ]
TL;DR: instead of letting rows and cols of stdin be the floor of 1/5 of their original values, we now only increment their respective values by 1. Note, however, that the former is a more noticeable change when # of rows, cols are fairly large.
Signed-off-by: Soumendra Ganguly soumendraganguly@gmail.com
https://bugs.python.org/issue41818