runtest.py: replace chromium_utils.RunCommand with simpler subprocess calls

BUG=506498

Review URL: https://codereview.chromium.org/1305753002

Cr-Commit-Position: refs/heads/master@{#344732}

3 files changed, 32 insertions, 327 deletions

diff --git a/infra/scripts/legacy/scripts/common/chromium_utils.py b/infra/scripts/legacy/scripts/common/chromium_utils.py
index 9371cb7..3aa1e2b 100644
--- a/infra/scripts/legacy/scripts/common/chromium_utils.py
+++ b/infra/scripts/legacy/scripts/common/chromium_utils.py
@@ -180,296 +180,6 @@ def FindUpward(start_dir, *desired_list):
   return os.path.join(parent, *desired_list)
 
 
-class RunCommandFilter(object):
-  """Class that should be subclassed to provide a filter for RunCommand."""
-  # Method could be a function
-  # pylint: disable=R0201
-
-  def FilterLine(self, a_line):
-    """Called for each line of input.  The \n is included on a_line.  Should
-    return what is to be recorded as the output for this line.  A result of
-    None suppresses the line."""
-    return a_line
-
-  def FilterDone(self, last_bits):
-    """Acts just like FilterLine, but is called with any data collected after
-    the last newline of the command."""
-    return last_bits
-
-
-def RunCommand(command, parser_func=None, filter_obj=None, pipes=None,
-               print_cmd=True, timeout=None, max_time=None, **kwargs):
-  """Runs the command list, printing its output and returning its exit status.
-
-  Prints the given command (which should be a list of one or more strings),
-  then runs it and writes its stdout and stderr to the appropriate file handles.
-
-  If timeout is set, the process will be killed if output is stopped after
-  timeout seconds. If max_time is set, the process will be killed if it runs for
-  more than max_time.
-
-  If parser_func is not given, the subprocess's output is passed to stdout
-  and stderr directly.  If the func is given, each line of the subprocess's
-  stdout/stderr is passed to the func and then written to stdout.
-
-  If filter_obj is given, all output is run through the filter a line
-  at a time before it is written to stdout.
-
-  We do not currently support parsing stdout and stderr independent of
-  each other.  In previous attempts, this led to output ordering issues.
-  By merging them when either needs to be parsed, we avoid those ordering
-  issues completely.
-
-  pipes is a list of commands (also a list) that will receive the output of
-  the intial command. For example, if you want to run "python a | python b | c",
-  the "command" will be set to ['python', 'a'], while pipes will be set to
-  [['python', 'b'],['c']]
-  """
-
-  def TimedFlush(timeout, fh, kill_event):
-    """Flush fh every timeout seconds until kill_event is true."""
-    while True:
-      try:
-        fh.flush()
-      # File handle is closed, exit.
-      except ValueError:
-        break
-      # Wait for kill signal or timeout.
-      if kill_event.wait(timeout):
-        break
-
-  # TODO(all): nsylvain's CommandRunner in buildbot_slave is based on this
-  # method.  Update it when changes are introduced here.
-  def ProcessRead(readfh, writefh, parser_func=None, filter_obj=None,
-                  log_event=None):
-    writefh.flush()
-
-    # Python on Windows writes the buffer only when it reaches 4k.  Ideally
-    # we would flush a minimum of 10 seconds.  However, we only write and
-    # flush no more often than 20 seconds to avoid flooding the master with
-    # network traffic from unbuffered output.
-    kill_event = threading.Event()
-    flush_thread = threading.Thread(
-        target=TimedFlush, args=(20, writefh, kill_event))
-    flush_thread.daemon = True
-    flush_thread.start()
-
-    try:
-      in_byte = readfh.read(1)
-      in_line = cStringIO.StringIO()
-      while in_byte:
-        # Capture all characters except \r.
-        if in_byte != '\r':
-          in_line.write(in_byte)
-
-        # Write and flush on newline.
-        if in_byte == '\n':
-          if log_event:
-            log_event.set()
-          if parser_func:
-            parser_func(in_line.getvalue().strip())
-
-          if filter_obj:
-            filtered_line = filter_obj.FilterLine(in_line.getvalue())
-            if filtered_line is not None:
-              writefh.write(filtered_line)
-          else:
-            writefh.write(in_line.getvalue())
-          in_line = cStringIO.StringIO()
-        in_byte = readfh.read(1)
-
-      if log_event and in_line.getvalue():
-        log_event.set()
-
-      # Write remaining data and flush on EOF.
-      if parser_func:
-        parser_func(in_line.getvalue().strip())
-
-      if filter_obj:
-        if in_line.getvalue():
-          filtered_line = filter_obj.FilterDone(in_line.getvalue())
-          if filtered_line is not None:
-            writefh.write(filtered_line)
-      else:
-        if in_line.getvalue():
-          writefh.write(in_line.getvalue())
-    finally:
-      kill_event.set()
-      flush_thread.join()
-      writefh.flush()
-
-  pipes = pipes or []
-
-  # Print the given command (which should be a list of one or more strings).
-  if print_cmd:
-    print '\n' + subprocess.list2cmdline(command) + '\n',
-    for pipe in pipes:
-      print '     | ' + subprocess.list2cmdline(pipe) + '\n',
-
-  sys.stdout.flush()
-  sys.stderr.flush()
-
-  if not (parser_func or filter_obj or pipes or timeout or max_time):
-    # Run the command.  The stdout and stderr file handles are passed to the
-    # subprocess directly for writing.  No processing happens on the output of
-    # the subprocess.
-    proc = subprocess.Popen(command, stdout=sys.stdout, stderr=sys.stderr,
-                            bufsize=0, **kwargs)
-
-  else:
-    if not (parser_func or filter_obj):
-      filter_obj = RunCommandFilter()
-
-    # Start the initial process.
-    proc = subprocess.Popen(command, stdout=subprocess.PIPE,
-                            stderr=subprocess.STDOUT, bufsize=0, **kwargs)
-    proc_handles = [proc]
-
-    if pipes:
-      pipe_number = 0
-      for pipe in pipes:
-        pipe_number = pipe_number + 1
-        if pipe_number == len(pipes) and not (parser_func or filter_obj):
-          # The last pipe process needs to output to sys.stdout or filter
-          stdout = sys.stdout
-        else:
-          # Output to a pipe, since another pipe is on top of us.
-          stdout = subprocess.PIPE
-        pipe_proc = subprocess.Popen(pipe, stdin=proc_handles[0].stdout,
-                                     stdout=stdout, stderr=subprocess.STDOUT)
-        proc_handles.insert(0, pipe_proc)
-
-      # Allow proc to receive a SIGPIPE if the piped process exits.
-      for handle in proc_handles[1:]:
-        handle.stdout.close()
-
-    log_event = threading.Event()
-
-    # Launch and start the reader thread.
-    thread = threading.Thread(target=ProcessRead,
-                              args=(proc_handles[0].stdout, sys.stdout),
-                              kwargs={'parser_func': parser_func,
-                                      'filter_obj': filter_obj,
-                                      'log_event': log_event})
-
-    kill_lock = threading.Lock()
-
-
-    def term_then_kill(handle, initial_timeout, numtimeouts, interval):
-      def timed_check():
-        for _ in range(numtimeouts):
-          if handle.poll() is not None:
-            return True
-          time.sleep(interval)
-
-      handle.terminate()
-      time.sleep(initial_timeout)
-      timed_check()
-      if handle.poll() is None:
-        handle.kill()
-      timed_check()
-      return handle.poll() is not None
-
-
-    def kill_proc(proc_handles, message=None):
-      with kill_lock:
-        if proc_handles:
-          killed = term_then_kill(proc_handles[0], 0.1, 5, 1)
-
-          if message:
-            print >> sys.stderr, message
-
-          if not killed:
-            print >> sys.stderr, 'could not kill pid %d!' % proc_handles[0].pid
-          else:
-            print >> sys.stderr, 'program finished with exit code %d' % (
-                proc_handles[0].returncode)
-
-          # Prevent other timeouts from double-killing.
-          del proc_handles[:]
-
-    def timeout_func(timeout, proc_handles, log_event, finished_event):
-      while log_event.wait(timeout):
-        log_event.clear()
-        if finished_event.is_set():
-          return
-
-      message = ('command timed out: %d seconds without output, attempting to '
-                 'kill' % timeout)
-      kill_proc(proc_handles, message)
-
-    def maxtimeout_func(timeout, proc_handles, finished_event):
-      if not finished_event.wait(timeout):
-        message = ('command timed out: %d seconds elapsed' % timeout)
-        kill_proc(proc_handles, message)
-
-    timeout_thread = None
-    maxtimeout_thread = None
-    finished_event = threading.Event()
-
-    if timeout:
-      timeout_thread = threading.Thread(target=timeout_func,
-                                        args=(timeout, proc_handles, log_event,
-                                              finished_event))
-      timeout_thread.daemon = True
-    if max_time:
-      maxtimeout_thread = threading.Thread(target=maxtimeout_func,
-                                           args=(max_time, proc_handles,
-                                                 finished_event))
-      maxtimeout_thread.daemon = True
-
-    thread.start()
-    if timeout_thread:
-      timeout_thread.start()
-    if maxtimeout_thread:
-      maxtimeout_thread.start()
-
-    # Wait for the commands to terminate.
-    for handle in proc_handles:
-      handle.wait()
-
-    # Wake up timeout threads.
-    finished_event.set()
-    log_event.set()
-
-    # Wait for the reader thread to complete (implies EOF reached on stdout/
-    # stderr pipes).
-    thread.join()
-
-    # Check whether any of the sub commands has failed.
-    for handle in proc_handles:
-      if handle.returncode:
-        return handle.returncode
-
-  # Wait for the command to terminate.
-  proc.wait()
-  return proc.returncode
-
-
-def GetStatusOutput(command, **kwargs):
-  """Runs the command list, returning its result and output."""
-  proc = subprocess.Popen(command, stdout=subprocess.PIPE,
-                          stderr=subprocess.STDOUT, bufsize=1,
-                          **kwargs)
-  output = proc.communicate()[0]
-  result = proc.returncode
-
-  return (result, output)
-
-
-def GetCommandOutput(command):
-  """Runs the command list, returning its output.
-
-  Run the command and returns its output (stdout and stderr) as a string.
-
-  If the command exits with an error, raises ExternalError.
-  """
-  (result, output) = GetStatusOutput(command)
-  if result:
-    raise ExternalError('%s: %s' % (subprocess.list2cmdline(command), output))
-  return output
-
-
 def convert_json(option, _, value, parser):
   """Provide an OptionParser callback to unmarshal a JSON string."""
   setattr(parser.values, option.dest, json.loads(value))
diff --git a/infra/scripts/legacy/scripts/slave/runtest.py b/infra/scripts/legacy/scripts/slave/runtest.py
index 3e09630..cd6ad75 100755
--- a/infra/scripts/legacy/scripts/slave/runtest.py
+++ b/infra/scripts/legacy/scripts/slave/runtest.py
@@ -94,36 +94,6 @@ def _ShutdownDBus():
     print ' cleared DBUS_SESSION_BUS_ADDRESS environment variable'
 
 
-def _RunGTestCommand(
-    options, command, extra_env, pipes=None):
-  """Runs a test, printing and possibly processing the output.
-
-  Args:
-    options: Options passed for this invocation of runtest.py.
-    command: A list of strings in a command (the command and its arguments).
-    extra_env: A dictionary of extra environment variables to set.
-    pipes: A list of command string lists which the output will be piped to.
-
-  Returns:
-    The process return code.
-  """
-  env = os.environ.copy()
-  if extra_env:
-    print 'Additional test environment:'
-    for k, v in sorted(extra_env.items()):
-      print '  %s=%s' % (k, v)
-  env.update(extra_env or {})
-
-  # Trigger bot mode (test retries, redirection of stdio, possibly faster,
-  # etc.) - using an environment variable instead of command-line flags because
-  # some internal waterfalls run this (_RunGTestCommand) for totally non-gtest
-  # code.
-  # TODO(phajdan.jr): Clean this up when internal waterfalls are fixed.
-  env.update({'CHROMIUM_TEST_LAUNCHER_BOT_MODE': '1'})
-
-  return chromium_utils.RunCommand(command, pipes=pipes, env=env)
-
-
 def _BuildTestBinaryCommand(_build_dir, test_exe_path, options):
   """Builds a command to run a test binary.
 
@@ -307,16 +277,40 @@ def _Main(options, args, extra_env):
           options.test_launcher_summary_output)
       command.append('--test-launcher-summary-output=%s' % json_file_name)
 
-    pipes = []
-    # See the comment in main() regarding offline symbolization.
+    command = _GenerateRunIsolatedCommand(build_dir, test_exe_path, options,
+                                          command)
+
+    env = os.environ.copy()
+    if extra_env:
+      print 'Additional test environment:'
+      for k, v in sorted(extra_env.items()):
+        print '  %s=%s' % (k, v)
+    env.update(extra_env or {})
+
+    # Trigger bot mode (test retries, redirection of stdio, possibly faster,
+    # etc.) - using an environment variable instead of command-line flags
+    # because some internal waterfalls run this for totally non-gtest code.
+    # TODO(phajdan.jr): Clean this up when internal waterfalls are fixed.
+    env.update({'CHROMIUM_TEST_LAUNCHER_BOT_MODE': '1'})
+
     if options.use_symbolization_script:
       symbolize_command = _GetSanitizerSymbolizeCommand(
           strip_path_prefix=options.strip_path_prefix)
-      pipes = [symbolize_command]
 
-    command = _GenerateRunIsolatedCommand(build_dir, test_exe_path, options,
-                                          command)
-    result = _RunGTestCommand(options, command, extra_env, pipes=pipes)
+      command_process = subprocess.Popen(
+          command, env=env, stdout=subprocess.PIPE)
+      symbolize_process = subprocess.Popen(
+          symbolize_command, env=env, stdin=command_process.stdout)
+      command_process.stdout.close()
+
+      command_process.wait()
+      symbolize_process.wait()
+
+      result = command_process.returncode
+      if result == 0:
+        result = symbolize_process.returncode
+    else:
+      result = subprocess.call(command, env=env)
   finally:
     if start_xvfb:
       xvfb.StopVirtualX(slave_name)
diff --git a/infra/scripts/legacy/scripts/slave/slave_utils.py b/infra/scripts/legacy/scripts/slave/slave_utils.py
index 76cf4e3..72a192d 100644
--- a/infra/scripts/legacy/scripts/slave/slave_utils.py
+++ b/infra/scripts/legacy/scripts/slave/slave_utils.py
@@ -10,6 +10,7 @@ import glob
 import os
 import re
 import shutil
+import subprocess
 import sys
 import tempfile
 import time
@@ -212,7 +213,7 @@ def RemoveChromeTemporaryFiles():
   elif chromium_utils.IsMac():
     nstempdir_path = '/usr/local/libexec/nstempdir'
     if os.path.exists(nstempdir_path):
-      ns_temp_dir = chromium_utils.GetCommandOutput([nstempdir_path]).strip()
+      ns_temp_dir = subprocess.check_output([nstempdir_path]).strip()
       if ns_temp_dir:
         LogAndRemoveFiles(ns_temp_dir, kLogRegex)
     for i in ('Chromium', 'Google Chrome'):