[Findit] Use step level analysis to exclude flaky test failures.

appengine/findit/waterfall/extract_signal_pipeline.py

 # Copyright 2015 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.
 
 import logging
+import json
 
 from google.appengine.api.urlfetch import ResponseTooLargeError
 
 from pipeline_utils.appengine_third_party_pipeline_src_pipeline import pipeline
 
 from common.http_client_appengine import HttpClientAppengine as HttpClient
 from model.wf_step import WfStep
 from waterfall import buildbot
 from waterfall import extractors
 from waterfall import lock_util
@@ skipping 22 matching lines @@
 
     lines = log_data.split('\n')
     size = 0
     for line_index in reversed(range(len(lines))):
       size += len(lines[line_index]) + 1
       if size > ExtractSignalPipeline.LOG_DATA_BYTE_LIMIT:
         return '\n'.join(lines[line_index + 1:])
     else:
       return log_data  # pragma: no cover - this won't be reached.
 
+  @staticmethod
+  def _GetTestLevelFailures(step_log):
+    """Analyze the step log and extract reliable failures only.
+
+    Args:
+      step_log (file): A JSON file for failed step log.
+
+    Returns:
+      A dict like below:
+      {
+        'test_name1': [
+          {
+            "elapsed_time_ms": ..,
+            "losless_snippet": ..,
+            "output_snippet": ..,
+            "status": "FAILURE",
+            "output_snippet_base64":..
+          },
+          ..
+        ],
+        ..
+      }
+    """
+    failed_test_log = {}
+    step_failure_data = json.loads(step_log)
+
+    for iteration in step_failure_data['gtest_results']['per_iteration_data']:
+      for key in iteration.keys():  # Keys are test names.
+        is_reliable_failure = True
+
+        for test in iteration[key]:
+          # We will ignore the test if one of the attempts passes.
+          if test['status'] != 'FAILURE':

[stgao, 2015/05/21 00:29:56]

This code doesn't match the comment.
I think status has more values, like TIMEOUT, CRASHED, etc.

You may check
https://code.google.com/p/chromium/codesearch#chromium/build/scripts/slave/ru...

+            is_reliable_failure = False
+            break
+
+        if is_reliable_failure: # All attempts failed, it's a reliable failure.
+          failed_test_log[key] = iteration[key]
+
+    return failed_test_log
 
   # Arguments number differs from overridden method - pylint: disable=W0221
   def run(self, failure_info):
     """
     Args:
       failure_info (dict): Output of pipeline DetectFirstFailurePipeline.run().
 
     Returns:
       A dict like below:
       {
         'step_name1': waterfall.failure_signal.FailureSignal.ToJson(),
         ...
       }
     """
     signals = {}
 
     master_name = failure_info['master_name']
     builder_name = failure_info['builder_name']
     build_number = failure_info['build_number']
     for step_name in failure_info.get('failed_steps', []):
       step = WfStep.Get(master_name, builder_name, build_number, step_name)
       if step and step.log_data:
-        stdio_log = step.log_data
+        test_failure_log = step.log_data
       else:
         if not lock_util.WaitUntilDownloadAllowed(

[stgao, 2015/05/21 00:29:56]

We don't have to wait unless the request is to the build master.

             master_name):  # pragma: no cover
-          raise pipeline.Retry('Failed to pull stdio of step %s of master %s'
+          raise pipeline.Retry('Failed to pull log of step %s of master %s'
                                % (step_name, master_name))
 
-        # TODO: do test-level analysis instead of step-level.
-        try:
-          stdio_log = buildbot.GetStepStdio(
-              master_name, builder_name, build_number, step_name,
-              self.HTTP_CLIENT)
-        except ResponseTooLargeError:  # pragma: no cover.
-          logging.exception(
-              'Log of step "%s" is too large for urlfetch.', step_name)
-          # If the stdio log of a step is too large, we don't want to pull it
-          # again in next run, because that might lead to DDoS to the master.
-          # TODO: Use archived stdio logs in Google Storage instead.

[stgao, 2015/05/21 00:29:56]

Please still keep this TODO. Because we still pull logs from the build master if
gtest result is not available.

-          stdio_log = 'Stdio log is too large for urlfetch.'
+        # TODO: Add test level log info to signal

[stgao, 2015/05/21 00:29:56]

style.

+        step_log = buildbot.GetGsStepLog(
+              master_name, builder_name, build_number, step_name)
+        if step_log:
+          test_failure_log = str(self._GetTestLevelFailures(step_log))

[stgao, 2015/05/21 00:29:56]

This is a little hacky.

How about using StringIO in self._GetTestLevelFailures to build it?

+        else:
+          try:
+            test_failure_log = buildbot.GetStepStdio(
+                master_name, builder_name, build_number, step_name,
+                self.HTTP_CLIENT)
+          except ResponseTooLargeError:  # pragma: no cover.
+            logging.exception(
+                'Log of step "%s" is too large for urlfetch.', step_name)
+            # If the stdio log of a step is too large, we don't want to pull it
+            # again in next run, because that might lead to DDoS to the master.
+            test_failure_log = 'Stdio log is too large for urlfetch.'
 
-        if not stdio_log:  # pragma: no cover
-          raise pipeline.Retry('Failed to pull stdio of step %s of master %s'
-                               % (step_name, master_name))
+          if not test_failure_log:  # pragma: no cover
+            raise pipeline.Retry('Failed to pull stdio of step %s of master %s'
+                                 % (step_name, master_name))
 
-        # Save stdio in datastore and avoid downloading again during retry.
+        # Save step log in datastore and avoid downloading again during retry.
         if not step:  # pragma: no cover
           step = WfStep.Create(
               master_name, builder_name, build_number, step_name)
 
-        step.log_data = self._ExtractStorablePortionOfLog(stdio_log)
+        step.log_data = self._ExtractStorablePortionOfLog(test_failure_log)
+
         try:
           step.put()
         except Exception as e:  # pragma: no cover
-          # Sometimes, the stdio log is too large to save in datastore.
+          # Sometimes, the step log is too large to save in datastore.
           logging.exception(e)
 
       # TODO: save result in datastore?
       signals[step_name] = extractors.ExtractSignal(
-          master_name, builder_name, step_name, None, stdio_log).ToJson()
+          master_name, builder_name, step_name, None,
+          str(test_failure_log)).ToJson()
 
     return signals