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

Issues Search
    My Issues | Starred     Open | Closed | All

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

Issue 2613153006: [Predator] Add TouchCrashedFileMetaFeature. (Closed)
Patch Set: Add comments. 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 side-by-side diff with in-line comments
Download patch
« no previous file with comments | « appengine/findit/crash/loglinear/feature.py ('k') | appengine/findit/crash/loglinear/test/feature_test.py » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: appengine/findit/crash/loglinear/test/changelist_classifier_test.py
diff --git a/appengine/findit/crash/loglinear/test/changelist_classifier_test.py b/appengine/findit/crash/loglinear/test/changelist_classifier_test.py
index 580f07e8b75688c2d09a0196f8b07e5b00e9be99..f1817b57da98096f3c2f8fb38501e0b08a0cdb62 100644
--- a/appengine/findit/crash/loglinear/test/changelist_classifier_test.py
+++ b/appengine/findit/crash/loglinear/test/changelist_classifier_test.py
@@ -13,9 +13,8 @@ from common.chrome_dependency_fetcher import ChromeDependencyFetcher
import crash.changelist_classifier as scorer_changelist_classifier
from crash.crash_report import CrashReport
from crash.loglinear.changelist_classifier import LogLinearChangelistClassifier
-from crash.loglinear.changelist_features.min_distance import MinDistanceFeature
-from crash.loglinear.changelist_features.top_frame_index import (
- TopFrameIndexFeature)
+from crash.loglinear.changelist_features.touch_crashed_file_meta import (
+ TouchCrashedFileMetaFeature)
from crash.loglinear.feature import WrapperMetaFeature
from crash.loglinear.weight import Weight
from crash.loglinear.weight import MetaWeight
@@ -28,6 +27,8 @@ 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
@@ -127,15 +128,18 @@ class LogLinearChangelistClassifierTest(CrashTestSuite):
def setUp(self):
super(LogLinearChangelistClassifierTest, self).setUp()
meta_weight = MetaWeight({
- 'MinDistance': Weight(1.),
- 'TopFrameIndex': Weight(1.),
+ 'TouchCrashedFileMeta': MetaWeight({
+ 'MinDistance': Weight(1.),
+ 'TopFrameIndex': Weight(1.),
+ 'TouchCrashedFile': Weight(1.),
+ })
})
- meta_feature = WrapperMetaFeature([MinDistanceFeature(),
- TopFrameIndexFeature()])
+ get_repository = GitilesRepository.Factory(self.GetMockHttpClient())
+ meta_feature = WrapperMetaFeature(
+ [TouchCrashedFileMetaFeature(get_repository)])
self.changelist_classifier = LogLinearChangelistClassifier(
- GitilesRepository.Factory(self.GetMockHttpClient()),
- meta_feature, meta_weight)
+ get_repository, meta_feature, meta_weight)
# TODO(http://crbug.com/659346): why do these mocks give coverage
# failures? That's almost surely hiding a bug in the tests themselves.
@@ -143,7 +147,9 @@ class LogLinearChangelistClassifierTest(CrashTestSuite):
self.mock(ChromeDependencyFetcher, 'GetDependencyRollsDict', lambda *_: {})
self.mock(ChromeDependencyFetcher, 'GetDependency', lambda *_: {})
# N.B., for this one test we really do want regression_range=None.
- report = DUMMY_REPORT._replace(regression_range=None)
+ report = CrashReport(None, None, None, Stacktrace(DUMMY_CALLSTACKS,
+ DUMMY_CALLSTACKS[0]),
+ None)
self.assertListEqual(self.changelist_classifier(report), [])
def testFindItForCrashNoMatchFound(self):
@@ -155,131 +161,115 @@ class LogLinearChangelistClassifierTest(CrashTestSuite):
def testFindItForCrash(self):
suspect1 = Suspect(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])
- 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/')
- 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)
- }
- self.mock(scorer_changelist_classifier, 'FindSuspects', lambda *_:
- [suspect1, suspect2])
- self.mock(ChromeDependencyFetcher, 'GetDependencyRollsDict', lambda *_:
- {'src/': DependencyRoll('src/', 'https://repo', '1', '2')})
- self.mock(ChromeDependencyFetcher, 'GetDependency', lambda *_:
- {'src/': Dependency('src/', 'https://repo', '2')})
-
- # N.B., In order to get complete coverage for the code computing
- # ``dep_to_file_to_stack_infos`` we must (a) have frames on at
- # least one stack, (b) have frames with dependencies in the
- # CrashReportWithDependency's ``dependencies``, and (c) have frames
- # with dependencies *not* in CrashReportWithDependency.
+ a_cc_blame = Blame('6', 'src/')
+ a_cc_blame.AddRegions([Region(0, 10, suspect1.changelog.revision,
+ suspect1.changelog.author.name,
+ suspect1.changelog.author.email,
+ suspect1.changelog.author.time)])
+ f_cc_blame = Blame('6', 'src/')
+ f_cc_blame.AddRegions([Region(21, 10, suspect2.changelog.revision,
+ suspect2.changelog.author.name,
+ suspect2.changelog.author.email,
+ suspect2.changelog.author.time)])
+ url_to_blame = {'6/a.cc': a_cc_blame,
+ '6/f.cc': f_cc_blame}
+
+ def _MockGetBlame(_, path, revision):
+ revision_path = '%s/%s' % (revision, path)
+ return url_to_blame.get(revision_path)
+
+ self.mock(GitilesRepository, 'GetBlame', _MockGetBlame)
+ self.mock(scorer_changelist_classifier,
+ 'GetChangeLogsForFilesGroupedByDeps',
+ lambda *_: (None, None))
+ self.mock(scorer_changelist_classifier, 'FindSuspects',
+ lambda *_: [suspect1, suspect2])
+ frame1 = StackFrame(0, 'src/', 'func', 'a.cc', 'src/a.cc', [1])
+ frame2 = StackFrame(1, 'src/', 'func', 'a.cc', 'src/a.cc', [7])
+ frame3 = StackFrame(15, 'src/', 'func', 'f.cc', 'src/f.cc', [1])
frame4 = StackFrame(3, 'src/dep1', 'func', 'f.cc', 'src/dep1/f.cc', [1])
- stack0 = CallStack(0, [frame1, frame2])
- stack1 = CallStack(1, [frame3, frame4])
- report = DUMMY_REPORT._replace(
- stacktrace=Stacktrace([stack0, stack1], stack0))
-
- # TODO(katesonia): this mocking is to cover up a bug where
- # ``_SendRequestForJsonResponse`` returns None (due to MockHttpClient
- # returning 404), which in turn causes ``GitilesRepository.GetChangeLogs``
- # to raise an exception. Really we should fix the bug rather than
- # hiding it.
- self.mock(
- scorer_changelist_classifier,
- 'GetChangeLogsForFilesGroupedByDeps',
- lambda *_: ({}, []))
+ stacks = [CallStack(0, frame_list=[frame1, frame2, frame3, frame4])]
+ stacktrace = Stacktrace(stacks, stacks[0])
+ report = CrashReport('6', 'sig', 'win', stacktrace, ('0', '4'))
+ self.mock(ChromeDependencyFetcher, 'GetDependency',
+ lambda *_: {'src/': Dependency('src/', 'https://repo', '6')})
+ self.mock(ChromeDependencyFetcher, 'GetDependencyRollsDict',
+ lambda *_: {'src/': DependencyRoll('src/', 'https://repo',
+ '0', '4')})
suspects = self.changelist_classifier(report)
self.assertTrue(suspects,
- "Expected suspects, but the classifier didn't return any")
+ 'Expected suspects, but the classifier didn\'t return any')
expected_suspects = [
{
- 'review_url': 'https://codereview.chromium.org/3281',
- 'url': 'https://repo.test/+/3',
- 'author': 'e@chromium.org',
- 'time': 'Thu Apr 1 21:24:43 2016',
- 'project_path': 'src/',
- 'revision': '3',
- 'confidence': math.log(0.2857142857142857 * 0.6),
- 'reasons': ('MinDistance: -0.510826 -- Minimum distance is 20\n'
- 'TopFrameIndex: -1.252763 -- Top frame is #5'),
+ 'author': 'r@chromium.org',
'changed_files': [
{
- 'file': 'a.cc',
'blame_url': None,
- 'info': 'Minimum distance (LOC) 20, frame #5',
- }],
- }, {
- 'review_url': 'https://codereview.chromium.org/3281',
- 'url': 'https://repo.test/+/1',
- 'author': 'r@chromium.org',
- 'time': 'Thu Mar 31 21:24:43 2016',
+ 'file': 'a.cc',
+ 'info': ('Distance from touched lines and crashed lines is '
+ '0, in frame #0')
+ }
+ ],
+ 'confidence': 0.,
'project_path': 'src/',
+ 'reasons': ('MinDistance: 0.000000 -- Minimum distance is '
+ '0\nTopFrameIndex: 0.000000 -- Top frame is #0\n'
+ 'TouchCrashedFile: 0.000000 -- Touched files - a.cc'),
+ 'review_url': 'https://codereview.chromium.org/3281',
'revision': '1',
- 'confidence': 0.,
- 'reasons': ('MinDistance: 0.000000 -- Minimum distance is 0\n'
- 'TopFrameIndex: 0.000000 -- Top frame is #0'),
- 'changed_files': [
- {
- 'file': 'a.cc',
- 'blame_url': None,
- 'info': 'Minimum distance (LOC) 0, frame #0',
- }],
+ 'time': 'Thu Mar 31 21:24:43 2016',
+ 'url': 'https://repo.test/+/1'
},
]
self.assertListEqual([suspect.ToDict() for suspect in suspects],
expected_suspects)
def testFinditForCrashFilterZeroConfidenceSuspects(self):
- def _MockFindSuspects(*_):
- suspect1 = Suspect(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])
- 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/')
- 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/')
- 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]
+ suspect1 = Suspect(DUMMY_CHANGELOG1, 'src/')
+ suspect2 = Suspect(DUMMY_CHANGELOG3, 'src/')
- self.mock(scorer_changelist_classifier, 'FindSuspects', _MockFindSuspects)
+ a_cc_blame = Blame('6', 'src/')
+ a_cc_blame.AddRegions([Region(0, 10, suspect1.changelog.revision,
+ suspect1.changelog.author.name,
+ suspect1.changelog.author.email,
+ suspect1.changelog.author.time)])
+ f_cc_blame = Blame('6', 'src/')
+ f_cc_blame.AddRegions([Region(21, 10, suspect2.changelog.revision,
+ suspect2.changelog.author.name,
+ suspect2.changelog.author.email,
+ suspect2.changelog.author.time)])
+ url_to_blame = {'6/a.cc': a_cc_blame,
+ '6/f.cc': f_cc_blame}
+
+ def _MockGetBlame(_, path, revision):
+ revision_path = '%s/%s' % (revision, path)
+ return url_to_blame.get(revision_path)
+
+ self.mock(GitilesRepository, 'GetBlame', _MockGetBlame)
+ self.mock(scorer_changelist_classifier,
+ 'GetChangeLogsForFilesGroupedByDeps',
+ lambda *_: (None, None))
+ self.mock(scorer_changelist_classifier, 'FindSuspects',
+ lambda *_: [suspect1, suspect2])
+ frame1 = StackFrame(0, 'src/', 'func', 'a.cc', 'src/a.cc', [1])
+ frame2 = StackFrame(1, 'src/', 'func', 'a.cc', 'src/a.cc', [7])
+ frame3 = StackFrame(15, 'src/', 'func', 'f.cc', 'src/f.cc', [1])
+ stacks = [CallStack(0, frame_list=[frame1, frame2, frame3])]
+ stacktrace = Stacktrace(stacks, stacks[0])
+ report = CrashReport('6', 'sig', 'win', stacktrace, ('0', '4'))
+ self.mock(ChromeDependencyFetcher, 'GetDependency',
+ lambda *_: {'src/': Dependency('src/', 'https://repo', '6')})
+ self.mock(ChromeDependencyFetcher, 'GetDependencyRollsDict',
+ lambda *_: {'src/': DependencyRoll('src/', 'https://repo',
+ '0', '4')})
- suspects = self.changelist_classifier(DUMMY_REPORT)
+ suspects = self.changelist_classifier(report)
self.assertTrue(suspects,
- "Expected suspects, but the classifier didn't return any")
+ 'Expected suspects, but the classifier didn\'t return any')
expected_suspects = [
{
@@ -288,13 +278,15 @@ class LogLinearChangelistClassifierTest(CrashTestSuite):
{
'blame_url': None,
'file': 'a.cc',
- 'info': 'Minimum distance (LOC) 1, frame #0'
+ 'info': ('Distance from touched lines and crashed lines is '
+ '0, in frame #0')
}
],
- 'confidence': math.log(0.98),
+ 'confidence': 0.,
'project_path': 'src/',
- 'reasons': ('MinDistance: -0.020203 -- Minimum distance is 1\n'
- 'TopFrameIndex: 0.000000 -- Top frame is #0'),
+ 'reasons': ('MinDistance: 0.000000 -- Minimum distance is '
+ '0\nTopFrameIndex: 0.000000 -- Top frame is #0\n'
+ 'TouchCrashedFile: 0.000000 -- Touched files - a.cc'),
'review_url': 'https://codereview.chromium.org/3281',
'revision': '1',
'time': 'Thu Mar 31 21:24:43 2016',
@@ -303,40 +295,3 @@ class LogLinearChangelistClassifierTest(CrashTestSuite):
]
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
- ``TopFrameIndexFeature``. Therefore we should get a score of zero
- for that feature, which should cause the suspects to be filtered out.
- """
- def _MockFindSuspects(*_):
- suspect1 = Suspect(DUMMY_CHANGELOG1, 'src/')
- 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/')
- 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(scorer_changelist_classifier, 'FindSuspects', _MockFindSuspects)
-
- suspects = self.changelist_classifier(DUMMY_REPORT)
- self.assertFalse(suspects, 'Expected zero suspects, but found some:\n%s'
- % pprint.pformat([suspect.ToDict() for suspect in suspects]))
« no previous file with comments | « appengine/findit/crash/loglinear/feature.py ('k') | appengine/findit/crash/loglinear/test/feature_test.py » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698