[Findit] Factoring out the components, so they can classify themselves

appengine/findit/crash/findit_for_fracas.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.
 
 """Process crashes from Chrome crash server and find culprits for them."""
 
 import logging
 
 from common import chromium_deps
 from crash import detect_regression_range
 from crash import findit_for_crash
 from crash.fracas_parser import FracasParser
 from crash.project_classifier import ProjectClassifier
+from crash.component import Component
 from crash.component_classifier import ComponentClassifier
 from model.crash.crash_config import CrashConfig
 
 # TODO(katesonia): Remove the default value after adding validity check to
 # config.
 _DEFAULT_TOP_N = 7
 
 
 def FindCulpritForChromeCrash(signature, platform,
                               stack_trace, crashed_version, historic_metadata):
@@ skipping 89 matching lines @@
         last_good_version, first_bad_version, platform)
 
   crash_config = CrashConfig.Get()
   culprit_results = findit_for_crash.FindItForCrash(
       stacktrace, regression_deps_rolls, crash_deps, crash_config.fracas.get(
           'top_n', _DEFAULT_TOP_N))
 
   crash_stack = stacktrace.crash_stack
   suspected_project = ProjectClassifier().Classify(
       culprit_results, crash_stack)
-  suspected_components = ComponentClassifier().Classify(
-      culprit_results, crash_stack)
+
+  component_classifier_config = CrashConfig.Get().compiled_component_classifier
+  suspected_components = ComponentClassifier(
+      # TODO(wrengr): have the config return Component objects directly,
+      # rather than needing to convert them here.
+      [ Component(component_name, path_regex, function_regex)

[Martin Barbella, 2016/09/19 21:17:18]

Nit: no space after [

[Sharu Jiang, 2016/09/20 00:31:29]

The path_regex, function_regex in
CrashConfig.Get().compiled_component_classifier have already been compiled, we
don't need to re-compile them again every time we analyze a crash.

[wrengr, 2016/09/27 22:00:39]

The current code does not recompile them. Calling re.compile on a regex object
(i.e. the result of previous calls to re.compile) is a no-op.

My desired end-goal is to have the construction of the ComponentClassifier moved
outside of the function so it can be shared. However, the current structure of
the code is extremely fragile, and innocuous restructuring like that causes
obscure breakage due to how mocking is done in the unit tests.

[wrengr, 2016/09/27 22:00:39]

Done.

+          for path_regex, function_regex, component_name
+          in component_classifier_config['path_function_component'] ],

[Martin Barbella, 2016/09/19 21:17:18]

and none before ]

[stgao, 2016/09/21 21:57:27]

Not sure if the intent is correct here, mind a check by running pylint?

[wrengr, 2016/09/27 22:00:39]

Pylint says everything is fine

[wrengr, 2016/09/27 22:00:39]

Done.

+      component_classifier_config['top_n']
+    ).Classify(culprit_results, crash_stack)
 
   # TODO(http://crbug.com/644411): the caller should convert things to
   # JSON, not us.
   culprit_results_list = [result.ToDict() for result in culprit_results]
 
   return (
       {
           'found': (bool(suspected_project) or bool(suspected_components) or
                     bool(culprit_results_list)),
           'regression_range': regression_versions,
           'suspected_project': suspected_project,
           'suspected_components': suspected_components,
           'suspected_cls': culprit_results_list,
       },
       {
           'found_suspects': bool(culprit_results_list),
           'has_regression_range': bool(regression_versions),
           'solution': 'core_algorithm',
       }
   )