[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}

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 []

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

@@ -0,0 +1,177 @@
+# 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

@@ -1,152 +0,0 @@
-# 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.
+def MakeProcessContext(sancov_dir):
+  return ProcessContext(sancov_dir)
 
-  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)
+# 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 execute(*args, **kwargs):
-      return test_outputs.pop()
-    commands.Execute = MagicMock(side_effect=execute)
+    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)
 
     # 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)