Skip to content

Conversation

@8vasu
Copy link
Contributor

@8vasu8vasu commented Nov 28, 2020

This replaces #23533. Build was failing on Solaris since, like the BSDs and Darwin, Solaris does not raise OSError in test_master_read(). Therefore, now we make test_master_read() succeed on all platforms that either raise OSError [ such as Linux ] or return b"" [ such as BSDs, Darwin, and possibly Solaris ] upon reading from master when the slave is closed. Any platform that does not exhibit such behavior can now be detected using test_master_read(). On any given platform, exiting from pty.spawn()'s copy loop depends on our exact knowledge of the behavior of test_master_read() on that platform.

Signed-off-by: Soumendra Ganguly soumendraganguly@gmail.com

https://bugs.python.org/issue41818

either raise OSError or return b"" upon reading from master Signed-off-by: Soumendra Ganguly <soumendraganguly@gmail.com>
@8vasu
Copy link
ContributorAuthor

8vasu commented Nov 28, 2020

@asvetlov this follows #23526. My original solution was #23533, but this is much better.

Edit: to be extra safe, I request you to run the buildbots again.

Copy link
Contributor

@kulikjakkulikjak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With this patch, test_pty on Solaris is green again.

@asvetlovasvetlov added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Nov 28, 2020
@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @asvetlov for commit f994d09 🤖

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

@bedevere-botbedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Nov 28, 2020
@asvetlov
Copy link
Contributor

to be extra safe, I request you to run the buildbots again.

Done

@asvetlov
Copy link
Contributor

Thanks!

@8vasu
Copy link
ContributorAuthor

You're welcome!

@8vasu
Copy link
ContributorAuthor

@asvetlov#23546 is the next one. Of course, you can take your time to review it :D The tty module will now need a documentation update; I have not done that yet. Also, please let me know if I need to make tests for the tty module. After we finish working on that one, and before starting to make changes to the pty module, I will add one more nice test for the pty module which will check if pty.spawn() is capable of adjusting slave window size dynamically [ SIGWINCH ].

adorilson pushed a commit to adorilson/cpython that referenced this pull request Mar 13, 2021
…s that either raise OSError or return b"" upon reading from master (pythonGH-23536) Signed-off-by: Soumendra Ganguly <soumendraganguly@gmail.com>
@8vasu8vasumannequin mentioned this pull request May 5, 2022
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

testsTests in the Lib/test dir

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@8vasu@bedevere-bot@asvetlov@kulikjak@the-knights-who-say-ni