[test] Refactor commands into OS-specific versions

This seperates OS-specific code of the Command class into a Posix and a
WindowsCommand to simplify scattered OS checks.

This also removes some temporary mac debug output that's obsolete after
https://crbug.com/v8/6927 got resolved.

TBR=sergiyb@chromium.org

Bug: v8:6917
Change-Id: Iaab3b527ce556dfba797a164ae58e8dd358eb56f
Reviewed-on: https://chromium-review.googlesource.com/847000Reviewed-by: Michael Achenbach <machenbach@chromium.org>
Commit-Queue: Michael Achenbach <machenbach@chromium.org>
Cr-Commit-Position: refs/heads/master@{#50346}

tools/testrunner/local/command.py

@@ -3,7 +3,6 @@
 # found in the LICENSE file.
 
 
-from contextlib import contextmanager
 import os
 import subprocess
 import sys
@@ -14,38 +13,11 @@ from ..local import utils
 from ..objects import output
 
 
-@contextmanager
-def win_error_mode():
-  """ Try to change the error mode to avoid dialogs on fatal errors. Don't
-  touch any existing error mode flags by merging the existing error mode.
-  See http://blogs.msdn.com/oldnewthing/archive/2004/07/27/198410.aspx.
-  """
+SEM_INVALID_VALUE = -1
+SEM_NOGPFAULTERRORBOX = 0x0002  # Microsoft Platform SDK WinBase.h
 
-  def set_error_mode(mode):
-    prev_error_mode = SEM_INVALID_VALUE
-    try:
-      import ctypes
-      prev_error_mode = (
-          ctypes.windll.kernel32.SetErrorMode(mode))  #@UndefinedVariable
-    except ImportError:
-      pass
-    return prev_error_mode
-
-  SEM_INVALID_VALUE = -1
-  SEM_NOGPFAULTERRORBOX = 0x0002  # Microsoft Platform SDK WinBase.h
-
-  if utils.IsWindows():
-    error_mode = SEM_NOGPFAULTERRORBOX
-    prev_error_mode = set_error_mode(error_mode)
-    set_error_mode(error_mode | prev_error_mode)
-
-  yield
-
-  if utils.IsWindows() and prev_error_mode != SEM_INVALID_VALUE:
-    set_error_mode(prev_error_mode)
 
-
-class Command(object):
+class BaseCommand(object):
   def __init__(self, shell, args=None, cmd_prefix=None, timeout=60, env=None,
                verbose=False):
     assert(timeout > 0)
@@ -61,23 +33,12 @@ class Command(object):
     if self.verbose:
       print '# %s' % self
 
-    with win_error_mode():
-      try:
-        process = subprocess.Popen(
-          args=self._get_popen_args(),
-          stdout=subprocess.PIPE,
-          stderr=subprocess.PIPE,
-          env=self._get_env(),
-          **additional_popen_kwargs
-        )
-      except Exception as e:
-        sys.stderr.write('Error executing: %s\n' % self)
-        raise e
+    process = self._start_process(**additional_popen_kwargs)
 
     # Variable to communicate with the timer.
     timeout_occured = [False]
     timer = threading.Timer(
-        self.timeout, self._kill_process, [process, timeout_occured])
+        self.timeout, self._on_timeout, [process, timeout_occured])
     timer.start()
 
     start_time = time.time()
@@ -95,11 +56,21 @@ class Command(object):
       duration
     )
 
+  def _start_process(self, **additional_popen_kwargs):
+    try:
+      return subprocess.Popen(
+        args=self._get_popen_args(),
+        stdout=subprocess.PIPE,
+        stderr=subprocess.PIPE,
+        env=self._get_env(),
+        **additional_popen_kwargs
+      )
+    except Exception as e:
+      sys.stderr.write('Error executing: %s\n' % self)
+      raise e
+
   def _get_popen_args(self):
-    args = self._to_args_list()
-    if utils.IsWindows():
-      return subprocess.list2cmdline(args)
-    return args
+    return self._to_args_list()
 
   def _get_env(self):
     env = os.environ.copy()
@@ -111,55 +82,16 @@ class Command(object):
     env.pop('GTEST_SHARD_INDEX', None)
     return env
 
-  def _kill_process(self, process, timeout_occured):
+  def _kill_process(self, process):
+    raise NotImplementedError()
+
+  def _on_timeout(self, process, timeout_occured):
     timeout_occured[0] = True
     try:
-      if utils.IsWindows():
-        self._kill_process_windows(process)
-      else:
-        self._kill_process_posix(process)
-
+      self._kill_process(process)
     except OSError:
       sys.stderr.write('Error: Process %s already ended.\n' % process.pid)
 
-  def _kill_process_windows(self, process):
-    if self.verbose:
-      print 'Attempting to kill process %d' % process.pid
-      sys.stdout.flush()
-    tk = subprocess.Popen(
-        'taskkill /T /F /PID %d' % process.pid,
-        stdout=subprocess.PIPE,
-        stderr=subprocess.PIPE,
-    )
-    stdout, stderr = tk.communicate()
-    if self.verbose:
-      print 'Taskkill results for %d' % process.pid
-      print stdout
-      print stderr
-      print 'Return code: %d' % tk.returncode
-      sys.stdout.flush()
-
-  def _kill_process_posix(self, process):
-    if utils.GuessOS() == 'macos':
-      # TODO(machenbach): Temporary output for investigating hanging test
-      # driver on mac.
-      print 'Attempting to kill process %d - cmd %s' % (process.pid, self)
-      try:
-        print subprocess.check_output(
-          'ps -e | egrep "d8|cctest|unittests"', shell=True)
-      except Exception:
-        pass
-      sys.stdout.flush()
-
-    process.kill()
-
-    if utils.GuessOS() == 'macos':
-      # TODO(machenbach): Temporary output for investigating hanging test
-      # driver on mac. This will probably not print much, since kill only
-      # sends the signal.
-      print 'Return code after signalling the kill: %s' % process.returncode
-      sys.stdout.flush()
-
   def __str__(self):
     return self.to_string()
 
@@ -179,3 +111,61 @@ class Command(object):
 
   def _to_args_list(self):
     return self.cmd_prefix + [self.shell] + self.args
+
+
+class PosixCommand(BaseCommand):
+  def _kill_process(self, process):
+    process.kill()
+
+
+class WindowsCommand(BaseCommand):
+  def _start_process(self, **kwargs):
+    # Try to change the error mode to avoid dialogs on fatal errors. Don't
+    # touch any existing error mode flags by merging the existing error mode.
+    # See http://blogs.msdn.com/oldnewthing/archive/2004/07/27/198410.aspx.
+    def set_error_mode(mode):
+      prev_error_mode = SEM_INVALID_VALUE
+      try:
+        import ctypes
+        prev_error_mode = (
+            ctypes.windll.kernel32.SetErrorMode(mode))  #@UndefinedVariable
+      except ImportError:
+        pass
+      return prev_error_mode
+
+    error_mode = SEM_NOGPFAULTERRORBOX
+    prev_error_mode = set_error_mode(error_mode)
+    set_error_mode(error_mode | prev_error_mode)
+
+    try:
+      return super(WindowsCommand, self)._start_process(**kwargs)
+    finally:
+      if prev_error_mode != SEM_INVALID_VALUE:
+        set_error_mode(prev_error_mode)
+
+  def _get_popen_args(self):
+    return subprocess.list2cmdline(self._to_args_list())
+
+  def _kill_process(self, process):
+    if self.verbose:
+      print 'Attempting to kill process %d' % process.pid
+      sys.stdout.flush()
+    tk = subprocess.Popen(
+        'taskkill /T /F /PID %d' % process.pid,
+        stdout=subprocess.PIPE,
+        stderr=subprocess.PIPE,
+    )
+    stdout, stderr = tk.communicate()
+    if self.verbose:
+      print 'Taskkill results for %d' % process.pid
+      print stdout
+      print stderr
+      print 'Return code: %d' % tk.returncode
+      sys.stdout.flush()
+
+
+# Set the Command class to the OS-specific version.
+if utils.IsWindows():
+  Command = WindowsCommand
+else:
+  Command = PosixCommand