[Findit] Trigger swarming tasks after detech_first_faliure_pipeline

appengine/findit/waterfall/detect_first_failure_pipeline.py

Index: appengine/findit/waterfall/detect_first_failure_pipeline.py
diff --git a/appengine/findit/waterfall/detect_first_failure_pipeline.py b/appengine/findit/waterfall/detect_first_failure_pipeline.py
index 06cc093a024b4eacbe62398ac14c82f166031df6..a662f351beb0d7642c5d71f550cb87ec699d4651 100644
--- a/appengine/findit/waterfall/detect_first_failure_pipeline.py
+++ b/appengine/findit/waterfall/detect_first_failure_pipeline.py
@@ -19,6 +19,29 @@ from waterfall import swarming_util
 
 _MAX_BUILDS_TO_CHECK = 20
 _NON_FAILURE_STATUS = ['SUCCESS', 'SKIPPED', 'UNKNOWN']
+_PRE_TEST_PREFIX = 'PRE_'
+
+
+def _RemoveAnyPrefixes(test):

[lijeffrey, 2016/07/12 22:36:27]

Nit: should this be renamed to _RemoveAllPrefixes, since the while loop in line
40 seems to remove all of them?

[chanli, 2016/07/12 22:54:56]

Done.

+  """Remove prefixes from test names.
+
+  Args:
+    test (str): A test's name, eg: 'suite1.PRE_test1'.
+
+  Returns:
+    base_test (str): A base test name, eg: 'suite1.test1'.
+  """
+  test_name_start = test.find('.') if test.find('.') > -1 else 0

[lijeffrey, 2016/07/12 22:36:27]

I think we can do this with just 1 call to test.find() by doing something like

test_name_start = max(test.find('.'), 0)

[chanli, 2016/07/12 22:54:56]

Good idea. Done

+  if test_name_start == 0:
+    return test

[lijeffrey, 2016/07/12 22:36:27]

nit: 1 empty line after return test

[chanli, 2016/07/12 22:54:56]

Done.

+  test_suite = test[ : test_name_start]

[lijeffrey, 2016/07/12 22:36:28]

nit: is the extra space in 'test[ :' necessary? Same with line 38

[chanli, 2016/07/12 22:54:56]

I honestly don't know... I'll remove them for now.

+  test_name = test[test_name_start + 1 : ]
+  pre_position = test_name.find(_PRE_TEST_PREFIX)
+  while pre_position == 0:
+    test_name = test_name[len(_PRE_TEST_PREFIX):]
+    pre_position = test_name.find(_PRE_TEST_PREFIX)
+  base_test = test_suite + '.' + test_name

[lijeffrey, 2016/07/12 22:36:28]

nit: How about

base_test = '%s.%s' % (test_suite, test_name)?

I think using %s should be more robust than the + operator which breaks if
anything is None

[chanli, 2016/07/12 22:54:56]

Done.

+  return base_test
 
 
 class DetectFirstFailurePipeline(BasePipeline):
@@ -195,6 +218,7 @@ class DetectFirstFailurePipeline(BasePipeline):
             failed_step['tests'][test_name] = {
                 'current_failure': failed_step['current_failure'],
                 'first_failure': failed_step['current_failure'],
+                'base_test_name': _RemoveAnyPrefixes(test_name)
             }
             if failed_step.get('last_pass'):
               failed_step['tests'][test_name]['last_pass'] = (