Refactoring and minor corrections:

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 2192 matching lines @@
     Returns:
       A tuple with the new bad and good revisions.
     """
     # DONOT perform nudge because at revision 291563 .DEPS.git was removed
     # and source contain only DEPS file for dependency changes.
     if good_svn_revision >= 291563:
       return (bad_revision, good_revision)
 
     if self.source_control.IsGit() and self.opts.target_platform == 'chromium':
       changes_to_deps = self.source_control.QueryFileRevisionHistory(
-          'DEPS', good_revision, bad_revision)
+          FILE_DEPS, good_revision, bad_revision)
 
       if changes_to_deps:
         # DEPS file was changed, search from the oldest change to DEPS file to
         # bad_revision to see if there are matching .DEPS.git changes.
         oldest_deps_change = changes_to_deps[-1]
         changes_to_gitdeps = self.source_control.QueryFileRevisionHistory(
             bisect_utils.FILE_DEPS_GIT, oldest_deps_change, bad_revision)
 
         if len(changes_to_deps) != len(changes_to_gitdeps):
           # Grab the timestamp of the last DEPS change
@@ skipping 22 matching lines @@
     """Checks that |good_revision| is an earlier revision than |bad_revision|.
 
     Args:
       good_revision: Number/tag of the known good revision.
       bad_revision: Number/tag of the known bad revision.
 
     Returns:
       True if the revisions are in the proper order (good earlier than bad).
     """
     if self.source_control.IsGit() and target_depot != 'cros':
-      cmd = ['log', '--format=%ct', '-1', good_revision]
       cwd = self._GetDepotDirectory(target_depot)
 
+      cmd = ['log', '--format=%ct', '-1', good_revision]

[qyearsley, 2014/09/15 20:30:40]

Does this make a difference?

[Sergiy Byelozyorov, 2014/09/16 11:24:42]

Not functionally, but it is consistent with grouping of statements below (cmd
goes before CheckRunGit), while cwd should be separate as it's used in both
pieces.

[qyearsley, 2014/09/16 18:59:18]

I see; makes sense.

[Sergiy Byelozyorov, 2014/09/17 15:45:18]

Acknowledged.

       output = bisect_utils.CheckRunGit(cmd, cwd=cwd)
       good_commit_time = int(output)
 
       cmd = ['log', '--format=%ct', '-1', bad_revision]
       output = bisect_utils.CheckRunGit(cmd, cwd=cwd)
       bad_commit_time = int(output)
 
       return good_commit_time <= bad_commit_time
     else:
       # CrOS and SVN use integers.
@@ skipping 1015 matching lines @@
 
         if not opts.cros_board:
           raise RuntimeError('missing required parameter: --cros_board')
 
         if not opts.cros_remote_ip:
           raise RuntimeError('missing required parameter: --cros_remote_ip')
 
         if not opts.working_directory:
           raise RuntimeError('missing required parameter: --working_directory')
 
-      metric_values = opts.metric.split('/')
-      if (len(metric_values) != 2 and
-          opts.bisect_mode != BISECT_MODE_RETURN_CODE):
-        raise RuntimeError('Invalid metric specified: [%s]' % opts.metric)
+      if opts.bisect_mode != BISECT_MODE_RETURN_CODE:
+        metric_values = opts.metric.split('/')
+        if len(metric_values) != 2:
+          raise RuntimeError('Invalid metric specified: [%s]' % opts.metric)
+        opts.metric = metric_values
 
-      opts.metric = metric_values
       opts.repeat_test_count = min(max(opts.repeat_test_count, 1), 100)
       opts.max_time_minutes = min(max(opts.max_time_minutes, 1), 60)
       opts.truncate_percent = min(max(opts.truncate_percent, 0), 25)
       opts.truncate_percent = opts.truncate_percent / 100.0
 
       for k, v in opts.__dict__.iteritems():
         assert hasattr(self, k), 'Invalid %s attribute in BisectOptions.' % k
         setattr(self, k, v)
     except RuntimeError, e:
       output_string = StringIO.StringIO()
@@ skipping 10 matching lines @@
       values: a dict containing options to set.
 
     Returns:
       An instance of BisectOptions.
     """
     opts = BisectOptions()
     for k, v in values.iteritems():
       assert hasattr(opts, k), 'Invalid %s attribute in BisectOptions.' % k
       setattr(opts, k, v)
 
-    if opts.metric:
+    if opts.metric and opts.bisect_mode != BISECT_MODE_RETURN_CODE:
       metric_values = opts.metric.split('/')
       if len(metric_values) != 2:
         raise RuntimeError('Invalid metric specified: [%s]' % opts.metric)
       opts.metric = metric_values
 
     opts.repeat_test_count = min(max(opts.repeat_test_count, 1), 100)
     opts.max_time_minutes = min(max(opts.max_time_minutes, 1), 60)
     opts.truncate_percent = min(max(opts.truncate_percent, 0), 25)
     opts.truncate_percent = opts.truncate_percent / 100.0
 
@@ skipping 57 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())