Adds saner behavior for git try -t foo to not fail in a inexplicable way.

Enforces that each builder and builder:test association must be in its own list item in PRESUBMIT.py/GetPreferredTrySlaves().

If a user run git try -t foo, trychange.py will now take the list of all the
slaves, will skip any :compile builder, and apply the test filter to them.

Currently, git try -t foo on a chromium checkout throws an exception because of
PRESUBMIT.py files containing test specification.

R=petermayo@chromium.org
BUG=
TEST=


Review URL: http://codereview.chromium.org/9664015

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

presubmit_support.py

@@ -926,6 +926,9 @@ class GetTrySlavesExecuter(object):
         if item != item.strip():
           raise PresubmitFailure(
               'Try slave names cannot start/end with whitespace')
+        if ',' in item:
+          raise PresubmitFailure(
+              'Do not use \',\' separated builder or test names: %s' % item)
     else:
       result = []
     return result

tests/presubmit_unittest.py

@@ -792,6 +792,25 @@ def CheckChangeOnCommit(input_api, output_api):
                                               self.fake_root_dir, None, None,
                                               False, output))
 
+  def testGetTrySlavesExecuter_ok(self):
+    script_text = (
+        'def GetPreferredTrySlaves():\n'
+        '  return ["foo", "bar"]\n')
+    results = presubmit.GetTrySlavesExecuter.ExecPresubmitScript(
+        script_text, 'path', 'project', None)
+    self.assertEquals(['foo', 'bar'], results)
+
+  def testGetTrySlavesExecuter_comma(self):
+    script_text = (
+        'def GetPreferredTrySlaves():\n'
+        '  return ["foo,bar"]\n')
+    try:
+      presubmit.GetTrySlavesExecuter.ExecPresubmitScript(
+          script_text, 'path', 'project', None)
+      self.fail()
+    except presubmit.PresubmitFailure:
+      pass
+
   def testMainUnversioned(self):
     # OptParser calls presubmit.os.path.exists and is a pain when mocked.
     self.UnMock(presubmit.os.path, 'exists')

trychange.py

@@ -805,6 +805,16 @@ def TryChange(argv,
             sys.stdout)
       except ImportError:
         pass
+      if options.testfilter:
+        bots = set()
+        for bot in options.bot:
+          assert ',' not in bot
+          if bot.endswith(':compile'):
+            # Skip over compile-only builders for now.
+            continue
+          bots.add(bot.split(':', 1)[0])
+        options.bot = list(bots)
+
       # If no bot is specified, either the default pool will be selected or the
       # try server will refuse the job. Either case we don't need to interfere.