Trigger build request using git try instead of try_job_http on bisect bots

tools/auto_bisect/bisect_perf_regression_test.py

Index: tools/auto_bisect/bisect_perf_regression_test.py
diff --git a/tools/auto_bisect/bisect_perf_regression_test.py b/tools/auto_bisect/bisect_perf_regression_test.py
index b8f32266c92a66ae086d838718c2a61f2e0c51ff..e1a00364fe9c52aa4e575d7e89ad3e01c9eea61f 100644
--- a/tools/auto_bisect/bisect_perf_regression_test.py
+++ b/tools/auto_bisect/bisect_perf_regression_test.py
@@ -5,12 +5,18 @@
 import os
 import re
 import shutil
+import sys
 import unittest
 
+SRC = os.path.join(os.path.dirname(__file__), os.path.pardir, os.path.pardir)
+sys.path.append(os.path.join(SRC, 'third_party', 'pymock'))
+
 import bisect_perf_regression
 import bisect_results
+import mock
 import source_control as source_control_module
 
+
 def _GetBisectPerformanceMetricsInstance():
   """Returns an instance of the BisectPerformanceMetrics class."""
   options_dict = {
@@ -347,6 +353,114 @@ class DepotDirectoryRegistryTest(unittest.TestCase):
     self.registry.ChangeToDepotDir('mock_depot')
     self.assertEqual(self.cur_dir, '/mock/src/foo')
 

[qyearsley, 2014/10/02 01:07:53]

Two newlines between top-level classes.

[prasadv, 2014/10/02 17:27:27]

Done.

+# The tests below test private functions (W0212).
+# pylint: disable=W0212
+class GitTryJobTestCases(unittest.TestCase):
+  """Test case for bisect try job."""
+  def setUp(self):
+    bisect_utils_patcher = mock.patch('bisect_perf_regression.bisect_utils')
+    self.mock_bisect_utils = bisect_utils_patcher.start()
+    self.addCleanup(bisect_utils_patcher.stop)
+
+  def _AssertRunGitExceptions(self, git_cmds, func, *args):

[qyearsley, 2014/10/02 01:07:53]

I think it would be helpful to explain the parameters to this funciton in a
docstring.

In particular, git_cmds is not just a list of commands, it's a sequence of pairs
of input and output, right?

[prasadv, 2014/10/02 17:27:27]

Done.

+    def side_effect(values):

[qyearsley, 2014/10/02 01:07:53]

A name like git_cmd_args or something might be better than "values".

[prasadv, 2014/10/02 17:27:27]

Done.

+      for val in git_cmds:

[qyearsley, 2014/10/02 01:07:53]

If the items you're iterating over are pairs, you can directly name the parts in
the for loop, for example:

for input, output in git_cmds:
  if input == values:
    return output

[prasadv, 2014/10/02 17:27:27]

"git_cmds" is a list of tuples with git commands and expected output.

+        if set(val[0]) == set(values):
+          return val[1]
+    self.mock_bisect_utils.RunGit = mock.Mock(side_effect=side_effect)
+    self.assertRaises(bisect_perf_regression.RunGitError,

[qyearsley, 2014/10/02 01:07:53]

If you wanted to add another test method below that passes when no exceptions
are thrown, it might be easier if you take the "assertRaises" out of this
function (and put it into the sub-functions below) -- then the purpose of this
function is only to be a helper function for setting up the mock RunGit with
some canned inputs and outputs.

[prasadv, 2014/10/02 17:27:27]

Done.

+                      func,
+                      *args)
+
+  def testNotGitRepo(self):
+    new_branch = bisect_perf_regression.BISECT_TRYJOB_BRANCH
+    parent_branch = bisect_perf_regression.BISECT_MASTER_BRANCH
+    cmds = (

[qyearsley, 2014/10/02 01:07:53]

This could also be a list (with square brackets) right? Then there's no need for
that comma before the closing parenthesis.

[prasadv, 2014/10/02 17:27:27]

Done.

+      (['rev-parse', '--abbrev-ref', 'HEAD'], (None, 128)),)

[qyearsley, 2014/10/02 01:07:53]

I think indentation here should be 4 spaces.

[prasadv, 2014/10/02 17:27:27]

Done.

+    self._AssertRunGitExceptions(cmds,
+                                 bisect_perf_regression._PrepareBisectBranch,
+                                 parent_branch, new_branch)
+
+  def testFailedCheckoutMaster(self):
+    new_branch = bisect_perf_regression.BISECT_TRYJOB_BRANCH
+    parent_branch = bisect_perf_regression.BISECT_MASTER_BRANCH
+    cmds = (
+       (['rev-parse', '--abbrev-ref', 'HEAD'], (new_branch, 0)),
+      (['checkout', '-f', parent_branch], ('Checkout Failed', 1)),)

[qyearsley, 2014/10/02 01:07:53]

Nit: inconsistent indentation.

[prasadv, 2014/10/02 17:27:27]

Done.

+    self._AssertRunGitExceptions(cmds,
+                                 bisect_perf_regression._PrepareBisectBranch,
+                                 parent_branch, new_branch)
+
+  def testDeleteBisectBranchIfExists(self):
+    new_branch = bisect_perf_regression.BISECT_TRYJOB_BRANCH
+    parent_branch = bisect_perf_regression.BISECT_MASTER_BRANCH
+    cmds = (
+       (['rev-parse', '--abbrev-ref', 'HEAD'], (parent_branch, 0)),
+      (['branch', '--list' ], ('bisect-tryjob\n*master\nsomebranch', 0)),
+      (['branch', '-D', new_branch], ('Failed to delete branch', 128)),)
+    self._AssertRunGitExceptions(cmds,
+                                 bisect_perf_regression._PrepareBisectBranch,
+                                 parent_branch, new_branch)
+
+  def testCreatNewBranchFails(self):
+    new_branch = bisect_perf_regression.BISECT_TRYJOB_BRANCH
+    parent_branch = bisect_perf_regression.BISECT_MASTER_BRANCH
+    cmds = (
+       (['rev-parse', '--abbrev-ref', 'HEAD'], (parent_branch, 0)),
+      (['branch', '--list' ], ('bisect-tryjob\n*master\nsomebranch', 0)),
+      (['branch', '-D', new_branch], ('None', 0)),
+      (['update-index', '--refresh', '-q'], (None, 0)),
+      (['diff-index', 'HEAD'], (None, 0)),
+      (['checkout', '-b', new_branch], ('Failed to create branch', 128)),)
+
+    self._AssertRunGitExceptions(cmds,
+                                 bisect_perf_regression._PrepareBisectBranch,
+                                 parent_branch, new_branch)
+
+  def testSetUpstreamToFails(self):
+    new_branch = bisect_perf_regression.BISECT_TRYJOB_BRANCH
+    parent_branch = bisect_perf_regression.BISECT_MASTER_BRANCH
+    cmds = (
+       (['rev-parse', '--abbrev-ref', 'HEAD'], (parent_branch, 0)),
+      (['branch', '--list' ], ('bisect-tryjob\n*master\nsomebranch', 0)),
+      (['branch', '-D', new_branch], ('None', 0)),
+      (['update-index', '--refresh', '-q'], (None, 0)),
+      (['diff-index', 'HEAD'], (None, 0)),
+      (['checkout', '-b', new_branch], ('None', 0)),
+      (['branch', '--set-upstream-to', parent_branch], ('Setuptream fails', 1)),
+      )
+    self._AssertRunGitExceptions(cmds,
+                                 bisect_perf_regression._PrepareBisectBranch,
+                                 parent_branch, new_branch)
+  def testBuilderTryJob(self):
+    git_revision = 'ac4a9f31fe2610bd146857bbd55d7a260003a888'
+    bot_name = 'linux_perf_bisect_builder'
+    bisect_job_name = 'testBisectJobname'
+    patch = None
+    patch_content = '/dev/null'
+    new_branch = bisect_perf_regression.BISECT_TRYJOB_BRANCH
+    parent_branch = bisect_perf_regression.BISECT_MASTER_BRANCH
+    try_cmd = (
+        (['rev-parse', '--abbrev-ref', 'HEAD'], (parent_branch, 0)),
+        (['branch', '--list' ], ('bisect-tryjob\n*master\nsomebranch', 0)),
+        (['branch', '-D', new_branch], ('None', 0)),
+        (['update-index', '--refresh', '-q'], (None, 0)),
+        (['diff-index', 'HEAD'], (None, 0)),
+        (['checkout', '-b', new_branch], ('None', 0)),
+        (['branch', '--set-upstream-to', parent_branch],
+         ('Setuptream fails', 0)),
+        (['try',
+          '-b', bot_name,
+          '-r', git_revision,
+          '-n', bisect_job_name,
+          '--svn_repo=%s' % bisect_perf_regression.SVN_REPO_URL,
+          '--diff=%s' % patch_content
+          ], (None, 1)),
+        )
+    self._AssertRunGitExceptions(try_cmd,
+                                 bisect_perf_regression._BuilderTryjob,
+                                 git_revision, bot_name, bisect_job_name, patch)
+
 
 if __name__ == '__main__':
   unittest.main()