Add support for parallel presubmit unit testing.

presubmit_support.py

Index: presubmit_support.py
diff --git a/presubmit_support.py b/presubmit_support.py
index fe1c2878c5c27b64e75dfd36e2b456ac84aa43af..98b065925bec49db82f45c81c13e6e18c1a71fc9 100755
--- a/presubmit_support.py
+++ b/presubmit_support.py
@@ -15,6 +15,7 @@ __version__ = '1.6.2'
 import cpplint
 import cPickle  # Exposed through the API.
 import cStringIO  # Exposed through the API.
+import collections
 import contextlib
 import fnmatch
 import glob
@@ -22,6 +23,7 @@ import inspect
 import json  # Exposed through the API.
 import logging
 import marshal  # Exposed through the API.
+import multiprocessing
 import optparse
 import os  # Somewhat exposed through the API.
 import pickle  # Exposed through the API.
@@ -54,6 +56,9 @@ class PresubmitFailure(Exception):
   pass
 
 
+CommandData = collections.namedtuple('CommandData',
+                                     ['name', 'cmd', 'kwargs', 'message'])
+
 def normpath(path):
   '''Version of os.path.normpath that also changes backward slashes to
   forward slashes when not running on Windows.
@@ -104,81 +109,89 @@ class PresubmitOutput(object):
     return ''.join(self.written_output)
 
 
+class _PresubmitResult(object):
+  """Base class for result objects."""
+  fatal = False
+  should_prompt = False
+
+  def __init__(self, message, items=None, long_text=''):
+    """
+    message: A short one-line message to indicate errors.
+    items: A list of short strings to indicate where errors occurred.
+    long_text: multi-line text output, e.g. from another tool
+    """
+    self._message = message
+    self._items = []
+    if items:
+      self._items = items
+    self._long_text = long_text.rstrip()
+
+  def handle(self, output):
+    output.write(self._message)
+    output.write('\n')
+    for index, item in enumerate(self._items):
+      output.write('  ')
+      # Write separately in case it's unicode.
+      output.write(str(item))
+      if index < len(self._items) - 1:
+        output.write(' \\')
+      output.write('\n')
+    if self._long_text:
+      output.write('\n***************\n')
+      # Write separately in case it's unicode.
+      output.write(self._long_text)
+      output.write('\n***************\n')
+    if self.fatal:
+      output.fail()
+

[M-A Ruel, 2013/04/19 16:49:39]

two lines between file level symbols

[Isaac (away), 2013/04/20 00:41:36]

Done.

+class _PresubmitAddReviewers(_PresubmitResult):
+  """Add some suggested reviewers to the change."""
+  def __init__(self, reviewers):
+    super(OutputApi.PresubmitAddReviewers, self).__init__('')

[M-A Ruel, 2013/04/19 16:49:39]

Stale reference to OutputApi (and below)

[Isaac (away), 2013/04/20 00:41:36]

Done.

+    self.reviewers = reviewers
+
+  def handle(self, output):
+    output.reviewers.extend(self.reviewers)
+
+class _PresubmitError(_PresubmitResult):
+  """A hard presubmit error."""
+  fatal = True
+
+class _PresubmitPromptWarning(_PresubmitResult):
+  """An warning that prompts the user if they want to continue."""
+  should_prompt = True
+
+class _PresubmitNotifyResult(_PresubmitResult):
+  """Just print something to the screen -- but it's not even a warning."""
+  pass
+
+class _MailTextResult(_PresubmitResult):
+  """A warning that should be included in the review request email."""
+  def __init__(self, *args, **kwargs):
+    super(OutputApi.MailTextResult, self).__init__()
+    raise NotImplementedError()
+
 class OutputApi(object):
   """An instance of OutputApi gets passed to presubmit scripts so that they
   can output various types of results.
   """
+  # multiprocessing requires these be top level modules
+  PresubmitResult = _PresubmitResult
+  PresubmitAddReviewers = _PresubmitAddReviewers
+  PresubmitError = _PresubmitError
+  PresubmitPromptWarning = _PresubmitPromptWarning
+  PresubmitNotifyResult = _PresubmitNotifyResult
+  MailTextResult = _MailTextResult
+
   def __init__(self, is_committing):
     self.is_committing = is_committing
 
-  class PresubmitResult(object):
-    """Base class for result objects."""
-    fatal = False
-    should_prompt = False
-
-    def __init__(self, message, items=None, long_text=''):
-      """
-      message: A short one-line message to indicate errors.
-      items: A list of short strings to indicate where errors occurred.
-      long_text: multi-line text output, e.g. from another tool
-      """
-      self._message = message
-      self._items = []
-      if items:
-        self._items = items
-      self._long_text = long_text.rstrip()
-
-    def handle(self, output):
-      output.write(self._message)
-      output.write('\n')
-      for index, item in enumerate(self._items):
-        output.write('  ')
-        # Write separately in case it's unicode.
-        output.write(str(item))
-        if index < len(self._items) - 1:
-          output.write(' \\')
-        output.write('\n')
-      if self._long_text:
-        output.write('\n***************\n')
-        # Write separately in case it's unicode.
-        output.write(self._long_text)
-        output.write('\n***************\n')
-      if self.fatal:
-        output.fail()
-
-  class PresubmitAddReviewers(PresubmitResult):
-    """Add some suggested reviewers to the change."""
-    def __init__(self, reviewers):
-      super(OutputApi.PresubmitAddReviewers, self).__init__('')
-      self.reviewers = reviewers
-
-    def handle(self, output):
-      output.reviewers.extend(self.reviewers)
-
-  class PresubmitError(PresubmitResult):
-    """A hard presubmit error."""
-    fatal = True
-
-  class PresubmitPromptWarning(PresubmitResult):
-    """An warning that prompts the user if they want to continue."""
-    should_prompt = True
-
-  class PresubmitNotifyResult(PresubmitResult):
-    """Just print something to the screen -- but it's not even a warning."""
-    pass
-
   def PresubmitPromptOrNotify(self, *args, **kwargs):
     """Warn the user when uploading, but only notify if committing."""
     if self.is_committing:
       return self.PresubmitNotifyResult(*args, **kwargs)
     return self.PresubmitPromptWarning(*args, **kwargs)
 
-  class MailTextResult(PresubmitResult):
-    """A warning that should be included in the review request email."""
-    def __init__(self, *args, **kwargs):
-      super(OutputApi.MailTextResult, self).__init__()
-      raise NotImplementedError()
-
 
 class InputApi(object):
   """An instance of this object is passed to presubmit scripts so they can
@@ -284,6 +297,7 @@ class InputApi(object):
     self.owners_db = owners.Database(change.RepositoryRoot(),
         fopen=file, os_path=self.os_path, glob=self.glob)
     self.verbose = verbose
+    self.Command = CommandData
 
     # Replace <hash_map> and <hash_set> as headers that need to be included
     # with "base/hash_tables.h" instead.
@@ -437,6 +451,26 @@ class InputApi(object):
     """Returns if a change is TBR'ed."""
     return 'TBR' in self.change.tags
 
+  def RunTests(self, tests_mix, parallel=True):

[M-A Ruel, 2013/04/19 16:49:39]

Will parallel=False ever make sense?

[Isaac (away), 2013/04/19 19:52:04]

Yes, it is used by the legacy commands in presubmit_canned_checks.  I'll
probably switch those to parallell execution later and PRESUBMITs needing serial
execution can call input_api.RunUnitTests() multiple times.

+    tests = []
+    msgs = []
+    for t in tests_mix:
+      if isinstance(t, OutputApi.PresubmitResult):
+        msgs.append(t)
+      else:
+        t = t._replace(kwargs=t.kwargs.copy())
+        t.kwargs.setdefault('env', self.environ)
+        t.kwargs.setdefault('cwd', self.PresubmitLocalPath())
+        tests.append(t)
+    if parallel:
+      pool = multiprocessing.Pool()
+      msgs.extend(pool.map_async(CallCommand, tests).get(99999))

[M-A Ruel, 2013/04/19 16:49:39]

Can you note why this is used? (e.g. the Ctrl-C handling)

[Isaac (away), 2013/04/20 00:41:36]

Done.

+      pool.close()
+      pool.join()
+    else:
+      msgs.extend(map(CallCommand, tests))
+    return [m for m in msgs if m is not None]

[M-A Ruel, 2013/04/19 16:49:39]

return [m for m in msgs if m]
or
return filter(None, msgs)

I don't think '' messages are valuable (?)

[Isaac (away), 2013/04/19 19:52:04]

The intention is that each item in msg list is either an instance of
PresubmitResult or None, so your suggestion should work fine.  I will add an
assertion somewhere to verify my precondition.

+
 
 class AffectedFile(object):
   """Representation of a file in a change."""
@@ -1238,6 +1272,19 @@ def canned_check_filter(method_names):
     for name, method in filtered.iteritems():
       setattr(presubmit_canned_checks, name, method)
 
+# multiprocessing requires a top level fun with a single argument

[M-A Ruel, 2013/04/19 16:49:39]

fun -> function
I'd prefer the code to be inside the function.

[Isaac (away), 2013/04/20 00:41:36]

Done.

+def CallCommand(cmd_data):
+  cmd_data.kwargs['stdout'] = subprocess.PIPE
+  cmd_data.kwargs['stderr'] = subprocess.STDOUT
+  try:
+    (out, _), code = subprocess.communicate(cmd_data.cmd, **cmd_data.kwargs)
+    if code != 0:
+      #import pdb; pdb.set_trace()

[M-A Ruel, 2013/04/19 16:49:39]

Remove before committing

[Isaac (away), 2013/04/20 00:41:36]

Done.

+      return cmd_data.message('%s failed\n%s' % (cmd_data.name, out))
+  except OSError as e:
+    return cmd_data.message(
+        '%s exec failure\n   %s\n%s' % (cmd_data.name, e, out))
+
 
 def Main(argv):
   parser = optparse.OptionParser(usage="%prog [options] <files...>",