Add blink support to --browser=trybot

BUG=401180

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

Cr-Commit-Position: refs/heads/master@{#291789}

tools/telemetry/telemetry/core/backends/remote/trybot_browser_finder.py

@@ -6,6 +6,7 @@
 
 import json
 import logging
+import os
 import re
 import subprocess
 import sys
@@ -15,8 +16,9 @@ from telemetry import decorators
 from telemetry.core import platform
 from telemetry.core import possible_browser
 
-# TODO(sullivan): Check for blink changes
-CONFIG_FILENAME = 'tools/run-perf-test.cfg'
+CHROMIUM_CONFIG_FILENAME = 'tools/run-perf-test.cfg'
+BLINK_CONFIG_FILENAME = 'Tools/run-perf-test.cfg'
+SUCCESS, NO_CHANGES, ERROR = range(3)
 
 
 class PossibleTrybotBrowser(possible_browser.PossibleBrowser):
@@ -51,20 +53,29 @@ class PossibleTrybotBrowser(possible_browser.PossibleBrowser):
     returncode = proc.poll()
     return (returncode, out, err)
 
-  def RunRemote(self):
-    """Sends a tryjob to a perf trybot.
+  def _AttemptTryjob(self, cfg_file_path):
+    """Attempts to run a tryjob from the current directory.
 
-    This creates a branch, telemetry-tryjob, switches to that branch, edits
-    the bisect config, commits it, uploads the CL to rietveld, and runs a
-    tryjob on the given bot.
+    This is run once for chromium, and if it returns NO_CHANGES, once for blink.
+
+    Args:
+      cfg_file_path: Path to the config file for the try job.
+
+    Returns:
+      (result, msg) where result is one of:
+          SUCCESS if a tryjob was sent
+          NO_CHANGES if there was nothing to try,
+          ERROR if a tryjob was attempted but an error encountered
+          and msg is an error message if an error was encountered, or rietveld
+          url if success.
     """
     returncode, original_branchname, err = self._RunProcess(
         ['git', 'rev-parse', '--abbrev-ref', 'HEAD'])
     if returncode:
-      logging.error('Must be in a git repository to send changes to trybots.')
+      msg = 'Must be in a git repository to send changes to trybots.'
       if err:
-        logging.error('Git error: %s', err)
-      return
+        msg += '\nGit error: %s' % err
+      return (ERROR, msg)
     original_branchname = original_branchname.strip()
 
     # Check if the tree is dirty: make sure the index is up to date and then
@@ -72,28 +83,23 @@ class PossibleTrybotBrowser(possible_browser.PossibleBrowser):
     self._RunProcess(['git', 'update-index', '--refresh', '-q'])
     returncode, out, err = self._RunProcess(['git', 'diff-index', 'HEAD'])
     if out:
-      logging.error(
-          'Cannot send a try job with a dirty tree. Commit locally first.')
-      return
+      msg = 'Cannot send a try job with a dirty tree. Commit locally first.'
+      return (ERROR, msg)
 
     # Make sure the tree does have local commits.
     returncode, out, err = self._RunProcess(
         ['git', 'log', 'origin/master..HEAD'])
     if not out:
-      logging.error('No local changes on branch %s. browser=%s argument sends '
-                    'local changes to the %s perf trybot.', original_branchname,
-                    self._browser_type, self._buildername)
-      return
+      return (NO_CHANGES, None)
 
     # Create/check out the telemetry-tryjob branch, and edit the configs
     # for the tryjob there.
     returncode, out, err = self._RunProcess(
         ['git', 'checkout', '-b', 'telemetry-tryjob'])
     if returncode:
-      logging.error('Error creating branch telemetry-tryjob. '
-                    'Please delete it if it exists.')
-      logging.error(err)
-      return
+      msg = ('Error creating branch telemetry-tryjob. '
+             'Please delete it if it exists.\n%s' % err)
+      return (ERROR, msg)
 
     # Generate the command line for the perf trybots
     arguments = sys.argv
@@ -117,18 +123,18 @@ class PossibleTrybotBrowser(possible_browser.PossibleBrowser):
         'truncate_percent': '0',
     }
     try:
-      config_file = open(CONFIG_FILENAME, 'w')
+      config_file = open(cfg_file_path, 'w')
     except IOError:
-      logging.error('Cannot find %s. Please run from src dir.', CONFIG_FILENAME)
-      return
+      msg = 'Cannot find %s. Please run from src dir.' % cfg_file_path
+      return (ERROR, msg)
     config_file.write('config = %s' % json.dumps(
         config, sort_keys=True, indent=2, separators=(',', ': ')))
     config_file.close()
     returncode, out, err = self._RunProcess(
         ['git', 'commit', '-a', '-m', 'bisect config'])
     if returncode:
-      logging.error('Could not commit bisect config change, error %s', err)
-      return
+      msg = 'Could not commit bisect config change, error %s' % err
+      return (ERROR, msg)
 
     # Upload the CL to rietveld and run a try job.
     returncode, out, err = self._RunProcess([
@@ -136,34 +142,67 @@ class PossibleTrybotBrowser(possible_browser.PossibleBrowser):
         'CL for perf tryjob', 'origin/master'
     ])
     if returncode:
-      logging.error('Could upload to reitveld, error %s', err)
-      return
+      msg = 'Could upload to reitveld, error %s', err
+      return (ERROR, msg)
     match = re.search(r'https://codereview.chromium.org/[\d]+', out)
     if not match:
-      logging.error('Could not upload CL to reitveld! Output %s', out)
-      return
-    print 'Uploaded try job to reitveld. View progress at %s' % match.group(0)
+      msg = 'Could not upload CL to reitveld! Output %s' % out
+      return (ERROR, msg)
+    rietveld_url = match.group(0)
     returncode, out, err = self._RunProcess([
-          'git', 'cl', 'try', '-m', 'tryserver.chromium.perf',
-          '-b', self._buildername])
+        'git', 'cl', 'try', '-m', 'tryserver.chromium.perf', '-b',
+        self._buildername])
     if returncode:
-      logging.error('Could not try CL, error %s', err)
-      return
+      msg = 'Could not try CL, error %s' % err
+      return (ERROR, msg)
 
     # Checkout original branch and delete telemetry-tryjob branch.
     returncode, out, err = self._RunProcess(
         ['git', 'checkout', original_branchname])
     if returncode:
-      logging.error(
+      msg = (
           ('Could not check out %s. Please check it out and manually '
-           'delete the telemetry-tryjob branch. Error message: %s'),
-          original_branchname, err)
-      return
+           'delete the telemetry-tryjob branch. Error message: %s') %
+          (original_branchname, err))
+      return (ERROR, msg)
     returncode, out, err = self._RunProcess(
         ['git', 'branch', '-D', 'telemetry-tryjob'])
     if returncode:
-      logging.error(('Could not delete telemetry-tryjob branch. '
+      msg = (('Could not delete telemetry-tryjob branch. '
               'Please delete it manually. Error %s'), err)
+      return (ERROR, msg)
+    return (SUCCESS, rietveld_url)
+
+  def RunRemote(self):
+    """Sends a tryjob to a perf trybot.
+
+    This creates a branch, telemetry-tryjob, switches to that branch, edits
+    the bisect config, commits it, uploads the CL to rietveld, and runs a
+    tryjob on the given bot.
+    """
+    # First check if there are chromium changes to upload.
+    status, msg = self._AttemptTryjob(CHROMIUM_CONFIG_FILENAME)
+    if status == SUCCESS:
+      print 'Uploaded chromium try job to reitveld. View progress at %s' % msg
+      return
+    elif status == ERROR:
+      logging.error(msg)
+      return
+
+    # If we got here, there are no chromium changes to upload. Try blink.
+    os.chdir('third_party/WebKit/')
+    status, msg = self._AttemptTryjob(BLINK_CONFIG_FILENAME)
+    os.chdir('../..')
+    if status == SUCCESS:
+      print 'Uploaded blink try job to reitveld. View progress at %s' % msg
+      return
+    elif status == ERROR:
+      logging.error(msg)
+      return
+    else:
+      logging.error('No local changes found in chromium or blink trees. '
+                    'browser=%s argument sends local changes to the %s '
+                    'perf trybot.', self._browser_type, self._buildername)
       return
 
   def _InitPlatformIfNeeded(self):

tools/telemetry/telemetry/core/backends/remote/trybot_browser_finder_unittest.py

@@ -21,7 +21,8 @@ class TrybotBrowserFinderTest(unittest.TestCase):
     logging.getLogger().addHandler(self.stream_handler)
     self._real_subprocess = trybot_browser_finder.subprocess
     self._real_urllib2 = trybot_browser_finder.urllib2
-    self._stubs = system_stub.Override(trybot_browser_finder, ['sys', 'open'])
+    self._stubs = system_stub.Override(trybot_browser_finder,
+                                       ['sys', 'open', 'os'])
 
   def tearDown(self):
     logging.getLogger().removeHandler(self.stream_handler)
@@ -126,13 +127,17 @@ class TrybotBrowserFinderTest(unittest.TestCase):
         (['git', 'update-index', '--refresh', '-q'], (0, None, None,)),
         (['git', 'diff-index', 'HEAD'], (0, '', None)),
         (['git', 'log', 'origin/master..HEAD'], (0, '', None)),
+        (['git', 'rev-parse', '--abbrev-ref', 'HEAD'], (0, 'br', None)),
+        (['git', 'update-index', '--refresh', '-q'], (0, None, None,)),
+        (['git', 'diff-index', 'HEAD'], (0, '', None)),
+        (['git', 'log', 'origin/master..HEAD'], (0, '', None)),
     ))
 
     browser.RunRemote()
     self.assertEquals(
-        ('No local changes on branch br. browser=trybot-android-nexus4 '
-         'argument sends local changes to the android_nexus4_perf_bisect '
-         'perf trybot.\n'),
+        ('No local changes found in chromium or blink trees. '
+         'browser=trybot-android-nexus4 argument sends local changes to the '
+         'android_nexus4_perf_bisect perf trybot.\n'),
         self.log_output.getvalue())
 
   def test_branch_checkout_fails(self):
@@ -155,11 +160,19 @@ class TrybotBrowserFinderTest(unittest.TestCase):
          'fatal: A branch named \'telemetry-try\' already exists.\n'),
         self.log_output.getvalue())
 
-  def _GetConfigForBrowser(self, name, branch):
+  def _GetConfigForBrowser(self, name, branch, cfg_filename, is_blink=False):
     finder_options = browser_options.BrowserFinderOptions()
     browser = trybot_browser_finder.PossibleTrybotBrowser(name, finder_options)
     bot = '%s_perf_bisect' % name.replace('trybot-', '').replace('-', '_')
-    self._ExpectProcesses((
+    first_processes = ()
+    if is_blink:
+      first_processes = (
+          (['git', 'rev-parse', '--abbrev-ref', 'HEAD'], (0, 'br', None)),
+          (['git', 'update-index', '--refresh', '-q'], (0, None, None,)),
+          (['git', 'diff-index', 'HEAD'], (0, '', None)),
+          (['git', 'log', 'origin/master..HEAD'], (0, '', None))
+      )
+    self._ExpectProcesses(first_processes + (
         (['git', 'rev-parse', '--abbrev-ref', 'HEAD'], (0, branch, None)),
         (['git', 'update-index', '--refresh', '-q'], (0, None, None,)),
         (['git', 'diff-index', 'HEAD'], (0, '', None)),
@@ -179,13 +192,14 @@ class TrybotBrowserFinderTest(unittest.TestCase):
         '--browser=%s' % browser,
         'sunspider']
     cfg = StringIO.StringIO()
-    self._stubs.open.files = {'tools/run-perf-test.cfg': cfg}
+    self._stubs.open.files = {cfg_filename: cfg}
 
     browser.RunRemote()
     return cfg.getvalue()
 
   def test_config_android(self):
-    config = self._GetConfigForBrowser('trybot-android-nexus4', 'somebranch')
+    config = self._GetConfigForBrowser(
+        'trybot-android-nexus4', 'somebranch', 'tools/run-perf-test.cfg')
     self.assertEquals(
         ('config = {\n'
          '  "command": "./tools/perf/run_measurement '
@@ -196,7 +210,20 @@ class TrybotBrowserFinderTest(unittest.TestCase):
          '}'), config)
 
   def test_config_mac(self):
-    config = self._GetConfigForBrowser('trybot-mac-10-9', 'currentwork')
+    config = self._GetConfigForBrowser(
+        'trybot-mac-10-9', 'currentwork', 'tools/run-perf-test.cfg')
+    self.assertEquals(
+        ('config = {\n'
+         '  "command": "./tools/perf/run_measurement '
+         '--browser=release sunspider",\n'
+         '  "max_time_minutes": "120",\n'
+         '  "repeat_count": "1",\n'
+         '  "truncate_percent": "0"\n'
+         '}'), config)
+
+  def test_config_blink(self):
+    config = self._GetConfigForBrowser(
+        'trybot-mac-10-9', 'blinkbranch', 'Tools/run-perf-test.cfg', True)
     self.assertEquals(
         ('config = {\n'
          '  "command": "./tools/perf/run_measurement '

tools/telemetry/telemetry/unittest/system_stub.py

@@ -233,6 +233,9 @@ class OsModuleStub(object):
       raise NotImplementedError('Unsupported getenv')
     return env if env else value
 
+  def chdir(self, path):
+    pass
+
 
 class PerfControlModuleStub(object):
   class PerfControlStub(object):