From c32577295a184d2c8699fb0e7eb6334eb4eead00 Mon Sep 17 00:00:00 2001 From: Alex Willmer Date: Sun, 19 Jun 2022 12:54:35 +0100 Subject: [PATCH] tests: Check and/or suppress stderr of subprocesses, reduce shell=True uses --- .ci/ci_lib.py | 48 +++++++++++++++++------------------------ tests/fork_test.py | 22 +++++++++++++++---- tests/requirements.txt | 1 + tests/responder_test.py | 6 ++++-- tests/testlib.py | 37 ++++++------------------------- tests/unix_test.py | 7 ++++-- 6 files changed, 54 insertions(+), 67 deletions(-) diff --git a/.ci/ci_lib.py b/.ci/ci_lib.py index 000a7a83..2587e859 100644 --- a/.ci/ci_lib.py +++ b/.ci/ci_lib.py @@ -2,13 +2,15 @@ from __future__ import absolute_import 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 ( ( diff --git a/tests/fork_test.py b/tests/fork_test.py index 86e60cfc..35acfe13 100644 --- a/tests/fork_test.py +++ b/tests/fork_test.py @@ -1,5 +1,6 @@ import os import random +import subprocess import sys import unittest @@ -26,15 +27,28 @@ import plain_old_module 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] diff --git a/tests/requirements.txt b/tests/requirements.txt index 319e0f51..9ed07645 100644 --- a/tests/requirements.txt +++ b/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 diff --git a/tests/responder_test.py b/tests/responder_test.py index 3ab60be3..5552be98 100644 --- a/tests/responder_test.py +++ b/tests/responder_test.py @@ -93,8 +93,10 @@ class GoodModulesTest(testlib.RouterMixin, testlib.TestCase): # 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): diff --git a/tests/testlib.py b/tests/testlib.py index 8ab895c4..1e27be9a 100644 --- a/tests/testlib.py +++ b/tests/testlib.py @@ -3,9 +3,7 @@ import logging import os import random import re -import signal import socket -import subprocess import sys import threading import time @@ -13,6 +11,7 @@ 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 @@ class DockerizedSshDaemon(object): 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 @@ class DockerizedSshDaemon(object): '--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 @@ class DockerizedSshDaemon(object): 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 @@ class DockerizedSshDaemon(object): def close(self): args = ['docker', 'rm', '-f', self.container_name] - subprocess__check_output(args) + subprocess.check_output(args) class BrokerMixin(object): diff --git a/tests/unix_test.py b/tests/unix_test.py index aecc62ba..14fc54ae 100644 --- a/tests/unix_test.py +++ b/tests/unix_test.py @@ -130,7 +130,8 @@ class ClientTest(testlib.TestCase): 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 @@ class ClientTest(testlib.TestCase): 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__':