replacing the NullCulprit class by None

appengine/findit/crash/test/findit_for_chromecrash_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 common import chrome_dependency_fetcher
 from common.dependency import DependencyRoll
 from common.http_client_appengine import HttpClientAppengine
 from crash import chromecrash_parser
 from crash import detect_regression_range
 from crash import findit_for_chromecrash
 from crash.changelist_classifier import ChangelistClassifier
 from crash.chromecrash_parser import ChromeCrashParser
 from crash.component_classifier import ComponentClassifier
 from crash.crash_report import CrashReport
 from crash.culprit import Culprit
-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.project_classifier import ProjectClassifier
 from crash.results import MatchResult
 from crash.stacktrace import CallStack
 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
@@ skipping 50 matching lines @@
 class FinditForChromeCrashTest(CrashTestCase):
 
   chrome_dep_fetcher = chrome_dependency_fetcher.ChromeDependencyFetcher(
       gitiles_repository.GitilesRepository(http_client=HttpClientAppengine()))
 
   # 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())
+    self.mock(FinditForChromeCrash, 'FindCulprit', lambda self, *_: None)
 
     # 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
 
     findit_client = _FinditForChromeCrash(
         gitiles_repository.GitilesRepository(http_client=HttpClientAppengine()))
-    result, tags = findit_client.FindCulprit(analysis).ToDicts()
-    # TODO(wrengr): just test for the NullCulprit directly; instead of
-    # going through ``ToDicts``.
-    expected_result, expected_tags = NullCulprit().ToDicts()
-    self.assertDictEqual(result, expected_result)
-    self.assertDictEqual(tags, expected_tags)
+    self.assertIsNone(findit_client.FindCulprit(analysis))
 
 
 class FinditForFracasTest(CrashTestCase):
 
   def testPlatformRename(self):
     self.assertEqual(_FinditForFracas().RenamePlatform('linux'), 'unix')
 
   def testCheckPolicyUnsupportedPlatform(self):
     self.assertIsNone(_FinditForFracas().CheckPolicy(DummyCrashData(
         platform = 'unsupported_platform')))
@@ skipping 92 matching lines @@
     analysis = findit_client.CreateAnalysis(crash_identifiers)
     analysis.status = analysis_status.COMPLETED
     analysis.put()
     self.assertFalse(findit_client.ScheduleNewAnalysis(crash_data))
 
   def testFindCulpritForChromeCrashEmptyStacktrace(self):
     self.mock(chrome_dependency_fetcher.ChromeDependencyFetcher,
         'GetDependency', lambda *_: {})
     self.mock(ChromeCrashParser, 'Parse', lambda *_: Stacktrace())
 
-    # TODO(wrengr): use NullCulprit instead
-    expected_results = {'found': False,
-                        'regression_range': None,
-                        'suspected_cls': [],
-                        'suspected_components': [],
-                        'suspected_project': ''}
-    expected_tag = {'found_suspects': False,
-                    'found_components': False,
-                    'found_project': False,
-                    'has_regression_range': False,
-                    'solution': 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}]
-    results, tag = _FinditForChromeCrash().FindCulprit(analysis).ToDicts()
-
-    self.assertDictEqual(expected_results, results)
-    self.assertDictEqual(expected_tag, tag)
+    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 *_: {})
     self.mock(ChromeCrashParser, 'Parse', lambda *_: Stacktrace([CallStack(0)]))
     self.mock(chrome_dependency_fetcher.ChromeDependencyFetcher,
         'GetDependencyRollsDict',
         lambda *_: {
@@ skipping 18 matching lines @@
     self._testFindCulpritForChromeCrashFails()
 
   def _testFindCulpritForChromeCrashSucceeds(self, dummy_match_result):
     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
-    results, tag = _FinditForChromeCrash().FindCulprit(analysis).ToDicts()
+    culprit = _FinditForChromeCrash().FindCulprit(analysis)
+    self.assertIsNotNone(culprit, 'FindCulprit failed unexpectedly')
+    results, tag = culprit.ToDicts()
 
     expected_results = {
           'found': True,
           'suspected_project': '',
           'suspected_components': [],
           'suspected_cls': [dummy_match_result.ToDict()],
           'regression_range': dummy_regression_range
     }
     expected_tag = {
           'found_suspects': True,
@@ skipping 12 matching lines @@
     analysis.platform = 'win'
     analysis.stack_trace = 'frame1\nframe2'
     analysis.crashed_version = '50.0.1234.0'
     results, tag = _FinditForChromeCrash().FindCulprit(analysis).ToDicts()
 
     expected_results = {
           'found': False,
           'suspected_project': '',
           'suspected_components': [],
           'suspected_cls': [],
-          'regression_range': None
     }
     expected_tag = {
           'found_suspects': False,
           'found_project': False,
           'found_components': False,
           'has_regression_range': False,
           'solution': 'core_algorithm',
     }
 
     self.assertDictEqual(expected_results, results)
     self.assertDictEqual(expected_tag, tag)