Patchwork [1,of,5] tests: proof test-stdio.py against buffer fill-up

login
register
mail settings
Submitter Manuel Jacob
Date July 10, 2020, 4:46 a.m.
Message ID <7c5896871c7e9e8e510c.1594356398@tmp>
Download mbox | patch
Permalink /patch/46680/
State Accepted
Headers show

Comments

Manuel Jacob - July 10, 2020, 4:46 a.m.
# HG changeset patch
# User Manuel Jacob <me@manueljacob.de>
# Date 1594112797 -7200
#      Tue Jul 07 11:06:37 2020 +0200
# Node ID 7c5896871c7e9e8e510c0436f5c2bcc6018e8177
# Parent  f43bc4ce0d697051135ff21080b7794e6aaea8bf
# EXP-Topic stdio
tests: proof test-stdio.py against buffer fill-up

With the previous code, it could in theory happen that the pipe / PTY buffer of
the child stdout / stderr fills up and the process never finishes.

To prevent that, we read all of the stream before waiting for the end of the
process. To ensure that the stream reaches EOF when the child finishes, we must
close the parent "copy" of the child stdout / stderr.

Patch

diff --git a/tests/test-stdio.py b/tests/test-stdio.py
--- a/tests/test-stdio.py
+++ b/tests/test-stdio.py
@@ -5,6 +5,7 @@ 
 from __future__ import absolute_import
 
 import contextlib
+import errno
 import os
 import subprocess
 import sys
@@ -62,6 +63,23 @@ 
         yield rwpair
 
 
+def _readall(fd, buffer_size):
+    buf = []
+    while True:
+        try:
+            s = os.read(fd, buffer_size)
+        except OSError as e:
+            if e.errno == errno.EIO:
+                # If the child-facing PTY got closed, reading from the
+                # parent-facing PTY raises EIO.
+                break
+            raise
+        if not s:
+            break
+        buf.append(s)
+    return b''.join(buf)
+
+
 class TestStdio(unittest.TestCase):
     def _test(self, stream, rwpair_generator, expected_output, python_args=[]):
         assert stream in ('stdout', 'stderr')
@@ -76,9 +94,14 @@ 
                 stdout=child_stream if stream == 'stdout' else None,
                 stderr=child_stream if stream == 'stderr' else None,
             )
-            retcode = proc.wait()
+            try:
+                os.close(child_stream)
+                self.assertEqual(
+                    _readall(stream_receiver, 1024), expected_output
+                )
+            finally:
+                retcode = proc.wait()
             self.assertEqual(retcode, 0)
-            self.assertEqual(os.read(stream_receiver, 1024), expected_output)
 
     def test_stdout_pipes(self):
         self._test('stdout', _pipes, FULLY_BUFFERED)