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

Issues Search
    My Issues | Starred     Open | Closed | All

Unified Diff: appengine/findit/crash/test/changelist_classifier_test.py

Issue 2707603002: [Predator] Generate all changelogs in regression ranges instead of only matched changelogs (Closed)
Patch Set: . Created 3 years, 10 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 side-by-side diff with in-line comments
Download patch
« no previous file with comments | « appengine/findit/crash/suspect.py ('k') | appengine/findit/crash/test/suspect_test.py » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: appengine/findit/crash/test/changelist_classifier_test.py
diff --git a/appengine/findit/crash/test/changelist_classifier_test.py b/appengine/findit/crash/test/changelist_classifier_test.py
deleted file mode 100644
index da410c50ab41086481800468c87d796df5850d01..0000000000000000000000000000000000000000
--- a/appengine/findit/crash/test/changelist_classifier_test.py
+++ /dev/null
@@ -1,517 +0,0 @@
-# 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
-from crash import changelist_classifier
-from crash.crash_report import CrashReport
-from crash.suspect import AnalysisInfo
-from crash.suspect import StackInfo
-from crash.suspect import Suspect
-from crash.stacktrace import CallStack
-from crash.stacktrace import StackFrame
-from crash.stacktrace import Stacktrace
-from crash.test.crash_test_suite import CrashTestSuite
-from crash.type_enums import CallStackFormatType
-from crash.type_enums import LanguageType
-from libs.gitiles.blame import Blame
-from libs.gitiles.blame import Region
-from libs.gitiles.change_log import ChangeLog
-from libs.gitiles.gitiles_repository import GitilesRepository
-
-DUMMY_CHANGELOG1 = ChangeLog.FromDict({
- 'author': {
- 'name': 'r@chromium.org',
- 'email': 'r@chromium.org',
- 'time': 'Thu Mar 31 21:24:43 2016',
- },
- 'committer': {
- 'email': 'r@chromium.org',
- 'time': 'Thu Mar 31 21:28:39 2016',
- 'name': 'example@chromium.org',
- },
- 'message': 'dummy',
- 'commit_position': 175900,
- 'touched_files': [
- {
- 'change_type': 'add',
- 'new_path': 'a.cc',
- 'old_path': None,
- },
- ],
- 'commit_url':
- 'https://repo.test/+/1',
- 'code_review_url': 'https://codereview.chromium.org/3281',
- 'revision': '1',
- 'reverted_revision': None
-})
-
-DUMMY_CHANGELOG2 = ChangeLog.FromDict({
- 'author': {
- 'name': 'example@chromium.org',
- 'email': 'example@chromium.org',
- 'time': 'Thu Mar 31 21:24:43 2016',
- },
- 'committer': {
- 'name': 'example@chromium.org',
- 'email': 'example@chromium.org',
- 'time': 'Thu Mar 31 21:28:39 2016',
- },
- 'message': 'dummy',
- 'commit_position': 175976,
- 'touched_files': [
- {
- 'change_type': 'add',
- 'new_path': 'f0.cc',
- 'old_path': 'b/f0.cc'
- },
- ],
- 'commit_url':
- 'https://repo.test/+/2',
- 'code_review_url': 'https://codereview.chromium.org/3281',
- 'revision': '2',
- 'reverted_revision': '1'
-})
-
-DUMMY_CHANGELOG3 = ChangeLog.FromDict({
- 'author': {
- 'name': 'e@chromium.org',
- 'email': 'e@chromium.org',
- 'time': 'Thu Apr 1 21:24:43 2016',
- },
- 'committer': {
- 'name': 'example@chromium.org',
- 'email': 'e@chromium.org',
- 'time': 'Thu Apr 1 21:28:39 2016',
- },
- 'message': 'dummy',
- 'commit_position': 176000,
- 'touched_files': [
- {
- 'change_type': 'modify',
- 'new_path': 'f.cc',
- 'old_path': 'f.cc'
- },
- {
- 'change_type': 'delete',
- 'new_path': None,
- 'old_path': 'f1.cc'
- },
- ],
- 'commit_url':
- 'https://repo.test/+/3',
- 'code_review_url': 'https://codereview.chromium.org/3281',
- 'revision': '3',
- 'reverted_revision': None
-})
-
-# TODO(wrengr): re crrev.com/2414523002: we need to have a specified
-# revision_range (even if the versions therein are None), because
-# ChangelistClassifier.__call__ will take it apart in order to call
-# GetDEPSRollsDict; if it can't then it will immediately return the
-# empty list of suspects, breaking many of the tests here. Of course,
-# taking revision_range apart isn't actually required for the tests,
-# since we mock GetDEPSRollsDict. So, really what we ought to do in the
-# long run is redesign things so that GetDEPSRollsDict takes the
-# CrashReport directly and pulls out the revision_range and platform
-# itself; that way ChangelistClassifier.__call__ needn't worry about it,
-# allowing us to clean up the tests here.
-
-DUMMY_CALLSTACKS = [
- CallStack(0, [], CallStackFormatType.DEFAULT, LanguageType.CPP),
- CallStack(1, [], CallStackFormatType.DEFAULT, LanguageType.CPP)]
-DUMMY_REPORT = CrashReport(
- 'rev', 'sig', 'win', Stacktrace(DUMMY_CALLSTACKS, DUMMY_CALLSTACKS[0]),
- (None, None), None, None)
-
-
-class ChangelistClassifierTest(CrashTestSuite):
-
- @property
- def _mock_http_gitiles_repo_factory(self):
- # N.B., we must return a lambda rather than taking ``repo_url``
- # as an additional argument to the method/property, to ensure that
- # things have the right arity when this is eventually called. That
- # is, we want a function of ``repo_url`` which closes over ``self``,
- # rather than a function taking two arguments at once.
- return lambda repo_url: GitilesRepository(
- self.GetMockHttpClient(), repo_url)
-
- def setUp(self):
- super(ChangelistClassifierTest, self).setUp()
- self.changelist_classifier = changelist_classifier.ChangelistClassifier(
- self._mock_http_gitiles_repo_factory, 7)
-
- def testSkipAddedAndDeletedRegressionRolls(self):
- self.mock(chrome_dependency_fetcher.ChromeDependencyFetcher,
- 'GetDependency', lambda *_: {})
- dep_rolls = {
- 'src/dep': DependencyRoll('src/dep1', 'https://url_dep1', None, '9'),
- 'src/': DependencyRoll('src/', ('https://chromium.googlesource.com/'
- 'chromium/src.git'), '4', '5')
- }
- self.mock(chrome_dependency_fetcher.ChromeDependencyFetcher,
- 'GetDependencyRollsDict', lambda *_: dep_rolls)
-
- passed_in_regression_deps_rolls = []
- def _MockGetChangeLogsForFilesGroupedByDeps(regression_deps_rolls, *_):
- 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, 'FindSuspects', lambda *_: None)
-
- stack = CallStack(0)
- self.changelist_classifier(
- CrashReport(crashed_version = '5',
- signature = 'sig',
- platform = 'canary',
- stacktrace = Stacktrace([stack], stack),
- regression_range = ('4', '5'),
- dependencies = {},
- dependency_rolls = {}))
- 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.
- del expected_regression_deps_rolls['src/dep']
- self.assertEqual(passed_in_regression_deps_rolls[0],
- expected_regression_deps_rolls)
-
- def testGetDepsInCrashStack(self):
- crash_stack = CallStack(0, frame_list=[
- StackFrame(0, 'src/', 'func0', 'f0.cc', 'src/f0.cc', [1]),
- StackFrame(1, 'src/', 'func1', 'f1.cc', 'src/f1.cc', [2, 3]),
- StackFrame(1, '', 'func2', 'f2.cc', 'src/f2.cc', [2, 3])])
- crash_deps = {'src/': Dependency('src/', 'https://chromium_repo', '1'),
- 'src/v8/': Dependency('src/v8/', 'https://v8_repo', '2')}
-
- expected_stack_deps = {'src/': crash_deps['src/']}
-
- self.assertDictEqual(
- changelist_classifier.GetDepsInCrashStack(crash_stack, crash_deps),
- expected_stack_deps)
-
- def testGetChangeLogsForFilesGroupedByDeps(self):
- regression_deps_rolls = {
- 'src/dep': DependencyRoll('src/dep1', 'https://url_dep1', '7', '9'),
- 'src/': DependencyRoll('src/', ('https://chromium.googlesource.com/'
- 'chromium/src.git'), '4', '5')
- }
-
- stack_deps = {
- 'src/': Dependency('src/', 'https://url_src', 'rev1', 'DEPS'),
- 'src/new': Dependency('src/new', 'https://new', 'rev2', 'DEPS'),
- 'src/dep': Dependency('src/dep', 'https://url_dep', 'rev', 'DEPS'),
- }
-
- 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,
- self._mock_http_gitiles_repo_factory))
- 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()]
- }
- }
- self.assertDictEqual(dep_file_to_changelogs_json,
- expected_dep_file_to_changelogs_json)
- self.assertSetEqual(ignore_cls, set(['1']))
-
- def testGetStackInfosForFilesGroupedByDeps(self):
- main_stack = CallStack(0, frame_list=[
- StackFrame(0, 'src/', 'c(p* &d)', 'a.cc', 'src/a.cc', [177]),
- StackFrame(1, 'src/', 'd(a* c)', 'a.cc', 'src/a.cc', [227, 228, 229]),
- StackFrame(2, 'src/v8/', 'e(int)', 'b.cc', 'src/v8/b.cc', [89, 90])])
-
- low_priority_stack = CallStack(1, frame_list=[
- StackFrame(0, 'src/dummy/', 'c(p* &d)', 'd.cc', 'src/dummy/d.cc',
- [17])])
-
- stacktrace = Stacktrace([main_stack, low_priority_stack], main_stack)
-
- crashed_deps = {'src/': Dependency('src/', 'https//repo', '2'),
- 'src/v8/': Dependency('src/v8', 'https//repo', '1')}
-
- expected_dep_file_to_stack_infos = {
- 'src/': {
- 'a.cc': [
- StackInfo(main_stack.frames[0], 0),
- StackInfo(main_stack.frames[1], 0),
- ],
- },
- 'src/v8/': {
- 'b.cc': [
- StackInfo(main_stack.frames[2], 0),
- ]
- }
- }
-
- dep_file_to_stack_infos = (
- changelist_classifier.GetStackInfosForFilesGroupedByDeps(
- stacktrace, crashed_deps))
-
- self.assertEqual(len(dep_file_to_stack_infos),
- len(expected_dep_file_to_stack_infos))
-
- for dep, file_to_stack_infos in dep_file_to_stack_infos.iteritems():
- self.assertTrue(dep in expected_dep_file_to_stack_infos)
- expected_file_to_stack_infos = expected_dep_file_to_stack_infos[dep]
-
- for file_path, stack_infos in file_to_stack_infos.iteritems():
- self.assertTrue(file_path in expected_file_to_stack_infos)
- expected_stack_infos = expected_file_to_stack_infos[file_path]
-
- self._VerifyTwoStackInfosEqual(stack_infos, expected_stack_infos)
-
- def testFindSuspects(self):
- dep_file_to_changelogs = {
- 'src/': {
- 'a.cc': [
- DUMMY_CHANGELOG1,
- ]
- }
- }
-
- dep_file_to_stack_infos = {
- 'src/': {
- 'a.cc': [
- StackInfo(StackFrame(
- 0, 'src/', 'func', 'a.cc', 'src/a.cc', [1]), 0),
- StackInfo(StackFrame(
- 1, 'src/', 'func', 'a.cc', 'src/a.cc', [7]), 0),
- ],
- 'b.cc': [
- StackInfo(StackFrame(
- 2, 'src/', 'func', 'b.cc', 'src/b.cc', [36]), 0),
- ]
- }
- }
-
- dummy_blame = Blame('9', 'a.cc')
- dummy_blame.AddRegions([
- Region(1, 5, '6', 'a', 'a@chromium.org', 'Thu Mar 31 21:24:43 2016'),
- Region(6, 10, '1', 'b', 'b@chromium.org', 'Thu Jun 19 12:11:40 2015')])
-
- self.mock(GitilesRepository, 'GetBlame', lambda *_: dummy_blame)
-
- stack_deps = {
- 'src/': Dependency('src/', 'https://url_src', 'rev1', 'DEPS'),
- }
-
- expected_suspects = [{
- 'url': 'https://repo.test/+/1',
- 'review_url': 'https://codereview.chromium.org/3281',
- 'revision': '1',
- 'project_path': 'src/',
- 'author': 'r@chromium.org',
- 'time': 'Thu Mar 31 21:24:43 2016',
- 'reasons': None,
- 'confidence': None,
- 'changed_files': None
- }]
-
- suspects = changelist_classifier.FindSuspects(
- dep_file_to_changelogs, dep_file_to_stack_infos, stack_deps,
- self._mock_http_gitiles_repo_factory)
- self.assertListEqual([suspect.ToDict() for suspect in suspects],
- expected_suspects)
-
- # 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 *_: {})
- # N.B., for this one test we really do want regression_range=None.
- report = DUMMY_REPORT._replace(regression_range=None)
- self.assertListEqual(self.changelist_classifier(report), [])
-
- def testFindItForCrashNoMatchFound(self):
- self.mock(changelist_classifier, 'FindSuspects', lambda *_: [])
- self.mock(chrome_dependency_fetcher.ChromeDependencyFetcher,
- 'GetDependencyRollsDict',
- lambda *_: {'src/': DependencyRoll('src/', 'https://repo', '1', '2')})
- self.mock(chrome_dependency_fetcher.ChromeDependencyFetcher,
- 'GetDependency', lambda *_: {})
- self.assertListEqual(self.changelist_classifier(DUMMY_REPORT), [])
-
- def testFindItForCrash(self):
-
- def _MockFindSuspects(*_):
- suspect1 = Suspect(DUMMY_CHANGELOG1, 'src/', confidence=0.)
- frame1 = StackFrame(0, 'src/', 'func', 'a.cc', 'src/a.cc', [1])
- frame2 = StackFrame(1, 'src/', 'func', 'a.cc', 'src/a.cc', [7])
- suspect1.file_to_stack_infos = {
- 'a.cc': [StackInfo(frame1, 0), StackInfo(frame2, 0)]
- }
- suspect1.file_to_analysis_info = {
- 'a.cc': AnalysisInfo(min_distance=0, min_distance_frame=frame1)
- }
-
- suspect2 = Suspect(DUMMY_CHANGELOG3, 'src/', confidence=0.)
- frame3 = StackFrame(5, 'src/', 'func', 'f.cc', 'src/f.cc', [1])
- suspect2.file_to_stack_infos = {
- 'f.cc': [StackInfo(frame3, 0)]
- }
- suspect2.file_to_analysis_info = {
- 'a.cc': AnalysisInfo(min_distance=20, min_distance_frame=frame3)
- }
-
- return [suspect1, suspect2]
-
- self.mock(changelist_classifier, 'FindSuspects', _MockFindSuspects)
- self.mock(chrome_dependency_fetcher.ChromeDependencyFetcher,
- 'GetDependencyRollsDict',
- lambda *_: {'src/': DependencyRoll('src/', 'https://repo', '1', '2')})
- self.mock(chrome_dependency_fetcher.ChromeDependencyFetcher,
- 'GetDependency', lambda *_: {})
- suspects = self.changelist_classifier(DUMMY_REPORT)
- expected_suspects = [
- {
- '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',
- 'project_path': 'src/',
- 'review_url': 'https://codereview.chromium.org/3281',
- 'confidence': 1.0, 'revision': '1'
- },
- ]
- self.assertListEqual([suspect.ToDict() for suspect in suspects],
- expected_suspects)
-
- def testFinditForCrashFilterZeroConfidentResults(self):
- def _MockFindSuspects(*_):
- suspect1 = Suspect(DUMMY_CHANGELOG1, 'src/', confidence=0.)
- frame1 = StackFrame(0, 'src/', 'func', 'a.cc', 'src/a.cc', [1])
- frame2 = StackFrame(1, 'src/', 'func', 'a.cc', 'src/a.cc', [7])
- suspect1.file_to_stack_infos = {
- 'a.cc': [StackInfo(frame1, 0), StackInfo(frame2, 0)]
- }
- suspect1.file_to_analysis_info = {
- 'a.cc': AnalysisInfo(min_distance=1, min_distance_frame=frame1)
- }
-
- suspect2 = Suspect(DUMMY_CHANGELOG3, 'src/', confidence=0.)
- frame3 = StackFrame(15, 'src/', 'func', 'f.cc', 'src/f.cc', [1])
- suspect2.file_to_stack_infos = {
- 'f.cc': [StackInfo(frame3, 0)]
- }
- suspect2.file_to_analysis_info = {
- 'f.cc': AnalysisInfo(min_distance=20, min_distance_frame=frame3)
- }
-
- suspect3 = Suspect(DUMMY_CHANGELOG3, 'src/', confidence=0.)
- frame4 = StackFrame(3, 'src/', 'func', 'ff.cc', 'src/ff.cc', [1])
- suspect3.file_to_stack_infos = {
- 'f.cc': [StackInfo(frame4, 0)]
- }
- suspect3.file_to_analysis_info = {
- 'f.cc': AnalysisInfo(min_distance=60, min_distance_frame=frame4)
- }
-
- return [suspect1, suspect2, suspect3]
-
- self.mock(changelist_classifier, 'FindSuspects', _MockFindSuspects)
- self.mock(chrome_dependency_fetcher.ChromeDependencyFetcher,
- 'GetDependencyRollsDict',
- lambda *_: {'src/': DependencyRoll('src/', 'https://repo', '1', '2')})
- self.mock(chrome_dependency_fetcher.ChromeDependencyFetcher,
- 'GetDependency', lambda *_: {})
-
- suspects = self.changelist_classifier(DUMMY_REPORT)
- expected_suspects = [
- {
- 'author': 'r@chromium.org',
- 'changed_files': [
- {
- 'blame_url': None,
- 'file': 'a.cc',
- 'info': 'Minimum distance (LOC) 1, frame #0'
- }
- ],
- 'confidence': 0.8,
- 'project_path': 'src/',
- 'reasons': [
- ('TopFrameIndex', 1.0, 'Top frame is #0'),
- ('MinDistance', 0.8, 'Minimum distance is 1')
- ],
- 'review_url': 'https://codereview.chromium.org/3281',
- 'revision': '1',
- 'time': 'Thu Mar 31 21:24:43 2016',
- 'url': 'https://repo.test/+/1'
- }
- ]
- self.assertListEqual([suspect.ToDict() for suspect in suspects],
- expected_suspects)
-
- def testFinditForCrashAllSuspectsWithZeroConfidences(self):
- """Test that we filter out suspects with too-large frame indices.
-
- In the mock suspects below we return frames with indices
- 15, 20, 21 which are all larger than the ``max_top_n`` of
- ``TopFrameIndexScorer``. Therefore we should get a score of zero
- for that feature, which causes the total score to be zero, and so
- we should not return these suspects.
- """
- def _MockFindSuspects(*_):
- suspect1 = Suspect(DUMMY_CHANGELOG1, 'src/', confidence=0.)
- frame1 = StackFrame(20, 'src/', '', 'func', 'a.cc', [1])
- frame2 = StackFrame(21, 'src/', '', 'func', 'a.cc', [7])
- suspect1.file_to_stack_infos = {
- 'a.cc': [StackInfo(frame1, 0), StackInfo(frame2, 0)]
- }
- suspect1.file_to_analysis_info = {
- 'a.cc': AnalysisInfo(min_distance=1, min_distance_frame=frame1)
- }
-
- suspect2 = Suspect(DUMMY_CHANGELOG3, 'src/', confidence=0.)
- frame3 = StackFrame(15, 'src/', '', 'func', 'f.cc', [1])
- suspect2.file_to_stack_infos = {
- 'f.cc': [StackInfo(frame3, 0)]
- }
- suspect2.min_distance = 20
- suspect2.file_to_analysis_info = {
- 'f.cc': AnalysisInfo(min_distance=20, min_distance_frame=frame3)
- }
-
- return [suspect1, suspect2]
-
- self.mock(changelist_classifier, 'FindSuspects', _MockFindSuspects)
- self.mock(chrome_dependency_fetcher.ChromeDependencyFetcher,
- 'GetDependencyRollsDict',
- lambda *_: {'src/': DependencyRoll('src/', 'https://repo', '1', '2')})
- self.mock(chrome_dependency_fetcher.ChromeDependencyFetcher,
- 'GetDependency', lambda *_: {})
-
- self.assertListEqual(self.changelist_classifier(DUMMY_REPORT), [])
« no previous file with comments | « appengine/findit/crash/suspect.py ('k') | appengine/findit/crash/test/suspect_test.py » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698