Fix style in tools/auto_bisect.

BUG=

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

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

tools/auto_bisect/PRESUBMIT.py

@@ -18,6 +18,7 @@ CONFIG_FILES = [
     os.path.join(os.path.pardir, 'run-perf-test.cfg'),
 ]
 
+
 def CheckChangeOnUpload(input_api, output_api):
   return _CommonChecks(input_api, output_api)
 
@@ -93,7 +94,7 @@ def _RunPyLint(input_api, output_api):
       'third_party', 'pymock')
   disabled_warnings = [
       'relative-import',
-      ]
+  ]
   tests = input_api.canned_checks.GetPylint(
       input_api, output_api, disabled_warnings=disabled_warnings,
       extra_paths_list=[telemetry_path, mock_path])

tools/auto_bisect/bisect_perf_regression_test.py

@@ -70,31 +70,31 @@ CLEAR_REGRESSION = [
 MULTIPLE_VALUES = [
     [
         [18.916, 22.371, 8.527, 5.877, 5.407, 9.476, 8.100, 5.334,
-        4.507, 4.842, 8.485, 8.308, 27.490, 4.560, 4.804, 23.068, 17.577,
-        17.346, 26.738, 60.330, 32.307, 5.468, 27.803, 27.373, 17.823,
-        5.158, 27.439, 5.236, 11.413],
+         4.507, 4.842, 8.485, 8.308, 27.490, 4.560, 4.804, 23.068, 17.577,
+         17.346, 26.738, 60.330, 32.307, 5.468, 27.803, 27.373, 17.823,
+         5.158, 27.439, 5.236, 11.413],
         [18.999, 22.642, 8.158, 5.995, 5.495, 9.499, 8.092, 5.324,
-        4.468, 4.788, 8.248, 7.853, 27.533, 4.410, 4.622, 22.341, 22.313,
-        17.072, 26.731, 57.513, 33.001, 5.500, 28.297, 27.277, 26.462,
-        5.009, 27.361, 5.130, 10.955]
+         4.468, 4.788, 8.248, 7.853, 27.533, 4.410, 4.622, 22.341, 22.313,
+         17.072, 26.731, 57.513, 33.001, 5.500, 28.297, 27.277, 26.462,
+         5.009, 27.361, 5.130, 10.955]
     ],
     [
         [18.238, 22.365, 8.555, 5.939, 5.437, 9.463, 7.047, 5.345, 4.517,
-        4.796, 8.593, 7.901, 27.499, 4.378, 5.040, 4.904, 4.816, 4.828,
-        4.853, 57.363, 34.184, 5.482, 28.190, 27.290, 26.694, 5.099,
-        4.905, 5.290, 4.813],
+         4.796, 8.593, 7.901, 27.499, 4.378, 5.040, 4.904, 4.816, 4.828,
+         4.853, 57.363, 34.184, 5.482, 28.190, 27.290, 26.694, 5.099,
+         4.905, 5.290, 4.813],
         [18.301, 22.522, 8.035, 6.021, 5.565, 9.037, 6.998, 5.321, 4.485,
-        4.768, 8.397, 7.865, 27.636, 4.640, 5.015, 4.962, 4.933, 4.977,
-        4.961, 60.648, 34.593, 5.538, 28.454, 27.297, 26.490, 5.099, 5,
-        5.247, 4.945],
+         4.768, 8.397, 7.865, 27.636, 4.640, 5.015, 4.962, 4.933, 4.977,
+         4.961, 60.648, 34.593, 5.538, 28.454, 27.297, 26.490, 5.099, 5,
+         5.247, 4.945],
         [18.907, 23.368, 8.100, 6.169, 5.621, 9.971, 8.161, 5.331, 4.513,
-        4.837, 8.255, 7.852, 26.209, 4.388, 5.045, 5.029, 5.032, 4.946,
-        4.973, 60.334, 33.377, 5.499, 28.275, 27.550, 26.103, 5.108,
-        4.951, 5.285, 4.910],
+         4.837, 8.255, 7.852, 26.209, 4.388, 5.045, 5.029, 5.032, 4.946,
+         4.973, 60.334, 33.377, 5.499, 28.275, 27.550, 26.103, 5.108,
+         4.951, 5.285, 4.910],
         [18.715, 23.748, 8.128, 6.148, 5.691, 9.361, 8.106, 5.334, 4.528,
-        4.965, 8.261, 7.851, 27.282, 4.391, 4.949, 4.981, 4.964, 4.935,
-        4.933, 60.231, 33.361, 5.489, 28.106, 27.457, 26.648, 5.108,
-        4.963, 5.272, 4.954]
+         4.965, 8.261, 7.851, 27.282, 4.391, 4.949, 4.981, 4.964, 4.935,
+         4.933, 60.231, 33.361, 5.489, 28.106, 27.457, 26.648, 5.108,
+         4.963, 5.272, 4.954]
     ]
 ]
 
@@ -115,8 +115,7 @@ DEFAULT_OPTIONS = {
 _MockResultsGenerator = (x for x in [])
 
 
-def _MockRunTests(*args, **kwargs):
-  _, _ = args, kwargs
+def _MockRunTests(*args, **kwargs):  # pylint: disable=unused-argument
   return _FakeTestResult(_MockResultsGenerator.next())
 
 
@@ -379,7 +378,7 @@ class BisectPerfRegressionTest(unittest.TestCase):
     bisect_class.RunPerformanceTestAndParseResults = _MockRunTests
 
     try:
-      _GenericDryRun(_GetExtendedOptions(0, 0, False))
+      dry_run_results = _GenericDryRun(_GetExtendedOptions(0, 0, False))
     except StopIteration:
       # If StopIteration was raised, that means that the next value after
       # the first two values was requested, so the job was not aborted.
@@ -388,23 +387,23 @@ class BisectPerfRegressionTest(unittest.TestCase):
       bisect_class.RunPerformanceTestAndParseResults = original_run_tests
 
     # If the job was aborted, there should be a warning about it.
-    assert [w for w in results.warnings
+    assert [w for w in dry_run_results.warnings
             if 'could not reproduce the regression' in w]
     return True
 
-  def testBisectStopsOnClearUnclearRegression(self):
+  def testBisectAbortedOnClearNonRegression(self):
     self.assertTrue(self._CheckAbortsEarly(CLEAR_NON_REGRESSION))
 
-  def testBisectStopsOnClearUnclearRegression(self):
+  def testBisectNotAborted_AlmostRegression(self):
     self.assertFalse(self._CheckAbortsEarly(ALMOST_REGRESSION))
 
-  def testBisectStopsOnClearUnclearRegression(self):
+  def testBisectNotAborted_ClearRegression(self):
     self.assertFalse(self._CheckAbortsEarly(CLEAR_REGRESSION))
 
-  def testBisectStopsOnClearUnclearRegression(self):
+  def testBisectNotAborted_BarelyRegression(self):
     self.assertFalse(self._CheckAbortsEarly(BARELY_REGRESSION))
 
-  def testBisectStopsOnClearUnclearRegression(self):
+  def testBisectNotAborted_MultipleValues(self):
     self.assertFalse(self._CheckAbortsEarly(MULTIPLE_VALUES))
 
   def testGetCommitPosition(self):
@@ -467,8 +466,8 @@ class BisectPerfRegressionTest(unittest.TestCase):
         '--force',
         '--delete_unversioned_trees',
         '--revision',
-        'src@e6db23a037cad47299a94b155b95eebd1ee61a58'
-        ]
+        'src@e6db23a037cad47299a94b155b95eebd1ee61a58',
+    ]
 
     mock_RunGClient.assert_called_with(expected_params, cwd=None)
 
@@ -477,16 +476,18 @@ class BisectPerfRegressionTest(unittest.TestCase):
     bisect_instance = _GetBisectPerformanceMetricsInstance(DEFAULT_OPTIONS)
     mock_RunGit.return_value = None, None
     bisect_instance._SyncRevision(
-        'webkit', 'a94d028e0f2c77f159b3dac95eb90c3b4cf48c61' , None)
+        'webkit', 'a94d028e0f2c77f159b3dac95eb90c3b4cf48c61', None)
     expected_params = ['checkout', 'a94d028e0f2c77f159b3dac95eb90c3b4cf48c61']
     mock_RunGit.assert_called_with(expected_params)
 
   def testTryJobSvnRepo_PerfBuilderType_ReturnsRepoUrl(self):
-    self.assertEqual(bisect_perf_regression.PERF_SVN_REPO_URL,
+    self.assertEqual(
+        bisect_perf_regression.PERF_SVN_REPO_URL,
         bisect_perf_regression._TryJobSvnRepo(fetch_build.PERF_BUILDER))
 
   def testTryJobSvnRepo_FullBuilderType_ReturnsRepoUrl(self):
-    self.assertEqual(bisect_perf_regression.FULL_SVN_REPO_URL,
+    self.assertEqual(
+        bisect_perf_regression.FULL_SVN_REPO_URL,
         bisect_perf_regression._TryJobSvnRepo(fetch_build.FULL_BUILDER))
 
   def testTryJobSvnRepo_WithUnknownBuilderType_ThrowsError(self):
@@ -672,8 +673,8 @@ class GitTryJobTestCases(unittest.TestCase):
           '--revision=%s' % git_revision,
           '--name=%s' % bisect_job_name,
           '--svn_repo=%s' % bisect_perf_regression.PERF_SVN_REPO_URL,
-          '--diff=%s' % patch_content
-         ], (None, 1)),
+          '--diff=%s' % patch_content],
+         (None, 1)),
     ]
     self._AssertRunGitExceptions(
         try_cmd, bisect_perf_regression._StartBuilderTryJob,
@@ -702,8 +703,8 @@ class GitTryJobTestCases(unittest.TestCase):
           '--revision=%s' % git_revision,
           '--name=%s' % bisect_job_name,
           '--svn_repo=%s' % bisect_perf_regression.PERF_SVN_REPO_URL,
-          '--diff=%s' % patch_content
-         ], (None, 0)),
+          '--diff=%s' % patch_content],
+         (None, 0)),
     ]
     self._SetupRunGitMock(try_cmd)
     bisect_perf_regression._StartBuilderTryJob(

tools/auto_bisect/bisect_printer.py

@@ -204,7 +204,7 @@ class BisectPrinter(object):
   @staticmethod
   def _GetViewVCLinkFromDepotAndHash(git_revision, depot):
     """Gets link to the repository browser."""
-    if depot and bisect_utils.DEPOT_DEPS_NAME[depot].has_key('viewvc'):
+    if depot and 'viewvc' in bisect_utils.DEPOT_DEPS_NAME[depot]:
       return bisect_utils.DEPOT_DEPS_NAME[depot]['viewvc'] + git_revision
     return ''
 

tools/auto_bisect/bisect_results.py

@@ -58,7 +58,6 @@ class BisectResults(object):
       runtime_warnings: A list of warnings from the bisect run.
       error: Error message. When error is not None, other arguments are ignored.
     """
-
     self.error = error
     self.abort_reason = abort_reason
     if error is not None or abort_reason is not None:
@@ -66,8 +65,8 @@ class BisectResults(object):
 
     assert (bisect_state is not None and depot_registry is not None and
             opts is not None and runtime_warnings is not None), (
-            'Incorrect use of the BisectResults constructor. When error is '
-            'None, all other arguments are required')
+                'Incorrect use of the BisectResults constructor. '
+                'When error is None, all other arguments are required.')
 
     self.state = bisect_state
 
@@ -113,7 +112,7 @@ class BisectResults(object):
       return
 
     confidence_params = (results_reverted[0]['values'],
-        results_tot[0]['values'])
+                         results_tot[0]['values'])
     confidence = BisectResults.ConfidenceScore(*confidence_params)
 
     self.retest_results_tot = RevisionState('ToT', 'n/a', 0)
@@ -127,7 +126,7 @@ class BisectResults(object):
           'Confidence of re-test with reverted CL is not high.'
           ' Check that the regression hasn\'t already recovered. '
           ' There\'s still a chance this is a regression, as performance of'
-          ' local builds may not match official builds.' )
+          ' local builds may not match official builds.')
 
   @staticmethod
   def _GetResultBasedWarnings(culprit_revisions, opts, confidence):
@@ -218,8 +217,10 @@ class BisectResults(object):
           # mean of the current runs, this local regression is in same
           # direction.
           prev_greater_than_current = mean_of_prev_runs > mean_of_current_runs
-          is_same_direction = (prev_greater_than_current if
-              bad_greater_than_good else not prev_greater_than_current)
+          if bad_greater_than_good:
+            is_same_direction = prev_greater_than_current
+          else:
+            is_same_direction = not prev_greater_than_current
 
           # Only report potential regressions with high confidence.
           if is_same_direction and confidence > 50:

tools/auto_bisect/bisect_results_test.py

@@ -235,7 +235,7 @@ class BisectResultsTest(unittest.TestCase):
     revision_states[2].depot = 'webkit'
 
     results = BisectResults(self.mock_bisect_state, self.mock_depot_registry,
-                          self.mock_opts, self.mock_warnings)
+                            self.mock_opts, self.mock_warnings)
 
     self.assertEqual(1, len(results.culprit_revisions))
     self.assertEqual(('b', {'test': 'b'}, 'chromium'),

tools/auto_bisect/bisect_utils.py

@@ -325,7 +325,6 @@ def RunGClientAndCreateConfig(opts, custom_deps=None, cwd=None):
   return return_code
 
 
-
 def OnAccessError(func, path, _):
   """Error handler for shutil.rmtree.
 
@@ -431,7 +430,7 @@ def CheckRunGit(command, cwd=None):
   Returns:
     A tuple of the output and return code.
   """
-  (output, return_code) = RunGit(command, cwd=cwd)
+  output, return_code = RunGit(command, cwd=cwd)
 
   assert not return_code, 'An error occurred while running'\
                           ' "git %s"' % ' '.join(command)
@@ -463,8 +462,7 @@ def CreateBisectDirectoryAndSetupDepot(opts, custom_deps):
   if CheckIfBisectDepotExists(opts):
     path_to_dir = os.path.join(os.path.abspath(opts.working_directory),
                                BISECT_DIR, 'src')
-    (output, _) = RunGit(['rev-parse', '--is-inside-work-tree'],
-                         cwd=path_to_dir)
+    output, _ = RunGit(['rev-parse', '--is-inside-work-tree'], cwd=path_to_dir)
     if output.strip() == 'true':
       # Before checking out master, cleanup up any leftover index.lock files.
       _CleanupPreviousGitRuns(path_to_dir)
@@ -515,9 +513,10 @@ def RunProcessAndRetrieveOutput(command, cwd=None):
 
   # On Windows, use shell=True to get PATH interpretation.
   shell = IsWindowsHost()
-  proc = subprocess.Popen(command, shell=shell, stdout=subprocess.PIPE,
+  proc = subprocess.Popen(
+      command, shell=shell, stdout=subprocess.PIPE,
       stderr=subprocess.STDOUT)
-  (output, _) = proc.communicate()
+  output, _ = proc.communicate()
 
   if cwd:
     os.chdir(original_cwd)

tools/auto_bisect/fetch_build_test.py

@@ -71,7 +71,8 @@ class BuildArchiveTest(unittest.TestCase):
     archive = fetch_build.FullBuildArchive()
     archive._platform = 'android'
     self.assertEqual('chromium-android', archive.BucketName())
-    self.assertEqual('android_main_rel/full-build-linux_1234567890abcdef.zip',
+    self.assertEqual(
+        'android_main_rel/full-build-linux_1234567890abcdef.zip',
         archive.FilePath('1234567890abcdef'))
 
   def test_FullBuildArchive_Linux_BuilderName(self):
@@ -167,13 +168,16 @@ class BuildArchiveTest(unittest.TestCase):
     self.assertEqual(14400, archive.GetBuilderBuildTime())
 
   def test_GetBuildBotUrl_Perf(self):
-    self.assertEqual(fetch_build.PERF_TRY_SERVER_URL,
+    self.assertEqual(
+        fetch_build.PERF_TRY_SERVER_URL,
         fetch_build.GetBuildBotUrl(fetch_build.PERF_BUILDER))
 
   def test_GetBuildBotUrl_full(self):
-    self.assertEqual(fetch_build.LINUX_TRY_SERVER_URL,
+    self.assertEqual(
+        fetch_build.LINUX_TRY_SERVER_URL,
         fetch_build.GetBuildBotUrl(fetch_build.FULL_BUILDER))
 
+
 class UnzipTest(unittest.TestCase):
 
   def setUp(self):

tools/auto_bisect/math_utils.py

@@ -136,4 +136,3 @@ def StandardError(values):
     return 0.0
   std_dev = StandardDeviation(values)
   return std_dev / math.sqrt(len(values))
-

tools/auto_bisect/query_crbug_test.py

@@ -27,7 +27,7 @@ UNEXPECTED_FORMAT_DATA = CLOSED_ISSUE_DATA.replace('issues$state', 'gibberish')
 BROKEN_ISSUE_DATA = "\n<HTML><HEAD><TITLE>Not a JSON Doc</TITLE></HEAD></HTML>"
 
 
-class mockResponse(object):
+class MockResponse(object):
   def __init__(self, result):
     self._result = result
 
@@ -35,45 +35,45 @@ class mockResponse(object):
     return self._result
 
 
-def mockUrlOpen(url):
+def MockUrlOpen(url):
   # Note that these strings DO NOT represent http responses. They are just
   # memorable numeric bug ids to use.
   if '200' in url:
-    return mockResponse(CLOSED_ISSUE_DATA)
+    return MockResponse(CLOSED_ISSUE_DATA)
   elif '201' in url:
-    return mockResponse(OPEN_ISSUE_DATA)
+    return MockResponse(OPEN_ISSUE_DATA)
   elif '300' in url:
-    return mockResponse(UNEXPECTED_FORMAT_DATA)
+    return MockResponse(UNEXPECTED_FORMAT_DATA)
   elif '403' in url:
     raise urllib2.URLError('')
   elif '404' in url:
-    return mockResponse('')
+    return MockResponse('')
   elif '500' in url:
-    return mockResponse(BROKEN_ISSUE_DATA)
+    return MockResponse(BROKEN_ISSUE_DATA)
 
 
 class crbugQueryTest(unittest.TestCase):
-  @mock.patch('urllib2.urlopen',mockUrlOpen)
+  @mock.patch('urllib2.urlopen', MockUrlOpen)
   def testClosedIssueIsClosed(self):
     self.assertTrue(CheckIssueClosed(200))
 
-  @mock.patch('urllib2.urlopen',mockUrlOpen)
+  @mock.patch('urllib2.urlopen', MockUrlOpen)
   def testOpenIssueIsNotClosed(self):
     self.assertFalse(CheckIssueClosed(201))
 
-  @mock.patch('urllib2.urlopen',mockUrlOpen)
+  @mock.patch('urllib2.urlopen', MockUrlOpen)
   def testUnexpectedFormat(self):
     self.assertFalse(CheckIssueClosed(300))
 
-  @mock.patch('urllib2.urlopen',mockUrlOpen)
+  @mock.patch('urllib2.urlopen', MockUrlOpen)
   def testUrlError(self):
     self.assertFalse(CheckIssueClosed(403))
 
-  @mock.patch('urllib2.urlopen',mockUrlOpen)
+  @mock.patch('urllib2.urlopen', MockUrlOpen)
   def testEmptyResponse(self):
     self.assertFalse(CheckIssueClosed(404))
 
-  @mock.patch('urllib2.urlopen',mockUrlOpen)
+  @mock.patch('urllib2.urlopen', MockUrlOpen)
   def testBrokenResponse(self):
     self.assertFalse(CheckIssueClosed(500))
 

tools/auto_bisect/request_build.py

@@ -11,16 +11,9 @@ This module can be either run as a stand-alone script to send a request to a
 builder, or imported and used by calling the public functions below.
 """
 
-import getpass
 import json
-import optparse
-import os
-import sys
-import urllib
 import urllib2
 
-import fetch_build
-
 # URL template for fetching JSON data about builds.
 BUILDER_JSON_URL = ('%(server_url)s/json/builders/%(bot_name)s/builds/'
                     '%(build_num)s?as_text=1&filter=0')
@@ -109,13 +102,13 @@ def _IsBuildSuccessful(build_data):
 
 
 def _FetchBuilderData(builder_url):
-  """Fetches JSON data for the all the builds from the tryserver.
+  """Fetches JSON data for the all the builds from the try server.
 
   Args:
-    builder_url: A tryserver URL to fetch builds information.
+    builder_url: A try server URL to fetch builds information.
 
   Returns:
-    A dictionary with information of all build on the tryserver.
+    A dictionary with information of all build on the try server.
   """
   data = None
   try:
@@ -133,10 +126,10 @@ def _FetchBuilderData(builder_url):
 
 
 def _GetBuildData(buildbot_url):
-  """Gets build information for the given build id from the tryserver.
+  """Gets build information for the given build id from the try server.
 
   Args:
-    buildbot_url: A tryserver URL to fetch build information.
+    buildbot_url: A try server URL to fetch build information.
 
   Returns:
     A dictionary with build information if build exists, otherwise None.
@@ -151,7 +144,7 @@ def GetBuildStatus(build_num, bot_name, server_url):
   """Gets build status from the buildbot status page for a given build number.
 
   Args:
-    build_num: A build number on tryserver to determine its status.
+    build_num: A build number on try server to determine its status.
     bot_name: Name of the bot where the build information is scanned.
     server_url: URL of the buildbot.
 
@@ -188,13 +181,13 @@ def GetBuildNumFromBuilder(build_reason, bot_name, server_url):
 
   This function parses the JSON data from buildbot page and collects basic
   information about the all the builds, and then uniquely identifies the build
-  based on the 'reason' attribute in builds's JSON data.
+  based on the 'reason' attribute in the JSON data about the build.
 
   The 'reason' attribute set is when a build request is posted, and it is used
   to identify the build on status page.
 
   Args:
-    build_reason: A unique build name set to build on tryserver.
+    build_reason: A unique build name set to build on try server.
     bot_name: Name of the bot where the build information is scanned.
     server_url: URL of the buildbot.
 

tools/auto_bisect/source_control.py

@@ -137,10 +137,10 @@ def GetCommitPosition(git_revision, cwd=None):
   Returns:
     Git commit position as integer or None.
   """
-  # Some of the respositories are pure git based, unlike other repositories
+  # Some of the repositories are pure git based, unlike other repositories
   # they doesn't have commit position. e.g., skia, angle.
   cmd = ['footers', '--position-num', git_revision]
-  output, return_code  = bisect_utils.RunGit(cmd, cwd)
+  output, return_code = bisect_utils.RunGit(cmd, cwd)
   if not return_code:
     commit_position = output.strip()
     if bisect_utils.IsStringInt(commit_position):
@@ -230,4 +230,3 @@ def QueryFileRevisionHistory(filename, revision_start, revision_end):
   output = bisect_utils.CheckRunGit(cmd)
   lines = output.split('\n')
   return [o for o in lines if o]
-

tools/auto_bisect/source_control_test.py

@@ -81,4 +81,3 @@ def _SetMockCheckRunGitBehavior(mock_obj, command_output_map):
 
 if __name__ == '__main__':
   unittest.main()
-