Allow PRESUBMIT.py files to have old and new-style trybot specifications (as long as each file is consistent).

BUG=278554

Review URL: https://codereview.chromium.org/74163002

git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@241590 0039d316-1c4b-4281-b951-d872f2087c98

presubmit_support.py

@@ -1055,6 +1055,22 @@ class GetTrySlavesExecuter(object):
               'Do not use \',\' separated builder or test names: %s' % botname)
     else:
       result = []
+
+    def valid_oldstyle(result):
+      return all(isinstance(i, basestring) for i in result)
+
+    def valid_newstyle(result):
+      return (all(isinstance(i, tuple) for i in result) and
+              all(len(i) == 2 for i in result) and
+              all(isinstance(i[0], basestring) for i in result) and
+              all(isinstance(i[1], set) for i in result)
+             )
+
+    # 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
 
 
@@ -1083,35 +1099,35 @@ def DoGetTrySlaves(change,
     output_stream.write("Warning, no presubmit.py found.\n")
   results = []
   executer = GetTrySlavesExecuter()
+
   if default_presubmit:
     if verbose:
       output_stream.write("Running default presubmit script.\n")
     fake_path = os.path.join(repository_root, 'PRESUBMIT.py')
-    results += executer.ExecPresubmitScript(
-        default_presubmit, fake_path, project, change)
+    results.extend(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 += executer.ExecPresubmitScript(
-        presubmit_script, filename, project, change)
+    results.extend(executer.ExecPresubmitScript(
+        presubmit_script, filename, project, change))
 
-  if all(isinstance(i, tuple) for i in results):
-    # 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())
-  elif all(isinstance(i, basestring) for i in results):
-    # Old-style ['bot'] format.
-    slaves = list(set(results))
-  else:
-    raise ValueError('PRESUBMIT.py returned invalid trybot specification!')
+
+  slave_dict = {}
+  old_style = filter(lambda x: isinstance(x, basestring), results)
+  new_style = filter(lambda x: isinstance(x, tuple), results)
+
+  for result in new_style:
+    slave_dict.setdefault(result[0], set()).update(result[1])
+  slaves = list(slave_dict.items())
+
+  slaves.extend(set(old_style))
 
   if slaves and verbose:
-    output_stream.write(', '.join(slaves))
+    output_stream.write(', '.join((str(x) for x in slaves)))
     output_stream.write('\n')
   return slaves
 

tests/presubmit_unittest.py

@@ -923,7 +923,11 @@ def CheckChangeOnCommit(input_api, output_api):
     starts_with_space_result = ['  starts_with_space']
     not_list_result1 = "'foo'"
     not_list_result2 = "('a', 'tuple')"
-    for result in starts_with_space_result, not_list_result1, not_list_result2:
+    mixed_old_and_new = ['bot', ('bot2', set(['test']))]
+    not_set = [('bot2', ['test'])]
+    for result in (
+        starts_with_space_result, not_list_result1, not_list_result2,
+        mixed_old_and_new, not_set):
       self.assertRaises(presubmit.PresubmitFailure,
                         executer.ExecPresubmitScript,
                         self.presubmit_tryslave % result, '', '', change)
@@ -932,7 +936,9 @@ def CheckChangeOnCommit(input_api, output_api):
     expected_result = ['1', '2', '3']
     empty_result = []
     space_in_name_result = ['foo bar', '1\t2 3']
-    for result in expected_result, empty_result, space_in_name_result:
+    new_style = [('bot', set(['cool', 'tests']))]
+    for result in (
+        expected_result, empty_result, space_in_name_result, new_style):
       self.assertEqual(
           result,
           executer.ExecPresubmitScript(

trychange.py

@@ -381,14 +381,14 @@ def _GenTSBotSpec(checkouts, change, changed_files, options):
         options.verbose,
         sys.stdout)
     if trybots:
-      if isinstance(trybots[0], basestring):
-        # PRESUBMIT.py sent us an old-style string list of bots.
-        # _ParseBotList's testfilter is set to None otherwise it will complain.
-        bot_spec = _ApplyTestFilter(options.testfilter,
-                                    _ParseBotList(trybots, None))
-      else:
-        # PRESUBMIT.py sent us a new-style (bot, set()) specification.
-        bot_spec = _ApplyTestFilter(options.testfilter, trybots)
+      old_style = filter(lambda x: isinstance(x, basestring), trybots)
+      new_style = filter(lambda x: isinstance(x, tuple), trybots)
+
+      # _ParseBotList's testfilter is set to None otherwise it will complain.
+      bot_spec = _ApplyTestFilter(options.testfilter,
+                                  _ParseBotList(old_style, None))
+
+      bot_spec.extend(_ApplyTestFilter(options.testfilter, new_style))
 
   except ImportError:
     pass