Reorganizing detect_regression_range.py

appengine/findit/crash/test/detect_regression_range_test.py

Index: appengine/findit/crash/test/detect_regression_range_test.py
diff --git a/appengine/findit/crash/test/detect_regression_range_test.py b/appengine/findit/crash/test/detect_regression_range_test.py
index 03d0db54be4d34162b8610d39deef630161f4fc3..2f4cd95e09b9b5da7a53d248b43231109b468c7f 100644
--- a/appengine/findit/crash/test/detect_regression_range_test.py
+++ b/appengine/findit/crash/test/detect_regression_range_test.py
@@ -4,9 +4,7 @@
 
 from testing_utils import testing
 
-from crash.detect_regression_range import GetAttributesListFromHistoricData
-from crash.detect_regression_range import GetSpikeIndexes
-from crash.detect_regression_range import GetRegressionRangeFromSpike
+from crash.detect_regression_range import GetSpikes
 from crash.detect_regression_range import DetectRegressionRange
 
 
@@ -22,21 +20,18 @@ class DetectRegressionRangeTest(testing.AppengineTestCase):
                                                  result))
 
 
-  def testGetSpikeIndexes(self):
-    self.assertEqual(GetSpikeIndexes([]),
+  def testGetSpikes(self):
+    snd = lambda x: x[1]

[stgao, 2016/09/09 18:13:59]

style nit: usually we avoid abbreviation.

+    e0 = ('1', 0.5)

[Sharu Jiang, 2016/09/08 21:33:14]

Shouldn't this be {'chrome_version': '1', 'cpm': 0.5}?
This test will break.

[wrengr (wrong one), 2016/09/08 22:28:50]

The GetSpikes function takes a list of whatever type of objects the caller
likes. The DetectRegressionRange function chooses to have the objects be
historic metadata dicts, but that's not required. For this test we choose the
objects to be pairs where the value is the second component of the pair.

I ran the tests and this works as written.

[Sharu Jiang, 2016/09/08 23:55:19]

Ok, didn't notice the snd function...

+    e1 = ('1', 0)
+    e2 = ('2', 0.5)
+    self.assertEqual(GetSpikes([], snd),
                      [])
-    self.assertEqual(GetSpikeIndexes([('1', 0.5)]),
+    self.assertEqual(GetSpikes([e0], snd),
                      [])
-    self.assertEqual(GetSpikeIndexes([('1', 0), ('2', 0.5)]),
-                     [1])
+    self.assertEqual(GetSpikes([e1, e2], snd),
+                     [(e1, e2)])
 
-  def testGetRegressionRangeFromSpike(self):
-    self.assertEqual(GetRegressionRangeFromSpike(0, ['1', '2']),
-                     None)
-    self.assertEqual(GetRegressionRangeFromSpike(3, ['1', '2']),
-                     None)
-    self.assertEqual(GetRegressionRangeFromSpike(1, ['1', '2']),
-                     ('1', '2'))
 
   def testReturnNoneForEmptyCrashData(self):
     self.assertEqual(DetectRegressionRange([]), None)
@@ -80,14 +75,3 @@ class DetectRegressionRangeTest(testing.AppengineTestCase):
 
     self._VerifyCasesForDetectRegressonRange(cases)
 
-  def testGetAttributesListFromHistoricData(self):
-    historic_metadata = [{'chrome_version': '1', 'cpm': 0},
-                         {'chrome_version': '2', 'cpm': 0}]
-
-    attribute_list = GetAttributesListFromHistoricData(historic_metadata,
-                                                       ['chrome_version'])
-    expected_list = ['1', '2']
-    self.assertEqual(attribute_list, expected_list)
-
-    attribute_list = GetAttributesListFromHistoricData(historic_metadata, [])
-    self.assertEqual(attribute_list, [])