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

This is a reland of 3b065110
Original change's description:
> Reland "[test] Creating command before execution phase."
> 
> This is a reland of 98cc9e86
> 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}
> 
> Bug: v8:6917
> Change-Id: I49c29a8db813c47909f2cc45070ac7721a447c7a
> Reviewed-on: https://chromium-review.googlesource.com/800370
> Reviewed-by: Michael Achenbach <machenbach@chromium.org>
> Commit-Queue: Michał Majewski <majeski@google.com>
> Cr-Commit-Position: refs/heads/master@{#49756}

Bug: v8:6917
Change-Id: Ia39010a0a0f63537ad12490dfab17897d70d4930
Reviewed-on: https://chromium-review.googlesource.com/806034Reviewed-by: Michael Achenbach <machenbach@chromium.org>
Commit-Queue: Michał Majewski <majeski@google.com>
Cr-Commit-Position: refs/heads/master@{#49830}

test/cctest/testcfg.py

@@ -28,7 +28,7 @@
 import os
 import shutil
 
-from testrunner.local import commands
+from testrunner.local import command
 from testrunner.local import testsuite
 from testrunner.local import utils
 from testrunner.objects import testcase
@@ -37,21 +37,17 @@ 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 = context.command_prefix + [shell, "--list"] + context.extra_flags
-    output = commands.Execute(cmd)
+    cmd = command.Command(
+        cmd_prefix=context.command_prefix,
+        shell=shell,
+        args=["--list"] + context.extra_flags)
+    output = cmd.execute()
     if output.exit_code != 0:
-      print ' '.join(cmd)
+      print cmd
       print output.stdout
       print output.stderr
       return []

test/perf.isolate

@@ -10,7 +10,7 @@
       '../tools/run_perf.py',
       # TODO(machenbach): These files are referenced by the perf runner.
       # They should be transformed into a proper python module.
-      '../tools/testrunner/local/commands.py',
+      '../tools/testrunner/local/command.py',
       '../tools/testrunner/local/utils.py',
       '../tools/testrunner/objects/output.py',
       # This is often used to trigger performance bots. We include it in the

tools/perf-compare.py

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

tools/perf-to-html.py

@@ -12,7 +12,6 @@ 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 commands
+from testrunner.local import command
 from testrunner.local import utils
 
 ARCH_GUESS = utils.DefaultArch()
@@ -493,15 +493,23 @@ class RunnableConfig(GraphConfig):
     suffix = ["--"] + self.test_flags if self.test_flags else []
     return self.flags + (extra_flags or []) + [self.main] + suffix
 
-  def GetCommand(self, shell_dir, extra_flags=None):
+  def GetCommand(self, cmd_prefix, 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"
-    return cmd + self.GetCommandFlags(extra_flags=extra_flags)
+
+    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)
 
   def Run(self, runner, trybot):
     """Iterates over several runs and handles the output for all traces."""
@@ -677,18 +685,9 @@ 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)
-    if runnable.process_size:
-      command = ["/usr/bin/time", "--format=MaxMemory: %MKB"]
-    else:
-      command = []
-
-    command += self.command_prefix + runnable.GetCommand(shell_dir,
-                                                        self.extra_flags)
+    cmd = runnable.GetCommand(self.command_prefix, shell_dir, self.extra_flags)
     try:
-      output = commands.Execute(
-        command,
-        timeout=runnable.timeout,
-      )
+      output = cmd.execute()
     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,10 +34,9 @@ import sys
 import time
 
 from pool import Pool
-from . import commands
+from . import command
 from . import perfdata
 from . import statusfile
-from . import testsuite
 from . import utils
 from ..objects import output
 
@@ -48,76 +47,18 @@ 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", ["suites", "context"])
-
+    'process_context', ['sancov_dir'])
 
-def MakeProcessContext(context, suite_names):
-  """Generate a process-local context.
 
-  This reloads all suites per process and stores the global context.
+def MakeProcessContext(sancov_dir):
+  return ProcessContext(sancov_dir)
 
-  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)
+# 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)
 
 
 class Job(object):
@@ -126,31 +67,17 @@ class Job(object):
   All contained fields will be pickled/unpickled.
   """
 
-  def Run(self, process_context):
-    """Executes the job.
-
-    Args:
-      process_context: Process-local information that is initialized by the
-                       executing worker.
-    """
+  def run(self, process_context):
     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):
-    self.test = test
+  def __init__(self, test_id, cmd, run_num):
+    self.test_id = test_id
+    self.cmd = cmd
+    self.run_num = run_num
 
-  def _rename_coverage_data(self, output, context):
+  def _rename_coverage_data(self, out, sancov_dir):
     """Rename coverage data.
 
     Rename files with PIDs to files with unique test IDs, because the number
@@ -159,41 +86,27 @@ class TestJob(Job):
     42 is the test ID and 1 is the attempt (the same test might be rerun on
     failures).
     """
-    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))
+    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))
 
       # 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.test.run)] +
+            ["test", str(self.test_id), str(self.run_num)] +
             parts[-1:]
         )
         assert not os.path.exists(new_sancov_file)
         os.rename(sancov_file, new_sancov_file)
 
-  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)
-
+  def run(self, context):
     start_time = time.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)
+    out = self.cmd.execute()
+    self._rename_coverage_data(out, context.sancov_dir)
+    return (self.test_id, out, time.time() - start_time)
 
 
 class Runner(object):
@@ -262,7 +175,7 @@ class Runner(object):
       test.duration = None
       test.output = None
       test.run += 1
-      pool.add([TestJob(test)])
+      pool.add([TestJob(test.id, test.cmd, test.run)])
       self.remaining += 1
       self.total += 1
 
@@ -327,7 +240,7 @@ class Runner(object):
       # remember the output for comparison.
       test.run += 1
       test.output = result[1]
-      pool.add([TestJob(test)])
+      pool.add([TestJob(test.id, test.cmd, test.run)])
     # Always update the perf database.
     return True
 
@@ -350,7 +263,7 @@ class Runner(object):
         assert test.id >= 0
         test_map[test.id] = test
         try:
-          yield [TestJob(test)]
+          yield [TestJob(test.id, test.cmd, test.run)]
         except Exception, e:
           # If this failed, save the exception and re-raise it later (after
           # all other tests have had a chance to run).
@@ -358,10 +271,10 @@ class Runner(object):
           continue
     try:
       it = pool.imap_unordered(
-          fn=RunTest,
+          fn=run_job,
           gen=gen_tests(),
           process_context_fn=MakeProcessContext,
-          process_context_args=[self.context, self.suite_names],
+          process_context_args=[self.context.sancov_dir],
       )
       for result in it:
         if result.heartbeat:
@@ -378,7 +291,7 @@ class Runner(object):
       self._VerbosePrint("Closing process pool.")
       pool.terminate()
       self._VerbosePrint("Closing database connection.")
-      self._RunPerfSafe(lambda: self.perf_data_manager.close())
+      self._RunPerfSafe(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.
@@ -403,6 +316,8 @@ 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))
-        self.processes.append(p)
         p.start()
+        self.processes.append(p)
 
       self.advance(gen)
       while self.count > 0:
@@ -145,6 +145,11 @@ 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,14 +32,10 @@ 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):
@@ -70,18 +66,6 @@ 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."""
@@ -123,7 +107,7 @@ class SimpleProgressIndicator(ProgressIndicator):
       if failed.output.stdout:
         print "--- stdout ---"
         print failed.output.stdout.strip()
-      print "Command: %s" % self._EscapeCommand(failed)
+      print "Command: %s" % failed.cmd.to_string()
       if failed.output.HasCrashed():
         print "exit code: %d" % failed.output.exit_code
         print "--- CRASHED ---"
@@ -205,7 +189,7 @@ class CompactProgressIndicator(ProgressIndicator):
       stderr = test.output.stderr.strip()
       if len(stderr):
         print self.templates['stderr'] % stderr
-      print "Command: %s" % self._EscapeCommand(test)
+      print "Command: %s" % test.cmd.to_string()
       if test.output.HasCrashed():
         print "exit code: %d" % test.output.exit_code
         print "--- CRASHED ---"
@@ -273,6 +257,7 @@ 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")
@@ -293,7 +278,7 @@ class JUnitTestProgressIndicator(ProgressIndicator):
       stderr = test.output.stderr.strip()
       if len(stderr):
         fail_text += "stderr:\n%s\n" % stderr
-      fail_text += "Command: %s" % self._EscapeCommand(test)
+      fail_text += "Command: %s" % self.test.cmd.to_string()
       if test.output.HasCrashed():
         fail_text += "exit code: %d\n--- CRASHED ---" % test.output.exit_code
       if test.output.HasTimedOut():
@@ -307,6 +292,7 @@ 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
@@ -334,7 +320,7 @@ class JsonTestProgressIndicator(ProgressIndicator):
       {
         "name": test.GetLabel(),
         "flags": test.flags,
-        "command": self._EscapeCommand(test).replace(ABS_PATH_PREFIX, ""),
+        "command": test.cmd.to_string(relative=True),
         "duration": test.duration,
         "marked_slow": statusfile.IsSlow(
           test.suite.GetStatusFileOutcomes(test)),
@@ -364,7 +350,7 @@ class JsonTestProgressIndicator(ProgressIndicator):
     self.results.append({
       "name": test.GetLabel(),
       "flags": test.flags,
-      "command": self._EscapeCommand(test).replace(ABS_PATH_PREFIX, ""),
+      "command": test.cmd.to_string(relative=True),
       "run": test.run,
       "stdout": test.output.stdout,
       "stderr": test.output.stderr,
@@ -384,6 +370,7 @@ 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 commands
+from . import command
 from . import statusfile
 from . import utils
 from ..objects import testcase
@@ -310,6 +310,44 @@ 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'
@@ -376,16 +414,15 @@ class GoogleTestSuite(TestSuite):
 
     output = None
     for i in xrange(3): # Try 3 times in case of errors.
-      cmd = (
-          context.command_prefix +
-          [shell, "--gtest_list_tests"] +
-          context.extra_flags
-      )
-      output = commands.Execute(cmd)
+      cmd = command.Command(
+        cmd_prefix=context.command_prefix,
+        shell=shell,
+        args=['--gtest_list_tests'] + context.extra_flags)
+      output = cmd.execute()
       if output.exit_code == 0:
         break
       print "Test executable failed to list the tests (try %d).\n\nCmd:" % i
-      print ' '.join(cmd)
+      print cmd
       print "\nStdout:"
       print output.stdout
       print "\nStderr:"

tools/testrunner/objects/testcase.py

@@ -36,13 +36,11 @@ 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,6 +454,7 @@ 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 commands
-    global commands
+    from testrunner.local import command
+    global command
     global run_perf
 
   @classmethod
@@ -125,9 +125,14 @@ 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()
-    commands.Execute = MagicMock(side_effect=execute)
+      cmd.execute = MagicMock(side_effect=execute)
+      return cmd
+
+    command.Command = MagicMock(side_effect=create_cmd)
 
     # Check that d8 is called from the correct cwd for each test run.
     dirs = [path.join(TEST_WORKSPACE, arg) for arg in args[0]]
@@ -164,18 +169,23 @@ class PerfTest(unittest.TestCase):
     self.assertEquals(errors, self._LoadResults()["errors"])
 
   def _VerifyMock(self, binary, *args, **kwargs):
-    arg = [path.join(path.dirname(self.base), binary)]
-    arg += args
-    commands.Execute.assert_called_with(
-        arg, timeout=kwargs.get("timeout", 60))
+    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))
 
   def _VerifyMockMultiple(self, *args, **kwargs):
-    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)
+    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)
 
   def testOneRun(self):
     self._WriteTestInput(V8_JSON)