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

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

Issue 2449853012: [Predator] Fix bug in min_distance after refactor and add back skip added/deleted deps. (Closed)
Patch Set: Rebase Created 4 years, 1 month 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 from collections import defaultdict 5 from collections import defaultdict
6 import copy
6 7
7 from common.dependency import Dependency 8 from common.dependency import Dependency
8 from common.dependency import DependencyRoll 9 from common.dependency import DependencyRoll
9 from common.http_client_appengine import HttpClientAppengine
10 from common import chrome_dependency_fetcher 10 from common import chrome_dependency_fetcher
11 from crash import changelist_classifier
11 from crash.crash_report import CrashReport 12 from crash.crash_report import CrashReport
12 from crash import changelist_classifier 13 from crash.results import AnalysisInfo
13 from crash.results import MatchResult 14 from crash.results import MatchResult
14 from crash.stacktrace import CallStack 15 from crash.stacktrace import CallStack
15 from crash.stacktrace import StackFrame 16 from crash.stacktrace import StackFrame
16 from crash.stacktrace import Stacktrace 17 from crash.stacktrace import Stacktrace
17 from crash.test.crash_test_suite import CrashTestSuite 18 from crash.test.crash_test_suite import CrashTestSuite
18 from lib.gitiles.blame import Blame 19 from lib.gitiles.blame import Blame
19 from lib.gitiles.blame import Region 20 from lib.gitiles.blame import Region
20 from lib.gitiles.change_log import ChangeLog 21 from lib.gitiles.change_log import ChangeLog
21 from lib.gitiles.gitiles_repository import GitilesRepository 22 from lib.gitiles.gitiles_repository import GitilesRepository
22 23
(...skipping 79 matching lines...) Expand 10 before | Expand all | Expand 10 after
102 # taking revision_range apart isn't actually required for the tests, 103 # taking revision_range apart isn't actually required for the tests,
103 # since we mock GetDEPSRollsDict. So, really what we ought to do in the 104 # since we mock GetDEPSRollsDict. So, really what we ought to do in the
104 # long run is redesign things so that GetDEPSRollsDict takes the 105 # long run is redesign things so that GetDEPSRollsDict takes the
105 # CrashReport directly and pulls out the revision_range and platform 106 # CrashReport directly and pulls out the revision_range and platform
106 # itself; that way ChangelistClassifier.__call__ needn't worry about it, 107 # itself; that way ChangelistClassifier.__call__ needn't worry about it,
107 # allowing us to clean up the tests here. 108 # allowing us to clean up the tests here.
108 DUMMY_REPORT = CrashReport(None, None, None, Stacktrace(), (None, None)) 109 DUMMY_REPORT = CrashReport(None, None, None, Stacktrace(), (None, None))
109 110
110 class ChangelistClassifierTest(CrashTestSuite): 111 class ChangelistClassifierTest(CrashTestSuite):
111 112
113 def testSkipAddedAndDeletedRegressionRolls(self):
114 self.mock(chrome_dependency_fetcher.ChromeDependencyFetcher,
115 'GetDependency', lambda *_: {})
116 dep_rolls = {
117 'src/dep': DependencyRoll('src/dep1', 'https://url_dep1', None, '9'),
118 'src/': DependencyRoll('src/', ('https://chromium.googlesource.com/'
119 'chromium/src.git'), '4', '5')
120 }
121 self.mock(chrome_dependency_fetcher.ChromeDependencyFetcher,
122 'GetDependencyRollsDict', lambda *_: dep_rolls)
123
124 passed_in_regression_deps_rolls = []
125 def _MockGetChangeLogsForFilesGroupedByDeps(regression_deps_rolls, *_):
126 passed_in_regression_deps_rolls.append(regression_deps_rolls)
127 return {}, None
128
129 self.mock(changelist_classifier, 'GetChangeLogsForFilesGroupedByDeps',
130 _MockGetChangeLogsForFilesGroupedByDeps)
131 self.mock(changelist_classifier, 'GetStackInfosForFilesGroupedByDeps',
132 lambda *_: {})
133 self.mock(changelist_classifier, 'FindMatchResults', lambda *_: None)
134
135 cl_classifier = changelist_classifier.ChangelistClassifier(
136 GitilesRepository(), 7)
137 cl_classifier(CrashReport(crashed_version = '5',
138 signature = 'sig',
139 platform = 'canary',
140 stacktrace = Stacktrace([CallStack(0)]),
141 regression_range = ['4', '5']))
142 expected_regression_deps_rolls = copy.deepcopy(dep_rolls)
143
144 # Regression of a dep added/deleted (old_revision/new_revision is None) can
145 # not be known for sure and this case rarely happens, so just filter them
146 # out.
147 del expected_regression_deps_rolls['src/dep']
148 self.assertEqual(passed_in_regression_deps_rolls[0],
149 expected_regression_deps_rolls)
150
112 def testGetDepsInCrashStack(self): 151 def testGetDepsInCrashStack(self):
113 crash_stack = CallStack(0) 152 crash_stack = CallStack(0)
114 crash_stack.extend([ 153 crash_stack.extend([
115 StackFrame(0, 'src/', 'func0', 'f0.cc', 'src/f0.cc', [1]), 154 StackFrame(0, 'src/', 'func0', 'f0.cc', 'src/f0.cc', [1]),
116 StackFrame(1, 'src/', 'func1', 'f1.cc', 'src/f1.cc', [2, 3]), 155 StackFrame(1, 'src/', 'func1', 'f1.cc', 'src/f1.cc', [2, 3]),
117 StackFrame(1, '', 'func2', 'f2.cc', 'src/f2.cc', [2, 3]), 156 StackFrame(1, '', 'func2', 'f2.cc', 'src/f2.cc', [2, 3]),
118 ]) 157 ])
119 crash_deps = {'src/': Dependency('src/', 'https://chromium_repo', '1'), 158 crash_deps = {'src/': Dependency('src/', 'https://chromium_repo', '1'),
120 'src/v8/': Dependency('src/v8/', 'https://v8_repo', '2')} 159 'src/v8/': Dependency('src/v8/', 'https://v8_repo', '2')}
121 160
122 expected_stack_deps = {'src/': crash_deps['src/']} 161 expected_stack_deps = {'src/': crash_deps['src/']}
123 162
124 self.assertDictEqual( 163 self.assertDictEqual(
125 changelist_classifier.GetDepsInCrashStack(crash_stack, crash_deps), 164 changelist_classifier.GetDepsInCrashStack(crash_stack, crash_deps),
126 expected_stack_deps) 165 expected_stack_deps)
127 166
128 def testGetChangeLogsForFilesGroupedByDeps(self): 167 def testGetChangeLogsForFilesGroupedByDeps(self):
129 regression_deps_rolls = { 168 regression_deps_rolls = {
130 'src/dep1': DependencyRoll('src/dep1', 'https://url_dep1', '7', '9'), 169 'src/dep': DependencyRoll('src/dep1', 'https://url_dep1', '7', '9'),
131 'src/dep2': DependencyRoll('src/dep2', 'repo_url', '3', None),
132 'src/': DependencyRoll('src/', ('https://chromium.googlesource.com/' 170 'src/': DependencyRoll('src/', ('https://chromium.googlesource.com/'
133 'chromium/src.git'), '4', '5') 171 'chromium/src.git'), '4', '5')
134 } 172 }
135 173
136 stack_deps = { 174 stack_deps = {
137 'src/': Dependency('src/', 'https://url_src', 'rev1', 'DEPS'), 175 'src/': Dependency('src/', 'https://url_src', 'rev1', 'DEPS'),
138 'src/new': Dependency('src/new', 'https://new', 'rev2', 'DEPS'), 176 'src/new': Dependency('src/new', 'https://new', 'rev2', 'DEPS'),
177 'src/dep': Dependency('src/dep', 'https://url_dep', 'rev', 'DEPS'),
139 } 178 }
140 179
141 def _MockGetChangeLogs(*_): 180 def _MockGetChangeLogs(_, start_rev, end_rev):
142 return [DUMMY_CHANGELOG1, DUMMY_CHANGELOG2, DUMMY_CHANGELOG3] 181 if start_rev == '4' and end_rev == '5':
182 return [DUMMY_CHANGELOG1, DUMMY_CHANGELOG2, DUMMY_CHANGELOG3]
183
184 return []
143 185
144 self.mock(GitilesRepository, 'GetChangeLogs', _MockGetChangeLogs) 186 self.mock(GitilesRepository, 'GetChangeLogs', _MockGetChangeLogs)
145 187
146 dep_file_to_changelogs, ignore_cls = ( 188 dep_file_to_changelogs, ignore_cls = (
147 changelist_classifier.GetChangeLogsForFilesGroupedByDeps( 189 changelist_classifier.GetChangeLogsForFilesGroupedByDeps(
148 regression_deps_rolls, stack_deps, GitilesRepository())) 190 regression_deps_rolls, stack_deps, GitilesRepository()))
149 dep_file_to_changelogs_json = defaultdict(lambda: defaultdict(list)) 191 dep_file_to_changelogs_json = defaultdict(lambda: defaultdict(list))
150 for dep, file_to_changelogs in dep_file_to_changelogs.iteritems(): 192 for dep, file_to_changelogs in dep_file_to_changelogs.iteritems():
151 for file_path, changelogs in file_to_changelogs.iteritems(): 193 for file_path, changelogs in file_to_changelogs.iteritems():
152 for changelog in changelogs: 194 for changelog in changelogs:
(...skipping 135 matching lines...) Expand 10 before | Expand all | Expand 10 after
288 def testFindItForCrash(self): 330 def testFindItForCrash(self):
289 331
290 def _MockFindMatchResults(*_): 332 def _MockFindMatchResults(*_):
291 match_result1 = MatchResult(DUMMY_CHANGELOG1, 'src/', '') 333 match_result1 = MatchResult(DUMMY_CHANGELOG1, 'src/', '')
292 frame1 = StackFrame(0, 'src/', 'func', 'a.cc', 'src/a.cc', [1]) 334 frame1 = StackFrame(0, 'src/', 'func', 'a.cc', 'src/a.cc', [1])
293 frame2 = StackFrame(1, 'src/', 'func', 'a.cc', 'src/a.cc', [7]) 335 frame2 = StackFrame(1, 'src/', 'func', 'a.cc', 'src/a.cc', [7])
294 match_result1.file_to_stack_infos = { 336 match_result1.file_to_stack_infos = {
295 'a.cc': [(frame1, 0), (frame2, 0)] 337 'a.cc': [(frame1, 0), (frame2, 0)]
296 } 338 }
297 match_result1.file_to_analysis_info = { 339 match_result1.file_to_analysis_info = {
298 'a.cc': {'min_distance': 0, 'min_distance_frame': frame1} 340 'a.cc': AnalysisInfo(min_distance=0, min_distance_frame=frame1)
299 } 341 }
300 342
301 match_result2 = MatchResult(DUMMY_CHANGELOG3, 'src/', '') 343 match_result2 = MatchResult(DUMMY_CHANGELOG3, 'src/', '')
302 frame3 = StackFrame(5, 'src/', 'func', 'f.cc', 'src/f.cc', [1]) 344 frame3 = StackFrame(5, 'src/', 'func', 'f.cc', 'src/f.cc', [1])
303 match_result2.file_to_stack_infos = { 345 match_result2.file_to_stack_infos = {
304 'f.cc': [(frame3, 0)] 346 'f.cc': [(frame3, 0)]
305 } 347 }
306 match_result2.file_to_analysis_info = { 348 match_result2.file_to_analysis_info = {
307 'a.cc': {'min_distance': 20, 'min_distance_frame': frame3} 349 'a.cc': AnalysisInfo(min_distance=20, min_distance_frame=frame3)
308 } 350 }
309 351
310 return [match_result1, match_result2] 352 return [match_result1, match_result2]
311 353
312 self.mock(changelist_classifier, 'FindMatchResults', _MockFindMatchResults) 354 self.mock(changelist_classifier, 'FindMatchResults', _MockFindMatchResults)
313 self.mock(chrome_dependency_fetcher.ChromeDependencyFetcher, 355 self.mock(chrome_dependency_fetcher.ChromeDependencyFetcher,
314 'GetDependencyRollsDict', 356 'GetDependencyRollsDict',
315 lambda *_: {'src/': DependencyRoll('src/', 'https://repo', '1', '2')}) 357 lambda *_: {'src/': DependencyRoll('src/', 'https://repo', '1', '2')})
316 self.mock(chrome_dependency_fetcher.ChromeDependencyFetcher, 358 self.mock(chrome_dependency_fetcher.ChromeDependencyFetcher,
317 'GetDependency', lambda *_: {}) 359 'GetDependency', lambda *_: {})
(...skipping 19 matching lines...) Expand all
337 379
338 def testFinditForCrashFilterZeroConfidentResults(self): 380 def testFinditForCrashFilterZeroConfidentResults(self):
339 def _MockFindMatchResults(*_): 381 def _MockFindMatchResults(*_):
340 match_result1 = MatchResult(DUMMY_CHANGELOG1, 'src/', '') 382 match_result1 = MatchResult(DUMMY_CHANGELOG1, 'src/', '')
341 frame1 = StackFrame(0, 'src/', 'func', 'a.cc', 'src/a.cc', [1]) 383 frame1 = StackFrame(0, 'src/', 'func', 'a.cc', 'src/a.cc', [1])
342 frame2 = StackFrame(1, 'src/', 'func', 'a.cc', 'src/a.cc', [7]) 384 frame2 = StackFrame(1, 'src/', 'func', 'a.cc', 'src/a.cc', [7])
343 match_result1.file_to_stack_infos = { 385 match_result1.file_to_stack_infos = {
344 'a.cc': [(frame1, 0), (frame2, 0)] 386 'a.cc': [(frame1, 0), (frame2, 0)]
345 } 387 }
346 match_result1.file_to_analysis_info = { 388 match_result1.file_to_analysis_info = {
347 'a.cc': {'min_distance': 1, 'min_distance_frame': frame1} 389 'a.cc': AnalysisInfo(min_distance=1, min_distance_frame=frame1)
348 } 390 }
349 391
350 match_result2 = MatchResult(DUMMY_CHANGELOG3, 'src/', '') 392 match_result2 = MatchResult(DUMMY_CHANGELOG3, 'src/', '')
351 frame3 = StackFrame(15, 'src/', 'func', 'f.cc', 'src/f.cc', [1]) 393 frame3 = StackFrame(15, 'src/', 'func', 'f.cc', 'src/f.cc', [1])
352 match_result2.file_to_stack_infos = { 394 match_result2.file_to_stack_infos = {
353 'f.cc': [(frame3, 0)] 395 'f.cc': [(frame3, 0)]
354 } 396 }
355 match_result2.file_to_analysis_info = { 397 match_result2.file_to_analysis_info = {
356 'f.cc': {'min_distance': 20, 'min_distance_frame': frame3} 398 'f.cc': AnalysisInfo(min_distance=20, min_distance_frame=frame3)
357 } 399 }
358 400
359 match_result3 = MatchResult(DUMMY_CHANGELOG3, 'src/', '') 401 match_result3 = MatchResult(DUMMY_CHANGELOG3, 'src/', '')
360 frame4 = StackFrame(3, 'src/', 'func', 'ff.cc', 'src/ff.cc', [1]) 402 frame4 = StackFrame(3, 'src/', 'func', 'ff.cc', 'src/ff.cc', [1])
361 match_result3.file_to_stack_infos = { 403 match_result3.file_to_stack_infos = {
362 'f.cc': [(frame4, 0)] 404 'f.cc': [(frame4, 0)]
363 } 405 }
364 match_result3.file_to_analysis_info = { 406 match_result3.file_to_analysis_info = {
365 'f.cc': {'min_distance': 60, 'min_distance_frame': frame4} 407 'f.cc': AnalysisInfo(min_distance=60, min_distance_frame=frame4)
366 } 408 }
367 409
368 return [match_result1, match_result2, match_result3] 410 return [match_result1, match_result2, match_result3]
369 411
370 self.mock(changelist_classifier, 'FindMatchResults', _MockFindMatchResults) 412 self.mock(changelist_classifier, 'FindMatchResults', _MockFindMatchResults)
371 self.mock(chrome_dependency_fetcher.ChromeDependencyFetcher, 413 self.mock(chrome_dependency_fetcher.ChromeDependencyFetcher,
372 'GetDependencyRollsDict', 414 'GetDependencyRollsDict',
373 lambda *_: {'src/': DependencyRoll('src/', 'https://repo', '1', '2')}) 415 lambda *_: {'src/': DependencyRoll('src/', 'https://repo', '1', '2')})
374 self.mock(chrome_dependency_fetcher.ChromeDependencyFetcher, 416 self.mock(chrome_dependency_fetcher.ChromeDependencyFetcher,
375 'GetDependency', lambda *_: {}) 417 'GetDependency', lambda *_: {})
(...skipping 28 matching lines...) Expand all
404 446
405 def testFinditForCrashAllMatchResultsWithZeroConfidences(self): 447 def testFinditForCrashAllMatchResultsWithZeroConfidences(self):
406 def _MockFindMatchResults(*_): 448 def _MockFindMatchResults(*_):
407 match_result1 = MatchResult(DUMMY_CHANGELOG1, 'src/', '') 449 match_result1 = MatchResult(DUMMY_CHANGELOG1, 'src/', '')
408 frame1 = StackFrame(20, 'src/', '', 'func', 'a.cc', [1]) 450 frame1 = StackFrame(20, 'src/', '', 'func', 'a.cc', [1])
409 frame2 = StackFrame(21, 'src/', '', 'func', 'a.cc', [7]) 451 frame2 = StackFrame(21, 'src/', '', 'func', 'a.cc', [7])
410 match_result1.file_to_stack_infos = { 452 match_result1.file_to_stack_infos = {
411 'a.cc': [(frame1, 0), (frame2, 0)] 453 'a.cc': [(frame1, 0), (frame2, 0)]
412 } 454 }
413 match_result1.file_to_analysis_info = { 455 match_result1.file_to_analysis_info = {
414 'a.cc': {'min_distance': 1, 'min_distance_frame': frame1} 456 'a.cc': AnalysisInfo(min_distance=1, min_distance_frame=frame1)
415 } 457 }
416 458
417 match_result2 = MatchResult(DUMMY_CHANGELOG3, 'src/', '') 459 match_result2 = MatchResult(DUMMY_CHANGELOG3, 'src/', '')
418 frame3 = StackFrame(15, 'src/', '', 'func', 'f.cc', [1]) 460 frame3 = StackFrame(15, 'src/', '', 'func', 'f.cc', [1])
419 match_result2.file_to_stack_infos = { 461 match_result2.file_to_stack_infos = {
420 'f.cc': [(frame3, 0)] 462 'f.cc': [(frame3, 0)]
421 } 463 }
422 match_result2.min_distance = 20 464 match_result2.min_distance = 20
423 match_result2.file_to_analysis_info = { 465 match_result2.file_to_analysis_info = {
424 'f.cc': {'min_distance': 20, 'min_distance_frame': frame3} 466 'f.cc': AnalysisInfo(min_distance=20, min_distance_frame=frame3)
425 } 467 }
426 468
427 return [match_result1, match_result2] 469 return [match_result1, match_result2]
428 470
429 self.mock(changelist_classifier, 'FindMatchResults', _MockFindMatchResults) 471 self.mock(changelist_classifier, 'FindMatchResults', _MockFindMatchResults)
430 self.mock(chrome_dependency_fetcher.ChromeDependencyFetcher, 472 self.mock(chrome_dependency_fetcher.ChromeDependencyFetcher,
431 'GetDependencyRollsDict', 473 'GetDependencyRollsDict',
432 lambda *_: {'src/': DependencyRoll('src/', 'https://repo', '1', '2')}) 474 lambda *_: {'src/': DependencyRoll('src/', 'https://repo', '1', '2')})
433 self.mock(chrome_dependency_fetcher.ChromeDependencyFetcher, 475 self.mock(chrome_dependency_fetcher.ChromeDependencyFetcher,
434 'GetDependency', lambda *_: {}) 476 'GetDependency', lambda *_: {})
435 477
436 cl_classifier = changelist_classifier.ChangelistClassifier(7, 478 cl_classifier = changelist_classifier.ChangelistClassifier(7,
437 GitilesRepository()) 479 GitilesRepository())
438 self.assertListEqual(cl_classifier(DUMMY_REPORT), []) 480 self.assertListEqual(cl_classifier(DUMMY_REPORT), [])
OLDNEW
« no previous file with comments | « appengine/findit/crash/scorers/test/min_distance_test.py ('k') | appengine/findit/crash/test/culprit_test.py » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698