bpo-41818: Make test_openpty() avoid unexpected success due to number of rows and/or number of columns being == 0.#23526
Conversation
Sorry, something went wrong.
|
@zware thank you for suggesting that helpful command. @asvetlov @ethanfurman here is my proposed proper fix. |
Sorry, something went wrong.
|
Let me check on the buildbot fleet. Just in case. |
Sorry, something went wrong.
asvetlov
added
the
🔨 test-with-buildbots
Test PR w/ buildbots; report in status section
label
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. |
Sorry, something went wrong.
bedevere-bot
removed
the
🔨 test-with-buildbots
Test PR w/ buildbots; report in status section
label
Nov 27, 2020
asvetlov
left a comment
asvetlov
left a comment
There was a problem hiding this comment.
Looks good, a tiny polishing is needed
Sorry, something went wrong.
|
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! |
Sorry, something went wrong.
…d/or number of columns being == 0. Signed-off-by: Soumendra Ganguly <soumendraganguly@gmail.com>
8vasu
force-pushed
the
test_openpty_fix
branch
from
1178d94 to
3e2a4cc
Compare
November 27, 2020 09:14
|
I have made the requested changes; please review again. Also, I have rebased so that there is only one clean commit. |
Sorry, something went wrong.
|
Thanks for making the requested changes! @asvetlov: please review the changes made to this pull request. |
Sorry, something went wrong.
|
Cool, thanks! |
Sorry, something went wrong.
|
You're welcome! |
Sorry, something went wrong.
|
The next pull request will make some additions to the tty module as preparation for making changes to pty. |
Sorry, something went wrong.
|
@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. |
Sorry, something went wrong.
|
@asvetlov I am working on it now :) I have posted a comment in the bpo issue about the details. |
Sorry, something went wrong.
… of rows and/or number of columns being == 0. (pythonGH-23526)
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