FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

bpo-41818: Fix test_master_read() so that it succeeds on all platforms that either raise OSError or return b"" upon reading from master by 8vasu · Pull Request #23536 · python/cpython · GitHub

  1. python
  2. cpython

Repository navigation

bpo-41818: Fix test_master_read() so that it succeeds on all platforms that either raise OSError or return b"" upon reading from master - #23536

Merged
asvetlov merged 2 commits into
python:masterfrom
8vasu:fix_test_master_read
Nov 28, 2020
ConversationCommits (2)ChecksFiles changed
Merged

bpo-41818: Fix test_master_read() so that it succeeds on all platforms that either raise OSError or return b"" upon reading from master#23536
asvetlov merged 2 commits into
python:masterfrom
8vasu:fix_test_master_read

Conversation

8vasu commented Nov 28, 2020 •
edited by bedevere-bot
Loading

Copy link
Copy Markdown
Contributor

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

Fix test_master_read() so that it succeeds on all platforms that
a550369 
either raise OSError or return b"" upon reading from master

Signed-off-by: Soumendra Ganguly <soumendraganguly@gmail.com>
bedevere-bot added tests Tests in the Lib/test dir awaiting review labels Nov 28, 2020

8vasu commented Nov 28, 2020 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

@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.

kulikjak approved these changes Nov 28, 2020
View reviewed changes

kulikjak left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

With this patch, test_pty on Solaris is green again.

asvetlov added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Nov 28, 2020

bedevere-bot commented Nov 28, 2020

Copy link
Copy Markdown

🤖 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-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Nov 28, 2020

asvetlov commented Nov 28, 2020

Copy link
Copy Markdown
Contributor

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

Done

asvetlov merged commit 74311ae into python:master Nov 28, 2020

asvetlov commented Nov 28, 2020

Copy link
Copy Markdown
Contributor

Thanks!

8vasu commented Nov 28, 2020

Copy link
Copy Markdown
Contributor Author

You're welcome!

8vasu commented Nov 29, 2020

Copy link
Copy Markdown
Contributor Author

@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 ].

8vasu mentioned this pull request Nov 29, 2020
Closed
This was referenced Dec 8, 2020
Merged
Closed
adorilson pushed a commit to adorilson/cpython that referenced this pull request Mar 13, 2021
bpo-41818: Fix test_master_read() so that it succeeds on all platform…
1779aa3 
…s that either raise OSError or return b"" upon reading from master (pythonGH-23536)

Signed-off-by: Soumendra Ganguly <soumendraganguly@gmail.com>
8vasu mentioned this pull request Nov 20, 2021
Merged
8vasu mannequin mentioned this pull request May 5, 2022
Open
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

asvetlov asvetlov approved these changes
+1 more reviewer
kulikjak kulikjak approved these changes
Reviewers whose approvals may not affect merge requirements

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.

5 participants

Back | FazBrowse Home | New Git URL