bpo-31158: Fix nondeterministic read in test_pty#3808
bpo-31158: Fix nondeterministic read in test_pty#3808vstinner merged 3 commits intopython:masterfrom
Conversation
|
Why not use with io.FileIO(master_fd) as master:
...
s1 = master.readline()
... |
|
Thanks @pitrou for the suggestion. The following also works on my machine: diff --git a/Lib/test/test_pty.py b/Lib/test/test_pty.py
index f283e19..ee9b589 100644
--- a/Lib/test/test_pty.py
+++ b/Lib/test/test_pty.py
@@ -10,6 +10,7 @@ import sys
import select
import signal
import socket
+import io
import unittest
TEST_STRING_1 = b"I wish to buy a fish license.\n"
@@ -92,17 +93,18 @@ class PtyTest(unittest.TestCase):
# Restore the original flags.
os.set_blocking(master_fd, blocking)
- debug("Writing to slave_fd")
- os.write(slave_fd, TEST_STRING_1)
- s1 = os.read(master_fd, 1024)
- self.assertEqual(b'I wish to buy a fish license.\n',
- normalize_output(s1))
-
- debug("Writing chunked output")
- os.write(slave_fd, TEST_STRING_2[:5])
- os.write(slave_fd, TEST_STRING_2[5:])
- s2 = os.read(master_fd, 1024)
- self.assertEqual(b'For my pet fish, Eric.\n', normalize_output(s2))
+ with io.FileIO(master_fd, mode='rb', closefd=False) as master:
+ debug("Writing to slave_fd")
+ os.write(slave_fd, TEST_STRING_1)
+ s1 = master.readline()
+ self.assertEqual(b'I wish to buy a fish license.\n',
+ normalize_output(s1))
+
+ debug("Writing chunked output")
+ os.write(slave_fd, TEST_STRING_2[:5])
+ os.write(slave_fd, TEST_STRING_2[5:])
+ s2 = master.readline()
+ self.assertEqual(b'For my pet fish, Eric.\n', normalize_output(s2))
os.close(slave_fd)
os.close(master_fd)Which version is better? Problem in version 1:
Problem in version 2:
|
|
I hope we now have the best of both worlds. |
|
|
||
| return data | ||
|
|
||
| def _readline(fd): |
There was a problem hiding this comment.
I'm not sure you still need a helper function. Just use the FileIO object directly from the test method.
Also, if you remove closefd=False, you don't need the os.close(master_fd) at the end.
There was a problem hiding this comment.
The idea of keeping the helper function was to have it close to the comment I added above. And the comment is both about _readline and normalize_output.
We have os.close(slave_fd). I think it would be very asymmetric if we don't close(master_fd) as well in the code explicitly?
Ideally, this commit is fixuped into the previous commit. Since there is already a comment on github, I won't rebase.
|
GH-3852 is a backport of this pull request to the 3.6 branch. |
|
While the test is not perfect, I don't think that we need perfect quality here. I merged the PR to fix this very old bug: test_pty.test_basic() was failing randomly since 1 or 2 years! (well, probably since the test was written) |
|
Oh. I should have fixed the commit message. I just hate the UI for merge a PR. I always miss the commit message edit area. |
|
I backported the change manually to 2.7: PR #3853. |
Sometimes, the test suite fails in test_pty.test_basic()
https://bugs.python.org/issue31158
Acks to @Haypo !!
https://bugs.python.org/issue31158