Support multiple try masters when sending tries to rietveld.

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.8.0'
 
@@ skipping 989 matching lines @@
       results.append(p)
 
   logging.debug('Presubmit files: %s' % ','.join(results))
   return results
 
 
 class GetTrySlavesExecuter(object):
   @staticmethod
   def ExecPresubmitScript(script_text, presubmit_path, project, change):
     """Executes GetPreferredTrySlaves() from a single presubmit script.
+    
+    This will soon be deprecated and replaced by GetPreferredTryMasters().
 
     Args:
       script_text: The text of the presubmit script.
       presubmit_path: Project script to run.
       project: Project name to pass to presubmit script for bot selection.
 
     Return:
       A list of try slaves.
     """
     context = {}
@@ skipping 47 matching lines @@
              )
 
     # Ensure it's either all old-style or all new-style.
     if not valid_oldstyle(result) and not valid_newstyle(result):
       raise PresubmitFailure(
           'PRESUBMIT.py returned invalid trybot specification!')
 
     return result
 
 
+class GetTryMastersExecuter(object):
+  @staticmethod
+  def ExecPresubmitScript(script_text, presubmit_path, project, change):
+    """Executes GetPreferredTryMasters() from a single presubmit script.
+
+    Args:
+      script_text: The text of the presubmit script.
+      presubmit_path: Project script to run.
+      project: Project name to pass to presubmit script for bot selection.
+
+    Return:
+      A map of try masters to (list of builders or map of builders to set of
+      tests).
+    """
+    context = {}
+    try:
+      exec script_text in context
+    except Exception, e:
+      raise PresubmitFailure('"%s" had an exception.\n%s'
+                             % (presubmit_path, e))
+
+    function_name = 'GetPreferredTryMasters'
+    if function_name not in context:
+      return {}
+    get_preferred_try_masters = context[function_name]
+    function_info = inspect.getargspec(get_preferred_try_masters)

[Paweł Hajdan Jr., 2014/02/27 02:20:24]

Since this is new, can we standardize on one signature of that function?

[Michael Achenbach, 2014/02/27 18:35:23]

Done.

+    if len(function_info[0]) == 1:
+      result = get_preferred_try_masters(project)
+    elif len(function_info[0]) == 2:
+      result = get_preferred_try_masters(project, change)
+    else:
+      result = get_preferred_try_masters()
+
+    def CheckType(item, t):

[Paweł Hajdan Jr., 2014/02/27 02:20:24]

It's not obvious to me how much these type checks in python are actually useful.
I'm fine with them, but please consider removing them.

[Michael Achenbach, 2014/02/27 18:35:23]

If presubmit hooks return the wrong formats, it'd first be detected deeper down
the code and not be so obvious anymore. Would be nice if there were a more
declarative way to express the expected format.

+      if not isinstance(item, t):
+        raise PresubmitFailure(
+            'Expected a %s, got a %s instead: %s' %
+            (str(t), type(item), str(item)))
+
+    def CheckString(item):
+      if not item:
+        raise PresubmitFailure(
+            'PRESUBMIT.py returned empty trymaster/builder string.')
+      CheckType(item, basestring)
+      if item != item.strip():
+        raise PresubmitFailure(
+            'Try master/builder names cannot start/end with whitespace')
+      if ',' in item:
+        raise PresubmitFailure(
+            'Do not use \',\' separated master, builder or test names: %s'
+            % item)
+
+    CheckType(result, dict)
+    for (master, builders) in result.iteritems():
+      CheckString(master)
+      if isinstance(builders, dict):

[Paweł Hajdan Jr., 2014/02/27 02:20:24]

Similarly, since this is new, can we standardize on specifying both builders and
tests? A dummy value for tests can always be specified.

[Michael Achenbach, 2014/02/27 18:35:23]

Done.

+        for (builder, tests) in builders.iteritems():
+          CheckString(builder)
+          CheckType(tests, set)
+      elif isinstance(builders, list):
+        for builder in builders:
+          CheckString(builder)
+      else:
+        raise PresubmitFailure(
+            'Expected a list, got a %s instead: %s' %
+            (type(builders), str(builders)))   
+    return result
+
+
 def DoGetTrySlaves(change,
                    changed_files,
                    repository_root,
                    default_presubmit,
                    project,
                    verbose,
                    output_stream):
-  """Get the list of try servers from the presubmit scripts.
+  """Get the list of try servers from the presubmit scripts (deprecated).
 
   Args:
     changed_files: List of modified files.
     repository_root: The repository root.
     default_presubmit: A default presubmit script to execute in any case.
     project: Optional name of a project used in selecting trybots.
     verbose: Prints debug info.
     output_stream: A stream to write debug output to.
 
   Return:
@@ skipping 30 matching lines @@
   slaves = list(slave_dict.items())
 
   slaves.extend(set(old_style))
 
   if slaves and verbose:
     output_stream.write(', '.join((str(x) for x in slaves)))
     output_stream.write('\n')
   return slaves
 
 
+def _ExpandBuilders(builders):
+  """Expands builders in list format to dict format."""
+  if isinstance(builders, list):

[Paweł Hajdan Jr., 2014/02/27 02:20:24]

Why do we need to make a decision based on type here?

[Michael Achenbach, 2014/02/27 18:35:23]

This is removed now, since now only dicts are allowed.

+    return dict((builder, set()) for builder in builders)
+  else:
+    return builders
+
+
+def _MergeMasters(results, masters):
+  """Merges masters into results.

[Paweł Hajdan Jr., 2014/02/27 02:20:24]

It's not obvious to me what this does, could you clarify or ideally simplify it?

I could spend more time trying to understand it, but ideally I shouldn't.

[Michael Achenbach, 2014/02/27 18:35:23]

I tried to simplify it and made it a function. The best would be to look at the
tests and check if they're complete.

+
+  Merges with side effects on the original results dict.
+  """
+  for (master, builders) in masters.iteritems():
+    if master in results:
+      result_builders = results[master]
+      for (builder, tests) in _ExpandBuilders(builders).iteritems():
+        if builder in result_builders:
+          for test in tests:
+            result_builders.add(test)
+        else:
+          result_builders[builder] = tests
+    else:
+      results[master] = _ExpandBuilders(builders)
+  return results
+
+
+def DoGetTryMasters(change,
+                    changed_files,
+                    repository_root,
+                    default_presubmit,
+                    project,
+                    verbose,
+                    output_stream):
+  """Get the list of try masters from the presubmit scripts.
+
+  Args:
+    changed_files: List of modified files.
+    repository_root: The repository root.
+    default_presubmit: A default presubmit script to execute in any case.
+    project: Optional name of a project used in selecting trybots.
+    verbose: Prints debug info.
+    output_stream: A stream to write debug output to.
+
+  Return:
+    Map of try masters to map of builders to set of tests.
+  """
+  presubmit_files = ListRelevantPresubmitFiles(changed_files, repository_root)
+  if not presubmit_files and verbose:
+    output_stream.write("Warning, no presubmit.py found.\n")
+  results = {}
+  executer = GetTryMastersExecuter()
+
+  if default_presubmit:
+    if verbose:
+      output_stream.write("Running default presubmit script.\n")
+    fake_path = os.path.join(repository_root, 'PRESUBMIT.py')
+    results = _MergeMasters(results, executer.ExecPresubmitScript(
+        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 = _MergeMasters(results, executer.ExecPresubmitScript(
+        presubmit_script, filename, project, change))
+
+  # Make sets to lists again for later JSON serialization. Add
+  # defaulttests where tests are missing.
+  for (_, builders) in results.iteritems():
+    for builder in builders:
+      builders[builder] = list(builders[builder])
+      if not builders[builder]:
+        builders[builder].append('defaulttests')
+
+  if results and verbose:
+    output_stream.write('%s\n' % str(results))
+  return results
+
+
 class PresubmitExecuter(object):
   def __init__(self, change, committing, rietveld_obj, verbose):
     """
     Args:
       change: The Change object.
       committing: True if 'gcl commit' is running, False if 'gcl upload' is.
       rietveld_obj: rietveld.Rietveld client object.
     """
     self.change = change
     self.committing = committing
@@ skipping 363 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))