[Findit] Reorganizing findit_for_*.py

appengine/findit/crash/test/findit_test.py

Index: appengine/findit/crash/test/findit_test.py
diff --git a/appengine/findit/crash/test/findit_test.py b/appengine/findit/crash/test/findit_test.py
new file mode 100644
index 0000000000000000000000000000000000000000..5e9718c8c163d80dcdd758c8daf386a6adabf376
--- /dev/null
+++ b/appengine/findit/crash/test/findit_test.py
@@ -0,0 +1,220 @@
+# 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.
+
+import copy
+import logging
+
+from crash.crash_report import CrashReport
+from crash.culprit import NullCulprit
+from crash.findit_for_chromecrash import FinditForChromeCrash
+from crash.findit_for_chromecrash import FinditForFracas
+from crash.findit import Findit
+from crash.stacktrace import Stacktrace
+from crash.test.crash_pipeline_test import DummyCrashData
+from crash.test.crash_testcase import CrashTestCase
+from crash.type_enums import CrashClient
+from model import analysis_status
+from model.crash.fracas_crash_analysis import FracasCrashAnalysis
+
+# In production we'd use CrashWrapperPipeline. And that'd work fine here,
+# since we never actually call the method that uses it. But just to be
+# absolutely sure we don't go over the wire due to some mocking failure,
+# we'll use this dummy class instead. (In fact, since it's never used,
+# we don't even need to give a real class; |None| works just fine.)
+MOCK_PIPELINE_CLS = None
+
+def _FinditForFracas():
+  """A helper to pass in the standard pipeline class."""
+  return FinditForFracas(MOCK_PIPELINE_CLS)
+
+
+class UnsupportedClient(Findit): # pylint: disable=W0223
+  @property
+  def client_id(self):
+    return self._client_id
+
+  def __init__(self, client_id=None):
+    super(UnsupportedClient, self).__init__(MOCK_PIPELINE_CLS)
+    if client_id is None:
+      client_id = 'unsupported_client'
+    self._client_id = client_id
+
+
+class FindTest(CrashTestCase):
+
+  # TODO(wrengr): make this test more straightforward/immediate; or give
+  # it a better name.
+  def testPlatformRename(self):
+    old_crash_data = DummyCrashData(
+        version = None,
+        platform = 'unix',
+        crash_identifiers = {})
+    # This is just here to match the old tests. Shouldn't actually be necessary.
+    del old_crash_data['customized_data']['historical_metadata']
+
+    testcase = self
+    class _MockFindit(FinditForFracas):
+      def __init__(self):
+        super(_MockFindit, self).__init__(MOCK_PIPELINE_CLS)
+
+      def _NeedsNewAnalysis(self, new_crash_data):
+        testcase.assertEqual(self.client_id, CrashClient.FRACAS)
+        testcase.assertDictEqual(new_crash_data, old_crash_data)
+        return False
+
+    new_crash_data = copy.deepcopy(old_crash_data)
+    new_crash_data['platform'] = 'linux'
+    self.assertFalse(_MockFindit().ScheduleNewAnalysis(new_crash_data))
+
+  def testCheckPolicyUnsupportedClient(self):
+    self.assertIsNone(UnsupportedClient().CheckPolicy(DummyCrashData(
+        platform = 'canary',
+        signature = 'sig',
+    )))
+
+  def testCheckPolicyUnsupportedPlatform(self):
+    self.assertIsNone(_FinditForFracas().CheckPolicy(DummyCrashData(
+        platform = 'unsupported_platform')))
+
+  def testCheckPolicyBlacklistedSignature(self):
+    self.assertIsNone(_FinditForFracas().CheckPolicy(DummyCrashData(
+        signature = 'Blacklist marker signature')))
+
+  def testCheckPolicyPlatformRename(self):
+    new_crash_data = _FinditForFracas().CheckPolicy(DummyCrashData(
+        platform = 'linux'))
+    self.assertIsNotNone(new_crash_data,
+        'FinditForFracas.CheckPolicy unexpectedly returned None')
+    self.assertEqual(new_crash_data['platform'], 'unix')
+
+  def testCreateAnalysisForFracas(self):
+    self.assertIsNotNone(_FinditForFracas().CreateAnalysis(
+        {'signature': 'sig'}))
+
+  def testCreateAnalysisForUnsupportedClientId(self):
+    self.assertIsNone(UnsupportedClient('unsupported_id').CreateAnalysis(
+        {'signature': 'sig'}))
+
+  def testGetAnalysisForFracas(self):
+    crash_identifiers = {'signature': 'sig'}
+    # TODO(wrengr): would be less fragile to call
+    # FinditForFracas.CreateAnalysis instead.
+    analysis = FracasCrashAnalysis.Create(crash_identifiers)
+    analysis.put()
+    self.assertEqual(_FinditForFracas().GetAnalysis(crash_identifiers),
+                     analysis)
+
+  def testGetAnalysisForUnsuportedClient(self):
+    crash_identifiers = {'signature': 'sig'}
+    # TODO(wrengr): it'd be less fragile to call FinditForFracas.CreateAnalysis
+    # instead. But we'd need to make UnsupportedClient inherit that
+    # implementation then, rather than inheriting the one from the Findit
+    # base class.
+    analysis = FracasCrashAnalysis.Create(crash_identifiers)
+    analysis.put()
+    self.assertIsNone(
+        UnsupportedClient('Unsupported_client').GetAnalysis(crash_identifiers),
+        'Unsupported client unexpectedly got analysis %s via identifiers %s'
+        % (analysis, crash_identifiers))
+
+  def testInitializeAnalysisForFracas(self):
+    crash_data = DummyCrashData(platform = 'linux')
+    crash_identifiers = crash_data['crash_identifiers']
+
+    findit_client = _FinditForFracas()
+    analysis = findit_client.CreateAnalysis(crash_identifiers)
+    findit_client._InitializeAnalysis(analysis, crash_data)
+    analysis.put()
+    analysis = findit_client.GetAnalysis(crash_identifiers)
+    self.assertIsNotNone(analysis,
+        'FinditForFracas.GetAnalysis unexpectedly returned None')
+
+    self.assertEqual(analysis.crashed_version, crash_data['crashed_version'])
+    self.assertEqual(analysis.signature, crash_data['signature'])
+    self.assertEqual(analysis.platform, crash_data['platform'])
+    self.assertEqual(analysis.stack_trace, crash_data['stack_trace'])
+    channel = crash_data['customized_data'].get('channel', None)
+    self.assertIsNotNone(channel,
+        'channel is unexpectedly not defined in crash_data')
+    self.assertEqual(analysis.channel, channel)
+
+  # TODO(wrengr): what was the purpose of this test? As written it's
+  # just testing that mocking works. I'm guessing it was to check that
+  # we fail when the analysis is for the wrong client_id; but if so,
+  # then we shouldn't need to mock FindCulprit...
+  def testFindCulprit(self):
+    self.mock(FinditForChromeCrash, 'FindCulprit',
+        lambda self, *_: NullCulprit())
+
+    # TODO(wrengr): would be less fragile to call
+    # FinditForFracas.CreateAnalysis instead; though if I'm right about
+    # the original purpose of this test, then this is one of the few
+    # places where calling FracasCrashAnalysis directly would actually
+    # make sense.
+    analysis = FracasCrashAnalysis.Create({'signature': 'sig'})
+    # TODO(wrengr): shouldn't FracasCrashAnalysis.Create already have set
+    # the client_id?
+    analysis.client_id = CrashClient.FRACAS
+
+    # TODO(wrengr): just test for the NullCulprit directly; instead of
+    # going through |ToDicts|.
+    result, tags = FinditForChromeCrash(MOCK_PIPELINE_CLS
+        ).FindCulprit(analysis).ToDicts()
+    expected_result, expected_tags = NullCulprit().ToDicts()
+    self.assertDictEqual(result, expected_result)
+    self.assertDictEqual(tags, expected_tags)
+
+  def testNeedsNewAnalysisIsTrueIfNoAnalysisYet(self):
+    self.assertTrue(_FinditForFracas()._NeedsNewAnalysis(DummyCrashData()))
+
+  def testNeedsNewAnalysisIsTrueIfLastOneFailed(self):
+    crash_data = DummyCrashData()
+    # TODO(wrengr): would be less fragile to call
+    # FinditForFracas.CreateAnalysis instead.
+    analysis = FracasCrashAnalysis.Create(crash_data['crash_identifiers'])
+    analysis.status = analysis_status.ERROR
+    analysis.put()
+    self.assertTrue(_FinditForFracas()._NeedsNewAnalysis(crash_data))
+
+  def testNeedsNewAnalysisIsFalseIfLastOneIsNotFailed(self):
+    crash_data = DummyCrashData()
+    crash_identifiers = crash_data['crash_identifiers']
+    for status in (analysis_status.PENDING, analysis_status.RUNNING,
+                   analysis_status.COMPLETED, analysis_status.SKIPPED):
+      # TODO(wrengr): would be less fragile to call
+      # FinditForFracas.CreateAnalysis instead.
+      analysis = FracasCrashAnalysis.Create(crash_identifiers)
+      analysis.status = status
+      analysis.put()
+      self.assertFalse(_FinditForFracas()._NeedsNewAnalysis(crash_data))
+
+  def testScheduleNewAnalysisSkipsUnsupportedChannel(self):
+    self.assertFalse(_FinditForFracas().ScheduleNewAnalysis(DummyCrashData(
+        version = None,
+        signature = None,
+        crash_identifiers = {},
+        channel = 'unsupported_channel')))
+
+  def testScheduleNewAnalysisSkipsUnsupportedPlatform(self):
+    self.assertFalse(_FinditForFracas().ScheduleNewAnalysis(DummyCrashData(
+        version = None,
+        signature = None,
+        platform = 'unsupported_platform',
+        crash_identifiers = {})))
+
+  def testScheduleNewAnalysisSkipsBlackListSignature(self):
+    self.assertFalse(_FinditForFracas().ScheduleNewAnalysis(DummyCrashData(
+        version = None,
+        signature = 'Blacklist marker signature',
+        crash_identifiers = {})))
+
+  def testScheduleNewAnalysisSkipsIfAlreadyCompleted(self):
+    crash_data = DummyCrashData()
+    crash_identifiers = crash_data['crash_identifiers']
+    # TODO(wrengr): would be less fragile to call
+    # FinditForFracas.CreateAnalysis instead.
+    analysis = FracasCrashAnalysis.Create(crash_identifiers)
+    analysis.status = analysis_status.COMPLETED
+    analysis.put()
+    self.assertFalse(_FinditForFracas().ScheduleNewAnalysis(crash_data))