Move bisect-perf-regression.py into auto_bisect directory.

The purpose of this is to continue gathering all the related code together, and to make it easier to run pylint/tests for the main module which still contains most of the code.

After doing this, pylint and the tests caught a couple of things -- one unused variable name, and after the script changed directory, some of the paths had to be modified as well.

BUG=

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

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

tools/auto_bisect/PRESUBMIT.py

@@ -86,5 +86,8 @@ def _RunUnitTests(input_api, output_api):
 
 def _RunPyLint(input_api, output_api):
   """Runs unit tests for auto-bisect."""
-  tests = input_api.canned_checks.GetPylint(input_api, output_api)
+  telemetry_path = os.path.join(
+      input_api.PresubmitLocalPath(), os.path.pardir, 'telemetry')
+  tests = input_api.canned_checks.GetPylint(
+      input_api, output_api, extra_paths_list=[telemetry_path])
   return input_api.RunTests(tests)

tools/auto_bisect/README

@@ -8,17 +8,11 @@ Documentation:
   http://www.chromium.org/developers/bisecting-bugs
   http://www.chromium.org/developers/tree-sheriffs/perf-sheriffs/bisecting-performance-regressions
 
-Overview of bisect-related files:
-  src/tools/run-bisect-perf-regression.py
-     -- the script used to kick off a normal performance regression bisect job.
-  src/tools/auto_bisect/bisect.cfg:
-     -- this file contains parameters for a bisect job, and is read by other
-        modules including run-bisect-perf-regression.py.
-  src/tools/run-bisect-manual-test.py
-     -- a script which is used to manually bisect regressions; this also
-        depends on bisect-perf-regression.py.
-  src/tools/bisect-perf-regression.py
-     -- the main module which the others depend on.
-  src/tools/bisect-manual-test.py
-     -- a helper module used when manually bisect regressions.
+Overview of bisect-related files in src/tools:
+  run-bisect-perf-regression.py -- used to kick off a bisect job
+  prepare-bisect-perf-regression.py -- run before the above to prepare the repo
+  run-bisect-manual-test.py -- used to manually bisect
+  bisect-manual-test.py -- helper module used by run-bisect-manual-test.py
+  auto_bisect/bisect.cfg -- config parameters for a bisect job
+  run-perf-test.cfg -- config parameters running a perf test once
 

tools/bisect-perf-regression.py → tools/auto_bisect/bisect_perf_regression.py

@@ -15,7 +15,7 @@ range.
 
 Example usage using SVN revisions:
 
-./tools/bisect-perf-regression.py -c\
+./tools/bisect_perf_regression.py -c\
   "out/Release/performance_ui_tests --gtest_filter=ShutdownTest.SimpleUserQuit"\
   -g 168222 -b 168232 -m shutdown/simple-user-quit
 
@@ -25,7 +25,7 @@ revision were merged in.
 
 Example usage using git hashes:
 
-./tools/bisect-perf-regression.py -c\
+./tools/bisect_perf_regression.py -c\
   "out/Release/performance_ui_tests --gtest_filter=ShutdownTest.SimpleUserQuit"\
   -g 1f6e67861535121c5c819c16a666f2436c207e7b\
   -b b732f23b4f81c382db0b23b9035f3dadc7d925bb\
@@ -47,14 +47,15 @@ import sys
 import time
 import zipfile
 
-sys.path.append(os.path.join(os.path.dirname(__file__), 'telemetry'))
+sys.path.append(os.path.join(
+    os.path.dirname(__file__), os.path.pardir, 'telemetry'))
 
-from auto_bisect import bisect_utils
-from auto_bisect import builder
-from auto_bisect import math_utils
-from auto_bisect import request_build
-from auto_bisect import source_control as source_control_module
-from auto_bisect import ttest
+import bisect_utils
+import builder
+import math_utils
+import request_build
+import source_control as source_control_module
+import ttest
 from telemetry.util import cloud_storage
 
 # Below is the map of "depot" names to information about each depot. Each depot
@@ -150,6 +151,9 @@ DEPOT_DEPS_NAME = {
 
 DEPOT_NAMES = DEPOT_DEPS_NAME.keys()
 
+# The script is in chromium/src/tools/auto_bisect. Throughout this script,
+# we use paths to other things in the chromium/src repository.
+
 CROS_CHROMEOS_PATTERN = 'chromeos-base/chromeos-chrome'
 
 # Possible return values from BisectPerformanceMetrics.RunTest.
@@ -913,6 +917,10 @@ class BisectPerformanceMetrics(object):
 
     self.opts = opts
     self.source_control = source_control
+
+    # The src directory here is NOT the src/ directory for the repository
+    # where the bisect script is running from. Instead, it's the src/ directory
+    # inside the bisect/ directory which is created before running.
     self.src_cwd = os.getcwd()
     self.cros_cwd = os.path.join(os.getcwd(), '..', 'cros')
     self.depot_cwd = {}
@@ -920,12 +928,11 @@ class BisectPerformanceMetrics(object):
     self.warnings = []
     self.builder = builder.Builder.FromOpts(opts)
 
-    for d in DEPOT_NAMES:
+    for depot in DEPOT_NAMES:
       # The working directory of each depot is just the path to the depot, but
       # since we're already in 'src', we can skip that part.
-
-      self.depot_cwd[d] = os.path.join(
-          self.src_cwd, DEPOT_DEPS_NAME[d]['src'][4:])
+      self.depot_cwd[depot] = os.path.join(
+          self.src_cwd, DEPOT_DEPS_NAME[depot]['src'][4:])
 
   def PerformCleanup(self):
     """Performs cleanup when script is finished."""
@@ -1090,8 +1097,8 @@ class BisectPerformanceMetrics(object):
           depot_data_src = depot_data.get('src') or depot_data.get('src_old')
           src_dir = deps_data.get(depot_data_src)
           if src_dir:
-            self.depot_cwd[depot_name] = os.path.join(self.src_cwd,
-                                                      depot_data_src[4:])
+            self.depot_cwd[depot_name] = os.path.join(
+                self.src_cwd, depot_data_src[4:])
             re_results = rxp.search(src_dir)
             if re_results:
               results[depot_name] = re_results.group('revision')
@@ -1567,11 +1574,14 @@ class BisectPerformanceMetrics(object):
     return self.opts.bisect_mode in [BISECT_MODE_STD_DEV]
 
   def GetCompatibleCommand(self, command_to_run, revision, depot):
-    # Prior to crrev.com/274857 *only* android-chromium-testshell
-    # Then until crrev.com/276628 *both* (android-chromium-testshell and
-    # android-chrome-shell) work. After that rev 276628 *only*
-    # android-chrome-shell works. bisect-perf-regression.py script should
-    # handle these cases and set appropriate browser type based on revision.
+    """Return a possibly modified test command depending on the revision.
+
+    Prior to crrev.com/274857 *only* android-chromium-testshell
+    Then until crrev.com/276628 *both* (android-chromium-testshell and
+    android-chrome-shell) work. After that rev 276628 *only*
+    android-chrome-shell works. The bisect_perf_regression.py script should
+    handle these cases and set appropriate browser type based on revision.
+    """
     if self.opts.target_platform in ['android']:
       # When its a third_party depot, get the chromium revision.
       if depot != 'chromium':
@@ -2283,7 +2293,7 @@ class BisectPerformanceMetrics(object):
       this will contain the field "error", otherwise None.
     """
     if self.opts.target_platform == 'android':
-      revision_to_check = self.source_control.GetCommitPosition(good_revision)
+      good_revision = self.source_control.GetCommitPosition(good_revision)
       if (bisect_utils.IsStringInt(good_revision)
           and good_revision < 265549):
         return {'error': (

tools/bisect-perf-regression_test.py → tools/auto_bisect/bisect_perf_regression_test.py

@@ -6,10 +6,8 @@ import os
 import re
 import unittest
 
-from auto_bisect import source_control as source_control_module
-
-# Special import necessary because filename contains dash characters.
-bisect_perf_module = __import__('bisect-perf-regression')
+import bisect_perf_regression
+import source_control as source_control_module
 
 def _GetBisectPerformanceMetricsInstance():
   """Returns an instance of the BisectPerformanceMetrics class."""
@@ -22,13 +20,13 @@ def _GetBisectPerformanceMetricsInstance():
     'good_revision': 280000,
     'bad_revision': 280005,
   }
-  bisect_options = bisect_perf_module.BisectOptions.FromDict(options_dict)
+  bisect_options = bisect_perf_regression.BisectOptions.FromDict(options_dict)
   source_control = source_control_module.DetermineAndCreateSourceControl(
       bisect_options)
-  bisect_instance = bisect_perf_module.BisectPerformanceMetrics(
+  bisect_instance = bisect_perf_regression.BisectPerformanceMetrics(
       source_control, bisect_options)
   bisect_instance.src_cwd = os.path.abspath(
-      os.path.join(os.path.dirname(__file__), os.path.pardir))
+      os.path.join(os.path.dirname(__file__), os.path.pardir, os.path.pardir))
   return bisect_instance
 
 
@@ -49,7 +47,7 @@ class BisectPerfRegressionTest(unittest.TestCase):
     """
     # ConfidenceScore takes a list of lists but these lists are flattened
     # inside the function.
-    confidence = bisect_perf_module.ConfidenceScore(
+    confidence = bisect_perf_regression.ConfidenceScore(
         [[v] for v in bad_values],
         [[v] for v in good_values])
     self.assertEqual(score, confidence)
@@ -128,7 +126,7 @@ class BisectPerfRegressionTest(unittest.TestCase):
     }
     # Testing private function.
     # pylint: disable=W0212
-    vars_dict = bisect_perf_module._ParseRevisionsFromDEPSFileManually(
+    vars_dict = bisect_perf_regression._ParseRevisionsFromDEPSFileManually(
         deps_file_contents)
     self.assertEqual(vars_dict, expected_vars_dict)
 
@@ -140,7 +138,8 @@ class BisectPerfRegressionTest(unittest.TestCase):
     metric = ['my_chart', 'my_trace']
     # Testing private function.
     # pylint: disable=W0212
-    values = bisect_perf_module._TryParseResultValuesFromOutput(metric, results)
+    values = bisect_perf_regression._TryParseResultValuesFromOutput(
+        metric, results)
     self.assertEqual(expected_values, values)
 
   def testTryParseResultValuesFromOutput_WithSingleValue(self):
@@ -192,15 +191,15 @@ class BisectPerfRegressionTest(unittest.TestCase):
       Prior to r274857, only android-chromium-testshell works.
       In the range [274857, 276628], both work.
     """
-    bisect_options = bisect_perf_module.BisectOptions()
+    bisect_options = bisect_perf_regression.BisectOptions()
     bisect_options.output_buildbot_annotations = None
     source_control = source_control_module.DetermineAndCreateSourceControl(
         bisect_options)
-    bisect_instance = bisect_perf_module.BisectPerformanceMetrics(
+    bisect_instance = bisect_perf_regression.BisectPerformanceMetrics(
         source_control, bisect_options)
     bisect_instance.opts.target_platform = target_platform
     git_revision = bisect_instance.source_control.ResolveToRevision(
-        revision, 'chromium', bisect_perf_module.DEPOT_DEPS_NAME, 100)
+        revision, 'chromium', bisect_perf_regression.DEPOT_DEPS_NAME, 100)
     depot = 'chromium'
     command = bisect_instance.GetCompatibleCommand(
         original_command, git_revision, depot)
@@ -268,7 +267,7 @@ class BisectPerfRegressionTest(unittest.TestCase):
   def testGetCommitPositionForV8(self):
     bisect_instance = _GetBisectPerformanceMetricsInstance()
     v8_rev = '21d700eedcdd6570eff22ece724b63a5eefe78cb'
-    depot_path = os.path.join(bisect_instance.src_cwd, 'src', 'v8')
+    depot_path = os.path.join(bisect_instance.src_cwd, 'v8')
     self.assertEqual(
         23634,
         bisect_instance.source_control.GetCommitPosition(v8_rev, depot_path))
@@ -276,8 +275,7 @@ class BisectPerfRegressionTest(unittest.TestCase):
   def testGetCommitPositionForWebKit(self):
     bisect_instance = _GetBisectPerformanceMetricsInstance()
     wk_rev = 'a94d028e0f2c77f159b3dac95eb90c3b4cf48c61'
-    depot_path = os.path.join(bisect_instance.src_cwd, 'src', 'third_party',
-                              'WebKit')
+    depot_path = os.path.join(bisect_instance.src_cwd, 'third_party', 'WebKit')
     self.assertEqual(
         181660,
         bisect_instance.source_control.GetCommitPosition(wk_rev, depot_path))
@@ -290,7 +288,7 @@ class BisectPerfRegressionTest(unittest.TestCase):
     # to search is not changed in DEPS content.
     # TODO (prasadv): Add a separate test to validate the DEPS contents with the
     # format that bisect script expects.
-    deps_contents = bisect_perf_module.ReadStringFromFile(deps_file)
+    deps_contents = bisect_perf_regression.ReadStringFromFile(deps_file)
     deps_key = 'v8_revision'
     depot = 'v8'
     git_revision = 'a12345789a23456789a123456789a123456789'

tools/auto_bisect/bisect_utils.py

@@ -440,7 +440,6 @@ def RunGit(command, cwd=None):
     A tuple of the output and return code.
   """
   command = ['git'] + command
-
   return RunProcessAndRetrieveOutput(command, cwd=cwd)
 
 

tools/auto_bisect/source_control.py

@@ -6,7 +6,7 @@
 
 import os
 
-from . import bisect_utils
+import bisect_utils
 
 CROS_VERSION_PATTERN = 'new version number from %s'
 

tools/run-bisect-manual-test.py

@@ -14,8 +14,8 @@ to setup the sandbox manually before running the script. Otherwise the script
 fails to launch Chrome and exits with an error.
 
 This script serves a similar function to bisect-builds.py, except it uses
-the bisect-perf-regression.py. This means that that it can actually check out
-and build revisions of Chromium that are not available in cloud storage.
+the bisect_perf_regression.py. This means that that it can obtain builds of
+Chromium for revisions where builds aren't available in cloud storage.
 """
 
 import os
@@ -24,26 +24,28 @@ import sys
 
 CROS_BOARD_ENV = 'BISECT_CROS_BOARD'
 CROS_IP_ENV = 'BISECT_CROS_IP'
-_DIR_TOOLS_ROOT = os.path.abspath(os.path.dirname(__file__))
+_TOOLS_DIR = os.path.abspath(os.path.dirname(__file__))
+_BISECT_SCRIPT_PATH = os.path.join(
+    _TOOLS_DIR, 'auto_bisect', 'bisect_perf_regression.py')
 
-sys.path.append(os.path.join(_DIR_TOOLS_ROOT, 'telemetry'))
+sys.path.append(os.path.join(_TOOLS_DIR, 'telemetry'))
 from telemetry.core import browser_options
 
 
 def _RunBisectionScript(options):
-  """Attempts to execute src/tools/bisect-perf-regression.py.
+  """Attempts to execute the bisect script (bisect_perf_regression.py).
 
   Args:
     options: The configuration options to pass to the bisect script.
 
   Returns:
-    The exit code of bisect-perf-regression.py: 0 on success, otherwise 1.
+    An exit code; 0 for success, 1 for failure.
   """
   test_command = ('python %s --browser=%s --chrome-root=.' %
-                  (os.path.join(_DIR_TOOLS_ROOT, 'bisect-manual-test.py'),
+                  (os.path.join(_TOOLS_DIR, 'bisect-manual-test.py'),
                    options.browser_type))
 
-  cmd = ['python', os.path.join(_DIR_TOOLS_ROOT, 'bisect-perf-regression.py'),
+  cmd = ['python', _BISECT_SCRIPT_PATH,
          '-c', test_command,
          '-g', options.good_revision,
          '-b', options.bad_revision,
@@ -82,8 +84,7 @@ def _RunBisectionScript(options):
   return_code = subprocess.call(cmd)
 
   if return_code:
-    print ('Error: bisect-perf-regression.py returned with error %d' %
-           return_code)
+    print 'Error: bisect_perf_regression.py had exit code %d.' % return_code
     print
 
   return return_code

tools/run-bisect-perf-regression.py

@@ -5,13 +5,12 @@
 
 """Run Performance Test Bisect Tool
 
-This script is used by a try bot to run the src/tools/bisect-perf-regression.py
-script with the parameters specified in src/tools/auto_bisect/bisect.cfg.
-It will check out a copy of the depot in a subdirectory 'bisect' of the working
-directory provided, and run the bisect-perf-regression.py script there.
+This script is used by a try bot to run the bisect script with the parameters
+specified in the bisect config file. It checks out a copy of the depot in
+a subdirectory 'bisect' of the working directory provided, annd runs the
+bisect scrip there.
 """
 
-import imp
 import optparse
 import os
 import platform
@@ -19,22 +18,20 @@ import subprocess
 import sys
 import traceback
 
+from auto_bisect import bisect_perf_regression
 from auto_bisect import bisect_utils
 from auto_bisect import math_utils
 
-bisect = imp.load_source('bisect-perf-regression',
-    os.path.join(os.path.abspath(os.path.dirname(sys.argv[0])),
-        'bisect-perf-regression.py'))
-
-
 CROS_BOARD_ENV = 'BISECT_CROS_BOARD'
 CROS_IP_ENV = 'BISECT_CROS_IP'
 
-# Default config file paths, relative to this script.
-BISECT_REGRESSION_CONFIG = os.path.join('auto_bisect', 'bisect.cfg')
-RUN_TEST_CONFIG = 'run-perf-test.cfg'
-WEBKIT_RUN_TEST_CONFIG = os.path.join(
-    '..', 'third_party', 'WebKit', 'Tools', 'run-perf-test.cfg')
+SCRIPT_DIR = os.path.dirname(__file__)
+SRC_DIR = os.path.join(SCRIPT_DIR, os.path.pardir)
+BISECT_CONFIG_PATH = os.path.join(SCRIPT_DIR, 'auto_bisect', 'bisect.cfg')
+RUN_TEST_CONFIG_PATH = os.path.join(SCRIPT_DIR, 'run-perf-test.cfg')
+WEBKIT_RUN_TEST_CONFIG_PATH = os.path.join(
+    SRC_DIR, 'third_party', 'WebKit', 'Tools', 'run-perf-test.cfg')
+BISECT_SCRIPT_DIR = os.path.join(SCRIPT_DIR, 'auto_bisect')
 
 
 class Goma(object):
@@ -230,26 +227,25 @@ def _CreateBisectOptionsFromConfig(config):
     else:
       opts_dict['target_platform'] = 'android'
 
-  return bisect.BisectOptions.FromDict(opts_dict)
+  return bisect_perf_regression.BisectOptions.FromDict(opts_dict)
 
 
-def _RunPerformanceTest(config, path_to_file):
+def _RunPerformanceTest(config):
   """Runs a performance test with and without the current patch.
 
   Args:
     config: Contents of the config file, a dictionary.
-    path_to_file: Path to the bisect-perf-regression.py script.
 
   Attempts to build and run the current revision with and without the
   current patch, with the parameters passed in.
   """
   # Bisect script expects to be run from the src directory
-  os.chdir(os.path.join(path_to_file, '..'))
+  os.chdir(SRC_DIR)
 
   bisect_utils.OutputAnnotationStepStart('Building With Patch')
 
   opts = _CreateBisectOptionsFromConfig(config)
-  b = bisect.BisectPerformanceMetrics(None, opts)
+  b = bisect_perf_regression.BisectPerformanceMetrics(None, opts)
 
   if bisect_utils.RunGClient(['runhooks']):
     raise RuntimeError('Failed to run gclient runhooks')
@@ -339,17 +335,16 @@ def _RunPerformanceTest(config, path_to_file):
     bisect_utils.OutputAnnotationStepLink('HTML Results', cloud_file_link)
 
 
-def _SetupAndRunPerformanceTest(config, path_to_file, path_to_goma):
+def _SetupAndRunPerformanceTest(config, path_to_goma):
   """Attempts to build and run the current revision with and without the
   current patch, with the parameters passed in.
 
   Args:
     config: The config read from run-perf-test.cfg.
-    path_to_file: Path to the bisect-perf-regression.py script.
     path_to_goma: Path to goma directory.
 
   Returns:
-    The exit code of bisect-perf-regression.py: 0 on success, otherwise 1.
+    An exit code: 0 on success, otherwise 1.
   """
   if platform.release() == 'XP':
     print 'Windows XP is not supported for perf try jobs because it lacks '
@@ -360,7 +355,7 @@ def _SetupAndRunPerformanceTest(config, path_to_file, path_to_goma):
       config['use_goma'] = bool(path_to_goma)
       if config['use_goma']:
         config['goma_dir'] = os.path.abspath(path_to_goma)
-      _RunPerformanceTest(config, path_to_file)
+      _RunPerformanceTest(config)
     return 0
   except RuntimeError, e:
     bisect_utils.OutputAnnotationStepClosed()
@@ -369,16 +364,13 @@ def _SetupAndRunPerformanceTest(config, path_to_file, path_to_goma):
 
 
 def _RunBisectionScript(
-    config, working_directory, path_to_file, path_to_goma, path_to_extra_src,
-    dry_run):
-  """Attempts to execute bisect-perf-regression.py with the given parameters.
+    config, working_directory, path_to_goma, path_to_extra_src, dry_run):
+  """Attempts to execute the bisect script with the given parameters.
 
   Args:
     config: A dict containing the parameters to pass to the script.
-    working_directory: A working directory to provide to the
-      bisect-perf-regression.py script, where it will store it's own copy of
-      the depot.
-    path_to_file: Path to the bisect-perf-regression.py script.
+    working_directory: A working directory to provide to the bisect script,
+      where it will store it's own copy of the depot.
     path_to_goma: Path to goma directory.
     path_to_extra_src: Path to extra source file.
     dry_run: Do a dry run, skipping sync, build, and performance testing steps.
@@ -388,7 +380,7 @@ def _RunBisectionScript(
   """
   _PrintConfigStep(config)
 
-  cmd = ['python', os.path.join(path_to_file, 'bisect-perf-regression.py'),
+  cmd = ['python', os.path.join(BISECT_SCRIPT_DIR, 'bisect_perf_regression.py'),
          '-c', config['command'],
          '-g', config['good_revision'],
          '-b', config['bad_revision'],
@@ -469,7 +461,7 @@ def _RunBisectionScript(
     return_code = subprocess.call(cmd)
 
   if return_code:
-    print ('Error: bisect-perf-regression.py returned with error %d\n'
+    print ('Error: bisect_perf_regression.py returned with error %d\n'
            % return_code)
 
   return return_code
@@ -525,14 +517,11 @@ def main():
   just run a performance test, depending on the particular config parameters
   specified in the config file.
   """
-
   parser = _OptionParser()
   opts, _ = parser.parse_args()
 
-  current_dir = os.path.abspath(os.path.dirname(sys.argv[0]))
-
   # Use the default config file path unless one was specified.
-  config_path = os.path.join(current_dir, BISECT_REGRESSION_CONFIG)
+  config_path = BISECT_CONFIG_PATH
   if opts.path_to_config:
     config_path = opts.path_to_config
   config = _LoadConfigFile(config_path)
@@ -547,13 +536,13 @@ def main():
       return 1
 
     return _RunBisectionScript(
-        config, opts.working_directory, current_dir,
-        opts.path_to_goma, opts.extra_src, opts.dry_run)
+        config, opts.working_directory, opts.path_to_goma, opts.extra_src,
+        opts.dry_run)
 
   # If it wasn't valid for running a bisect, then maybe the user wanted
   # to run a perf test instead of a bisect job. Try reading any possible
   # perf test config files.
-  perf_cfg_files = [RUN_TEST_CONFIG, WEBKIT_RUN_TEST_CONFIG]
+  perf_cfg_files = [RUN_TEST_CONFIG_PATH, WEBKIT_RUN_TEST_CONFIG_PATH]
   for current_perf_cfg_file in perf_cfg_files:
     if opts.path_to_config:
       path_to_perf_cfg = opts.path_to_config
@@ -566,12 +555,10 @@ def main():
     config_is_valid = _ValidatePerfConfigFile(config)
 
     if config and config_is_valid:
-      return _SetupAndRunPerformanceTest(
-          config, current_dir, opts.path_to_goma)
+      return _SetupAndRunPerformanceTest(config, opts.path_to_goma)
 
   print ('Error: Could not load config file. Double check your changes to '
-         'auto_bisect/bisect.cfg or run-perf-test.cfg for syntax '
-         'errors.\n')
+         'auto_bisect/bisect.cfg or run-perf-test.cfg for syntax errors.\n')
   return 1