[Predator] Switch from anonymous dict to CrashData.

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 1a27067bb85e26de42955c832d1a388345645d08..e20c187a784a5ab8d9fe7eac25fc1871fa07e583 100644
--- a/appengine/findit/crash/test/findit_for_chromecrash_test.py
+++ b/appengine/findit/crash/test/findit_for_chromecrash_test.py
@@ -27,15 +27,17 @@ from gae_libs.http.http_client_appengine import HttpClientAppengine
 from libs.gitiles.gitiles_repository import GitilesRepository
 from model import analysis_status
 from model.crash.crash_analysis import CrashAnalysis
+from model.crash.crash_config import CrashConfig
 from model.crash.fracas_crash_analysis import FracasCrashAnalysis
 
 MOCK_GET_REPOSITORY = lambda _: None # pragma: no cover
 
+
 class _FinditForChromeCrash(FinditForChromeCrash):  # pylint: disable = W
   # We allow overriding the default ``get_repository`` because one unittest
   # needs to.
-  def __init__(self, get_repository=MOCK_GET_REPOSITORY):
-    super(_FinditForChromeCrash, self).__init__(get_repository)
+  def __init__(self, get_repository=MOCK_GET_REPOSITORY, config=None):
+    super(_FinditForChromeCrash, self).__init__(get_repository, config or {})
 
   @classmethod
   def _ClientID(cls): # pragma: no cover
@@ -46,26 +48,12 @@ class _FinditForChromeCrash(FinditForChromeCrash):  # pylint: disable = W
     exceptions since we want to be able to test the FinditForChromeCrash
     class itself.
     """
-    return ''
-
-  @property
-  def config(self):
-    """Avoid returning None.
-
-    The default ``Findit.config`` will return None if the client
-    id is not found in the CrashConfig. This in turn will cause
-    ``FinditForChromeCrash.__init__`` to crash, since NoneType doesn't
-    have a ``get`` method. In general it's fine for things to crash, since
-    noone should make instances of Findit subclasses which don't define
-    ``_clientID``; but for this test suite, we want to permit instances
-    of FinditForChromeCrash, so that we can test that class directly.
-    """
-    return {}
+    return 'ChromeCrash'
 
 
-def _FinditForFracas():
+def _FinditForFracas(config=None):
   """A helper to pass in the standard pipeline class."""
-  return FinditForFracas(MOCK_GET_REPOSITORY)
+  return FinditForFracas(MOCK_GET_REPOSITORY, config or {})
 
 
 class FinditForChromeCrashTest(PredatorTestCase):
@@ -83,37 +71,45 @@ class FinditForChromeCrashTest(PredatorTestCase):
     # 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
-
-    findit_client = (
-        _FinditForChromeCrash(GitilesRepository.Factory(HttpClientAppengine())))
+    findit_client = _FinditForChromeCrash(
+        GitilesRepository.Factory(HttpClientAppengine()), CrashConfig.Get())
     self.assertIsNone(findit_client.FindCulprit(analysis))
 
 
 class FinditForFracasTest(PredatorTestCase):
 
-  def testPlatformRename(self):
-    self.assertEqual(_FinditForFracas().RenamePlatform('linux'), 'unix')
-
-  def testCheckPolicyUnsupportedPlatform(self):
-    self.assertIsNone(_FinditForFracas().CheckPolicy(self.GetDummyCrashData(
-        platform = 'unsupported_platform')))
+  def setUp(self):
+    super(FinditForFracasTest, self).setUp()
+    self._client = _FinditForFracas(config=CrashConfig.Get())
 
-  def testCheckPolicyBlacklistedSignature(self):
-    self.assertIsNone(_FinditForFracas().CheckPolicy(self.GetDummyCrashData(
-        signature = 'Blacklist marker signature')))
+  def testCheckPolicyBlacklistSignature(self):
+    crash_data = self.GetDummyCrashData(
+        client_id = CrashClient.FRACAS,
+        signature='Blacklist marker signature')
+    crash_buffer = self._client.GetCrashBuffer(crash_data)
+    self.assertFalse(self._client._CheckPolicy(crash_buffer))
 
-  def testCheckPolicyPlatformRename(self):
-    new_crash_data = _FinditForFracas().CheckPolicy(self.GetDummyCrashData(
-        platform = 'linux'))
-    self.assertIsNotNone(new_crash_data,
-        'FinditForFracas.CheckPolicy unexpectedly returned None')
-    self.assertEqual(new_crash_data['platform'], 'unix')
+  def testCheckPolicyUnsupportedPlatform(self):
+    crash_data = self.GetDummyCrashData(
+        client_id = CrashClient.FRACAS,
+        platform='unsupported_platform')
+    crash_buffer = self._client.GetCrashBuffer(crash_data)
+    self.assertFalse(self._client._CheckPolicy(crash_buffer))
+
+  def testScheduleNewAnalysisSkipsUnsupportedChannel(self):
+    crash_data = self.GetDummyCrashData(
+        client_id = CrashClient.FRACAS,
+        channel='unsupported_channel')
+    crash_buffer = self._client.GetCrashBuffer(crash_data)
+    self.assertFalse(self._client._CheckPolicy(crash_buffer))
+
+  def testCheckPolicySuccess(self):
+    crash_buffer = self._client.GetCrashBuffer(self.GetDummyCrashData(
+        client_id = CrashClient.FRACAS))
+    self.assertTrue(self._client._CheckPolicy(crash_buffer))
 
   def testCreateAnalysis(self):
-    self.assertIsNotNone(_FinditForFracas().CreateAnalysis(
+    self.assertIsNotNone(self._client.CreateAnalysis(
         {'signature': 'sig'}))
 
   def testGetAnalysis(self):
@@ -122,148 +118,7 @@ class FinditForFracasTest(PredatorTestCase):
     # FinditForFracas.CreateAnalysis instead.
     analysis = FracasCrashAnalysis.Create(crash_identifiers)
     analysis.put()
-    self.assertEqual(_FinditForFracas().GetAnalysis(crash_identifiers),
-                     analysis)
-
-  def testInitializeAnalysisForFracas(self):
-    crash_data = self.GetDummyCrashData(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['chrome_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)
-
-  def testNeedsNewAnalysisIsTrueIfNoAnalysisYet(self):
-    self.assertTrue(_FinditForFracas()._NeedsNewAnalysis(
-        self.GetDummyCrashData()))
-
-  def testNeedsNewAnalysisIsTrueIfLastOneFailed(self):
-    findit_client = _FinditForFracas()
-    crash_data = self.GetDummyCrashData()
-    analysis = findit_client.CreateAnalysis(crash_data['crash_identifiers'])
-    analysis.status = analysis_status.ERROR
-    analysis.put()
-    self.assertTrue(findit_client._NeedsNewAnalysis(crash_data))
-
-  def testNeedsNewAnalysisIsFalseIfLastOneIsNotFailed(self):
-    findit_client = _FinditForFracas()
-    crash_data = self.GetDummyCrashData()
-    crash_identifiers = crash_data['crash_identifiers']
-    for status in (analysis_status.PENDING, analysis_status.RUNNING,
-                   analysis_status.COMPLETED, analysis_status.SKIPPED):
-      analysis = findit_client.CreateAnalysis(crash_identifiers)
-      analysis.status = status
-      analysis.put()
-      self.assertFalse(findit_client._NeedsNewAnalysis(crash_data))
-
-  def testFindCulpritForChromeCrashEmptyStacktrace(self):
-    self.mock(chrome_dependency_fetcher.ChromeDependencyFetcher,
-        'GetDependency', lambda *_: {})
-    self.mock(ChromeCrashParser, 'Parse', lambda *_: None)
-
-    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}]
-    self.assertIsNone(_FinditForChromeCrash().FindCulprit(analysis))
-
-  # TODO(http://crbug.com/659346): why do these mocks give coverage
-  # failures? That's almost surely hiding a bug in the tests themselves.
-  def testFindCulpritForChromeCrash(self): # pragma: no cover
-    self.mock(chrome_dependency_fetcher.ChromeDependencyFetcher,
-        'GetDependency', lambda *_: {})
-    stack = CallStack(0)
-    self.mock(ChromeCrashParser, 'Parse',
-              lambda *_: Stacktrace([stack], stack))
-    self.mock(chrome_dependency_fetcher.ChromeDependencyFetcher,
-        'GetDependencyRollsDict',
-        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)
-    })
-
-    dummy_suspect = Suspect(self.GetDummyChangeLog(), 'src/')
-    self.mock(LogLinearChangelistClassifier, '__call__',
-        lambda _self, report:
-            [dummy_suspect] if report.regression_range else [])
-
-    self.mock(ComponentClassifier, 'ClassifySuspects', lambda *_: [])
-    self.mock(ComponentClassifier, 'ClassifyCallStack', lambda *_: [])
-    self.mock(ProjectClassifier, 'ClassifySuspects', lambda *_: '')
-    self.mock(ProjectClassifier, 'ClassifyCallStack', lambda *_: '')
-
-    # TODO(wrengr): for both these tests, we should compare Culprit
-    # objects directly rather than calling ToDicts and comparing the
-    # dictionaries.
-    self._testFindCulpritForChromeCrashSucceeds(dummy_suspect)
-    self._testFindCulpritForChromeCrashFails()
-
-  def _testFindCulpritForChromeCrashSucceeds(self, dummy_suspect):
-    analysis = CrashAnalysis()
-    analysis.signature = 'signature'
-    analysis.platform = 'win'
-    analysis.stack_trace = 'frame1\nframe2'
-    analysis.crashed_version = '50.0.1234.0'
-    dummy_regression_range = ('50.0.1233.0', '50.0.1234.0')
-    analysis.regression_range = dummy_regression_range
-    culprit = _FinditForChromeCrash().FindCulprit(analysis)
-    self.assertIsNotNone(culprit, 'FindCulprit failed unexpectedly')
-    suspects, tag = culprit.ToDicts()
-
-    expected_suspects = {
-          'found': True,
-          'suspected_cls': [dummy_suspect.ToDict()],
-          'regression_range': dummy_regression_range
-    }
-    expected_tag = {
-          'found_suspects': True,
-          'found_project': False,
-          'found_components': False,
-          'has_regression_range': True,
-          'solution': 'core_algorithm',
-    }
-
-    self.assertDictEqual(expected_suspects, suspects)
-    self.assertDictEqual(expected_tag, tag)
-
-  def _testFindCulpritForChromeCrashFails(self):
-    analysis = CrashAnalysis()
-    analysis.signature = 'signature'
-    analysis.platform = 'win'
-    analysis.stack_trace = 'frame1\nframe2'
-    analysis.crashed_version = '50.0.1234.0'
-    # N.B., analysis.regression_range is None
-    suspects, tag = _FinditForChromeCrash().FindCulprit(analysis).ToDicts()
-
-    expected_suspects = {'found': False}
-    expected_tag = {
-          'found_suspects': False,
-          'found_project': False,
-          'found_components': False,
-          'has_regression_range': False,
-          'solution': 'core_algorithm',
-    }
-
-    self.assertDictEqual(expected_suspects, suspects)
-    self.assertDictEqual(expected_tag, tag)
+    self.assertEqual(self._client.GetAnalysis(crash_identifiers), analysis)
 
   @mock.patch('google.appengine.ext.ndb.Key.urlsafe')
   @mock.patch('common.appengine_util.GetDefaultVersionHostname')
@@ -277,9 +132,8 @@ class FinditForFracasTest(PredatorTestCase):
     crash_identifiers = {'signature': 'sig'}
     analysis = FracasCrashAnalysis.Create(crash_identifiers)
     analysis.result = {'other': 'data'}
-    findit_object = FinditForFracas(MOCK_GET_REPOSITORY)
     expected_processed_suspect = {
-        'client_id': findit_object.client_id,
+        'client_id': self._client.client_id,
         'crash_identifiers': {'signature': 'sig'},
         'result': {
             'feedback_url': (
@@ -289,6 +143,6 @@ class FinditForFracasTest(PredatorTestCase):
         }
     }
 
-    self.assertDictEqual(findit_object.GetPublishableResult(crash_identifiers,
+    self.assertDictEqual(self._client.GetPublishableResult(crash_identifiers,
                                                             analysis),
                          expected_processed_suspect)