Refactor source_control.py and add a test.

Originally, Simon made SourceControl a class becaused we expected to
add a SvnSourceControl subclass which implemented the same methods in
different ways. However, after the git transition we're fairly sure
that we don't want to do this.

So, all of the functions in the SourceControl class can be made top-level
functions that don't need a "self" argument. This way, there's no need
to pass around a bogus SourceControl object; the functions can be called
directly.

I also added tests for some methods that seemed relatively testable.

PTAL and give suggestions :-)

BUG=

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

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

tools/auto_bisect/bisect_perf_regression_test.py

@@ -14,7 +14,7 @@ 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
+import source_control
 
 
 def _GetBisectPerformanceMetricsInstance():
@@ -29,10 +29,8 @@ def _GetBisectPerformanceMetricsInstance():
     'bad_revision': 280005,
   }
   bisect_options = bisect_perf_regression.BisectOptions.FromDict(options_dict)
-  source_control = source_control_module.DetermineAndCreateSourceControl(
-      bisect_options)
   bisect_instance = bisect_perf_regression.BisectPerformanceMetrics(
-      source_control, bisect_options)
+      bisect_options)
   return bisect_instance
 
 
@@ -207,12 +205,10 @@ class BisectPerfRegressionTest(unittest.TestCase):
     """
     bisect_options = bisect_perf_regression.BisectOptions()
     bisect_options.output_buildbot_annotations = None
-    source_control = source_control_module.DetermineAndCreateSourceControl(
-        bisect_options)
     bisect_instance = bisect_perf_regression.BisectPerformanceMetrics(
-        source_control, bisect_options)
+        bisect_options)
     bisect_instance.opts.target_platform = target_platform
-    git_revision = bisect_instance.source_control.ResolveToRevision(
+    git_revision = source_control.ResolveToRevision(
         revision, 'chromium', bisect_perf_regression.DEPOT_DEPS_NAME, 100)
     depot = 'chromium'
     command = bisect_instance.GetCompatibleCommand(
@@ -275,30 +271,25 @@ class BisectPerfRegressionTest(unittest.TestCase):
       shutil.rmtree = old_rmtree
 
   def testGetCommitPosition(self):
-    bisect_instance = _GetBisectPerformanceMetricsInstance()
     cp_git_rev = '7017a81991de983e12ab50dfc071c70e06979531'
-    self.assertEqual(
-        291765, bisect_instance.source_control.GetCommitPosition(cp_git_rev))
+    self.assertEqual(291765, source_control.GetCommitPosition(cp_git_rev))
 
     svn_git_rev = 'e6db23a037cad47299a94b155b95eebd1ee61a58'
-    self.assertEqual(
-        291467, bisect_instance.source_control.GetCommitPosition(svn_git_rev))
+    self.assertEqual(291467, source_control.GetCommitPosition(svn_git_rev))
 
   def testGetCommitPositionForV8(self):
     bisect_instance = _GetBisectPerformanceMetricsInstance()
     v8_rev = '21d700eedcdd6570eff22ece724b63a5eefe78cb'
     depot_path = os.path.join(bisect_instance.src_cwd, 'v8')
     self.assertEqual(
-        23634,
-        bisect_instance.source_control.GetCommitPosition(v8_rev, depot_path))
+        23634, source_control.GetCommitPosition(v8_rev, depot_path))
 
   def testGetCommitPositionForWebKit(self):
     bisect_instance = _GetBisectPerformanceMetricsInstance()
     wk_rev = 'a94d028e0f2c77f159b3dac95eb90c3b4cf48c61'
     depot_path = os.path.join(bisect_instance.src_cwd, 'third_party', 'WebKit')
     self.assertEqual(
-        181660,
-        bisect_instance.source_control.GetCommitPosition(wk_rev, depot_path))
+        181660, source_control.GetCommitPosition(wk_rev, depot_path))
 
   def testUpdateDepsContent(self):
     bisect_instance = _GetBisectPerformanceMetricsInstance()

tools/auto_bisect/bisect_results.py

@@ -7,6 +7,7 @@ import os
 
 import bisect_utils
 import math_utils
+import source_control
 import ttest
 
 
@@ -48,11 +49,10 @@ def ConfidenceScore(good_results_lists, bad_results_lists):
 
 class BisectResults(object):
 
-  def __init__(self, depot_registry, source_control):
+  def __init__(self, depot_registry):
     self._depot_registry = depot_registry
     self.revision_data = {}
     self.error = None
-    self._source_control = source_control
 
   @staticmethod
   def _FindOtherRegressions(revision_data_sorted, bad_greater_than_good):
@@ -232,7 +232,7 @@ class BisectResults(object):
                 changes.append([last_depot, contents[0]])
         for c in changes:
           os.chdir(c[0])
-          info = self._source_control.QueryRevisionInfo(c[1])
+          info = source_control.QueryRevisionInfo(c[1])
           culprit_revisions.append((c[1], info, None))
       else:
         for i in xrange(last_broken_revision_index, len(revision_data_sorted)):
@@ -240,7 +240,7 @@ class BisectResults(object):
           if k == first_working_revision:
             break
           self._depot_registry.ChangeToDepotDir(v['depot'])
-          info = self._source_control.QueryRevisionInfo(k)
+          info = source_control.QueryRevisionInfo(k)
           culprit_revisions.append((k, info, v['depot']))
       os.chdir(cwd)
 

tools/auto_bisect/source_control_test.py

+# 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.
+
+"""Unit tests for the source_control module."""
+
+import unittest
+import mock
+
+import source_control
+
+
+class SourceControlTest(unittest.TestCase):
+
+  @mock.patch('source_control.bisect_utils.CheckRunGit')
+  def testQueryRevisionInfo(self, mock_run_git):
+    # The QueryRevisionInfo function should run a sequence of git commands,
+    # then returns a dict with the results.
+    command_output_map = [
+        (['log', '--format=%aN', '-1', 'abcd1234'], 'Some Name\n'),
+        (['log', '--format=%aE', '-1', 'abcd1234'], 'somename@x.com'),
+        (['log', '--format=%s', '-1', 'abcd1234'], 'Commit subject '),
+        (['log', '--format=%cD', '-1', 'abcd1234'], 'Fri, 10 Oct 2014'),
+        (['log', '--format=%b', '-1', 'abcd1234'], 'Commit body\n'),
+    ]
+    _SetMockCheckRunGitBehavior(mock_run_git, command_output_map)
+    # The result of calling QueryRevisionInfo is a dictionary like that below.
+    # Trailing whitespace is stripped.
+    expected = {
+        'author': 'Some Name',
+        'email': 'somename@x.com',
+        'date': 'Fri, 10 Oct 2014',
+        'subject': 'Commit subject',
+        'body': 'Commit body',
+    }
+    self.assertEqual(expected, source_control.QueryRevisionInfo('abcd1234'))
+    self.assertEqual(5, mock_run_git.call_count)
+
+  def testResolveToRevision_InputGitHash(self):
+    # The ResolveToRevision function returns a git commit hash corresponding
+    # to the input, so if the input can't be parsed as an int, it is returned.
+    self.assertEqual(
+        'abcd1234',
+        source_control.ResolveToRevision('abcd1234', 'chromium', {}, 5))
+
+    # Note: It actually does this for any junk that isn't an int. This isn't
+    # necessarily desired behavior.
+    self.assertEqual(
+        'foo bar',
+        source_control.ResolveToRevision('foo bar', 'chromium', {}, 5))
+
+  @mock.patch('source_control.bisect_utils.CheckRunGit')
+  def testResolveToRevision_NotFound(self, mock_run_git):
+    # If no corresponding git hash was found, then None is returned.
+    mock_run_git.return_value = ''
+    self.assertIsNone(
+        source_control.ResolveToRevision('12345', 'chromium', {}, 5))
+
+  @mock.patch('source_control.bisect_utils.CheckRunGit')
+  def testResolveToRevision_Found(self, mock_run_git):
+    # In general, ResolveToRevision finds a git commit hash by repeatedly
+    # calling "git log --grep ..." with different numbers until something
+    # matches.
+    mock_run_git.return_value = 'abcd1234'
+    self.assertEqual(
+        'abcd1234',
+        source_control.ResolveToRevision('12345', 'chromium', {}, 5))
+    self.assertEqual(1, mock_run_git.call_count)
+
+
+def _SetMockCheckRunGitBehavior(mock_obj, command_output_map):
+  """Sets the behavior of a mock function according to the given mapping."""
+  # Unused argument 'cwd', expected in args list but not needed.
+  # pylint: disable=W0613
+  def FakeCheckRunGit(in_command, cwd=None):
+    for command, output in command_output_map:
+      if command == in_command:
+        return output
+  mock_obj.side_effect = FakeCheckRunGit
+
+
+if __name__ == '__main__':
+  unittest.main()
+