tests: Check and/or suppress stderr of subprocesses, reduce shell=True uses

Author: moreati

.ci/ci_lib.py

@@ -2,13 +2,15 @@
 from __future__ import print_function
 
 import atexit
+import errno
 import os
 import shlex
 import shutil
-import subprocess
 import sys
 import tempfile
 
+import subprocess32 as subprocess
+
 try:
     import urlparse
 except ImportError:
@@ -30,40 +32,30 @@ def print(*args, **kwargs):
         file.flush()
 
 
-#
-# check_output() monkeypatch cutpasted from testlib.py
-#
-
-def subprocess__check_output(*popenargs, **kwargs):
-    # Missing from 2.6.
-    process = subprocess.Popen(stdout=subprocess.PIPE, *popenargs, **kwargs)
-    output, _ = process.communicate()
-    retcode = process.poll()
-    if retcode:
-        cmd = kwargs.get("args")
-        if cmd is None:
-            cmd = popenargs[0]
-        raise subprocess.CalledProcessError(retcode, cmd)
-    return output
-
-if not hasattr(subprocess, 'check_output'):
-    subprocess.check_output = subprocess__check_output
-
+def _have_cmd(args):
+    try:
+        subprocess.run(
+            args, stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL,
+        )
+    except OSError as exc:
+        if exc.errno == errno.ENOENT:
+            return False
+        raise
+    except subprocess.CallProcessError:
+        return False
+    return True
 
-# ------------------
 
 def have_apt():
-    proc = subprocess.Popen('apt --help >/dev/null 2>/dev/null', shell=True)
-    return proc.wait() == 0
+    return _have_cmd(['apt', '--help'])
+
 
 def have_brew():
-    proc = subprocess.Popen('brew help >/dev/null 2>/dev/null', shell=True)
-    return proc.wait() == 0
+    return _have_cmd(['brew', 'help'])
 
 
 def have_docker():
-    proc = subprocess.Popen('docker info >/dev/null 2>/dev/null', shell=True)
-    return proc.wait() == 0
+    return _have_cmd(['docker', 'info'])
 
 
 def _argv(s, *args):
@@ -315,7 +307,7 @@ def get_interesting_procs(container_name=None):
         args = ['docker', 'exec', container_name] + args
 
     out = []
-    for line in subprocess__check_output(args).decode().splitlines():
+    for line in subprocess.check_output(args).decode().splitlines():
         ppid, pid, comm, rest = line.split(None, 3)
         if (
             (

tests/fork_test.py

@@ -1,5 +1,6 @@
 import os
 import random
+import subprocess
 import sys
 import unittest
 
@@ -26,15 +27,28 @@
 
 
 def _find_ssl_linux():
-    s = testlib.subprocess__check_output(['ldd', _ssl.__file__])
-    for line in s.decode().splitlines():
+    proc = subprocess.Popen(
+        ['ldd', _ssl.__file__],
+        stdout=subprocess.PIPE, stderr=subprocess.PIPE,
+    )
+    b_stdout, b_stderr = proc.communicate()
+    assert proc.returncode == 0
+    assert b_stderr.decode() == ''
+    for line in b_stdout.decode().splitlines():
         bits = line.split()
         if bits[0].startswith('libssl'):
             return bits[2]
 
+
 def _find_ssl_darwin():
-    s = testlib.subprocess__check_output(['otool', '-l', _ssl.__file__])
-    for line in s.decode().splitlines():
+    proc = subprocess.Popen(
+        ['otool', '-l', _ssl.__file__],
+        stdout=subprocess.PIPE, stderr=subprocess.PIPE,
+    )
+    b_stdout, b_stderr = proc.communicate()
+    assert proc.returncode == 0
+    assert b_stderr.decode() == ''
+    for line in b_stdout.decode().splitlines():
         bits = line.split()
         if bits[0] == 'name' and 'libssl' in bits[1]:
             return bits[1]

tests/requirements.txt

@@ -8,6 +8,7 @@ cffi==1.14.3  # Random pin to try and fix pyparser==2.18 not having effect
 pycparser==2.18  # Last version supporting 2.6.
 pytest-catchlog==1.2.2
 pytest==3.1.2
+subprocess32==3.5.4
 timeoutcontext==1.2.0
 # Fix InsecurePlatformWarning while creating py26 tox environment
 # https://urllib3.readthedocs.io/en/latest/advanced-usage.html#ssl-warnings

tests/responder_test.py

@@ -93,8 +93,10 @@ def test_self_contained_program(self):
         # Ensure a program composed of a single script can be imported
         # successfully.
         args = [sys.executable, testlib.data_path('self_contained_program.py')]
-        output = testlib.subprocess__check_output(args).decode()
-        self.assertEqual(output, "['__main__', 50]\n")
+        proc = subprocess.Popen(args, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+        b_stdout, _ = proc.communicate()
+        self.assertEqual(proc.returncode, 0)
+        self.assertEqual(b_stdout.decode(), "['__main__', 50]\n")
 
 
 class BrokenModulesTest(testlib.TestCase):

tests/testlib.py

@@ -3,16 +3,15 @@
 import os
 import random
 import re
-import signal
 import socket
-import subprocess
 import sys
 import threading
 import time
 import traceback
 import unittest
 
 import psutil
+import subprocess32 as subprocess
 
 import mitogen.core
 import mitogen.fork
@@ -71,30 +70,6 @@ def data_path(suffix):
     return path
 
 
-def subprocess__check_output(*popenargs, **kwargs):
-    # Missing from 2.6.
-    process = subprocess.Popen(stdout=subprocess.PIPE, *popenargs, **kwargs)
-    output, _ = process.communicate()
-    retcode = process.poll()
-    if retcode:
-        cmd = kwargs.get("args")
-        if cmd is None:
-            cmd = popenargs[0]
-        raise subprocess.CalledProcessError(retcode, cmd)
-    return output
-
-
-def Popen__terminate(proc):
-    os.kill(proc.pid, signal.SIGTERM)
-
-
-if hasattr(subprocess, 'check_output'):
-    subprocess__check_output = subprocess.check_output
-
-if hasattr(subprocess.Popen, 'terminate'):
-    Popen__terminate = subprocess.Popen.terminate
-
-
 def threading__thread_is_alive(thread):
     """Return whether the thread is alive (Python version compatibility shim).
 
@@ -457,7 +432,7 @@ def get_docker_host():
 
 class DockerizedSshDaemon(object):
     def _get_container_port(self):
-        s = subprocess__check_output(['docker', 'port', self.container_name])
+        s = subprocess.check_output(['docker', 'port', self.container_name])
         for line in s.decode().splitlines():
             m = self.PORT_RE.match(line)
             if not m:
@@ -472,7 +447,7 @@ def _get_container_port(self):
 
     def start_container(self):
         try:
-            subprocess__check_output(['docker', '--version'])
+            subprocess.check_output(['docker', '--version'])
         except Exception:
             raise unittest.SkipTest('Docker binary is unavailable')
 
@@ -486,7 +461,7 @@ def start_container(self):
             '--name', self.container_name,
             self.image,
         ]
-        subprocess__check_output(args)
+        subprocess.check_output(args)
         self._get_container_port()
 
     def __init__(self, mitogen_test_distro=os.environ.get('MITOGEN_TEST_DISTRO', 'debian9')):
@@ -518,7 +493,7 @@ def wait_for_sshd(self):
     def check_processes(self):
         args = ['docker', 'exec', self.container_name, 'ps', '-o', 'comm=']
         counts = {}
-        for comm in subprocess__check_output(args).decode().splitlines():
+        for comm in subprocess.check_output(args).decode().splitlines():
             comm = comm.strip()
             counts[comm] = counts.get(comm, 0) + 1
 
@@ -533,7 +508,7 @@ def check_processes(self):
 
     def close(self):
         args = ['docker', 'rm', '-f', self.container_name]
-        subprocess__check_output(args)
+        subprocess.check_output(args)
 
 
 class BrokerMixin(object):

tests/unix_test.py

@@ -130,7 +130,8 @@ def _test_simple_server(cls, path):
     def test_simple(self):
         path = mitogen.unix.make_socket_path()
         proc = subprocess.Popen(
-            [sys.executable, __file__, 'ClientTest_server', path]
+            [sys.executable, __file__, 'ClientTest_server', path],
+            stdout=subprocess.PIPE, stderr=subprocess.PIPE,
         )
         try:
             self._test_simple_client(path)
@@ -139,7 +140,9 @@ def test_simple(self):
             mitogen.context_id = 0
             mitogen.parent_id = None
             mitogen.parent_ids = []
-        proc.wait()
+        b_stdout, _ = proc.communicate()
+        self.assertEqual(proc.returncode, 0)
+        self.assertEqual(b_stdout.decode(), '')
 
 
 if __name__ == '__main__':