[Findit] Reorganizing findit_for_*.py

appengine/findit/crash/test/findit_for_chromecrash_test.py

Index: appengine/findit/crash/test/findit_for_chromecrash_test.py
diff --git a/appengine/findit/crash/test/findit_for_chromecrash_test.py b/appengine/findit/crash/test/findit_for_chromecrash_test.py
index 230d0711a311de8ac19c99ed4363d8ab5f9d536a..8d60bf97a3b00f67708c0de7d0afcb1ea7f60e51 100644
--- a/appengine/findit/crash/test/findit_for_chromecrash_test.py
+++ b/appengine/findit/crash/test/findit_for_chromecrash_test.py
@@ -4,84 +4,77 @@
 
 from common import chromium_deps
 from common.dependency import DependencyRoll
+from crash import changelist_classifier
 from crash import detect_regression_range
-from crash import findit_for_chromecrash
+from crash import findit
+from crash.findit_for_chromecrash import FinditForChromeCrash
 from crash import chromecrash_parser
-from crash import findit_for_crash
 from crash.component_classifier import ComponentClassifier
 from crash.project_classifier import ProjectClassifier
+from crash.changelist_classifier import ChangelistClassifier
 from crash.results import MatchResult
 from crash.stacktrace import CallStack
 from crash.stacktrace import Stacktrace
 from crash.test.crash_testcase import CrashTestCase
+from model.crash.crash_analysis import CrashAnalysis
 
 
 class FinditForChromeCrashTest(CrashTestCase):
 
   def testFindCulpritForChromeCrashEmptyStacktrace(self):
-    def _MockGetChromeDependency(*_):
-      return {}
-
-    def _MockParse(*_):
-      return Stacktrace()
-
-    self.mock(chromium_deps, 'GetChromeDependency', _MockGetChromeDependency)
-    self.mock(chromecrash_parser.ChromeCrashParser, 'Parse', _MockParse)
+    self.mock(chromium_deps, 'GetChromeDependency', lambda *_: {})
+    self.mock(chromecrash_parser.ChromeCrashParser, 'Parse',
+        lambda *_: Stacktrace())
 
     expected_results = {'found': False}
     expected_tag = {'found_suspects': False,
                     'has_regression_range': False}
 
-    results, tag = findit_for_chromecrash.FinditForChromeCrash().FindCulprit(
-        'signature', 'win', 'frame1\nframe2', '50.0.1234.0',
-        [{'chrome_version': '50.0.1234.0', 'cpm': 0.6}]).ToDicts()
+    analysis = CrashAnalysis()
+    analysis.signature = 'signature'
+    analysis.platform = 'win'
+    analysis.stack_trace = 'frame1\nframe2'
+    analysis.crashed_version = '50.0.1234.0'
+    analysis.historical_metadata = [{'chrome_version': '51.0.1234.0', 'cpm': 0.6}]
+    results, tag = FinditForChromeCrash().FindCulprit(analysis).ToDicts()
 
-    self.assertEqual(expected_results, results)
-    self.assertEqual(expected_tag, tag)
+    self.assertDictEqual(expected_results, results)
+    self.assertDictEqual(expected_tag, tag)
 
   def testFindCulpritForChromeCrash(self):
-    def _MockGetChromeDependency(*_):
-      return {}
+    self.mock(chromium_deps, 'GetChromeDependency', lambda *_: {})
 
-    def _MockParse(*_):
-      stack = Stacktrace()
-      stack.append(CallStack(0))
-      return stack
+    self.mock(chromecrash_parser.ChromeCrashParser, 'Parse',
+        lambda *_: Stacktrace([CallStack(0)]))
 
-    def _MockGetDEPSRollsDict(*_):
-      return {'src/': DependencyRoll('src/', 'https://repo', '1', '2'),
-              'src/add': DependencyRoll('src/add', 'https://repo1', None, '2'),
-              'src/delete': DependencyRoll('src/delete', 'https://repo2',
-                                           '2', None)}
-
-    dummy_match_result = MatchResult(self.GetDummyChangeLog(), 'src/')
-    def _MockFindItForCrash(*args):
-      regression_deps_rolls = args[1]
-      if regression_deps_rolls:
-        return [dummy_match_result]
+    self.mock(detect_regression_range, 'DetectRegressionRange',
+        lambda historic_metadata:
+            ('50.0.1233.0', '50.0.1234.0') if historic_metadata else None)
 
-      return []
+    self.mock(chromium_deps, 'GetDEPSRollsDict', lambda *_: {
+        'src/': DependencyRoll('src/', 'https://repo', '1', '2'),
+        'src/add': DependencyRoll('src/add', 'https://repo1', None, '2'),
+        'src/delete': DependencyRoll('src/delete', 'https://repo2', '2', None)
+      })
 
-    def _MockComponentClassify(*_):
-      return []
-
-    def _MockProjectClassify(*_):
-      return ''
-
-    self.mock(chromium_deps, 'GetChromeDependency', _MockGetChromeDependency)
-    self.mock(chromecrash_parser.ChromeCrashParser, 'Parse', _MockParse)
-    self.mock(chromium_deps, 'GetDEPSRollsDict', _MockGetDEPSRollsDict)
-    self.mock(findit_for_crash, 'FindItForCrash', _MockFindItForCrash)
+    dummy_match_result = MatchResult(self.GetDummyChangeLog(), 'src/')
+    self.mock(changelist_classifier.ChangelistClassifier, '__call__',
+        lambda _self, regression_deps_rolls, *args:
+            [dummy_match_result] if regression_deps_rolls else [])
 
-    self.mock(ComponentClassifier, 'Classify', _MockComponentClassify)
-    self.mock(ProjectClassifier, 'Classify', _MockProjectClassify)
+    self.mock(ComponentClassifier, 'Classify', lambda *_: [])
+    self.mock(ProjectClassifier, 'Classify', lambda *_: '')
 
     expected_results = {'found': False}
     expected_tag = {'found_suspects': False}
 
-    results, tag = findit_for_chromecrash.FinditForChromeCrash().FindCulprit(
-        'signature', 'win', 'frame1\nframe2', '50.0.1234.0',
-        ['50.0.1233.0', '50.0.1234.0']).ToDicts()
+    analysis = CrashAnalysis()
+    analysis.signature = 'signature'
+    analysis.platform = 'win'
+    analysis.stack_trace = 'frame1\nframe2'
+    analysis.crashed_version = '50.0.1234.0'
+    analysis.historical_metadata = ['50.0.1233.0', '50.0.1234.0']

[Sharu Jiang, 2016/10/19 00:20:06]

This should be analysis.regression_range = ['50.0.1233.0', '50.0.1234.0'] (this
argument is regression_range in the old FindCulprit.), then the test would be
fixed.

The mock of DetectRegressionRange should be removed since it's not called.

[wrengr, 2016/10/19 18:05:16]

Done. Thanks

+    results, tag = FinditForChromeCrash().FindCulprit(analysis).ToDicts()
 
     # TODO(wrengr): compare the Culprit object directly to these values,
     # rather than converting to dicts first. We can make a different
@@ -101,11 +94,18 @@ class FinditForChromeCrashTest(CrashTestCase):
           'solution': 'core_algorithm',
     }
 
-    self.assertEqual(expected_results, results)
-    self.assertEqual(expected_tag, tag)
+    # TODO(wrengr): our _MockDetectRegressionRange isn't working here...
+    self.assertDictEqual(expected_results, results)

[wrengr, 2016/10/18 23:28:10]

The mocking of DetectRegressionRange doesn't work for some reason. An example
stacktrace:

  File
"/usr/local/google/home/wrengr/chromium-srcs/infra/appengine/findit/crash/test/findit_for_chromecrash_test.py",
line 98, in testFindCulpritForChromeCrash
    self.assertDictEqual(expected_results, results)
  File "/usr/lib/python2.7/unittest/case.py", line 837, in assertDictEqual
    self.fail(self._formatMessage(msg, standardMsg))
  File "/usr/lib/python2.7/unittest/case.py", line 412, in fail
    raise self.failureException(msg)
AssertionError: {'found': True, 'suspected_project': '', 'suspected_cls':
[{'reasons': None, 'co [truncated]... != {'suspected_components': [], 'found':
True, 'suspected_project': '', 'suspected_ [truncated]...
  {'found': True,
-  'regression_range': ['50.0.1233.0', '50.0.1234.0'],
+  'regression_range': None,
   'suspected_cls': [{'author': 'r@chromium.org',
                      'changed_files': None,
                      'confidence': None,
                      'project_path': 'src/',
                      'reasons': None,
                      'review_url': 'https://codereview.chromium.org/3281',
                      'revision': '1',
                      'time': 'Thu Mar 31 21:24:43 2016',
                      'url': 'https://repo.test/+/1'}],
   'suspected_components': [],
   'suspected_project': ''}

==== captured logging output ====
LOG:
INFO: Config
CrashConfig<component_classifier=JsonProperty('component_classifier',
default={}, compressed=True), cracas=JsonProperty('cracas', default={}),
fracas=JsonProperty('fracas', default={}),
project_classifier=JsonProperty('project_classifier', default={}),
updated_by=StringProperty('updated_by', indexed=False),
updated_ts=DateTimeProperty('updated_ts', indexed=False, auto_now=True)> was
updated by admin

STDOUT:
(empty)

STDERR:
(empty)

+    self.assertDictEqual(expected_tag, tag)
 
-    results, tag = findit_for_chromecrash.FinditForChromeCrash().FindCulprit(
-        'signature', 'win', 'frame1\nframe2', '50.0.1234.0', None).ToDicts()
+    # TODO(wrengr): why isn't this one broken out as a separate unittest?
+    analysis = CrashAnalysis()
+    analysis.signature = 'signature'
+    analysis.platform = 'win'
+    analysis.stack_trace = 'frame1\nframe2'
+    analysis.crashed_version = '50.0.1234.0'
+    analysis.historical_metadata = []
+    results, tag = FinditForChromeCrash().FindCulprit(analysis).ToDicts()
 
     expected_results = {
           'found': False,
@@ -122,5 +122,5 @@ class FinditForChromeCrashTest(CrashTestCase):
           'solution': 'core_algorithm',
     }
 
-    self.assertEqual(expected_results, results)
-    self.assertEqual(expected_tag, tag)
+    self.assertDictEqual(expected_results, results)
+    self.assertDictEqual(expected_tag, tag)