Rework bot and test parsing to allow receipt of (bot, set(test)) specifications.

presubmit_support.py

 #!/usr/bin/env python
 # Copyright (c) 2012 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
 """Enables directory-specific presubmit checks to run at upload and/or commit.
 """
 
 __version__ = '1.6.2'
 
@@ skipping 1013 matching lines @@
         result = get_preferred_try_slaves(project)
       elif len(function_info[0]) == 2:
         result = get_preferred_try_slaves(project, change)
       else:
         result = get_preferred_try_slaves()
       if not isinstance(result, types.ListType):
         raise PresubmitFailure(
             'Presubmit functions must return a list, got a %s instead: %s' %
             (type(result), str(result)))
       for item in result:
         if not isinstance(item, basestring):

[M-A Ruel, 2013/11/01 13:13:57]

Use positive checks instead of negative checks.

if isinstance(item, tuple):
  ...
elif isinstance(item, basestring):
  ...
else:
  raise YoDawg()

-          raise PresubmitFailure('All try slaves names must be strings.')
-        if item != item.strip():
+          if not isinstance(item, tuple):
+            raise PresubmitFailure('PRESUBMIT.py returned invalid tryslave/test'
+                                   ' format.')
+          # New-style [('bot', set(['tests']))] format.
+          botname = item[0]
+        else:
+          # Old-style ['bot'] format.
+          botname = item
+
+        if botname != botname.strip():
           raise PresubmitFailure(
               'Try slave names cannot start/end with whitespace')
-        if ',' in item:
+        if ',' in botname:
           raise PresubmitFailure(
-              'Do not use \',\' separated builder or test names: %s' % item)
+              'Do not use \',\' separated builder or test names: %s' % botname)
     else:
       result = []
     return result
 
 
 def DoGetTrySlaves(change,
                    changed_files,
                    repository_root,
                    default_presubmit,
                    project,
@@ skipping 25 matching lines @@
         default_presubmit, fake_path, project, change)
   for filename in presubmit_files:
     filename = os.path.abspath(filename)
     if verbose:
       output_stream.write("Running %s\n" % filename)
     # Accept CRLF presubmit script.
     presubmit_script = gclient_utils.FileRead(filename, 'rU')
     results += executer.ExecPresubmitScript(
         presubmit_script, filename, project, change)
 
-  slaves = list(set(results))
+  if results and isinstance(results[0], tuple):

[M-A Ruel, 2013/11/01 13:13:57]

do you want to use all()?
I'd prefer to enforce:
 all(isinstance(i, basestring) for i in results) or all(isinstance(i, tuple) for
i in results)
but no mix and match. Your code is less resistant.

+    # New-style [('bot', set(['tests']))] format.
+    slave_dict = {}
+    for result in results:
+      slave_dict.setdefault(result[0], set()).update(result[1])
+    slaves = list(slave_dict.iteritems())
+  else:
+    # Old-style ['bot'] format.
+    slaves = list(set(results))
+
   if slaves and verbose:
     output_stream.write(', '.join(slaves))
     output_stream.write('\n')
   return slaves
 
 
 class PresubmitExecuter(object):
   def __init__(self, change, committing, rietveld_obj, verbose):
     """
     Args:
@@ skipping 329 matching lines @@
   except PresubmitFailure, e:
     print >> sys.stderr, e
     print >> sys.stderr, 'Maybe your depot_tools is out of date?'
     print >> sys.stderr, 'If all fails, contact maruel@'
     return 2
 
 
 if __name__ == '__main__':
   fix_encoding.fix_encoding()
   sys.exit(Main(None))