Revert "[test] Creating command before execution phase."

This reverts commit 98cc9e86.

Reason for revert: Breaks test isolation:
https://build.chromium.org/p/client.v8/builders/V8%20Linux%20-%20builder/builds/29746

Original change's description:
> [test] Creating command before execution phase.
> 
> Immutable command class with shell, flags and
> environment.
> 
> Command creation moved from worker to the main
> process. Because of that there is no need to send
> test cases beyond process boundaries and load test
> suites in worker processes.
> 
> Bug: v8:6917
> Change-Id: Ib6a44278095b4f7141eb9b96802fe3e8117678a6
> Reviewed-on: https://chromium-review.googlesource.com/791710
> Commit-Queue: Michał Majewski <majeski@google.com>
> Reviewed-by: Michael Achenbach <machenbach@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#49746}

TBR=machenbach@chromium.org,sergiyb@chromium.org,majeski@google.com

Change-Id: I44b99468d18fd093833f4185dad067a9eeaf2bc1
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: v8:6917
Reviewed-on: https://chromium-review.googlesource.com/800292Reviewed-by: Michael Achenbach <machenbach@chromium.org>
Commit-Queue: Michael Achenbach <machenbach@chromium.org>
Cr-Commit-Position: refs/heads/master@{#49747}

test/cctest/testcfg.py

@@ -28,7 +28,7 @@
 import os
 import shutil
 
-from testrunner.local import command
+from testrunner.local import commands
 from testrunner.local import testsuite
 from testrunner.local import utils
 from testrunner.objects import testcase
@@ -37,17 +37,21 @@ from testrunner.objects import testcase
 class CcTestSuite(testsuite.TestSuite):
   SHELL = 'cctest'
 
+  def __init__(self, name, root):
+    super(CcTestSuite, self).__init__(name, root)
+    if utils.IsWindows():
+      build_dir = "build"
+    else:
+      build_dir = "out"
+
   def ListTests(self, context):
     shell = os.path.abspath(os.path.join(context.shell_dir, self.SHELL))
     if utils.IsWindows():
       shell += ".exe"
-    cmd = command.Command(
-        cmd_prefix=context.command_prefix,
-        shell=shell,
-        args=["--list"] + context.extra_flags)
-    output = cmd.execute()
+    cmd = context.command_prefix + [shell, "--list"] + context.extra_flags
+    output = commands.Execute(cmd)
     if output.exit_code != 0:
-      print cmd
+      print ' '.join(cmd)
       print output.stdout
       print output.stderr
       return []

tools/perf-compare.py

@@ -12,6 +12,7 @@ Examples:
 '''
 
 from collections import OrderedDict
+import commands
 import json
 import math
 from argparse import ArgumentParser

tools/perf-to-html.py

@@ -12,6 +12,7 @@ from standard input or via the --filename option. Examples:
   %prog -f results.json -t "ia32 results" -o results.html
 '''
 
+import commands
 import json
 import math
 from optparse import OptionParser

tools/run_perf.py

@@ -106,7 +106,7 @@ import re
 import subprocess
 import sys
 
-from testrunner.local import command
+from testrunner.local import commands
 from testrunner.local import utils
 
 ARCH_GUESS = utils.DefaultArch()
@@ -493,23 +493,15 @@ class RunnableConfig(GraphConfig):
     suffix = ["--"] + self.test_flags if self.test_flags else []
     return self.flags + (extra_flags or []) + [self.main] + suffix
 
-  def GetCommand(self, cmd_prefix, shell_dir, extra_flags=None):
+  def GetCommand(self, shell_dir, extra_flags=None):
     # TODO(machenbach): This requires +.exe if run on windows.
     extra_flags = extra_flags or []
+    cmd = [os.path.join(shell_dir, self.binary)]
+    if self.binary.endswith(".py"):
+      cmd = [sys.executable] + cmd
     if self.binary != 'd8' and '--prof' in extra_flags:
       print "Profiler supported only on a benchmark run with d8"
-
-    if self.process_size:
-      cmd_prefix = ["/usr/bin/time", "--format=MaxMemory: %MKB"] + cmd_prefix
-    if self.binary.endswith('.py'):
-      # Copy cmd_prefix instead of update (+=).
-      cmd_prefix = cmd_prefix + [sys.executable]
-
-    return command.Command(
-        cmd_prefix=cmd_prefix,
-        shell=os.path.join(shell_dir, self.binary),
-        args=self.GetCommandFlags(extra_flags=extra_flags),
-        timeout=self.timeout or 60)
+    return cmd + self.GetCommandFlags(extra_flags=extra_flags)
 
   def Run(self, runner, trybot):
     """Iterates over several runs and handles the output for all traces."""
@@ -685,9 +677,18 @@ class DesktopPlatform(Platform):
     suffix = ' - secondary' if secondary else ''
     shell_dir = self.shell_dir_secondary if secondary else self.shell_dir
     title = ">>> %%s (#%d)%s:" % ((count + 1), suffix)
-    cmd = runnable.GetCommand(self.command_prefix, shell_dir, self.extra_flags)
+    if runnable.process_size:
+      command = ["/usr/bin/time", "--format=MaxMemory: %MKB"]
+    else:
+      command = []
+
+    command += self.command_prefix + runnable.GetCommand(shell_dir,
+                                                        self.extra_flags)
     try:
-      output = cmd.execute()
+      output = commands.Execute(
+        command,
+        timeout=runnable.timeout,
+      )
     except OSError as e:  # pragma: no cover
       print title % "OSError"
       print e

tools/testrunner/local/command.py

-# Copyright 2017 the V8 project authors. All rights reserved.
-# Use of this source code is governed by a BSD-style license that can be
-# found in the LICENSE file.
-
-
-from contextlib import contextmanager
-import os
-import subprocess
-import sys
-import threading
-
-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.
-  """
-
-  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):
-  def __init__(self, shell, args=None, cmd_prefix=None, timeout=60, env=None,
-               verbose=False):
-    assert(timeout > 0)
-
-    self.shell = shell
-    self.args = args or []
-    self.cmd_prefix = cmd_prefix or []
-    self.timeout = timeout
-    self.env = env or {}
-    self.verbose = verbose
-
-  def execute(self, **additional_popen_kwargs):
-    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
-
-    # Variable to communicate with the timer.
-    timeout_occured = [False]
-    timer = threading.Timer(
-        self.timeout, self._kill_process, [process, timeout_occured])
-    timer.start()
-
-    stdout, stderr = process.communicate()
-
-    timer.cancel()
-
-    return output.Output(
-      process.returncode,
-      timeout_occured[0],
-      stdout.decode('utf-8', 'replace').encode('utf-8'),
-      stderr.decode('utf-8', 'replace').encode('utf-8'),
-      process.pid,
-    )
-
-  def _get_popen_args(self):
-    args = self._to_args_list()
-    if utils.IsWindows():
-      return subprocess.list2cmdline(args)
-    return args
-
-  def _get_env(self):
-    env = os.environ.copy()
-    env.update(self.env)
-    # GTest shard information is read by the V8 tests runner. Make sure it
-    # doesn't leak into the execution of gtests we're wrapping. Those might
-    # otherwise apply a second level of sharding and as a result skip tests.
-    env.pop('GTEST_TOTAL_SHARDS', None)
-    env.pop('GTEST_SHARD_INDEX', None)
-    return env
-
-  def _kill_process(self, process, timeout_occured):
-    timeout_occured[0] = True
-    try:
-      if utils.IsWindows():
-        self._kill_process_windows(process)
-      else:
-        self._kill_process_posix(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()
-
-  def to_string(self, relative=False):
-    def escape(part):
-      # Escape spaces. We may need to escape more characters for this to work
-      # properly.
-      if ' ' in part:
-        return '"%s"' % part
-      return part
-
-    parts = map(escape, self._to_args_list())
-    cmd = ' '.join(parts)
-    if relative:
-      cmd = cmd.replace(os.getcwd() + os.sep, '')
-    return cmd
-
-  def _to_args_list(self):
-    return self.cmd_prefix + [self.shell] + self.args

tools/testrunner/local/commands.py

+# Copyright 2012 the V8 project authors. All rights reserved.
+# Redistribution and use in source and binary forms, with or without
+# modification, are permitted provided that the following conditions are
+# met:
+#
+#     * Redistributions of source code must retain the above copyright
+#       notice, this list of conditions and the following disclaimer.
+#     * Redistributions in binary form must reproduce the above
+#       copyright notice, this list of conditions and the following
+#       disclaimer in the documentation and/or other materials provided
+#       with the distribution.
+#     * Neither the name of Google Inc. nor the names of its
+#       contributors may be used to endorse or promote products derived
+#       from this software without specific prior written permission.
+#
+# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+
+import os
+import subprocess
+import sys
+from threading import Timer
+
+from ..local import utils
+from ..objects import output
+
+
+SEM_INVALID_VALUE = -1
+SEM_NOGPFAULTERRORBOX = 0x0002  # Microsoft Platform SDK WinBase.h
+
+
+def Win32SetErrorMode(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
+
+
+def RunProcess(verbose, timeout, args, additional_env, **rest):
+  if verbose: print "#", " ".join(args)
+  popen_args = args
+  prev_error_mode = SEM_INVALID_VALUE
+  if utils.IsWindows():
+    popen_args = subprocess.list2cmdline(args)
+    # 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.
+    error_mode = SEM_NOGPFAULTERRORBOX
+    prev_error_mode = Win32SetErrorMode(error_mode)
+    Win32SetErrorMode(error_mode | prev_error_mode)
+
+  env = os.environ.copy()
+  env.update(additional_env)
+  # GTest shard information is read by the V8 tests runner. Make sure it
+  # doesn't leak into the execution of gtests we're wrapping. Those might
+  # otherwise apply a second level of sharding and as a result skip tests.
+  env.pop('GTEST_TOTAL_SHARDS', None)
+  env.pop('GTEST_SHARD_INDEX', None)
+
+  try:
+    process = subprocess.Popen(
+      args=popen_args,
+      stdout=subprocess.PIPE,
+      stderr=subprocess.PIPE,
+      env=env,
+      **rest
+    )
+  except Exception as e:
+    sys.stderr.write("Error executing: %s\n" % popen_args)
+    raise e
+
+  if (utils.IsWindows() and prev_error_mode != SEM_INVALID_VALUE):
+    Win32SetErrorMode(prev_error_mode)
+
+  def kill_process(process, timeout_result):
+    timeout_result[0] = True
+    try:
+      if utils.IsWindows():
+        if 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 verbose:
+          print "Taskkill results for %d" % process.pid
+          print stdout
+          print stderr
+          print "Return code: %d" % tk.returncode
+          sys.stdout.flush()
+      else:
+        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, args)
+          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()
+
+    except OSError:
+      sys.stderr.write('Error: Process %s already ended.\n' % process.pid)
+
+  # Pseudo object to communicate with timer thread.
+  timeout_result = [False]
+
+  timer = Timer(timeout, kill_process, [process, timeout_result])
+  timer.start()
+  stdout, stderr = process.communicate()
+  timer.cancel()
+
+  return output.Output(
+      process.returncode,
+      timeout_result[0],
+      stdout.decode('utf-8', 'replace').encode('utf-8'),
+      stderr.decode('utf-8', 'replace').encode('utf-8'),
+      process.pid,
+  )
+
+
+# TODO(machenbach): Instead of passing args around, we should introduce an
+# immutable Command class (that just represents the command with all flags and
+# is pretty-printable) and a member method for running such a command.
+def Execute(args, verbose=False, timeout=None, env=None):
+  args = [ c for c in args if c != "" ]
+  return RunProcess(verbose, timeout, args, env or {})

tools/testrunner/local/execution.py

@@ -34,9 +34,10 @@ import sys
 import time
 
 from pool import Pool
-from . import command
+from . import commands
 from . import perfdata
 from . import statusfile
+from . import testsuite
 from . import utils
 from ..objects import output
 
@@ -47,18 +48,76 @@ BASE_DIR = os.path.dirname(os.path.dirname(os.path.dirname(os.path.dirname(
 TEST_DIR = os.path.join(BASE_DIR, "test")
 
 
+class Instructions(object):
+  def __init__(self, command, test_id, timeout, verbose, env):
+    self.command = command
+    self.id = test_id
+    self.timeout = timeout
+    self.verbose = verbose
+    self.env = env
+
+
 # Structure that keeps global information per worker process.
 ProcessContext = collections.namedtuple(
-    'process_context', ['sancov_dir'])
+    "process_context", ["suites", "context"])
 
 
-def MakeProcessContext(sancov_dir):
-  return ProcessContext(sancov_dir)
+def MakeProcessContext(context, suite_names):
+  """Generate a process-local context.
 
-# Global function for multiprocessing, because pickling a static method doesn't
-# work on Windows.
-def run_job(job, process_context):
-  return job.run(process_context)
+  This reloads all suites per process and stores the global context.
+
+  Args:
+    context: The global context from the test runner.
+    suite_names (list of str): Suite names as loaded by the parent process.
+        Load the same suites in each subprocess.
+  """
+  suites = {}
+  for root in suite_names:
+    # Don't reinitialize global state as this is concurrently called from
+    # different processes.
+    suite = testsuite.TestSuite.LoadTestSuite(
+        os.path.join(TEST_DIR, root), global_init=False)
+    if suite:
+      suites[suite.name] = suite
+  return ProcessContext(suites, context)
+
+
+def GetCommand(test, context):
+  d8testflag = []
+  shell = test.suite.GetShellForTestCase(test)
+  if shell == "d8":
+    d8testflag = ["--test"]
+  if utils.IsWindows():
+    shell += ".exe"
+  if context.random_seed:
+    d8testflag += ["--random-seed=%s" % context.random_seed]
+  files, flags, env = test.suite.GetParametersForTestCase(test, context)
+  cmd = (
+      context.command_prefix +
+      [os.path.abspath(os.path.join(context.shell_dir, shell))] +
+      d8testflag +
+      files +
+      context.extra_flags +
+      # Flags from test cases can overwrite extra cmd-line flags.
+      flags
+  )
+  return cmd, env
+
+
+def _GetInstructions(test, context):
+  command, env = GetCommand(test, context)
+  timeout = context.timeout
+  if ("--stress-opt" in test.flags or
+      "--stress-opt" in context.mode_flags or
+      "--stress-opt" in context.extra_flags):
+    timeout *= 4
+  if "--noenable-vfp3" in context.extra_flags:
+    timeout *= 2
+
+  # TODO(majeski): make it slow outcome dependent.
+  timeout *= 2
+  return Instructions(command, test.id, timeout, context.verbose, env)
 
 
 class Job(object):
@@ -67,17 +126,31 @@ class Job(object):
   All contained fields will be pickled/unpickled.
   """
 
-  def run(self, process_context):
+  def Run(self, process_context):
+    """Executes the job.
+
+    Args:
+      process_context: Process-local information that is initialized by the
+                       executing worker.
+    """
     raise NotImplementedError()
 
 
+def SetupProblem(exception, test):
+  stderr = ">>> EXCEPTION: %s\n" % exception
+  match = re.match(r"^.*No such file or directory: '(.*)'$", str(exception))
+  if match:
+    # Extra debuging information when files are claimed missing.
+    f = match.group(1)
+    stderr += ">>> File %s exists? -> %s\n" % (f, os.path.exists(f))
+  return test.id, output.Output(1, False, "", stderr, None), 0
+
+
 class TestJob(Job):
-  def __init__(self, test_id, cmd, run_num):
-    self.test_id = test_id
-    self.cmd = cmd
-    self.run_num = run_num
+  def __init__(self, test):
+    self.test = test
 
-  def _rename_coverage_data(self, out, sancov_dir):
+  def _rename_coverage_data(self, output, context):
     """Rename coverage data.
 
     Rename files with PIDs to files with unique test IDs, because the number
@@ -86,27 +159,41 @@ class TestJob(Job):
     42 is the test ID and 1 is the attempt (the same test might be rerun on
     failures).
     """
-    if sancov_dir and out.pid is not None:
-      # Doesn't work on windows so basename is sufficient to get the shell name.
-      shell = os.path.basename(self.cmd.shell)
-      sancov_file = os.path.join(sancov_dir, "%s.%d.sancov" % (shell, out.pid))
+    if context.sancov_dir and output.pid is not None:
+      shell = self.test.suite.GetShellForTestCase(self.test)
+      sancov_file = os.path.join(
+          context.sancov_dir, "%s.%d.sancov" % (shell, output.pid))
 
       # Some tests are expected to fail and don't produce coverage data.
       if os.path.exists(sancov_file):
         parts = sancov_file.split(".")
         new_sancov_file = ".".join(
             parts[:-2] +
-            ["test", str(self.test_id), str(self.run_num)] +
+            ["test", str(self.test.id), str(self.test.run)] +
             parts[-1:]
         )
         assert not os.path.exists(new_sancov_file)
         os.rename(sancov_file, new_sancov_file)
 
-  def run(self, context):
+  def Run(self, process_context):
+    try:
+      # Retrieve a new suite object on the worker-process side. The original
+      # suite object isn't pickled.
+      self.test.SetSuiteObject(process_context.suites)
+      instr = _GetInstructions(self.test, process_context.context)
+    except Exception, e:
+      # TODO(majeski): Better exception reporting.
+      return SetupProblem(e, self.test)
+
     start_time = time.time()
-    out = self.cmd.execute()
-    self._rename_coverage_data(out, context.sancov_dir)
-    return (self.test_id, out, time.time() - start_time)
+    output = commands.Execute(instr.command, instr.verbose, instr.timeout,
+                              instr.env)
+    self._rename_coverage_data(output, process_context.context)
+    return (instr.id, output, time.time() - start_time)
+
+
+def RunTest(job, process_context):
+  return job.Run(process_context)
 
 
 class Runner(object):
@@ -175,7 +262,7 @@ class Runner(object):
       test.duration = None
       test.output = None
       test.run += 1
-      pool.add([TestJob(test.id, test.cmd, test.run)])
+      pool.add([TestJob(test)])
       self.remaining += 1
       self.total += 1
 
@@ -240,7 +327,7 @@ class Runner(object):
       # remember the output for comparison.
       test.run += 1
       test.output = result[1]
-      pool.add([TestJob(test.id, test.cmd, test.run)])
+      pool.add([TestJob(test)])
     # Always update the perf database.
     return True
 
@@ -263,7 +350,7 @@ class Runner(object):
         assert test.id >= 0
         test_map[test.id] = test
         try:
-          yield [TestJob(test.id, test.cmd, test.run)]
+          yield [TestJob(test)]
         except Exception, e:
           # If this failed, save the exception and re-raise it later (after
           # all other tests have had a chance to run).
@@ -271,10 +358,10 @@ class Runner(object):
           continue
     try:
       it = pool.imap_unordered(
-          fn=run_job,
+          fn=RunTest,
           gen=gen_tests(),
           process_context_fn=MakeProcessContext,
-          process_context_args=[self.context.sancov_dir],
+          process_context_args=[self.context, self.suite_names],
       )
       for result in it:
         if result.heartbeat:
@@ -291,7 +378,7 @@ class Runner(object):
       self._VerbosePrint("Closing process pool.")
       pool.terminate()
       self._VerbosePrint("Closing database connection.")
-      self._RunPerfSafe(self.perf_data_manager.close)
+      self._RunPerfSafe(lambda: self.perf_data_manager.close())
       if self.perf_failures:
         # Nuke perf data in case of failures. This might not work on windows as
         # some files might still be open.
@@ -316,8 +403,6 @@ class Runner(object):
 
 class BreakNowException(Exception):
   def __init__(self, value):
-    super(BreakNowException, self).__init__()
     self.value = value
-
   def __str__(self):
     return repr(self.value)

tools/testrunner/local/pool.py

@@ -120,8 +120,8 @@ class Pool():
                                          self.done,
                                          process_context_fn,
                                          process_context_args))
-        p.start()
         self.processes.append(p)
+        p.start()
 
       self.advance(gen)
       while self.count > 0:
@@ -145,11 +145,6 @@ class Pool():
         else:
           yield MaybeResult.create_result(result.result)
         self.advance(gen)
-    except KeyboardInterrupt:
-      raise
-    except Exception as e:
-      traceback.print_exc()
-      print(">>> EXCEPTION: %s" % e)
     finally:
       self.terminate()
     if internal_error:

tools/testrunner/local/progress.py

@@ -32,10 +32,14 @@ import os
 import sys
 import time
 
+from . import execution
 from . import junit_output
 from . import statusfile
 
 
+ABS_PATH_PREFIX = os.getcwd() + os.sep
+
+
 class ProgressIndicator(object):
 
   def __init__(self):
@@ -66,6 +70,18 @@ class ProgressIndicator(object):
       'negative': negative_marker
     }
 
+  def _EscapeCommand(self, test):
+    command, _ = execution.GetCommand(test, self.runner.context)
+    parts = []
+    for part in command:
+      if ' ' in part:
+        # Escape spaces.  We may need to escape more characters for this
+        # to work properly.
+        parts.append('"%s"' % part)
+      else:
+        parts.append(part)
+    return " ".join(parts)
+
 
 class IndicatorNotifier(object):
   """Holds a list of progress indicators and notifies them all on events."""
@@ -107,7 +123,7 @@ class SimpleProgressIndicator(ProgressIndicator):
       if failed.output.stdout:
         print "--- stdout ---"
         print failed.output.stdout.strip()
-      print "Command: %s" % failed.cmd.to_string()
+      print "Command: %s" % self._EscapeCommand(failed)
       if failed.output.HasCrashed():
         print "exit code: %d" % failed.output.exit_code
         print "--- CRASHED ---"
@@ -189,7 +205,7 @@ class CompactProgressIndicator(ProgressIndicator):
       stderr = test.output.stderr.strip()
       if len(stderr):
         print self.templates['stderr'] % stderr
-      print "Command: %s" % test.cmd.to_string()
+      print "Command: %s" % self._EscapeCommand(test)
       if test.output.HasCrashed():
         print "exit code: %d" % test.output.exit_code
         print "--- CRASHED ---"
@@ -257,7 +273,6 @@ class MonochromeProgressIndicator(CompactProgressIndicator):
 class JUnitTestProgressIndicator(ProgressIndicator):
 
   def __init__(self, junitout, junittestsuite):
-    super(JUnitTestProgressIndicator, self).__init__()
     self.outputter = junit_output.JUnitTestOutput(junittestsuite)
     if junitout:
       self.outfile = open(junitout, "w")
@@ -278,7 +293,7 @@ class JUnitTestProgressIndicator(ProgressIndicator):
       stderr = test.output.stderr.strip()
       if len(stderr):
         fail_text += "stderr:\n%s\n" % stderr
-      fail_text += "Command: %s" % self.test.cmd.to_string()
+      fail_text += "Command: %s" % self._EscapeCommand(test)
       if test.output.HasCrashed():
         fail_text += "exit code: %d\n--- CRASHED ---" % test.output.exit_code
       if test.output.HasTimedOut():
@@ -292,7 +307,6 @@ class JUnitTestProgressIndicator(ProgressIndicator):
 class JsonTestProgressIndicator(ProgressIndicator):
 
   def __init__(self, json_test_results, arch, mode, random_seed):
-    super(JsonTestProgressIndicator, self).__init__()
     self.json_test_results = json_test_results
     self.arch = arch
     self.mode = mode
@@ -320,7 +334,7 @@ class JsonTestProgressIndicator(ProgressIndicator):
       {
         "name": test.GetLabel(),
         "flags": test.flags,
-        "command": test.cmd.to_string(relative=True),
+        "command": self._EscapeCommand(test).replace(ABS_PATH_PREFIX, ""),
         "duration": test.duration,
         "marked_slow": statusfile.IsSlow(
           test.suite.GetStatusFileOutcomes(test)),
@@ -350,7 +364,7 @@ class JsonTestProgressIndicator(ProgressIndicator):
     self.results.append({
       "name": test.GetLabel(),
       "flags": test.flags,
-      "command": test.cmd.to_string(relative=True),
+      "command": self._EscapeCommand(test).replace(ABS_PATH_PREFIX, ""),
       "run": test.run,
       "stdout": test.output.stdout,
       "stderr": test.output.stderr,
@@ -370,7 +384,6 @@ class JsonTestProgressIndicator(ProgressIndicator):
 class FlakinessTestProgressIndicator(ProgressIndicator):
 
   def __init__(self, json_test_results):
-    super(FlakinessTestProgressIndicator, self).__init__()
     self.json_test_results = json_test_results
     self.results = {}
     self.summary = {

tools/testrunner/local/testsuite.py

@@ -30,7 +30,7 @@ import fnmatch
 import imp
 import os
 
-from . import command
+from . import commands
 from . import statusfile
 from . import utils
 from ..objects import testcase
@@ -310,44 +310,6 @@ class TestSuite(object):
 
     return self._outcomes_cache[cache_key]
 
-  def GetCommand(self, test, context):
-    shell = self.GetShellForTestCase(test)
-    shell_flags = []
-    if shell == 'd8':
-      shell_flags.append('--test')
-    if utils.IsWindows():
-      shell += '.exe'
-    if context.random_seed:
-      shell_flags.append('--random-seed=%s' % context.random_seed)
-    files, flags, env = self.GetParametersForTestCase(test, context)
-
-    return command.Command(
-      cmd_prefix=context.command_prefix,
-      shell=os.path.abspath(os.path.join(context.shell_dir, shell)),
-      args=(
-          shell_flags +
-          files +
-          context.extra_flags +
-          flags
-      ),
-      env=env,
-      timeout=self.GetTimeout(test, context),
-      verbose=context.verbose
-    )
-
-  def GetTimeout(self, testcase, context):
-    timeout = context.timeout
-    if ("--stress-opt" in testcase.flags or
-        "--stress-opt" in context.mode_flags or
-        "--stress-opt" in context.extra_flags):
-      timeout *= 4
-    if "--noenable-vfp3" in context.extra_flags:
-      timeout *= 2
-
-    # TODO(majeski): make it slow outcome dependent.
-    timeout *= 2
-    return timeout
-
   def GetShellForTestCase(self, testcase):
     """Returns shell to be executed for this test case."""
     return 'd8'
@@ -414,15 +376,16 @@ class GoogleTestSuite(TestSuite):
 
     output = None
     for i in xrange(3): # Try 3 times in case of errors.
-      cmd = command.Command(
-        cmd_prefix=context.command_prefix,
-        shell=shell,
-        args=['--gtest_list_tests'] + context.extra_flags)
-      output = cmd.execute()
+      cmd = (
+          context.command_prefix +
+          [shell, "--gtest_list_tests"] +
+          context.extra_flags
+      )
+      output = commands.Execute(cmd)
       if output.exit_code == 0:
         break
       print "Test executable failed to list the tests (try %d).\n\nCmd:" % i
-      print cmd
+      print ' '.join(cmd)
       print "\nStdout:"
       print output.stdout
       print "\nStderr:"

tools/testrunner/objects/testcase.py

@@ -36,11 +36,13 @@ class TestCase(object):
     self.id = None  # int, used to map result back to TestCase instance
     self.duration = None  # assigned during execution
     self.run = 1  # The nth time this test is executed.
-    self.cmd = None
 
   def CopyAddingFlags(self, variant, flags):
     return TestCase(self.suite, self.path, variant, self.flags + flags)
 
+  def SetSuiteObject(self, suites):
+    self.suite = suites[self.suite]
+
   def suitename(self):
     return self.suite.name
 

tools/testrunner/standard_runner.py

@@ -454,7 +454,6 @@ class StandardTestRunner(base_runner.BaseTestRunner):
 
         for t in s.tests:
           t.flags += s.GetStatusfileFlags(t)
-          t.cmd = s.GetCommand(t, ctx)
 
         s.tests = self._shard_tests(s.tests, options)
         num_tests += len(s.tests)

tools/unittests/run_perf_test.py

@@ -94,8 +94,8 @@ class PerfTest(unittest.TestCase):
         include=([os.path.join(cls.base, "run_perf.py")]))
     cls._cov.start()
     import run_perf
-    from testrunner.local import command
-    global command
+    from testrunner.local import commands
+    global commands
     global run_perf
 
   @classmethod
@@ -125,14 +125,9 @@ class PerfTest(unittest.TestCase):
                            stderr=None,
                            timed_out=kwargs.get("timed_out", False))
                     for arg in args[1]]
-    def create_cmd(*args, **kwargs):
-      cmd = MagicMock()
-      def execute(*args, **kwargs):
-        return test_outputs.pop()
-      cmd.execute = MagicMock(side_effect=execute)
-      return cmd
-
-    command.Command = MagicMock(side_effect=create_cmd)
+    def execute(*args, **kwargs):
+      return test_outputs.pop()
+    commands.Execute = MagicMock(side_effect=execute)
 
     # Check that d8 is called from the correct cwd for each test run.
     dirs = [path.join(TEST_WORKSPACE, arg) for arg in args[0]]
@@ -169,23 +164,18 @@ class PerfTest(unittest.TestCase):
     self.assertEquals(errors, self._LoadResults()["errors"])
 
   def _VerifyMock(self, binary, *args, **kwargs):
-    shell = path.join(path.dirname(self.base), binary)
-    command.Command.assert_called_with(
-        cmd_prefix=[],
-        shell=shell,
-        args=list(args),
-        timeout=kwargs.get('timeout', 60))
+    arg = [path.join(path.dirname(self.base), binary)]
+    arg += args
+    commands.Execute.assert_called_with(
+        arg, timeout=kwargs.get("timeout", 60))
 
   def _VerifyMockMultiple(self, *args, **kwargs):
-    self.assertEquals(len(args), len(command.Command.call_args_list))
-    for arg, actual in zip(args, command.Command.call_args_list):
-      expected = {
-        'cmd_prefix': [],
-        'shell': path.join(path.dirname(self.base), arg[0]),
-        'args': list(arg[1:]),
-        'timeout': kwargs.get('timeout', 60)
-      }
-      self.assertEquals((expected, ), actual)
+    expected = []
+    for arg in args:
+      a = [path.join(path.dirname(self.base), arg[0])]
+      a += arg[1:]
+      expected.append(((a,), {"timeout": kwargs.get("timeout", 60)}))
+    self.assertEquals(expected, commands.Execute.call_args_list)
 
   def testOneRun(self):
     self._WriteTestInput(V8_JSON)