[Findit] Pipeline change to save suspected cls to data store.

appengine/findit/waterfall/build_failure_analysis.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 collections
+from collections import defaultdict
 import os
 import re
 
 from common.diff import ChangeType
 from common.git_repository import GitRepository
 from common.http_client_appengine import HttpClientAppengine as HttpClient
 from waterfall import waterfall_config
 from waterfall.failure_signal import FailureSignal
 
 
@@ skipping 210 matching lines @@
         appearing in the compile failure.
      2) If a hint shows that a CL is likely-suspected, the hint is given 1
         score point. Eg. a CL is just likely suspected if it only changed a
         related file (x_impl.cc vs. x.h) appearing in a failure.
   2. hints: each hint is a string describing a reason for suspecting a CL and
      could be shown to the user (eg., "added x_impl.cc (and it was in log)").
   """
 
   def __init__(self):
     self._score = 0
-    self._hints = collections.defaultdict(int)
+    self._hints = defaultdict(int)
 
   @property
   def score(self):
     return self._score
 
   def AddFileChange(self, change_action, changed_src_file_path,
                     file_path_in_log, score, num_file_name_occurrences,
                     changed_line_numbers=None):
     """Adds a suspected file change.
 
@@ skipping 311 matching lines @@
     failure_signal (FailureSignal): The failure signal of a failed step or test.
     change_log (dict): The change log of a CL as returned by
         common.change_log.ChangeLog.ToDict().
     deps_info (dict): Output of pipeline ExtractDEPSInfoPipeline.
 
   Returns:
     A dict as returned by _Justification.ToDict() if the CL is suspected for the
     failure; otherwise None.
   """
   # Use a dict to map each file name of the touched files to their occurrences.
-  file_name_occurrences = collections.defaultdict(int)
+  file_name_occurrences = defaultdict(int)
   for touched_file in change_log['touched_files']:
     change_type = touched_file['change_type']
     if (change_type in (ChangeType.ADD, ChangeType.COPY,
                         ChangeType.RENAME, ChangeType.MODIFY)):
       file_name = os.path.basename(touched_file['new_path'])
       file_name_occurrences[file_name] += 1
 
     if change_type in (ChangeType.DELETE, ChangeType.RENAME):
       file_name = os.path.basename(touched_file['old_path'])
       file_name_occurrences[file_name] += 1
@@ skipping 14 matching lines @@
 
     _CheckFileInDependencyRolls(file_path_in_log, rolls, justification,
                                 line_numbers)
 
   if not justification.score:
     return None
   else:
     return justification.ToDict()
 
 
+class _CLInfo(object):
+  """A object of information we need for a suspected CL.
+
+  The information is specific to current build.
+  """
+  def __init__(self):
+    self.failures = defaultdict(list)
+    self.top_score = 0
+    self.url = None
+
+  @property
+  def not_none_failures(self):

[stgao, 2016/09/28 00:13:27]

can we have a better name here too?

[chanli, 2016/09/30 20:41:01]

Removed it.

+    # For steps like compile, there is no test_name,
+    # to ensure we include them in the dict, used None as placeholder.

[stgao, 2016/09/28 00:13:27]

Can we use a singleton object like below?

NO_TEST_PLACEHOLDER = object()

[chanli, 2016/09/30 20:41:01]

I don't understand your suggestion... Used another way.

+    # Now removes those Nones.
+    failures = {
+        key: [t for t in self.failures[key] if t] for key in self.failures
+    }
+    return failures
+
+
+def _SaveFailureToMap(
+    cl_failure_map, new_suspected_cl_dict, step_name, test_name, top_score):
+  """Saves a failure's info to the cl that caused it."""
+  cl_key = (
+      new_suspected_cl_dict['repo_name'], new_suspected_cl_dict['revision'],
+      new_suspected_cl_dict['commit_position'])
+
+  cl_failure_map[cl_key].failures[step_name].append(test_name)
+  # Ignores the case where in the same build for the same cl,
+  # we have different scores.
+  # Not sure if we need to handle it since it should be rare.
+  cl_failure_map[cl_key].top_score = (
+      cl_failure_map[cl_key].top_score or top_score)
+  cl_failure_map[cl_key].url = (
+      cl_failure_map[cl_key].url or new_suspected_cl_dict['url'])
+
+
+def _ConvertCLFailureMapToList(cl_failure_map):
+  suspected_cls = []
+  for cl_key, cl_info in cl_failure_map.iteritems():
+    suspected_cl = {}
+    (suspected_cl['repo_name'], suspected_cl['revision'],
+     suspected_cl['commit_position']) = cl_key
+    suspected_cl['url'] = cl_info.url
+    suspected_cl['failures'] = cl_info.not_none_failures
+    suspected_cl['top_score'] = cl_info.top_score
+
+    suspected_cls.append(suspected_cl)
+  return suspected_cls
+
 def AnalyzeBuildFailure(
     failure_info, change_logs, deps_info, failure_signals):
-  """Analyze the given failure signals, and figure out culprit CLs.
+  """Analyzes the given failure signals, and figure out culprit CLs.
 
   Args:
     failure_info (dict): Output of pipeline DetectFirstFailurePipeline.
     change_logs (dict): Output of pipeline PullChangelogPipeline.
     deps_info (dict): Output of pipeline ExtractDEPSInfoPipeline.
     failure_signals (dict): Output of pipeline ExtractSignalPipeline.
 
   Returns:
     A dict with the following form:
     {
@@ skipping 16 matching lines @@
                 'modify e/f/z.cc': 1,
                 ...
               }
             },
             ...
           ],
         },
         ...
       ]
     }
+
+    And a list of suspected_cls format as below:
+    [
+        {
+            'repo_name': 'chromium',
+            'revision': 'r98_1',
+            'commit_position': None,
+            'url': None,
+            'failures': {
+                'b': ['Unittest2.Subtest1', 'Unittest3.Subtest2']
+            },
+            'top_score': 4
+        },
+        ...
+    ]
   """
   analysis_result = {
       'failures': []
   }
 
   if not failure_info['failed'] or not failure_info['chromium_revision']:
     # Bail out if no failed step or no chromium revision.
-    return analysis_result
+    return analysis_result, []
 
   def CreateCLInfoDict(justification_dict, build_number, change_log):
     # TODO(stgao): remove hard-coded 'chromium' when DEPS file parsing is
     # supported.
     cl_info = {
         'build_number': build_number,
         'repo_name': 'chromium',
         'revision': change_log['revision'],
         'commit_position': change_log.get('commit_position'),
         'url':
             change_log.get('code_review_url') or change_log.get('commit_url'),
     }
 
     cl_info.update(justification_dict)
     return cl_info
 
   failed_steps = failure_info['failed_steps']
   builds = failure_info['builds']
-  master_name = failure_info.get('master_name')
+  master_name = failure_info['master_name']
+
+  cl_failure_map = defaultdict(_CLInfo)
 
   for step_name, step_failure_info in failed_steps.iteritems():
     is_test_level = step_failure_info.get('tests') is not None
 
     failed_build_number = step_failure_info['current_failure']
     if step_failure_info.get('last_pass') is not None:
       start_build_number = step_failure_info.get('last_pass') + 1
     else:
       start_build_number = step_failure_info['first_failure']
     step_analysis_result = {
@@ skipping 30 matching lines @@
                   test_signal, change_logs[revision], deps_info)
 
               if not justification_dict:
                 continue
 
               new_suspected_cl_dict = CreateCLInfoDict(
                   justification_dict, build_number, change_logs[revision])
               test_analysis_result['suspected_cls'].append(
                   new_suspected_cl_dict)
 
+              _SaveFailureToMap(
+                  cl_failure_map, new_suspected_cl_dict, step_name, test_name,
+                  max(justification_dict['hints'].values()))
+
           # Checks Files on step level using step level signals
           # regardless of test level signals so we can make sure
           # no duplicate justifications added to the step result.
           failure_signal = FailureSignal.FromDict(failure_signals[step_name])
           justification_dict = _CheckFiles(
               failure_signal, change_logs[revision], deps_info)
 
           if not justification_dict:
             continue
 
-          step_analysis_result['suspected_cls'].append(
-              CreateCLInfoDict(justification_dict, build_number,
-                               change_logs[revision]))
+          new_suspected_cl_dict = CreateCLInfoDict(
+              justification_dict, build_number, change_logs[revision])
+          step_analysis_result['suspected_cls'].append(new_suspected_cl_dict)
+
+          if not is_test_level:
+            _SaveFailureToMap(
+                cl_failure_map, new_suspected_cl_dict, step_name, None,
+                max(justification_dict['hints'].values()))
 
     # TODO(stgao): sort CLs by score.
     analysis_result['failures'].append(step_analysis_result)
 
-  return analysis_result
+  suspected_cls = _ConvertCLFailureMapToList(cl_failure_map)
+
+  return analysis_result, suspected_cls