In the presubmit for auto-bisect, run pylint and unit tests.

Note: This only runs pylint and tests for the auto_bisect directory (this was much simpler than picking and choosing particular files from the parent tools directory).

I think the next step is to move bisect-perf-regression.py and related files into the auto_bisect directory.

BUG=

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

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

tools/PRESUBMIT.py → tools/auto_bisect/PRESUBMIT.py

@@ -5,10 +5,11 @@
 """Top-level presubmit script for auto-bisect.
 
 See http://dev.chromium.org/developers/how-tos/depottools/presubmit-scripts for
-details on the presubmit API built into gcl.
+details on the presubmit API.
 """
 
 import imp
+import subprocess
 import os
 
 # Paths to bisect config files relative to src/tools.
@@ -17,10 +18,6 @@ CONFIG_FILES = [
     'run-perf-test.cfg'
 ]
 
-PYLINT_BLACKLIST = []
-PYLINT_DISABLED_WARNINGS = []
-
-
 def CheckChangeOnUpload(input_api, output_api):
   return _CommonChecks(input_api, output_api)
 
@@ -31,10 +28,10 @@ def CheckChangeOnCommit(input_api, output_api):
 
 def _CommonChecks(input_api, output_api):
   """Does all presubmit checks for auto-bisect."""
-  # TODO(qyearsley) Run bisect unit test.
-  # TODO(qyearsley) Run pylint on all auto-bisect py files but not other files.
   results = []
   results.extend(_CheckAllConfigFiles(input_api, output_api))
+  results.extend(_RunUnitTests(input_api, output_api))
+  results.extend(_RunPyLint(input_api, output_api))
   return results
 
 
@@ -64,7 +61,7 @@ def _CheckConfigFile(file_path, output_api):
     warning = 'Config file "config" global variable is not dict: %s' % str(e)
     return [output_api.PresubmitError(warning, items=[file_path])]
 
-  for k, v in config_dict.iteritems():
+  for k, v in config_file.config.iteritems():
     if v != '':
       warning = 'Non-empty value in config dict: %s: %s' % (repr(k), repr(v))
       warning += ('\nThe bisection config file should only contain a config '
@@ -73,3 +70,21 @@ def _CheckConfigFile(file_path, output_api):
       return [output_api.PresubmitError(warning, items=[file_path])]
 
   return []
+
+
+def _RunUnitTests(input_api, output_api):
+  """Runs unit tests for auto-bisect."""
+  repo_root = input_api.change.RepositoryRoot()
+  auto_bisect_dir = os.path.join(repo_root, 'tools', 'auto_bisect')
+  test_runner = os.path.join(auto_bisect_dir, 'run_tests')
+  return_code = subprocess.call(['python', test_runner])
+  if return_code:
+    message = 'Auto-bisect unit tests did not all pass.'
+    return [output_api.PresubmitError(message)]
+  return []
+
+
+def _RunPyLint(input_api, output_api):
+  """Runs unit tests for auto-bisect."""
+  tests = input_api.canned_checks.GetPylint(input_api, output_api)
+  return input_api.RunTests(tests)

tools/auto_bisect/builder.py

@@ -142,6 +142,9 @@ class AndroidBuilder(Builder):
   def __init__(self, opts):
     super(AndroidBuilder, self).__init__(opts)
 
+  # TODO(qyearsley): Make this a class method and verify that it works with
+  # a unit test.
+  # pylint: disable=R0201
   def _GetTargets(self):
     """Returns a list of build targets."""
     return ['chrome_shell_apk', 'cc_perftests_apk', 'android_tools']
@@ -179,6 +182,9 @@ class AndroidChromeBuilder(AndroidBuilder):
   def __init__(self, opts):
     super(AndroidChromeBuilder, self).__init__(opts)
 
+  # TODO(qyearsley): Make this a class method and verify that it works with
+  # a unit test.
+  # pylint: disable=R0201
   def _GetTargets(self):
     """Returns a list of build targets."""
     return AndroidBuilder._GetTargets(self) + ['chrome_apk']

tools/auto_bisect/run_tests

+#!/usr/bin/env python
+# Copyright 2014 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.
+
+"""Runs all tests in all unit test modules in this directory."""
+
+import os
+import sys
+import unittest
+
+
+def main():
+  suite = unittest.TestSuite()
+  loader = unittest.TestLoader()
+
+  # Add all tests in the directory.
+  script_dir = os.path.dirname(__file__)
+  suite.addTests(loader.discover(start_dir=script_dir, pattern='*_test.py'))
+
+  print 'Running unit tests in %s...' % os.path.abspath(script_dir)
+  result = unittest.TextTestRunner(verbosity=1).run(suite)
+  return 0 if result.wasSuccessful() else 1
+
+
+if __name__ == '__main__':
+  sys.exit(main())

tools/auto_bisect/source_control.py

@@ -28,6 +28,9 @@ def DetermineAndCreateSourceControl(opts):
   return None
 
 
+# TODO(qyearsley): Almost all of the methods below could be top-level functions
+# (or class methods). Refactoring may make this simpler.
+# pylint: disable=R0201
 class SourceControl(object):
   """SourceControl is an abstraction over the source control system."""
 

tools/auto_bisect/ttest_test.py

@@ -9,6 +9,8 @@ import unittest
 import ttest
 
 
+# This test case accesses private functions of the ttest module.
+# pylint: disable=W0212
 class TTestTest(unittest.TestCase):
   """Tests for the t-test functions."""
 

tools/bisect-perf-regression.py

@@ -539,14 +539,14 @@ def _WaitUntilBuildIsReady(
       last_status_check = time.time()
       if not build_num:
         # Get the build number on try server for the current build.
-        build_num = bisect_builder.GetBuildNumFromBuilder(
+        build_num = request_build.GetBuildNumFromBuilder(
             build_request_id, bot_name, builder_host, builder_port)
       # Check the status of build using the build number.
       # Note: Build is treated as PENDING if build number is not found
       # on the the try server.
-      build_status, status_link = bisect_builder.GetBuildStatus(
+      build_status, status_link = request_build.GetBuildStatus(
           build_num, bot_name, builder_host, builder_port)
-      if build_status == bisect_builder.FAILED:
+      if build_status == request_build.FAILED:
         return (None, 'Failed to produce build, log: %s' % status_link)
     elapsed_time = time.time() - start_time
     if elapsed_time > max_timeout:
@@ -1369,7 +1369,7 @@ class BisectPerformanceMetrics(object):
     if patch:
       job_args['patch'] = patch
     # Posts job to build the revision on the server.
-    if bisect_builder.PostTryJob(builder_host, builder_port, job_args):
+    if request_build.PostTryJob(builder_host, builder_port, job_args):
       target_file, error_msg = _WaitUntilBuildIsReady(
           fetch_build, bot_name, builder_host, builder_port, build_request_id,
           build_timeout)