Converting various classes to namedtuples

appengine/findit/crash/results.py

Index: appengine/findit/crash/results.py
diff --git a/appengine/findit/crash/results.py b/appengine/findit/crash/results.py
index 066941e37dc4530799bb025cb466c49b81894599..4c270e9f2ebb8291bc9627cb4214c9484c09dabb 100644
--- a/appengine/findit/crash/results.py
+++ b/appengine/findit/crash/results.py
@@ -10,8 +10,17 @@ class AnalysisInfo(namedtuple('AnalysisInfo',
   __slots__ = ()
 
 
+# TODO(wrengr): it's not clear exactly what \"priority\" means nor why
+# it's necessary, given that every use in this file discards it.

[Sharu Jiang, 2016/11/21 23:31:57]

The priority is priority of callstack, this is conception from clusterfuzz, for
example for a msan crash, like in
https://cluster-fuzz.appspot.com/testcase?key=5293890036039680. the allocation
stack should have priority 0, and others have priority 1.

[wrengr, 2016/11/22 18:33:40]

That's what I was thinking. But since nothing here uses it (that I can see), why
keep it around? Why not just store the frames directly rather than making
StackInfo pairs?

[Sharu Jiang, 2016/11/22 18:53:51]

For Fracas/Crasas, this is not needed since they only provide one callstack.
However this part of code will be shared by cracas/fracas/clusterfuzz, so I
think we can keep it.

 # TODO(http://crbug.com/644476): this class needs a better name.
-# TODO(http://crbug.com/661822): convert this into a namedtuple.
+class StackInfo(namedtuple('StackInfo', ['frame', 'priority'])):
+  __slots__ = ()
+
+
+# TODO(http://crbug.com/644476): this class needs a better name.
+# TODO(http://crbug.com/661822): maybe convert this into a namedtuple? We
+# can do it here, but then the ``Update`` method on the subclass will be
+# busted. Do we actually need/want that method? If so, to what end?

[Sharu Jiang, 2016/11/21 23:31:57]

We are currently matching file changed by changelog and file crashed in
stacktrace, however the MatchResult is per changelog, say a changelog touches 3
files in the stacktrace, the ``Update`` needs to be called 3 times to update the
MatchResult for this changelog. 

So ``update`` is needed. 

Also, the reasons and confidence needs to be filled later based on the
changed_files, so I don't think this should be converted to a namedtuple.

[wrengr, 2016/11/22 18:33:40]

Acknowledged.

 class Result(object):
   """Represents findit culprit result."""
 
@@ -25,7 +34,6 @@ class Result(object):
 
     # TODO(wrengr): (a) make these two fields private/readonly
     # TODO(wrengr): (b) zip them together.
-    # TODO(http://crbug.com/661822): change stack_info pair to a namedtuple.
     self.file_to_stack_infos = {}
     self.file_to_analysis_info = {}
 
@@ -83,8 +91,8 @@ class MatchResult(Result):
 
     Args:
       file_path (str): File path of the crashed file.
-      stack_infos (list): List of (frame, stack_priority), represents frames of
-        this file and the callstack priorities of those frames.
+      stack_infos (list of StackInfo): List of the frames of this file
+        together with their callstack priorities.
       blame (Blame): Blame oject of this file.
     """
     self.file_to_stack_infos[file_path] = stack_infos