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

appengine/findit/crash/findit_for_fracas.py

Index: appengine/findit/crash/findit_for_fracas.py
diff --git a/appengine/findit/crash/findit_for_fracas.py b/appengine/findit/crash/findit_for_fracas.py
index 407f9f16420c609683915494f079ef9658abe2cc..ce6865b4dd0563b1df9dba87d9a79121bc24c9c7 100644
--- a/appengine/findit/crash/findit_for_fracas.py
+++ b/appengine/findit/crash/findit_for_fracas.py
@@ -11,6 +11,7 @@ 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
 
@@ -120,8 +121,16 @@ def FindCulpritForChromeCrash(signature, platform,
   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.