Refactor SyncBuildAndRunRevision to remove deep nesting.

tools/bisect-perf-regression.py

 #!/usr/bin/env python
 # Copyright (c) 2013 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.
 
 """Performance Test Bisect Tool
 
 This script bisects a series of changelists using binary search. It starts at
 a bad revision where a performance metric has regressed, and asks for a last
 known-good revision. It will then binary search across this revision range by
@@ skipping 134 matching lines @@
         'from': ['chromium'],
         'viewvc': 'https://code.google.com/p/skia/source/detail?r=',
         'deps_var': 'None'
     }
 }
 
 DEPOT_NAMES = DEPOT_DEPS_NAME.keys()
 
 CROS_CHROMEOS_PATTERN = 'chromeos-base/chromeos-chrome'
 
-# Possible return values from BisectPerformanceMetrics.SyncBuildAndRunRevision.
+# Possible return values from BisectPerformanceMetrics.RunTest.
 BUILD_RESULT_SUCCEED = 0
 BUILD_RESULT_FAIL = 1
 BUILD_RESULT_SKIPPED = 2
 
 # Maximum time in seconds to wait after posting build request to the try server.
 # TODO: Change these values based on the actual time taken by buildbots on
 # the try server.
 MAX_MAC_BUILD_TIME = 14400
 MAX_WIN_BUILD_TIME = 14400
 MAX_LINUX_BUILD_TIME = 14400
@@ skipping 1014 matching lines @@
           results['chromium'] = output.strip()
 
     if depot == 'v8':
       # We can't try to map the trunk revision to bleeding edge yet, because
       # we don't know which direction to try to search in. Have to wait until
       # the bisect has narrowed the results down to 2 v8 rolls.
       results['v8_bleeding_edge'] = None
 
     return results
 
-  def BackupOrRestoreOutputdirectory(self, restore=False, build_type='Release'):
+  def BackupOrRestoreOutputDirectory(self, restore=False, build_type='Release'):
     """Backs up or restores build output directory based on restore argument.
 
     Args:
       restore: Indicates whether to restore or backup. Default is False(Backup)
       build_type: Target build type ('Release', 'Debug', 'Release_x64' etc.)
 
     Returns:
       Path to backup or restored location as string. otherwise None if it fails.
     """
     build_dir = os.path.abspath(
@@ skipping 77 matching lines @@
           revision, fetch_build=fetch_build_func, patch=patch)
       if not downloaded_file:
         return False
 
     # Generic name for the archive, created when archive file is extracted.
     output_dir = os.path.join(
         abs_build_dir, GetZipFileName(target_arch=self.opts.target_arch))
     # Unzip build archive directory.
     try:
       RmTreeAndMkDir(output_dir, skip_makedir=True)
-      self.BackupOrRestoreOutputdirectory(restore=False)
+      self.BackupOrRestoreOutputDirectory(restore=False)
       # Build output directory based on target(e.g. out/Release, out/Debug).
       target_build_output_dir = os.path.join(abs_build_dir, build_type)
       ExtractZip(downloaded_file, abs_build_dir)
       if not os.path.exists(output_dir):
         # Due to recipe changes, the builds extract folder contains
         # out/Release instead of full-build-<platform>/Release.
         if os.path.exists(os.path.join(abs_build_dir, 'out', build_type)):
           output_dir = os.path.join(abs_build_dir, 'out', build_type)
         else:
           raise IOError('Missing extracted folder %s ' % output_dir)
 
       print 'Moving build from %s to %s' % (
           output_dir, target_build_output_dir)
       shutil.move(output_dir, target_build_output_dir)
       return True
     except Exception as e:
       print 'Something went wrong while extracting archive file: %s' % e
-      self.BackupOrRestoreOutputdirectory(restore=True)
+      self.BackupOrRestoreOutputDirectory(restore=True)
       # Cleanup any leftovers from unzipping.
       if os.path.exists(output_dir):
         RmTreeAndMkDir(output_dir, skip_makedir=True)
     finally:
       # Delete downloaded archive
       if os.path.exists(downloaded_file):
         os.remove(downloaded_file)
     return False
 
   def PostBuildRequestAndWait(self, git_revision, fetch_build, patch=None):
@@ skipping 387 matching lines @@
           'std_err': standard_err,
           'std_dev': standard_dev,
           'values': metric_values,
       }
 
       print 'Results of performance test: %12f %12f' % (
           truncated_mean, standard_err)
       print
     return (values, success_code, output_of_all_runs)
 
-  def FindAllRevisionsToSync(self, revision, depot):
+  def _FindAllRevisionsToSync(self, revision, depot):
     """Finds all dependent revisions and depots that need to be synced.
 
     For example skia is broken up into 3 git mirrors over skia/src,
     skia/gyp, and skia/include. To sync skia/src properly, one has to find
     the proper revisions in skia/gyp and skia/include.
 
     This is only useful in the git workflow, as an SVN depot may be split into
     multiple mirrors.
 
     Args:
@@ skipping 67 matching lines @@
     Returns:
       True if successful.
     """
     cwd = os.getcwd()
     self.ChangeToDepotWorkingDirectory('cros')
     cmd = [bisect_utils.CROS_SDK_PATH, '--create']
     return_code = bisect_utils.RunProcess(cmd)
     os.chdir(cwd)
     return not return_code
 
-  def PerformPreSyncCleanup(self, revision, depot):
+  def _PerformPreSyncCleanup(self, depot):
     """Performs any necessary cleanup before syncing.
 
+    Args:
+      depot: Depot name.
+
     Returns:
       True if successful.
     """
     if depot == 'chromium' or depot == 'android-chrome':
       # Removes third_party/libjingle. At some point, libjingle was causing
       # issues syncing when using the git workflow (crbug.com/266324).
       os.chdir(self.src_cwd)
       if not bisect_utils.RemoveThirdPartyDirectory('libjingle'):
         return False
       # Removes third_party/skia. At some point, skia was causing
       # issues syncing when using the git workflow (crbug.com/377951).
       if not bisect_utils.RemoveThirdPartyDirectory('skia'):
         return False
     elif depot == 'cros':
       return self.PerformCrosChrootCleanup()
     return True
 
-  def RunPostSync(self, depot):
+  def _RunPostSync(self, depot):
     """Performs any work after syncing.
 
+    Args:
+      depot: Depot name.
+
     Returns:
       True if successful.
     """
     if self.opts.target_platform == 'android':
       if not builder.SetupAndroidBuildEnvironment(self.opts,
           path_to_src=self.src_cwd):
         return False
 
     if depot == 'cros':
       return self.CreateCrosChroot()
@@ skipping 19 matching lines @@
         cmd = ['diff-tree', '--no-commit-id', '--name-only', '-r', revision]
         output = bisect_utils.CheckRunGit(cmd)
 
         files = output.splitlines()
 
         if len(files) == 1 and files[0] == 'DEPS':
           return True
 
     return False
 
-  def SyncBuildAndRunRevision(self, revision, depot, command_to_run, metric,
-                              skippable=False):
+  def RunTest(self, revision, depot, command, metric, skippable=False):
     """Performs a full sync/build/run of the specified revision.
 
     Args:
       revision: The revision to sync to.
       depot: The depot that's being used at the moment (src, webkit, etc.)
-      command_to_run: The command to execute the performance test.
+      command: The command to execute the performance test.
       metric: The performance metric being tested.
 
     Returns:
       On success, a tuple containing the results of the performance test.
       Otherwise, a tuple with the error message.
     """
+    # Decide which sync program to use.
     sync_client = None
     if depot == 'chromium' or depot == 'android-chrome':
       sync_client = 'gclient'
     elif depot == 'cros':
       sync_client = 'repo'
 
-    revisions_to_sync = self.FindAllRevisionsToSync(revision, depot)
-
+    # Decide what depots will need to be synced to what revisions.
+    revisions_to_sync = self._FindAllRevisionsToSync(revision, depot)
     if not revisions_to_sync:
       return ('Failed to resolve dependent depots.', BUILD_RESULT_FAIL)
 
-    if not self.PerformPreSyncCleanup(revision, depot):
+    if not self._PerformPreSyncCleanup(depot):
       return ('Failed to perform pre-sync cleanup.', BUILD_RESULT_FAIL)
 
-    success = True
+    # Do the syncing for all depots.
+    if not self.opts.debug_ignore_sync:
+      if not self._SyncAllRevisions(revisions_to_sync, sync_client):
+        return ('Failed to sync: [%s]' % str(revision), BUILD_RESULT_FAIL)
 
-    if not self.opts.debug_ignore_sync:
-      for r in revisions_to_sync:
-        self.ChangeToDepotWorkingDirectory(r[0])
+     # Try to do any post-sync steps. This may include "gclient runhooks".
+    if not self._RunPostSync(depot):
+      return ('Failed to run [gclient runhooks].', BUILD_RESULT_FAIL)
 
-        if sync_client:
-          self.PerformPreBuildCleanup()
+    # Skip this revision if it can be skipped.
+    if skippable and self.ShouldSkipRevision(depot, revision):
+      return ('Skipped revision: [%s]' % str(revision),
+              BUILD_RESULT_SKIPPED)
 
-        # If you're using gclient to sync, you need to specify the depot you
-        # want so that all the dependencies sync properly as well.
-        # i.e. gclient sync src@<SHA1>
-        current_revision = r[1]
-        if sync_client == 'gclient':
-          current_revision = '%s@%s' % (DEPOT_DEPS_NAME[depot]['src'],
-              current_revision)
-        if not self.source_control.SyncToRevision(current_revision,
-            sync_client):
-          success = False
+    # Obtain a build for this revision. This may be done by requesting a build
+    # from another builder, waiting for it and downloading it.
+    start_build_time = time.time()
+    build_success = self.BuildCurrentRevision(depot, revision)
+    if not build_success:
+      return ('Failed to build revision: [%s]' % str(revision),
+              BUILD_RESULT_FAIL)
+    after_build_time = time.time()
 
-          break
+    # Possibly alter the command.
+    command = self.GetCompatibleCommand(command, revision, depot)
 
-    if success:
-      success = self.RunPostSync(depot)
-      if success:
-        if skippable and self.ShouldSkipRevision(depot, revision):
-          return ('Skipped revision: [%s]' % str(revision),
-                  BUILD_RESULT_SKIPPED)
+    # Run the command and get the results.
+    results = self.RunPerformanceTestAndParseResults(command, metric)
 
-        start_build_time = time.time()
-        if self.BuildCurrentRevision(depot, revision):
-          after_build_time = time.time()
-          # Hack to support things that got changed.
-          command_to_run = self.GetCompatibleCommand(
-              command_to_run, revision, depot)
-          results = self.RunPerformanceTestAndParseResults(command_to_run,
-                                                           metric)
-          # Restore build output directory once the tests are done, to avoid
-          # any discrepancies.
-          if self.IsDownloadable(depot) and revision:
-            self.BackupOrRestoreOutputdirectory(restore=True)
+    # Restore build output directory once the tests are done, to avoid
+    # any discrepancies.
+    if self.IsDownloadable(depot) and revision:
+      self.BackupOrRestoreOutputDirectory(restore=True)
 
-          if results[1] == 0:
-            external_revisions = self._Get3rdPartyRevisions(depot)
+    # A value other than 0 indicates that the test couldn't be run, and results
+    # should also include an error message.
+    if results[1] != 0:
+      return results
 
-            if not external_revisions is None:
-              return (results[0], results[1], external_revisions,
-                  time.time() - after_build_time, after_build_time -
-                  start_build_time)
-            else:
-              return ('Failed to parse DEPS file for external revisions.',
-                      BUILD_RESULT_FAIL)
-          else:
-            return results
-        else:
-          return ('Failed to build revision: [%s]' % str(revision),
-                  BUILD_RESULT_FAIL)
-      else:
-        return ('Failed to run [gclient runhooks].', BUILD_RESULT_FAIL)
+    external_revisions = self._Get3rdPartyRevisions(depot)
+
+    if not external_revisions is None:
+      return (results[0], results[1], external_revisions,
+          time.time() - after_build_time, after_build_time -
+          start_build_time)
     else:
-      return ('Failed to sync revision: [%s]' % str(revision),
-              BUILD_RESULT_FAIL)
+      return ('Failed to parse DEPS file for external revisions.',
+               BUILD_RESULT_FAIL)
+
+  def _SyncAllRevisions(self, revisions_to_sync, sync_client):
+    """Syncs multiple depots to particular revisions.
+
+    Args:
+      revisions_to_sync: A list of (depot, revision) pairs to be synced.
+      sync_client: Program used to sync, e.g. "gclient", "repo". Can be None.
+
+    Returns:
+      True if successful, False otherwise.
+    """
+    for depot, revision in revisions_to_sync:
+      self.ChangeToDepotWorkingDirectory(depot)
+
+      if sync_client:
+        self.PerformPreBuildCleanup()
+
+      # When using gclient to sync, you need to specify the depot you
+      # want so that all the dependencies sync properly as well.
+      # i.e. gclient sync src@<SHA1>
+      if sync_client == 'gclient':
+        revision = '%s@%s' % (DEPOT_DEPS_NAME[depot]['src'], revision)
+
+      sync_success = self.source_control.SyncToRevision(revision, sync_client)
+      if not sync_success:
+        return False
+
+    return True
 
   def _CheckIfRunPassed(self, current_value, known_good_value, known_bad_value):
     """Given known good and bad values, decide if the current_value passed
     or failed.
 
     Args:
       current_value: The value of the metric being checked.
       known_bad_value: The reference value for a "failed" run.
       known_good_value: The reference value for a "passed" run.
 
@@ skipping 148 matching lines @@
     Args:
       good_rev: The last known good revision where the performance regression
         has not occurred yet.
       bad_rev: A revision where the performance regression has already occurred.
       cmd: The command to execute the performance test.
       metric: The metric being tested for regression.
 
     Returns:
       A tuple with the results of building and running each revision.
     """
-    bad_run_results = self.SyncBuildAndRunRevision(
-        bad_rev, target_depot, cmd, metric)
+    bad_run_results = self.RunTest(bad_rev, target_depot, cmd, metric)
 
     good_run_results = None
 
     if not bad_run_results[1]:
-      good_run_results = self.SyncBuildAndRunRevision(
-          good_rev, target_depot, cmd, metric)
+      good_run_results = self.RunTest(good_rev, target_depot, cmd, metric)
 
     return (bad_run_results, good_run_results)
 
   def PrintRevisionsToBisectMessage(self, revision_list, depot):
     if self.opts.output_buildbot_annotations:
       step_name = 'Bisection Range: [%s - %s]' % (
           revision_list[len(revision_list)-1], revision_list[0])
       bisect_utils.OutputAnnotationStepStart(step_name)
 
     print
@@ skipping 363 matching lines @@
         next_revision_depot = next_revision_data['depot']
 
         self.ChangeToDepotWorkingDirectory(next_revision_depot)
 
         if self.opts.output_buildbot_annotations:
           step_name = 'Working on [%s]' % next_revision_id
           bisect_utils.OutputAnnotationStepStart(step_name)
 
         print 'Working on revision: [%s]' % next_revision_id
 
-        run_results = self.SyncBuildAndRunRevision(next_revision_id,
-                                                   next_revision_depot,
-                                                   command_to_run,
-                                                   metric, skippable=True)
+        run_results = self.RunTest(
+            next_revision_id, next_revision_depot, command_to_run, metric,
+            skippable=True)
 
         # If the build is successful, check whether or not the metric
         # had regressed.
         if not run_results[1]:
           if len(run_results) > 2:
             next_revision_data['external'] = run_results[2]
             next_revision_data['perf_time'] = run_results[3]
             next_revision_data['build_time'] = run_results[4]
 
           passed_regression = self._CheckIfRunPassed(run_results[0],
@@ skipping 815 matching lines @@
       # bugs. If you change this, please update the perf dashboard as well.
       bisect_utils.OutputAnnotationStepStart('Results')
     print 'Error: %s' % e.message
     if opts.output_buildbot_annotations:
       bisect_utils.OutputAnnotationStepClosed()
   return 1
 
 
 if __name__ == '__main__':
   sys.exit(main())