[Culprit-Finder] Refactor GitilesRepostory to make http_client required argument.

appengine/findit/crash/test/changelist_classifier_test.py

 # Copyright 2016 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.
 
 from collections import defaultdict
 import copy
 
 from common.dependency import Dependency
 from common.dependency import DependencyRoll
 from common import chrome_dependency_fetcher
@@ skipping 115 matching lines @@
       passed_in_regression_deps_rolls.append(regression_deps_rolls)
       return {}, None
 
     self.mock(changelist_classifier, 'GetChangeLogsForFilesGroupedByDeps',
               _MockGetChangeLogsForFilesGroupedByDeps)
     self.mock(changelist_classifier, 'GetStackInfosForFilesGroupedByDeps',
               lambda *_: {})
     self.mock(changelist_classifier, 'FindMatchResults', lambda *_: None)
 
     cl_classifier = changelist_classifier.ChangelistClassifier(
-        GitilesRepository(), 7)
+        GitilesRepository(self.GetMockHttpClient()), 7)
     cl_classifier(CrashReport(crashed_version = '5',
                               signature = 'sig',
                               platform = 'canary',
                               stacktrace = Stacktrace([CallStack(0)]),
                               regression_range = ['4', '5']))
     expected_regression_deps_rolls = copy.deepcopy(dep_rolls)
 
     # Regression of a dep added/deleted (old_revision/new_revision is None) can
     # not be known for sure and this case rarely happens, so just filter them
     # out.
@@ skipping 33 matching lines @@
     def _MockGetChangeLogs(_, start_rev, end_rev):
       if start_rev == '4' and end_rev == '5':
         return [DUMMY_CHANGELOG1, DUMMY_CHANGELOG2, DUMMY_CHANGELOG3]
 
       return []
 
     self.mock(GitilesRepository, 'GetChangeLogs', _MockGetChangeLogs)
 
     dep_file_to_changelogs, ignore_cls = (
         changelist_classifier.GetChangeLogsForFilesGroupedByDeps(
-            regression_deps_rolls, stack_deps, GitilesRepository()))
+            regression_deps_rolls, stack_deps,
+            GitilesRepository(self.GetMockHttpClient())))
     dep_file_to_changelogs_json = defaultdict(lambda: defaultdict(list))
     for dep, file_to_changelogs in dep_file_to_changelogs.iteritems():
       for file_path, changelogs in file_to_changelogs.iteritems():
         for changelog in changelogs:
           dep_file_to_changelogs_json[dep][file_path].append(changelog.ToDict())
 
     expected_dep_file_to_changelogs_json = {
         'src/': {
             'a.cc': [DUMMY_CHANGELOG1.ToDict()],
             'f.cc': [DUMMY_CHANGELOG3.ToDict()]
@@ skipping 91 matching lines @@
         'project_path': 'src/',
         'author': 'r@chromium.org',
         'time': 'Thu Mar 31 21:24:43 2016',
         'reasons': None,
         'confidence': None,
         'changed_files': None
     }]
 
     match_results = changelist_classifier.FindMatchResults(
         dep_file_to_changelogs, dep_file_to_stack_infos, stack_deps,
-        GitilesRepository())
+        GitilesRepository(self.GetMockHttpClient()))
     self.assertListEqual([result.ToDict() for result in match_results],
                          expected_match_results)
 
   # TODO(http://crbug.com/659346): why do these mocks give coverage
   # failures? That's almost surely hiding a bug in the tests themselves.
   def testFindItForCrashNoRegressionRange(self): # pragma: no cover
     self.mock(chrome_dependency_fetcher.ChromeDependencyFetcher,
         'GetDependencyRollsDict', lambda *_: {})
     self.mock(chrome_dependency_fetcher.ChromeDependencyFetcher,
         'GetDependency', lambda *_: {})
-    cl_classifier = changelist_classifier.ChangelistClassifier(7,
-        GitilesRepository())
+    cl_classifier = changelist_classifier.ChangelistClassifier(
+        7, GitilesRepository(self.GetMockHttpClient()))

[wrengr, 2016/11/22 19:11:53]

Since we do this line a lot in a bunch of tests, maybe we should just have a
self.GetChangelistClassifier() method

[Sharu Jiang, 2016/11/22 20:15:38]

Done.

     # N.B., for this one test we really do want regression_range=None.
     report = DUMMY_REPORT._replace(regression_range=None)
     self.assertListEqual(cl_classifier(report), [])
 
   def testFindItForCrashNoMatchFound(self):
     self.mock(changelist_classifier, 'FindMatchResults', lambda *_: [])
     self.mock(chrome_dependency_fetcher.ChromeDependencyFetcher,
         'GetDependencyRollsDict',
         lambda *_: {'src/': DependencyRoll('src/', 'https://repo', '1', '2')})
     self.mock(chrome_dependency_fetcher.ChromeDependencyFetcher,
         'GetDependency', lambda *_: {})
-    cl_classifier = changelist_classifier.ChangelistClassifier(7,
-        GitilesRepository())
+    cl_classifier = changelist_classifier.ChangelistClassifier(
+        7, GitilesRepository(self.GetMockHttpClient()))
     self.assertListEqual(cl_classifier(DUMMY_REPORT), [])
 
   def testFindItForCrash(self):
 
     def _MockFindMatchResults(*_):
       match_result1 = MatchResult(DUMMY_CHANGELOG1, 'src/', '')
       frame1 = StackFrame(0, 'src/', 'func', 'a.cc', 'src/a.cc', [1])
       frame2 = StackFrame(1, 'src/', 'func', 'a.cc', 'src/a.cc', [7])
       match_result1.file_to_stack_infos = {
           'a.cc': [(frame1, 0), (frame2, 0)]
@@ skipping 12 matching lines @@
       }
 
       return [match_result1, match_result2]
 
     self.mock(changelist_classifier, 'FindMatchResults', _MockFindMatchResults)
     self.mock(chrome_dependency_fetcher.ChromeDependencyFetcher,
         'GetDependencyRollsDict',
         lambda *_: {'src/': DependencyRoll('src/', 'https://repo', '1', '2')})
     self.mock(chrome_dependency_fetcher.ChromeDependencyFetcher,
         'GetDependency', lambda *_: {})
-    cl_classifier = changelist_classifier.ChangelistClassifier(7,
-        GitilesRepository())
+    cl_classifier = changelist_classifier.ChangelistClassifier(
+        7, GitilesRepository(self.GetMockHttpClient()))
     results = cl_classifier(DUMMY_REPORT)
     expected_match_results = [
         {
             'reasons': [('TopFrameIndex', 1.0, 'Top frame is #0'),
                         ('MinDistance', 1, 'Minimum distance is 0')],
             'changed_files': [{'info': 'Minimum distance (LOC) 0, frame #0',
                                'blame_url': None, 'file': 'a.cc'}],
             'time': 'Thu Mar 31 21:24:43 2016',
             'author': 'r@chromium.org',
             'url': 'https://repo.test/+/1',
@@ skipping 37 matching lines @@
 
       return [match_result1, match_result2, match_result3]
 
     self.mock(changelist_classifier, 'FindMatchResults', _MockFindMatchResults)
     self.mock(chrome_dependency_fetcher.ChromeDependencyFetcher,
         'GetDependencyRollsDict',
         lambda *_: {'src/': DependencyRoll('src/', 'https://repo', '1', '2')})
     self.mock(chrome_dependency_fetcher.ChromeDependencyFetcher,
         'GetDependency', lambda *_: {})
 
-    cl_classifier = changelist_classifier.ChangelistClassifier(7,
-        GitilesRepository())
+    cl_classifier = changelist_classifier.ChangelistClassifier(
+        7, GitilesRepository(self.GetMockHttpClient()))
     results = cl_classifier(DUMMY_REPORT)
     expected_match_results = [
         {
             'author': 'r@chromium.org',
             'changed_files': [
                 {
                     'blame_url': None,
                     'file': 'a.cc',
                     'info': 'Minimum distance (LOC) 1, frame #0'
                 }
@@ skipping 37 matching lines @@
 
       return [match_result1, match_result2]
 
     self.mock(changelist_classifier, 'FindMatchResults', _MockFindMatchResults)
     self.mock(chrome_dependency_fetcher.ChromeDependencyFetcher,
         'GetDependencyRollsDict',
         lambda *_: {'src/': DependencyRoll('src/', 'https://repo', '1', '2')})
     self.mock(chrome_dependency_fetcher.ChromeDependencyFetcher,
         'GetDependency', lambda *_: {})
 
-    cl_classifier = changelist_classifier.ChangelistClassifier(7,
-        GitilesRepository())
+    cl_classifier = changelist_classifier.ChangelistClassifier(
+        7, GitilesRepository(self.GetMockHttpClient()))
     self.assertListEqual(cl_classifier(DUMMY_REPORT), [])