[Predator] Fix bug in min_distance after refactor and add back skip added/deleted deps.

appengine/findit/crash/test/changelist_classifier_test.py

 # 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.http_client_appengine import HttpClientAppengine
 from common import chrome_dependency_fetcher
+from crash import changelist_classifier
 from crash.crash_report import CrashReport
-from crash import changelist_classifier
+from crash.results import AnalysisInfo
 from crash.results import MatchResult
 from crash.stacktrace import CallStack
 from crash.stacktrace import StackFrame
 from crash.stacktrace import Stacktrace
 from crash.test.crash_test_suite import CrashTestSuite
 from lib.gitiles.blame import Blame
 from lib.gitiles.blame import Region
 from lib.gitiles.change_log import ChangeLog
 from lib.gitiles.gitiles_repository import GitilesRepository
 
@@ skipping 79 matching lines @@
 # 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_REPORT = CrashReport(None, None, None, Stacktrace(), (None, None))
 
 class ChangelistClassifierTest(CrashTestSuite):
 
+  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, 'FindMatchResults', lambda *_: None)
+
+    cl_classifier = changelist_classifier.ChangelistClassifier(
+        GitilesRepository(), 7)
+    cl_classifier(CrashReport(crashed_version = '5',
+                              signature = 'sig',
+                              platform = 'canary',
+                              stacktrace = Stacktrace([CallStack(0)]),
+                              regression_range = ['4', '5']))
+    expected_regression_deps_rolls = copy.deepcopy(dep_rolls)
+    del expected_regression_deps_rolls['src/dep']

[wrengr, 2016/10/31 23:48:35]

Could you add a comment about why this ``del`` is necessary?

[Sharu Jiang, 2016/11/01 23:54:09]

Done.

+    self.assertEqual(passed_in_regression_deps_rolls[0],
+                     expected_regression_deps_rolls)
+
   def testGetDepsInCrashStack(self):
     crash_stack = CallStack(0)
     crash_stack.extend([
         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/dep1': DependencyRoll('src/dep1', 'https://url_dep1', '7', '9'),
-        'src/dep2': DependencyRoll('src/dep2', 'repo_url', '3', None),
+        '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(*_):
-      return [DUMMY_CHANGELOG1, DUMMY_CHANGELOG2, DUMMY_CHANGELOG3]
+    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, GitilesRepository()))
     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:
@@ skipping 135 matching lines @@
   def testFindItForCrash(self):
 
     def _MockFindMatchResults(*_):
       match_result1 = MatchResult(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])
       match_result1.file_to_stack_infos = {
           'a.cc': [(frame1, 0), (frame2, 0)]
       }
       match_result1.file_to_analysis_info = {
-          'a.cc': {'min_distance': 0, 'min_distance_frame': frame1}
+          'a.cc': AnalysisInfo(min_distance=0, min_distance_frame=frame1)
       }
 
       match_result2 = MatchResult(DUMMY_CHANGELOG3, 'src/', '')
       frame3 = StackFrame(5, 'src/', 'func', 'f.cc', 'src/f.cc', [1])
       match_result2.file_to_stack_infos = {
           'f.cc': [(frame3, 0)]
       }
       match_result2.file_to_analysis_info = {
-          'a.cc': {'min_distance': 20, 'min_distance_frame': frame3}
+          'a.cc': AnalysisInfo(min_distance=20, min_distance_frame=frame3)
       }
 
       return [match_result1, match_result2]
 
     self.mock(changelist_classifier, 'FindMatchResults', _MockFindMatchResults)
     self.mock(chrome_dependency_fetcher.ChromeDependencyFetcher,
         'GetDependencyRollsDict',
         lambda *_: {'src/': DependencyRoll('src/', 'https://repo', '1', '2')})
     self.mock(chrome_dependency_fetcher.ChromeDependencyFetcher,
         'GetDependency', lambda *_: {})
@@ skipping 19 matching lines @@
 
   def testFinditForCrashFilterZeroConfidentResults(self):
     def _MockFindMatchResults(*_):
       match_result1 = MatchResult(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])
       match_result1.file_to_stack_infos = {
           'a.cc': [(frame1, 0), (frame2, 0)]
       }
       match_result1.file_to_analysis_info = {
-          'a.cc': {'min_distance': 1, 'min_distance_frame': frame1}
+          'a.cc': AnalysisInfo(min_distance=1, min_distance_frame=frame1)
       }
 
       match_result2 = MatchResult(DUMMY_CHANGELOG3, 'src/', '')
       frame3 = StackFrame(15, 'src/', 'func', 'f.cc', 'src/f.cc', [1])
       match_result2.file_to_stack_infos = {
           'f.cc': [(frame3, 0)]
       }
       match_result2.file_to_analysis_info = {
-          'f.cc': {'min_distance': 20, 'min_distance_frame': frame3}
+          'f.cc': AnalysisInfo(min_distance=20, min_distance_frame=frame3)
       }
 
       match_result3 = MatchResult(DUMMY_CHANGELOG3, 'src/', '')
       frame4 = StackFrame(3, 'src/', 'func', 'ff.cc', 'src/ff.cc', [1])
       match_result3.file_to_stack_infos = {
           'f.cc': [(frame4, 0)]
       }
       match_result3.file_to_analysis_info = {
-          'f.cc': {'min_distance': 60, 'min_distance_frame': frame4}
+          'f.cc': AnalysisInfo(min_distance=60, min_distance_frame=frame4)
       }
 
       return [match_result1, match_result2, match_result3]
 
     self.mock(changelist_classifier, 'FindMatchResults', _MockFindMatchResults)
     self.mock(chrome_dependency_fetcher.ChromeDependencyFetcher,
         'GetDependencyRollsDict',
         lambda *_: {'src/': DependencyRoll('src/', 'https://repo', '1', '2')})
     self.mock(chrome_dependency_fetcher.ChromeDependencyFetcher,
         'GetDependency', lambda *_: {})
@@ skipping 28 matching lines @@
 
   def testFinditForCrashAllMatchResultsWithZeroConfidences(self):
     def _MockFindMatchResults(*_):
       match_result1 = MatchResult(DUMMY_CHANGELOG1, 'src/', '')
       frame1 = StackFrame(20, 'src/', '', 'func', 'a.cc', [1])
       frame2 = StackFrame(21, 'src/', '', 'func', 'a.cc', [7])
       match_result1.file_to_stack_infos = {
           'a.cc': [(frame1, 0), (frame2, 0)]
       }
       match_result1.file_to_analysis_info = {
-          'a.cc': {'min_distance': 1, 'min_distance_frame': frame1}
+          'a.cc': AnalysisInfo(min_distance=1, min_distance_frame=frame1)
       }
 
       match_result2 = MatchResult(DUMMY_CHANGELOG3, 'src/', '')
       frame3 = StackFrame(15, 'src/', '', 'func', 'f.cc', [1])
       match_result2.file_to_stack_infos = {
           'f.cc': [(frame3, 0)]
       }
       match_result2.min_distance = 20
       match_result2.file_to_analysis_info = {
-          'f.cc': {'min_distance': 20, 'min_distance_frame': frame3}
+          'f.cc': AnalysisInfo(min_distance=20, min_distance_frame=frame3)
       }
 
       return [match_result1, match_result2]
 
     self.mock(changelist_classifier, 'FindMatchResults', _MockFindMatchResults)
     self.mock(chrome_dependency_fetcher.ChromeDependencyFetcher,
         'GetDependencyRollsDict',
         lambda *_: {'src/': DependencyRoll('src/', 'https://repo', '1', '2')})
     self.mock(chrome_dependency_fetcher.ChromeDependencyFetcher,
         'GetDependency', lambda *_: {})
 
     cl_classifier = changelist_classifier.ChangelistClassifier(7,
         GitilesRepository())
     self.assertListEqual(cl_classifier(DUMMY_REPORT), [])