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

trychange.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.
 
 """Client-side script to send a try job to the try server. It communicates to
 the try server by either writting to a svn repository or by directly connecting
 to the server by HTTP.
 """
 
 import datetime
 import errno
 import getpass
+import itertools
 import json
 import logging
 import optparse
 import os
 import posixpath
 import re
 import shutil
 import sys
 import tempfile
 import urllib
@@ skipping 283 matching lines @@
     if RunGit(['diff-index', 'HEAD']):
       print 'Cannot try with a dirty tree.  You must commit locally first.'
       return None
     return scm.GIT.GenerateDiff(
         self.checkout_root,
         files=self.files,
         full_move=True,
         branch=self.diff_against)
 
 
-def _ParseSendChangeOptions(options):
+def _ParseBotList(botlist, testfilter):
+  """Parses bot configurations from a list of strings."""
+  bots = []
+  if testfilter:
+    for bot in itertools.chain.from_iterable(botspec.split(',')
+                                             for botspec in botlist):
+      tests = set()
+      if ':' in bot:
+        if bot.endswith(':compile'):
+          tests |= set(['compile'])
+        else:
+          raise ValueError(
+              'Can\'t use both --testfilter and --bot builder:test formats '
+              'at the same time')
+
+      bots.append((bot, tests))
+  else:
+    for botspec in botlist:
+      botname = botspec.split(':')[0]
+      tests = set()
+      if ':' in botspec:
+        tests |= set(filter(None, botspec.split(':')[1].split(',')))
+      bots.append((botname, tests))
+  return bots
+
+
+def _ApplyTestFilter(testfilter, bot_spec):
+  """Applies testfilter from CLI.
+
+  Specifying a testfilter strips off any builder-specified tests (except for
+  compile).
+  """
+  if testfilter:
+    return [(botname, set(testfilter) | (tests & set(['compile'])))
+            for botname, tests in bot_spec]
+  else:
+    return bot_spec
+
+
+def _GenTSBotSpec(checkouts, change, changed_files, options):
+  bot_spec = []
+  # Get try slaves from PRESUBMIT.py files if not specified.
+  # Even if the diff comes from options.url, use the local checkout for bot
+  # selection.
+  try:
+    import presubmit_support
+    root_presubmit = checkouts[0].ReadRootFile('PRESUBMIT.py')
+    if not change:
+      if not changed_files:
+        changed_files = checkouts[0].file_tuples
+      change = presubmit_support.Change(options.name,
+                                        '',
+                                        checkouts[0].checkout_root,
+                                        changed_files,
+                                        options.issue,
+                                        options.patchset,
+                                        options.email)
+    trybots = presubmit_support.DoGetTrySlaves(
+        change,
+        checkouts[0].GetFileNames(),
+        checkouts[0].checkout_root,
+        root_presubmit,
+        options.project,
+        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)
+
+  except ImportError:
+    pass
+
+  return bot_spec
+
+
+def _ParseSendChangeOptions(bot_spec, options):
   """Parse common options passed to _SendChangeHTTP and _SendChangeSVN."""
   values = [
     ('user', options.user),
     ('name', options.name),
   ]
   if options.email:
     values.append(('email', options.email))
   if options.revision:
     values.append(('revision', options.revision))
   if options.clobber:
     values.append(('clobber', 'true'))
   if options.root:
     values.append(('root', options.root))
   if options.patchlevel:
     values.append(('patchlevel', options.patchlevel))
   if options.issue:
     values.append(('issue', options.issue))
   if options.patchset:
     values.append(('patchset', options.patchset))
   if options.target:
     values.append(('target', options.target))
   if options.project:
     values.append(('project', options.project))
 
-  filters = ','.join(options.testfilter)
-  if filters:
-    for botlist in options.bot:
-      for bot in botlist.split(','):
-        if ':' in bot:
-          raise ValueError(
-              'Can\'t use both --testfilter and --bot builder:test formats '
-              'at the same time')
-        else:
-          values.append(('bot', '%s:%s' % (bot, filters)))
-  else:
-    for bot in options.bot:
-      values.append(('bot', bot))
+  for bot, tests in bot_spec:
+    values.append(('bot', ('%s:%s' % (bot, ','.join(tests)))))
+
   return values
 
 
-def _SendChangeHTTP(options):
+def _SendChangeHTTP(bot_spec, options):
   """Send a change to the try server using the HTTP protocol."""
   if not options.host:
     raise NoTryServerAccess('Please use the --host option to specify the try '
         'server host to connect to.')
   if not options.port:
     raise NoTryServerAccess('Please use the --port option to specify the try '
         'server port to connect to.')
 
-  values = _ParseSendChangeOptions(options)
+  values = _ParseSendChangeOptions(bot_spec, options)
   values.append(('patch', options.diff))
 
   url = 'http://%s:%s/send_try_patch' % (options.host, options.port)
   proxies = None
   if options.proxy:
     if options.proxy.lower() == 'none':
       # Effectively disable HTTP_PROXY or Internet settings proxy setup.
       proxies = {}
     else:
       proxies = {'http': options.proxy, 'https': options.proxy}
 
   logging.info('Sending by HTTP')
   logging.info(''.join("%s=%s\n" % (k, v) for k, v in values))
   logging.info(url)
   logging.info(options.diff)
   if options.dry_run:
     return
 
   try:
     logging.info('Opening connection...')
     connection = urllib.urlopen(url, urllib.urlencode(values), proxies=proxies)
     logging.info('Done')
   except IOError, e:
     logging.info(str(e))
-    if options.bot and len(e.args) > 2 and e.args[2] == 'got a bad status line':
+    if bot_spec and len(e.args) > 2 and e.args[2] == 'got a bad status line':
       raise NoTryServerAccess('%s is unaccessible. Bad --bot argument?' % url)
     else:
       raise NoTryServerAccess('%s is unaccessible. Reason: %s' % (url,
                                                                   str(e.args)))
   if not connection:
     raise NoTryServerAccess('%s is unaccessible.' % url)
   logging.info('Reading response...')
   response = connection.read()
   logging.info('Done')
   if response != 'OK':
     raise NoTryServerAccess('%s is unaccessible. Got:\n%s' % (url, response))
 
 
-def _SendChangeSVN(options):
+def _SendChangeSVN(bot_spec, options):
   """Send a change to the try server by committing a diff file on a subversion
   server."""
   if not options.svn_repo:
     raise NoTryServerAccess('Please use the --svn_repo option to specify the'
                             ' try server svn repository to connect to.')
 
-  values = _ParseSendChangeOptions(options)
+  values = _ParseSendChangeOptions(bot_spec, options)
   description = ''.join("%s=%s\n" % (k, v) for k, v in values)
   logging.info('Sending by SVN')
   logging.info(description)
   logging.info(options.svn_repo)
   logging.info(options.diff)
   if options.dry_run:
     return
 
   # Create a temporary directory, put a uniquely named file in it with the diff
   # content and svn import that.
@@ skipping 29 matching lines @@
         command.append('--no-ignore')
 
       subprocess2.check_call(command)
     except subprocess2.CalledProcessError, e:
       raise NoTryServerAccess(str(e))
   finally:
     temp_file.close()
     shutil.rmtree(temp_dir, True)
 
 
-def PrintSuccess(options):
+def PrintSuccess(bot_spec, options):
   if not options.dry_run:
     text = 'Patch \'%s\' sent to try server' % options.name
-    if options.bot:
-      text += ': %s' % ', '.join(options.bot)
+    if bot_spec:
+      text += ': %s' % ', '.join(
+          '%s:%s' % (b[0], ','.join(b[1])) for b in bot_spec)
     print(text)
 
 
 def GuessVCS(options, path, file_list):
   """Helper to guess the version control system.
 
   NOTE: Very similar to upload.GuessVCS. Doesn't look for hg since we don't
   support it yet.
 
   This examines the path directory, guesses which SCM we're using, and
@@ skipping 326 matching lines @@
         options.name = 'Issue %s' % options.issue
       else:
         options.name = 'Unnamed'
         print('Note: use --name NAME to change the try job name.')
 
     if not options.email:
       parser.error('Using an anonymous checkout. Please use --email or set '
                    'the TRYBOT_RESULTS_EMAIL_ADDRESS environment variable.')
     print('Results will be emailed to: ' + options.email)
 
-    if not options.bot:
-      # Get try slaves from PRESUBMIT.py files if not specified.
-      # Even if the diff comes from options.url, use the local checkout for bot
-      # selection.
-      try:
-        import presubmit_support
-        root_presubmit = checkouts[0].ReadRootFile('PRESUBMIT.py')
-        if not change:
-          if not changed_files:
-            changed_files = checkouts[0].file_tuples
-          change = presubmit_support.Change(options.name,
-                                            '',
-                                            checkouts[0].checkout_root,
-                                            changed_files,
-                                            options.issue,
-                                            options.patchset,
-                                            options.email)
-        options.bot = presubmit_support.DoGetTrySlaves(
-            change,
-            checkouts[0].GetFileNames(),
-            checkouts[0].checkout_root,
-            root_presubmit,
-            options.project,
-            options.verbose,
-            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 options.bot:
+      bot_spec = _ApplyTestFilter(
+          options.testfilter, _ParseBotList(options.bot, options.testfilter))
+    else:
+      bot_spec = _GenTSBotSpec(checkouts, change, changed_files, options)
 
-      # 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.
+    if options.testfilter:
+      bot_spec = _ApplyTestFilter(options.testfilter, bot_spec)
 
-    if any('triggered' in b.split(':', 1)[0] for b in options.bot):
+    if any('triggered' in b[0] for b in bot_spec):
       print >> sys.stderr, (
           'ERROR You are trying to send a job to a triggered bot.  This type of'
           ' bot requires an\ninitial job from a parent (usually a builder).  '
-          'Instead send your job to the parent.\nBot list: %s' % options.bot)
+          'Instead send your job to the parent.\nBot list: %s' % bot_spec)
       return 1
 
     if options.print_bots:
       print 'Bots which would be used:'
-      for bot in options.bot:
-        print '  %s' % bot
+      for bot in bot_spec:
+        if bot[1]:
+          print '  %s:%s' % (bot[0], ','.join(bot[1]))
+        else:
+          print '  %s' % (bot[0])
       return 0
 
     # Send the patch.
     if options.send_patch:
       # If forced.
-      options.send_patch(options)
-      PrintSuccess(options)
+      options.send_patch(bot_spec, options)
+      PrintSuccess(bot_spec, options)
       return 0
     try:
       if can_http:
-        _SendChangeHTTP(options)
-        PrintSuccess(options)
+        _SendChangeHTTP(bot_spec, options)
+        PrintSuccess(bot_spec, options)
         return 0
     except NoTryServerAccess:
       if not can_svn:
         raise
-    _SendChangeSVN(options)
-    PrintSuccess(options)
+    _SendChangeSVN(bot_spec, options)
+    PrintSuccess(bot_spec, options)
     return 0
   except (InvalidScript, NoTryServerAccess), e:
     if swallow_exception:
       return 1
     print >> sys.stderr, e
     return 1
   except (gclient_utils.Error, subprocess2.CalledProcessError), e:
     print >> sys.stderr, e
     return 1
   return 0
 
 
 if __name__ == "__main__":
   fix_encoding.fix_encoding()
   sys.exit(TryChange(None, None, False))