GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — bpo-41818: Updated tests for the standard pty library by 8vasu · Pull Request #22962 · python/cpython

Conversation

@8vasu

@8vasu 8vasu commented Oct 25, 2020 •
edited by bedevere-bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

This converts the test for pty.master_open() to a test for pty.openpty() with additional tests such as checking slave termios, slave winsize; these additional tests are expected to fail.

This also adds a small test that is expected to fail on FreeBSD and should pass on Linux; reflecting pty.spawn()'s FreeBSD hang issue [ bpo-26228 ].

Solutions to these problems have been implemented here: https://github.com/8vasu/pypty2

If and when these tests are acknowledged, the solutions will be submitted in parts.

This follows the conversation here: #21861

Signed-off-by: Soumendra Ganguly [email protected]

https://bugs.python.org/issue41818

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@8vasu
Updated tests for the pty standard library
323fc9c 
Signed-off-by: Soumendra Ganguly <[email protected]>
@the-knights-who-say-ni the-knights-who-say-ni added the CLA signed label Oct 25, 2020
@bedevere-bot bedevere-bot added tests Tests in the Lib/test dir awaiting review labels Oct 25, 2020
@8vasu 8vasu mentioned this pull request Oct 25, 2020
bpo-41541: Make pty.spawn set window size #21861
Closed
@8vasu

8vasu commented Oct 25, 2020

Copy link
Copy Markdown
Contributor Author

@ethanfurman @aeros

Sir, thank you very much for being patient. Please run the tests when you have time. I welcome suggestions/criticism. I have also sent a message to the teaching email thread.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@8vasu
In Lib/test/test_pty.py, wrap @expectedfailure to check if pty.STDIN_…
4bb22ca 
…FILENO is a tty

Signed-off-by: Soumendra Ganguly <[email protected]>
@8vasu
8vasu force-pushed the new_pty_tests branch from 057f14b to 4bb22ca Compare October 25, 2020 07:36
@8vasu
In Lib/test/test_pty.py, add a check for "Darwin" in expectedFailureO…
a8723e7 
…nBSD

Signed-off-by: Soumendra Ganguly <[email protected]>
@8vasu
8vasu force-pushed the new_pty_tests branch from 10f2db4 to a8723e7 Compare October 25, 2020 08:37
@gpshead gpshead added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Oct 26, 2020
@bedevere-bot

bedevere-bot commented Oct 26, 2020

Copy link
Copy Markdown

🤖 New build scheduled with the buildbot fleet by @gpshead for commit 2a3cba4d8e906af233fc22b1be648bae05e7f08b 🤖

If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Oct 26, 2020
gpshead
gpshead reviewed Oct 26, 2020
View reviewed changes

@gpshead gpshead left a comment

Copy link
Copy Markdown
Member

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.

i kicked off a test on the buildbots for some additional platform coverage out out curiosity. Note: our buildbot fleet may generate a lot of noise, they aren't all reliably and some were already failing on the main branch to begin with right now. so don't fret if it claims there are failures. only look for obviously related things within them if you go looking at those. :)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Lib/test/test_pty.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
@gpshead
gpshead requested review from aeros and ethanfurman October 26, 2020 06:10
@8vasu
Remove some comments.
26c0928 
Signed-off-by: Soumendra Ganguly <[email protected]>
@8vasu
8vasu force-pushed the new_pty_tests branch from 2a3cba4 to 26c0928 Compare October 26, 2020 14:16
@8vasu

8vasu commented Oct 26, 2020

Copy link
Copy Markdown
Contributor Author

@gpshead I have removed those comments :)
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
aeros
aeros reviewed Oct 26, 2020
View reviewed changes

@aeros aeros left a comment

Copy link
Copy Markdown
Contributor

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.

I've completed a preliminary review of the test changes. Most of the suggested changes are fairly minor, for the most part I think this is good to go (although for the final approval/merge, I'd prefer to leave it to @ethanfurman or @Yhg1s since I'm no expert with pty or tty).

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Lib/test/test_pty.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Lib/test/test_pty.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Lib/test/test_pty.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Lib/test/test_pty.py Outdated

@aeros aeros Oct 26, 2020

Copy link
Copy Markdown
Contributor

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.

Shouldn't this at least result in a warning via warnings.warn()? I suspect it would get buried otherwise.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@8vasu 8vasu Oct 27, 2020

Copy link
Copy Markdown
Contributor Author

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.

You are absolutely correct. I will change this. I have one question before I change this. If I use warnings.warn() here, then it will always generate warnings when the test is not run with stdin being a tty; for example the automated tests done by GitHub. Is that going to be an issue?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@aeros aeros Oct 30, 2020

Copy link
Copy Markdown
Contributor

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 may cause a slight bit of extra noise on the tests, but it shouldn't cause the GitHub CI to fail when the warning is emitted (I don't believe we have any of our CI set to fail on warning, but it can be ran with that setting locally).

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Lib/test/test_pty.py Outdated

@aeros aeros Oct 26, 2020

Copy link
Copy Markdown
Contributor

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.

This existing whitespace line should be re-added, since generally there are two lines in between the last line of a class's body and functions after it.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@8vasu 8vasu Oct 27, 2020

Copy link
Copy Markdown
Contributor Author

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.

Thank you for teaching me this. I will make this change.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Misc/NEWS.d/next/Library/2020-10-26-14-17-44.bpo-41818.5MgaEc.rst Outdated

@aeros aeros Oct 26, 2020

Copy link
Copy Markdown
Contributor

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.

The news entry should optimally refer to the tests that were changed (in this case test_opentty was updated and a new test_master_read() was added.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@8vasu 8vasu Oct 27, 2020

Copy link
Copy Markdown
Contributor Author

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.

Sure!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Lib/test/test_pty.py Outdated

@aeros aeros Oct 26, 2020

Copy link
Copy Markdown
Contributor

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.

I don't think this comment adds any additional context for the reader, and can be removed.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@8vasu 8vasu Oct 27, 2020

Copy link
Copy Markdown
Contributor Author

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.

Gotcha. I did this because the test_fork() function also has a similar comment at the end thereof. Should I remove that one as well?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Lib/test/test_pty.py Outdated

@aeros aeros Oct 26, 2020

Copy link
Copy Markdown
Contributor

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.

Maybe @ethanfurman or @Yhg1s could clarify, but I don't think we should add a new "RELEVANT ANYMORE?" comment.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@8vasu 8vasu Oct 27, 2020

Copy link
Copy Markdown
Contributor Author

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.

Thank you for noticing this. I agree that they are annoying; I apologize for this. Yes, I need clarification. The plan was to remove the annoying "RELEVANT ANYMORE?" comments after clarification.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@8vasu 8vasu Oct 27, 2020

Copy link
Copy Markdown
Contributor Author

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.

I myself actually did not put much thought into the blocks that I marked with "RELEVANT ANYMORE?"; I will think about them in greater detail on Saturday. They were just supposed to be there as reminders; anyway, they do not hurt the tests :)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@8vasu 8vasu Nov 2, 2020

Copy link
Copy Markdown
Contributor Author

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.

Aah. Now I realize why I added the extra "RELEVANT ANYMORE" here. Notice that the following comment mentions pty.master_open(), which opens the slave and subsequently closes it. As a result, the original test_basic() function was having to call pty.slave_open() to re-open the slave. However, the new test_openpty() calls pty.openpty(), which, unlike pty.master_open(), does not close the slave end. Anyway, that part does not seem to affect the test on Linux and FreeBSD.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Lib/test/test_pty.py Outdated

@aeros aeros Oct 26, 2020

Copy link
Copy Markdown
Contributor

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.

Could we potentially surround with a try...finally to avoid this? A single test failure should really not result in the other tests within test_pty being unreliable.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@8vasu 8vasu Oct 27, 2020

Copy link
Copy Markdown
Contributor Author

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.

Good point. Thank you. I will work on this.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@8vasu

8vasu commented Oct 27, 2020

Copy link
Copy Markdown
Contributor Author

@aeros I am grateful for the reviews. I will keep working on this with the hope that we can finally put these old bugs to rest.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@aeros
aeros requested a review from Yhg1s October 30, 2020 18:03
@8vasu
Restructure the tests, perform cleanup.
9a6e70f 
Signed-off-by: Soumendra Ganguly <[email protected]>
@8vasu
8vasu force-pushed the new_pty_tests branch from c431bd0 to 9a6e70f Compare November 2, 2020 21:47
@8vasu

8vasu commented Nov 2, 2020

Copy link
Copy Markdown
Contributor Author

@aeros Thank you for being patient. I have made the following changes as per your suggestions:

  1. I had added a comment saying that the tty could be left in an abnormal state [ only winsize, not termios/terminfo ]. I have solved the problem by adding an addCleanup(). I have also renamed some variables, added functions [ _get_term_winsz() and _set_term_winsz() ] to increase readability.

  2. I have changed the debug() in the except clause of the second try-except block in test_openpty() to a warnings.warn() as per your suggestion.

  3. I realized why I added the extra "RELEVANT ANYMORE" comment. The following comment mentions pty.master _open(), which opens the slave and subsequently closes it. As a result, the original test_basic() function was having t
    o call pty.slave_open() to re-open the slave. However, the new test_openpty() calls pty.openpty(), which, unlike pty.ma ster_open(), does not close the slave end. Anyway, that part does not seem to affect the test on Linux and FreeBSD.
    I have removed the "RELEVANT ANYMORE" comments.

  4. Removed the "pty.openpty() passed" and "pty.fork() passed" comments.

  5. Replaced % with f-strings. However, I have not trimmed down the debug messages yet because test_fork() has similar debug messages in it. I will remove them all at once if you wish.

  6. Added the existing empty line between class and subsequent function definition.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
8vasu added 3 commits November 2, 2020 16:16
@8vasu
Restructure test_openpty().
f6d01f8 
Signed-off-by: Soumendra Ganguly <[email protected]>
@8vasu
Restructure test_openpty().
85f173b 
Signed-off-by: Soumendra Ganguly <[email protected]>
@8vasu
Restructure test_openpty().
6546938 
Signed-off-by: Soumendra Ganguly <[email protected]>
ethanfurman
ethanfurman reviewed Nov 17, 2020
View reviewed changes
Comment thread Lib/test/test_pty.py Outdated

@ethanfurman ethanfurman Nov 17, 2020

Copy link
Copy Markdown
Member

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.

I've been away from the code, so this might be a dumb question, but can't we determine if stdin is a tty?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@8vasu 8vasu Nov 19, 2020

Copy link
Copy Markdown
Contributor Author

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.

Sorry for the late reply. That is actually a great question. tcgetattr() can fail due to 2 reasons: errno == EBADF ( not a valid file descriptor ), errno == ENOTTY ( not a tty ). See https://pubs.opengroup.org/onlinepubs/9699919799/. In either case, your stdin is not a tty. In fact, all glibc's isatty() does is call tcgetattr() to see if it fails; see https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/posix/isatty.c.

I have verified that Linux, FreeBSD, and NetBSD follow this approach for their isatty() implementations, while OpenBSD calls fcntl()+F_ISATTY. Anyway, the tcgetattr() approach works on all the above platforms. I avoid isatty(), which is often followed by a separate tcgetattr() call, thereby avoiding an extra tcgetattr() call [ OCD :D ].

In our code, if tcgetattr() fails, then either pty.STDIN_FILENO, which is 0, is a valid fd but not a tty (ENOTTY), or it is not a valid fd at all (EBADF). EBADF will not happen unless something closes 0 [ uncommon scenario ]; for example, by using [ p close(0) ] in gdb, or if your script itself voluntarily closes 0 for some reason.

The "possible" in the comment # possible reason: current stdin is not a tty is not because of lack of confidence, but because I did not want to discount the (uncommon) chance of an EBADF, in which case 0 is not a valid fd at all [ OCD ]. Anyway, that comment can be safely removed. Do you want me to remove it?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@ethanfurman ethanfurman Nov 20, 2020

Copy link
Copy Markdown
Member

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.

I would say update it:

# possible reason ... to # not a tty or bad/closed fd

and, of course, make that change throughout.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@8vasu 8vasu Nov 20, 2020

Copy link
Copy Markdown
Contributor Author

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.

Done.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@8vasu
Remove/change some comments.
0cde3f2 
Signed-off-by: Soumendra Ganguly <[email protected]>
@8vasu
8vasu force-pushed the new_pty_tests branch from 4a3c962 to 0cde3f2 Compare November 20, 2020 14:39
@blurb-it
📜🤖 Added by blurb_it.
e990b79
asvetlov
asvetlov approved these changes Nov 24, 2020
View reviewed changes

@asvetlov asvetlov left a comment

Copy link
Copy Markdown
Contributor

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.

LGTM, I think we can merge the PR and unblock @8vasu to work on future PTY improvements.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@bedevere-bot bedevere-bot added awaiting merge and removed awaiting review labels Nov 24, 2020
@asvetlov
asvetlov merged commit c13d899 into python:master Nov 25, 2020
@bedevere-bot bedevere-bot removed the awaiting merge label Nov 25, 2020
@bedevere-bot

bedevere-bot commented Nov 25, 2020

Copy link
Copy Markdown

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot x86 Gentoo Installed with X 3.x has failed when building commit c13d899.

What do you need to do:

  1. Don't panic.
  2. Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
  3. Go to the page of the buildbot that failed (https://buildbot.python.org/all/#builders/464/builds/445) and take a look at the build logs.
  4. Check if the failure is related to this commit (c13d899) or if it is a false positive.
  5. If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/all/#builders/464/builds/445

Failed tests:

  • test_pty

Summary of the results of the build (if available):

== Tests result: FAILURE then FAILURE ==

411 tests OK.

1 test failed:
test_pty

13 tests skipped:
test_asdl_parser test_check_c_globals test_clinic test_devpoll
test_gdb test_ioctl test_kqueue test_msilib test_startfile
test_winconsoleio test_winreg test_winsound test_zipfile64

1 re-run test:
test_pty

Total duration: 28 min 6 sec

Click to see traceback logs 
remote: Enumerating objects: 18, done.        
remote: Counting objects:   5% (1/18)        
remote: Counting objects:  11% (2/18)        
remote: Counting objects:  16% (3/18)        
remote: Counting objects:  22% (4/18)        
remote: Counting objects:  27% (5/18)        
remote: Counting objects:  33% (6/18)        
remote: Counting objects:  38% (7/18)        
remote: Counting objects:  44% (8/18)        
remote: Counting objects:  50% (9/18)        
remote: Counting objects:  55% (10/18)        
remote: Counting objects:  61% (11/18)        
remote: Counting objects:  66% (12/18)        
remote: Counting objects:  72% (13/18)        
remote: Counting objects:  77% (14/18)        
remote: Counting objects:  83% (15/18)        
remote: Counting objects:  88% (16/18)        
remote: Counting objects:  94% (17/18)        
remote: Counting objects: 100% (18/18)        
remote: Counting objects: 100% (18/18), done.        
remote: Compressing objects:  10% (1/10)        
remote: Compressing objects:  20% (2/10)        
remote: Compressing objects:  30% (3/10)        
remote: Compressing objects:  40% (4/10)        
remote: Compressing objects:  50% (5/10)        
remote: Compressing objects:  60% (6/10)        
remote: Compressing objects:  70% (7/10)        
remote: Compressing objects:  80% (8/10)        
remote: Compressing objects:  90% (9/10)        
remote: Compressing objects: 100% (10/10)        
remote: Compressing objects: 100% (10/10), done.        
remote: Total 10 (delta 8), reused 1 (delta 0), pack-reused 0        
From https://github.com/python/cpython
 * branch                  master     -> FETCH_HEAD
Reset branch 'master'

	not a dynamic executable

	not a dynamic executable
	not a dynamic executable
  WARNING: The script easy_install-3.10 is installed in '/buildbot/buildarea/cpython/3.x.ware-gentoo-x86.installed/build/target/bin' which is not on PATH.
  Consider adding this directory to PATH or, if you prefer to suppress this warning, use --no-warn-script-location.
  WARNING: The scripts pip3 and pip3.10 are installed in '/buildbot/buildarea/cpython/3.x.ware-gentoo-x86.installed/build/target/bin' which is not on PATH.
  Consider adding this directory to PATH or, if you prefer to suppress this warning, use --no-warn-script-location.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot

bedevere-bot commented Nov 25, 2020

Copy link
Copy Markdown

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot x86 Gentoo Non-Debug with X 3.x has failed when building commit c13d899.

What do you need to do:

  1. Don't panic.
  2. Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
  3. Go to the page of the buildbot that failed (https://buildbot.python.org/all/#builders/58/builds/442) and take a look at the build logs.
  4. Check if the failure is related to this commit (c13d899) or if it is a false positive.
  5. If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/all/#builders/58/builds/442

Failed tests:

  • test_pty

Summary of the results of the build (if available):

== Tests result: FAILURE then FAILURE ==

414 tests OK.

10 slowest tests:

  • test_peg_generator: 7 min 5 sec
  • test_tokenize: 3 min 45 sec
  • test_concurrent_futures: 3 min 45 sec
  • test_multiprocessing_spawn: 2 min 58 sec
  • test_asyncio: 2 min 39 sec
  • test_lib2to3: 2 min 31 sec
  • test_unparse: 2 min 25 sec
  • test_multiprocessing_forkserver: 1 min 54 sec
  • test_multiprocessing_fork: 1 min 21 sec
  • test_io: 1 min 15 sec

1 test failed:
test_pty

10 tests skipped:
test_devpoll test_gdb test_ioctl test_kqueue test_msilib
test_startfile test_winconsoleio test_winreg test_winsound
test_zipfile64

1 re-run test:
test_pty

Total duration: 35 min 23 sec

Click to see traceback logs 
remote: Enumerating objects: 18, done.        
remote: Counting objects:   5% (1/18)        
remote: Counting objects:  11% (2/18)        
remote: Counting objects:  16% (3/18)        
remote: Counting objects:  22% (4/18)        
remote: Counting objects:  27% (5/18)        
remote: Counting objects:  33% (6/18)        
remote: Counting objects:  38% (7/18)        
remote: Counting objects:  44% (8/18)        
remote: Counting objects:  50% (9/18)        
remote: Counting objects:  55% (10/18)        
remote: Counting objects:  61% (11/18)        
remote: Counting objects:  66% (12/18)        
remote: Counting objects:  72% (13/18)        
remote: Counting objects:  77% (14/18)        
remote: Counting objects:  83% (15/18)        
remote: Counting objects:  88% (16/18)        
remote: Counting objects:  94% (17/18)        
remote: Counting objects: 100% (18/18)        
remote: Counting objects: 100% (18/18), done.        
remote: Compressing objects:  10% (1/10)        
remote: Compressing objects:  20% (2/10)        
remote: Compressing objects:  30% (3/10)        
remote: Compressing objects:  40% (4/10)        
remote: Compressing objects:  50% (5/10)        
remote: Compressing objects:  60% (6/10)        
remote: Compressing objects:  70% (7/10)        
remote: Compressing objects:  80% (8/10)        
remote: Compressing objects:  90% (9/10)        
remote: Compressing objects: 100% (10/10)        
remote: Compressing objects: 100% (10/10), done.        
remote: Total 10 (delta 8), reused 1 (delta 0), pack-reused 0        
From https://github.com/python/cpython
 * branch                  master     -> FETCH_HEAD
Reset branch 'master'

	not a dynamic executable

	not a dynamic executable
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@asvetlov

asvetlov commented Nov 25, 2020

Copy link
Copy Markdown
Contributor

The test fails with 'unexpected success' error, perhaps expectedFailure() should not be applied for this configuration.
Investigating.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@ethanfurman

ethanfurman commented Nov 25, 2020

Copy link
Copy Markdown
Member

Or perhaps Gentoo is one of the cases where it succeeds, and the the expected failure wrapper should not be applied in that case.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@asvetlov asvetlov mentioned this pull request Nov 25, 2020
bpo-41818: test_openpty succeed on Gentoo, don't expect to fail on this platform #23514
Merged
@asvetlov

asvetlov commented Nov 25, 2020

Copy link
Copy Markdown
Contributor

#23514 is created for fix
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@8vasu 8vasu mentioned this pull request Nov 26, 2020
bpo-41818: Make test_openpty() avoid unexpected success due to number of rows and/or number of columns being == 0. #23526
Merged
adorilson pushed a commit to adorilson/cpython that referenced this pull request Mar 13, 2021
@8vasu @adorilson
bpo-41818: Updated tests for the standard pty library (pythonGH-22962)
267725e
@8vasu 8vasu mentioned this pull request Aug 12, 2021
bpo-41818: Add termios.tcgetwinsize(), termios.tcsetwinsize(). Update docs. #23686
Merged
@8vasu 8vasu mannequin mentioned this pull request May 5, 2022
Lib/pty.py major revision #85984
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gpshead gpshead gpshead left review comments
@ethanfurman ethanfurman ethanfurman left review comments
@asvetlov asvetlov asvetlov approved these changes
@Yhg1s Yhg1s Awaiting requested review from Yhg1s
+1 more reviewer
@aeros aeros aeros left review comments
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

tests Tests in the Lib/test dir

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

7 participants

@8vasu @bedevere-bot @asvetlov @ethanfurman @gpshead @aeros @the-knights-who-say-ni
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.