[Findit] Reorganizing findit_for_*.py

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
 
+from common import chromium_deps
+from common.blame import Blame
 from common.blame import Region
-from common.blame import Blame
 from common.change_log import ChangeLog
 from common.dependency import Dependency
 from common.dependency import DependencyRoll
 from common.git_repository import GitRepository
-from crash import findit_for_crash
+from crash import changelist_classifier
+from crash.crash_report import CrashReport
+from crash.results import MatchResult
 from crash.stacktrace import StackFrame
 from crash.stacktrace import CallStack
 from crash.stacktrace import Stacktrace
-from crash.results import MatchResult
 from crash.test.crash_test_suite import CrashTestSuite
 
-
 DUMMY_CHANGELOG1 = ChangeLog.FromDict({
     'author_name': 'r@chromium.org',
     'message': 'dummy',
     'committer_email': 'r@chromium.org',
     'commit_position': 175900,
     'author_email': 'r@chromium.org',
     'touched_files': [
         {
             'change_type': 'add',
             'new_path': 'a.cc',
@@ skipping 54 matching lines @@
     'author_time': 'Thu Apr 1 21:24:43 2016',
     'committer_time': 'Thu Apr 1 21:28:39 2016',
     'commit_url':
         'https://repo.test/+/3',
     'code_review_url': 'https://codereview.chromium.org/3281',
     'committer_name': 'example@chromium.org',
     '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 results, 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_REPORT = CrashReport(None, None, None, Stacktrace(), (None, None))
 
-class FinditForCrashTest(CrashTestSuite):
+class ChangelistClassifierTest(CrashTestSuite):
 
   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.assertEqual(
-        findit_for_crash.GetDepsInCrashStack(crash_stack, crash_deps),
+    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/': 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'),
     }
 
     def _MockGetChangeLogs(*_):
       return [DUMMY_CHANGELOG1, DUMMY_CHANGELOG2, DUMMY_CHANGELOG3]
 
     self.mock(GitRepository, 'GetChangeLogs', _MockGetChangeLogs)
 
     dep_file_to_changelogs, ignore_cls = (
-        findit_for_crash.GetChangeLogsForFilesGroupedByDeps(
+        changelist_classifier.GetChangeLogsForFilesGroupedByDeps(
             regression_deps_rolls, stack_deps))
     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.assertEqual(dep_file_to_changelogs_json,
+    self.assertDictEqual(dep_file_to_changelogs_json,
                      expected_dep_file_to_changelogs_json)
-    self.assertEqual(ignore_cls, set(['1']))
+    self.assertSetEqual(ignore_cls, set(['1']))
 
   def testGetStackInfosForFilesGroupedByDeps(self):
-
     main_stack = CallStack(0)
     main_stack.extend(
         [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)
     low_priority_stack.append(
         StackFrame(0, 'src/dummy/', 'c(p* &d)', 'd.cc', 'src/dummy/d.cc', [17]))
 
@@ skipping 11 matching lines @@
             ],
         },
         'src/v8/': {
             'b.cc': [
                 (main_stack[2], 0),
             ]
         }
     }
 
     dep_file_to_stack_infos = (
-        findit_for_crash.GetStackInfosForFilesGroupedByDeps(
+        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():
@@ skipping 22 matching lines @@
             ]
         }
     }
 
     dummy_blame = Blame('9', 'a.cc')
     dummy_blame.AddRegion(
         Region(1, 5, '6', 'a', 'a@chromium.org', 'Thu Mar 31 21:24:43 2016'))
     dummy_blame.AddRegion(
         Region(6, 10, '1', 'b', 'b@chromium.org', 'Thu Jun 19 12:11:40 2015'))
 
-    def _MockGetBlame(*_):
-      return dummy_blame
-
-    self.mock(GitRepository, 'GetBlame', _MockGetBlame)
+    self.mock(GitRepository, 'GetBlame', lambda *_: dummy_blame)
 
     stack_deps = {
         'src/': Dependency('src/', 'https://url_src', 'rev1', 'DEPS'),
     }
 
     expected_match_results = [{
         '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
     }]
 
-    match_results = findit_for_crash.FindMatchResults(
-            dep_file_to_changelogs, dep_file_to_stack_infos, stack_deps)
-    self.assertEqual([result.ToDict() for result in match_results],
-                     expected_match_results)
+    match_results = changelist_classifier.FindMatchResults(
+        dep_file_to_changelogs, dep_file_to_stack_infos, stack_deps)
+    self.assertListEqual([result.ToDict() for result in match_results],
+                         expected_match_results)
 
+  # TODO(wrengr): something about the mocks here triggers coverage failures...
   def testFindItForCrashNoRegressionRange(self):
-    self.assertEqual(
-        findit_for_crash.FindItForCrash(Stacktrace(), {}, {}, 7),
-        [])
+    self.mock(chromium_deps, 'GetDEPSRollsDict', lambda *_: {})
+    self.mock(chromium_deps, 'GetChromeDependency', lambda *_: {})
+    # N.B., for this one test we really do want regression_range=None.
+    report = DUMMY_REPORT._replace(regression_range=None)
+    self.assertListEqual(
+        changelist_classifier.ChangelistClassifier(7)(report), [])
 
   def testFindItForCrashNoMatchFound(self):
-
-    def _MockFindMatchResults(*_):
-      return []
-
-    self.mock(findit_for_crash, 'FindMatchResults', _MockFindMatchResults)
-
-    regression_deps_rolls = {'src/': DependencyRoll('src/', 'https://repo',
-                                                    '1', '2')}
-    self.assertEqual(findit_for_crash.FindItForCrash(
-        Stacktrace(), regression_deps_rolls, {}, 7), [])
+    self.mock(changelist_classifier, 'FindMatchResults', lambda *_: [])
+    self.mock(chromium_deps, 'GetDEPSRollsDict', lambda *_:
+        {'src/': DependencyRoll('src/', 'https://repo', '1', '2')})
+    self.mock(chromium_deps, 'GetChromeDependency', lambda *_: {})
+    self.assertListEqual(
+        changelist_classifier.ChangelistClassifier(7)(DUMMY_REPORT), [])
 
   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}
       }
 
       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}
       }
 
       return [match_result1, match_result2]
 
-    self.mock(findit_for_crash, 'FindMatchResults', _MockFindMatchResults)
-
+    self.mock(changelist_classifier, 'FindMatchResults', _MockFindMatchResults)
+    self.mock(chromium_deps, 'GetDEPSRollsDict', lambda *_:
+        {'src/': DependencyRoll('src/', 'https://repo', '1', '2')})
+    self.mock(chromium_deps, 'GetChromeDependency', lambda *_: {})
+    results = changelist_classifier.ChangelistClassifier(7)(DUMMY_REPORT)
     expected_match_results = [
         {
             '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'
         },
     ]
-
-    regression_deps_rolls = {'src/': DependencyRoll('src/', 'https://repo',
-                                                    '1', '2')}
-
-    results = findit_for_crash.FindItForCrash(Stacktrace(),
-                                              regression_deps_rolls, {}, 7)
-    self.assertEqual([result.ToDict() for result in results],
-                     expected_match_results)
+    self.assertListEqual([result.ToDict() for result in results],
+                         expected_match_results)
 
   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 = {
@@ skipping 13 matching lines @@
       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}
       }
 
       return [match_result1, match_result2, match_result3]
 
-    self.mock(findit_for_crash, 'FindMatchResults', _MockFindMatchResults)
+    self.mock(changelist_classifier, 'FindMatchResults', _MockFindMatchResults)
+    self.mock(chromium_deps, 'GetDEPSRollsDict', lambda *_:
+        {'src/': DependencyRoll('src/', 'https://repo', '1', '2')})
+    self.mock(chromium_deps, 'GetChromeDependency', lambda *_: {})
 
+    results = changelist_classifier.ChangelistClassifier(7)(DUMMY_REPORT)
     expected_match_results = [
         {
             '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'
         }
     ]
-
-    regression_deps_rolls = {'src/': DependencyRoll('src/', 'https://repo',
-                                                    '1', '2')}
-
-    results = findit_for_crash.FindItForCrash(Stacktrace(),
-                                              regression_deps_rolls, {}, 7)
-
-    self.assertEqual([result.ToDict() for result in results],
-                     expected_match_results)
+    self.assertListEqual([result.ToDict() for result in results],
+                         expected_match_results)
 
   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}
       }
 
       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}
       }
 
       return [match_result1, match_result2]
 
-    self.mock(findit_for_crash, 'FindMatchResults', _MockFindMatchResults)
+    self.mock(changelist_classifier, 'FindMatchResults', _MockFindMatchResults)
+    self.mock(chromium_deps, 'GetDEPSRollsDict', lambda *_:
+        {'src/': DependencyRoll('src/', 'https://repo', '1', '2')})
+    self.mock(chromium_deps, 'GetChromeDependency', lambda *_: {})
 
-    regression_deps_rolls = {'src/': DependencyRoll('src/', 'https://repo',
-                                                    '1', '2')}
-
-    self.assertEqual(findit_for_crash.FindItForCrash(
-        Stacktrace(), regression_deps_rolls, {}, 7), [])
+    self.assertListEqual(
+        changelist_classifier.ChangelistClassifier(7)(DUMMY_REPORT), [])