bpo-31158: Fix nondeterministic read in test_pty

[diekmann]

Sometimes, the test suite fails in test_pty.test_basic()

https://bugs.python.org/issue31158

Acks to @Haypo !!

https://bugs.python.org/issue31158

[pitrou]

Why not use FileIO instead? Something like:

with io.FileIO(master_fd) as master:
    ...
    s1 = master.readline()
    ...

[diekmann]

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:

• Re-implementing readline

Problem in version 2:

• Depends on the io module, while the pty module usually only uses the raw os.read and os.write calls. A failure could make debugging harder (is it the pty module or a change in the io module?).
• I don't like that we have both, master and master_fd hanging around

[diekmann]

I hope we now have the best of both worlds.

[pitrou]

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.

[diekmann]

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?

[miss-islington]

Thanks @diekmann for the PR, and @Haypo for merging it 🌮🎉.. I'm working now to backport this PR to: 3.6.
🐍🍒⛏🤖

[bedevere-bot]

GH-3852 is a backport of this pull request to the 3.6 branch.

[vstinner]

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)

[vstinner]

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.

[vstinner]

I backported the change manually to 2.7: PR #3853.