Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1184)

Side by Side Diff: appengine/findit/crash/test/changelist_classifier_test.py

Issue 2608483002: Changed FindSuspects to take a Repository factory, rather than mutating it (Closed)
Patch Set: rebase Created 3 years, 11 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View unified diff | Download patch
« no previous file with comments | « appengine/findit/crash/loglinear/test/changelist_classifier_test.py ('k') | no next file » | no next file with comments »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
OLDNEW
1 # Copyright 2016 The Chromium Authors. All rights reserved. 1 # Copyright 2016 The Chromium Authors. All rights reserved.
2 # Use of this source code is governed by a BSD-style license that can be 2 # Use of this source code is governed by a BSD-style license that can be
3 # found in the LICENSE file. 3 # found in the LICENSE file.
4 4
5 from collections import defaultdict 5 from collections import defaultdict
6 import copy 6 import copy
7 7
8 from common.dependency import Dependency 8 from common.dependency import Dependency
9 from common.dependency import DependencyRoll 9 from common.dependency import DependencyRoll
10 from common import chrome_dependency_fetcher 10 from common import chrome_dependency_fetcher
(...skipping 105 matching lines...) Expand 10 before | Expand all | Expand 10 after
116 CallStackFormatType.DEFAULT, LanguageType.CPP)] 116 CallStackFormatType.DEFAULT, LanguageType.CPP)]
117 DUMMY_REPORT = CrashReport(None, None, None, Stacktrace(DUMMY_CALLSTACKS, 117 DUMMY_REPORT = CrashReport(None, None, None, Stacktrace(DUMMY_CALLSTACKS,
118 DUMMY_CALLSTACKS[0]), 118 DUMMY_CALLSTACKS[0]),
119 (None, None)) 119 (None, None))
120 120
121 121
122 class ChangelistClassifierTest(CrashTestSuite): 122 class ChangelistClassifierTest(CrashTestSuite):
123 123
124 def setUp(self): 124 def setUp(self):
125 super(ChangelistClassifierTest, self).setUp() 125 super(ChangelistClassifierTest, self).setUp()
126
127 repository = GitilesRepository(self.GetMockHttpClient())
128
129 # TODO(crbug.com/677224): should replace this with an actual factory.
130 def MutateTheRepo(dep_url): # pragma: no cover
131 """A factory function for returning ``Repository`` objects.
132
133 The current definition captures the functionality of before
134 we factored out this factory method. That is, it's not really a
135 "factory" but rather mutates the main repo object in place. In
136 the future this should be changed to do the right thing instead.
137 """
138 repository.repo_url = dep_url
139 return repository
140
126 self.changelist_classifier = changelist_classifier.ChangelistClassifier( 141 self.changelist_classifier = changelist_classifier.ChangelistClassifier(
127 GitilesRepository(self.GetMockHttpClient()), 7) 142 repository, MutateTheRepo, 7)
128 143
129 def testSkipAddedAndDeletedRegressionRolls(self): 144 def testSkipAddedAndDeletedRegressionRolls(self):
130 self.mock(chrome_dependency_fetcher.ChromeDependencyFetcher, 145 self.mock(chrome_dependency_fetcher.ChromeDependencyFetcher,
131 'GetDependency', lambda *_: {}) 146 'GetDependency', lambda *_: {})
132 dep_rolls = { 147 dep_rolls = {
133 'src/dep': DependencyRoll('src/dep1', 'https://url_dep1', None, '9'), 148 'src/dep': DependencyRoll('src/dep1', 'https://url_dep1', None, '9'),
134 'src/': DependencyRoll('src/', ('https://chromium.googlesource.com/' 149 'src/': DependencyRoll('src/', ('https://chromium.googlesource.com/'
135 'chromium/src.git'), '4', '5') 150 'chromium/src.git'), '4', '5')
136 } 151 }
137 self.mock(chrome_dependency_fetcher.ChromeDependencyFetcher, 152 self.mock(chrome_dependency_fetcher.ChromeDependencyFetcher,
(...skipping 168 matching lines...) Expand 10 before | Expand all | Expand 10 after
306 'project_path': 'src/', 321 'project_path': 'src/',
307 'author': 'r@chromium.org', 322 'author': 'r@chromium.org',
308 'time': 'Thu Mar 31 21:24:43 2016', 323 'time': 'Thu Mar 31 21:24:43 2016',
309 'reasons': None, 324 'reasons': None,
310 'confidence': None, 325 'confidence': None,
311 'changed_files': None 326 'changed_files': None
312 }] 327 }]
313 328
314 suspects = changelist_classifier.FindSuspects( 329 suspects = changelist_classifier.FindSuspects(
315 dep_file_to_changelogs, dep_file_to_stack_infos, stack_deps, 330 dep_file_to_changelogs, dep_file_to_stack_infos, stack_deps,
316 GitilesRepository(self.GetMockHttpClient())) 331 lambda repo_url: GitilesRepository(self.GetMockHttpClient(), repo_url))
317 self.assertListEqual([suspect.ToDict() for suspect in suspects], 332 self.assertListEqual([suspect.ToDict() for suspect in suspects],
318 expected_suspects) 333 expected_suspects)
319 334
320 # TODO(http://crbug.com/659346): why do these mocks give coverage 335 # TODO(http://crbug.com/659346): why do these mocks give coverage
321 # failures? That's almost surely hiding a bug in the tests themselves. 336 # failures? That's almost surely hiding a bug in the tests themselves.
322 def testFindItForCrashNoRegressionRange(self): # pragma: no cover 337 def testFindItForCrashNoRegressionRange(self): # pragma: no cover
323 self.mock(chrome_dependency_fetcher.ChromeDependencyFetcher, 338 self.mock(chrome_dependency_fetcher.ChromeDependencyFetcher,
324 'GetDependencyRollsDict', lambda *_: {}) 339 'GetDependencyRollsDict', lambda *_: {})
325 self.mock(chrome_dependency_fetcher.ChromeDependencyFetcher, 340 self.mock(chrome_dependency_fetcher.ChromeDependencyFetcher,
326 'GetDependency', lambda *_: {}) 341 'GetDependency', lambda *_: {})
(...skipping 156 matching lines...) Expand 10 before | Expand all | Expand 10 after
483 return [suspect1, suspect2] 498 return [suspect1, suspect2]
484 499
485 self.mock(changelist_classifier, 'FindSuspects', _MockFindSuspects) 500 self.mock(changelist_classifier, 'FindSuspects', _MockFindSuspects)
486 self.mock(chrome_dependency_fetcher.ChromeDependencyFetcher, 501 self.mock(chrome_dependency_fetcher.ChromeDependencyFetcher,
487 'GetDependencyRollsDict', 502 'GetDependencyRollsDict',
488 lambda *_: {'src/': DependencyRoll('src/', 'https://repo', '1', '2')}) 503 lambda *_: {'src/': DependencyRoll('src/', 'https://repo', '1', '2')})
489 self.mock(chrome_dependency_fetcher.ChromeDependencyFetcher, 504 self.mock(chrome_dependency_fetcher.ChromeDependencyFetcher,
490 'GetDependency', lambda *_: {}) 505 'GetDependency', lambda *_: {})
491 506
492 self.assertListEqual(self.changelist_classifier(DUMMY_REPORT), []) 507 self.assertListEqual(self.changelist_classifier(DUMMY_REPORT), [])
OLDNEW
« no previous file with comments | « appengine/findit/crash/loglinear/test/changelist_classifier_test.py ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698