Refactor stages 1, 2 and test_api overhaul.

scripts/slave/recipe_modules/auto_bisect/api.py

 # Copyright 2015 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.
 
 """API for the bisect recipe module.
 
 This API is meant to enable the bisect recipe to bisect any chromium-supported
 platform for any test that can be run via buildbot, perf or otherwise.
 """
 
@@ skipping 39 matching lines @@
     # Keep track of working directory (which contains the checkout).
     # None means "default value".
     self._working_dir = None
 
   @property
   def working_dir(self):
    if not self._working_dir:
      self._working_dir = self.m.chromium_checkout.get_checkout_dir({})
    return self._working_dir or self.m.path['slave_build']
 
-  def perform_bisect(self, **flags):
-    return local_bisect.perform_bisect(self, **flags)
-
   def create_bisector(self, bisect_config_dict, dummy_mode=False, **flags):
     """Passes the api and the config dictionary to the Bisector constructor.
 
     For details about the keys in the bisect config dictionary go to:
     http://chromium.org/developers/speed-infra/perf-try-bots-bisect-bots/config
 
     Args:
       bisect_config_dict (dict): Contains the configuration for the bisect job.
       dummy_mode (bool): In dummy mode we prevent the bisector for expanding
         the revision range at construction, to avoid the need for lots of step
@@ skipping 24 matching lines @@
     depot_config.add_addition_depot_into(depot_info)  # pragma: no cover
 
   def set_deploy_script(self, path):  # pragma: no cover
     """Sets apk deployment script path for android-chrome."""
     self.full_deploy_script = path
 
   def set_svn_repo(self, svn_repo_url):  # pragma: no cover
     """Sets SVN repo url for triggering build jobs."""
     self.svn_repo_url = svn_repo_url
 
-  def gsutil_file_exists(self, path):
+  def gsutil_file_exists(self, path, **kwargs):

[eakuefner, 2016/09/09 20:35:42]

I don't like this idea of putting **kwargs everywhere -- do we have specific
motivation for threading particular arguments through? Can we just pick those
instead?

[RobertoCN, 2016/09/13 22:11:40]

I agree, but this is a pattern that is used all over recipes. I tried to thread
only step_test_data but the behavior resulting from that was unexpected. I think
it had something to do with `'step_test_data' not in kwargs` being different
from `step_test_data == None` or somesuch.

     """Returns True if a file exists at the given GS path."""
     try:
-      self.m.gsutil(['ls', path])
-    except self.m.step.StepFailure:  # pragma: no cover
+      self.m.gsutil( ['ls', path], **kwargs)
+    except self.m.step.StepFailure:
       # A step failure here simply means that the file does not exist, and
       # should not be treated as an error.
       self.m.step.active_result.presentation.status = self.m.step.SUCCESS
       return False
     return True
 
   def query_revision_info(self, revision):
     """Gathers information on a particular revision.
 
     Args:
       revision (str): A git commit hash.
 
     Returns:
       A dict with the keys "author", "email", "date", "subject" and "body",
       as output by fetch_revision_info.py.
     """
     step_name = 'Reading culprit cl information.'
     # Use gitiles to get commit information.
     if revision.depot_name == 'android-chrome':  # pragma: no cover
       commit_url = depot_config.DEPOT_DEPS_NAME[revision.depot_name]['url']
       return self._commit_info(revision.commit_hash, commit_url, step_name)
     result = self.m.python(
         step_name,
         self.resource('fetch_revision_info.py'),
         [revision.commit_hash, revision.depot_name],
-        stdout=self.m.json.output())
+        stdout=self.m.json.output(),
+        step_test_data=lambda: self._test_data['cl_info'][revision.commit_hash],
+    )
     return result.stdout
 
   def _commit_info(self, commit_hash, url, step_name=None):  # pragma: no cover
     """Fetches information about a given commit.
 
     Args:
       commit_hash (str): A git commit hash.
       url (str): The URL of a repository, e.g.
     "https://chromium.googlesource.com/chromium/src".
       step_name (str): Optional step name.
@@ skipping 14 matching lines @@
             'email': commit_info['author']['email'],
             'subject': subject,
             'body': body,
             'date': commit_info['committer']['time'],
        }
      return None
     except self.m.step.StepFailure:  # pragma: no cover
      self.surface_result('BAD_REV')
      raise
 
-  def run_bisect_script(self, **kwargs):
+  def run_bisect_script(self, **kwargs):  # pragma: no cover

[prasadv, 2016/09/09 18:52:30]

Do we still want to support this? As far as I know all all bisects kicked off by
dashboard are recipe based.

[RobertoCN, 2016/09/13 22:11:40]

Done.

     """Executes src/tools/run-perf-bisect-regression.py to perform bisection."""
     self.m.python(
         'Preparing for Bisection',
         script=self.m.path['checkout'].join(
             'tools', 'prepare-bisect-perf-regression.py'),
         args=['-w', self.m.path['cache'].join('bisect')])
     args = []
 
     kwargs['allow_subannotations'] = True
 
     # TODO(prasadv): Remove this once bisect runs are no longer running
     # against revisions from February 2016 or earlier.
-    if self.internal_bisect:  # pragma: no cover
+    if self.internal_bisect:
       kwargs['env'] = {'CHROMIUM_OUTPUT_DIR': self.m.chromium.output_dir}
 
-    if kwargs.get('extra_src'):
+    if kwargs.get('extra_src'): 
       args = args + ['--extra_src', kwargs.pop('extra_src')]
     if kwargs.get('path_to_config'):
       args = args + ['--path_to_config', kwargs.pop('path_to_config')]
     if self.m.chromium.c.TARGET_PLATFORM != 'android':
       goma_dir = self.m.goma.ensure_goma()
       args += ['--path_to_goma', goma_dir]
     args += [
         '--build-properties',
         self.m.json.dumps(dict(self.m.properties.legacy())),
     ]
     self.m.chromium.runtest(
         self.m.path['checkout'].join('tools', 'run-bisect-perf-regression.py'),
         ['-w', self.m.path['cache'].join('bisect')] + args,
         name='Running Bisection',
         xvfb=True, **kwargs)
 
   def run_local_test_run(self, test_config_params,
-                         skip_download=False):  # pragma: no cover
+                         skip_download=False, **kwargs):
     """Starts a test run on the same machine.
 
     This is for the merged director/tester flow.
     """
     if self.m.platform.is_win:
       self.m.chromium.taskkill()
 
-    if skip_download:
+    if skip_download:  # pragma: no cover
       update_step = None
     else:
       update_step = self._SyncRevisionToTest(test_config_params)
-    self.start_test_run_for_bisect(update_step, self.bot_db,
-                                   test_config_params, run_locally=True,
-                                   skip_download=skip_download)
+    return self.start_test_run_for_bisect(
+        update_step, self.bot_db, test_config_params,
+        skip_download=skip_download, **kwargs)
 
   def ensure_checkout(self, *args, **kwargs):
     if self.working_dir:
       kwargs.setdefault('cwd', self.working_dir)
 
     return self.m.bot_update.ensure_checkout(*args, **kwargs)
 
-  def _SyncRevisionToTest(self, test_config_params):  # pragma: no cover
+  def _SyncRevisionToTest(self, test_config_params):
     if not self.internal_bisect:
       return self.ensure_checkout(
           root_solution_revision=test_config_params['revision'])
-    else:
+    else:  # pragma: no cover
       return self._SyncRevisionsForAndroidChrome(
           test_config_params['revision_ladder'])
 
   def _SyncRevisionsForAndroidChrome(self, revision_ladder):  # pragma: no cover
     """Syncs android-chrome and chromium repos to particular revision."""
     revisions = []
     for d, r in revision_ladder.iteritems():
       revisions.append('%s@%s' % (depot_config.DEPOT_DEPS_NAME[d]['src'], r))
     params = ['sync', '--verbose', '--nohooks', '--force',
               '--delete_unversioned_trees']
     for revision in revisions:
       params.extend(['--revision', revision])
     self.m.gclient('sync %s' % '-'.join(revisions), params)
     return None
 
-  def start_test_run_for_bisect(self, update_step, bot_db,
-                                test_config_params, run_locally=False,
-                                skip_download=False):
+  def start_test_run_for_bisect(self, update_step, bot_db, test_config_params,
+                                skip_download=False, **kwargs):
     mastername = self.m.properties.get('mastername')
     buildername = self.m.properties.get('buildername')
     bot_config = bot_db.get_bot_config(mastername, buildername)
     build_archive_url = test_config_params['parent_build_archive_url']
-    if not run_locally:
-      self.m.bisect_tester.upload_job_url()
     if not skip_download:
       if self.m.chromium.c.TARGET_PLATFORM == 'android':
         # The best way to ensure the old build directory is not used is to
         # remove it.
         build_dir = self.m.chromium.c.build_dir.join(
             self.m.chromium.c.build_config_fs)
         self.m.file.rmtree('build directory', build_dir)
 
         # The way android builders on tryserver.chromium.perf are archived is
         # different from builders on chromium.perf. In order to support both
@@ skipping 22 matching lines @@
               shutil.move(sys.argv[1], sys.argv[2])
               """,
               args=[zip_dir, build_dir])
       else:
         self.m.chromium_tests.download_and_unzip_build(
             mastername, buildername, update_step, bot_db,
             build_archive_url=build_archive_url,
             build_revision=test_config_params['parent_got_revision'],
             override_bot_type='tester')
 
-    tests = [self.m.chromium_tests.steps.BisectTest(test_config_params)]
+    tests = [
+        self.m.chromium_tests.steps.BisectTest(
+            test_config_params, **kwargs)]
 
     if not tests:  # pragma: no cover
       return
     self.m.chromium_swarming.configure_swarming(
         'chromium', precommit=False, mastername=mastername)
     test_runner = self.m.chromium_tests.create_test_runner(self.m, tests)
 
     bot_config_object = self.m.chromium_tests.create_bot_config_object(
         mastername, buildername)
     with self.m.chromium_tests.wrap_chromium_tests(bot_config_object, tests):
       if self.m.chromium.c.TARGET_PLATFORM == 'android' and not skip_download:
         deploy_apks = []
         deploy_args = []
         if self.internal_bisect:  # pragma: no cover
           deploy_args = list(bot_config['deploy_args'])
           deploy_apks = bot_config.get('deploy_apks')
           if deploy_apks:
             deploy_args.extend([
               '--apks',
               ','.join(str(self.m.chromium_android.apk_path(apk))
                        for apk in deploy_apks)])
         else:
           if bot_config.get('webview'):
             deploy_apks = ['SystemWebView.apk', 'SystemWebViewShell.apk']
           else:
             deploy_apks = ['ChromePublic.apk']
         self.deploy_apk_on_device(
             self.full_deploy_script, deploy_apks, deploy_args)
       test_runner()
+      return tests[0].run_results
 
   def deploy_apk_on_device(self, deploy_script, deploy_apks, deploy_args):
     """Installs apk on the android device."""
     if deploy_script:  # pragma: no cover
       self.full_deploy_on_device(deploy_script, deploy_args)
     else:
       for apk in deploy_apks:
         self.m.chromium_android.adb_install_apk(apk)
 
   def full_deploy_on_device(self, deploy_script, args=None):  # pragma: no cover
@@ skipping 29 matching lines @@
       api: The recipe api object.
       update_step: Extra update_step to, used for some job types.
       bot_db: A BotConfigAndTestDB object, used for some job types.
       kwargs: Args to use only for legacy bisect.
     """
     flags = {}
     if kwargs.get('do_not_nest_wait_for_revision'):
       flags['do_not_nest_wait_for_revision'] = kwargs.pop(
           'do_not_nest_wait_for_revision')
     if bot_db is None:  # pragma: no cover
-      self.bot_db = api.chromium_tests.create_bot_db_from_master_dict(
-          '', None, None)
+      self.bot_db = api.chromium_tests.create_bot_db_from_master_dict('', None)
     else:
       self.bot_db = bot_db
 
     context = {}
     if self.working_dir:
       context['cwd'] = self.working_dir
 
     with api.step.context(context):
       affected_files = self.m.tryserver.get_files_affected_by_patch()
       # Skip device setup for internal bisect as it is taken care in
       # internal recipes.
       if (api.chromium.c.TARGET_PLATFORM == 'android' and
           not self.internal_bisect):
         api.chromium_android.common_tests_setup_steps(
             perf_setup=True, remove_system_webview=True)
         api.chromium.runhooks()
       try:
         # Run legacy bisect script if the patch contains bisect.cfg.
         if BISECT_CONFIG_FILE in affected_files:
           api.step('***LEGACY BISECT (deprecated)***', [])
           self.run_bisect_script(**kwargs)
         elif api.properties.get('bisect_config'):
           # We can distinguish between a config for a full bisect vs a single
           # test by checking for the presence of the good_revision key.
           if api.properties.get('bisect_config').get('good_revision'):
             api.step('***BISECT***', [])
-            local_bisect.perform_bisect(self, **flags)  # pragma: no cover
+            local_bisect.perform_bisect(self, **flags)
           else:
             api.step('***SINGLE TEST (deprecated)***', [])
             self.start_test_run_for_bisect(update_step, self.bot_db,
                                            api.properties)
         else:
           api.step('***PERF TRYJOB***', [])
           self.m.perf_try.start_perf_try_job(
               api, affected_files, update_step, self.bot_db)
       finally:
         if api.chromium.c.TARGET_PLATFORM == 'android':
           if self.internal_bisect:  # pragma: no cover
             api.chromium_android.init_and_sync(
                 gclient_config=api.chromium_android.c.internal_dir_name,
                 use_bot_update=True)
           else:
             self.ensure_checkout()
           api.chromium_android.common_tests_final_steps()
+
+  def stat_compare(self, values_a, values_b, metric,
+                   output_format='chartjson', **kwargs):
+    """Compares samples using catapult's statistics implementation.
+
+    Args:
+      values_a, values_b: lists of paths to the json files containing the values
+        produced by the test.
+      metric: the name of the metric as sent by dashboard.
+      output_format: either 'chartjson' or 'valueset'

[eakuefner, 2016/09/09 20:35:42]

will 'buildbot' be supported?

[RobertoCN, 2016/09/13 22:11:40]

yeah, done.

+
+    Returns:
+      a dict containing 'result' which may be True, False or 'needMoreData', as
+      well as details about each sample ('debug_values', 'mean' and 'std_dev').
+
+    """
+    args = [','.join(map(str, values_a)),
+            ','.join(map(str, values_b)),
+            metric,
+            '--' + output_format]
+
+    script = self.m.path['catapult'].join(
+        'tracing', 'bin', 'compare_samples')
+    return self.m.python(
+        'Compare samples',
+        script=script,
+        args=args,
+        stdout=self.m.json.output(),
+        **kwargs).stdout