[Predator] Fix bug in min_distance after refactor and add back skip added/deleted deps.

appengine/findit/crash/test/culprit_test.py

Index: appengine/findit/crash/test/culprit_test.py
diff --git a/appengine/findit/crash/test/culprit_test.py b/appengine/findit/crash/test/culprit_test.py
new file mode 100644
index 0000000000000000000000000000000000000000..cd6b5e4f5aec7ef56605e206b2260fe9b2777c73
--- /dev/null
+++ b/appengine/findit/crash/test/culprit_test.py
@@ -0,0 +1,38 @@
+# 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 crash.culprit import Culprit
+from crash.test.crash_testcase import CrashTestCase
+
+
+class CulpritTest(CrashTestCase):
+
+  def testFieldsProperty(self):
+    culprit = Culprit('', [], [], None, None)
+    self.assertEqual(culprit.fields, ('project', 'components', 'cls',

[wrengr, 2016/10/31 23:48:35]

N.B., this is fragile. In general Culprit is not defined when the
``regression_range`` is None rather than a pair

[Sharu Jiang, 2016/11/01 23:54:09]

Actually we can still get suspected_project and suspected_components from crash
stack even though we cannot find any regression_range.

We return None only when there is no parsed stacktrace object.

[wrengr, 2016/11/03 16:14:33]

No, FindCulprit should return None (or rather: not return a Culprit) whenever it
fails for any reason. The regression_range is a required field for the
CrashReport, which Predator.FindCulprit takes as an argument, therefore
regression_range is guaranteed to be provided in every Culprit that
Predator.FindCulprit returns, thus the semantics of Culprit also have
regression_range be required.

+                                      'regression_range', 'algorithm'))
+  def testToDicts(self):
+    culprit = Culprit(None, None, None, None, None)

[wrengr, 2016/10/31 23:48:35]

Again, this is fragile. Culprit expects all the fields to actually be defined.

[Sharu Jiang, 2016/11/01 23:54:09]

Still, it's possible that some of those fields are empty. This is just to test
we can successfully drop those empty fields.

So it's possible that we return Culprit('', [], [], [], 'core_algorithm') if we
failed detecting regression range and all the classifiers failed.

[wrengr, 2016/11/03 16:14:33]

No, again. The Culprit class is defined to encapsulate the successful results of
Predator.FindCulprit. The regression_range is provided by the CrashReport, and
therefore is always available. Similarly, the algorithm always knows about
itself, therefore Culprit.algorithm can always be filled in. For the cls and
components, "failure" means returning the empty list of alternatives; None
should not be used as it is not iterable. For the project, since the empty
string is an invalid project name, we do not want to distinguish between '' vs
None. Therefore, none of the fields should ever be None. Allowing any of the
fields to be None means all the consumers of Culprits must go through and add a
bunch of boilerplate to deal with ruling out Culprits that don't make sense in
the first place. The whole point of making Culprit a class rather than an
arbitrary dict is precisely to capture these invariants.

I can add code to Culprit.__init__ to check all these invariants, if you'd
prefer. I've avoided doing so only because that's against Python's spirit of
"consenting adults" (which I disagree with, but well, Python culture is Python
culture).

+    self.assertTupleEqual(culprit.ToDicts(),
+                          ({'found': False},
+                           {'found_suspects': False,
+                            'found_project': False,
+                            'found_components': False,
+                            'has_regression_range': False,
+                            'solution': None}))
+
+    cl = self.GetDummyChangeLog()

[wrengr, 2016/10/31 23:48:35]

Below from here should be broken out into a separate test from the stuff above
here.

+    culprit = Culprit('proj', ['comp'], [cl], ['50.0.1234.1', '50.0.1234.2'],
+                      'core_algorithm')
+    self.assertTupleEqual(culprit.ToDicts(),
+                          ({'found': True,
+                            'regression_range': ['50.0.1234.1', '50.0.1234.2'],
+                            'suspected_project': 'proj',
+                            'suspected_components': ['comp'],
+                            'suspected_cls': [cl.ToDict()]},
+                           {'found_suspects': True,
+                            'found_project': True,
+                            'found_components': True,
+                            'has_regression_range': True,
+                            'solution': 'core_algorithm'}))