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

Side by Side Diff: appengine/findit/crash/loglinear/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
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 import copy 5 import copy
6 import math 6 import math
7 import pprint 7 import pprint
8 8
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 99 matching lines...) Expand 10 before | Expand all | Expand 10 after
110 110
111 class LogLinearChangelistClassifierTest(CrashTestSuite): 111 class LogLinearChangelistClassifierTest(CrashTestSuite):
112 112
113 def setUp(self): 113 def setUp(self):
114 super(LogLinearChangelistClassifierTest, self).setUp() 114 super(LogLinearChangelistClassifierTest, self).setUp()
115 weights = { 115 weights = {
116 'MinDistance': 1., 116 'MinDistance': 1.,
117 'TopFrameIndex': 1., 117 'TopFrameIndex': 1.,
118 } 118 }
119 119
120 repository = GitilesRepository(self.GetMockHttpClient())
121
122 # TODO(crbug.com/677224): should replace this with an actual factory.
123 def MutateTheRepo(dep_url): # pragma: no cover
124 """A factory function for returning ``Repository`` objects.
125
126 The current definition captures the functionality of before
127 we factored out this factory method. That is, it's not really a
128 "factory" but rather mutates the main repo object in place. In
129 the future this should be changed to do the right thing instead.
130 """
131 repository.repo_url = dep_url
132 return repository
133
120 self.changelist_classifier = ( 134 self.changelist_classifier = (
121 loglinear_changelist_classifier.LogLinearChangelistClassifier( 135 loglinear_changelist_classifier.LogLinearChangelistClassifier(
122 GitilesRepository(self.GetMockHttpClient()), weights)) 136 repository, MutateTheRepo, weights))
123 137
124 def testAggregateChangedFilesAggreegates(self): 138 def testAggregateChangedFilesAggreegates(self):
125 """Test that ``AggregateChangedFiles`` does aggregate reasons per file. 139 """Test that ``AggregateChangedFiles`` does aggregate reasons per file.
126 140
127 In the main/inner loop of ``AggregateChangedFiles``: if multiple 141 In the main/inner loop of ``AggregateChangedFiles``: if multiple
128 features all blame the same file change, we try to aggregate those 142 features all blame the same file change, we try to aggregate those
129 reasons so that we only report the file once (with all reasons). None 143 reasons so that we only report the file once (with all reasons). None
130 of the other tests here actually check the case where the same file 144 of the other tests here actually check the case where the same file
131 is blamed multiple times, so we check that here. 145 is blamed multiple times, so we check that here.
132 146
(...skipping 257 matching lines...) Expand 10 before | Expand all | Expand 10 after
390 self.mock(changelist_classifier, 'FindSuspects', _MockFindSuspects) 404 self.mock(changelist_classifier, 'FindSuspects', _MockFindSuspects)
391 self.mock(chrome_dependency_fetcher.ChromeDependencyFetcher, 405 self.mock(chrome_dependency_fetcher.ChromeDependencyFetcher,
392 'GetDependencyRollsDict', 406 'GetDependencyRollsDict',
393 lambda *_: {'src/': DependencyRoll('src/', 'https://repo', '1', '2')}) 407 lambda *_: {'src/': DependencyRoll('src/', 'https://repo', '1', '2')})
394 self.mock(chrome_dependency_fetcher.ChromeDependencyFetcher, 408 self.mock(chrome_dependency_fetcher.ChromeDependencyFetcher,
395 'GetDependency', lambda *_: {}) 409 'GetDependency', lambda *_: {})
396 410
397 suspects = self.changelist_classifier(DUMMY_REPORT) 411 suspects = self.changelist_classifier(DUMMY_REPORT)
398 self.assertFalse(suspects, 'Expected zero suspects, but found some:\n%s' 412 self.assertFalse(suspects, 'Expected zero suspects, but found some:\n%s'
399 % pprint.pformat([suspect.ToDict() for suspect in suspects])) 413 % pprint.pformat([suspect.ToDict() for suspect in suspects]))
OLDNEW
« no previous file with comments | « appengine/findit/crash/loglinear/changelist_classifier.py ('k') | appengine/findit/crash/test/changelist_classifier_test.py » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698