[Findit] Adding Try Job suspected CLs and result status to analysis

appengine/findit/waterfall/identify_try_job_culprit_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.
 
 from common.git_repository import GitRepository
 from common.http_client_appengine import HttpClientAppengine as HttpClient
+from model import wf_analysis_result_status

[stgao, 2016/04/11 19:50:12]

you may want to rebase on the refactoring CL I committed.

 from model import wf_analysis_status
+from model.wf_analysis import WfAnalysis
 from model.wf_try_job import WfTryJob
 from model.wf_try_job_data import WfTryJobData
 from pipeline_wrapper import BasePipeline
 from waterfall.try_job_type import TryJobType
 
 
 GIT_REPO = GitRepository(
     'https://chromium.googlesource.com/chromium/src.git', HttpClient())
 
 
+def _GetResultAnalysisStatus(analysis, result):
+  """Returns the analysis status based on existing status and try job result.
+
+  Args:
+    analysis: The WfAnalysis entity corresponding to this try job.
+    result: A result dict containing the result of this try job.
+
+  Returns:
+    A wf_analysis_result_status code.
+  """
+  # Only return an updated analysis result status if no results were already
+  # found (by the heuristic-based approach). Note it is possible the
+  # heuristic-based result was triaged before the completion of this try job.
+  analysis_result_status = analysis.result_status
+
+  if (analysis_result_status is None or
+      analysis_result_status == wf_analysis_result_status.NOT_FOUND_UNTRIAGED or
+      analysis_result_status == wf_analysis_result_status.NOT_FOUND_INCORRECT or
+      analysis_result_status == wf_analysis_result_status.NOT_FOUND_CORRECT):
+    if result and result.get('culprit'):
+      return wf_analysis_result_status.FOUND_UNTRIAGED
+    return wf_analysis_result_status.NOT_FOUND_UNTRIAGED

[stgao, 2016/04/11 19:50:12]

Why we reset wf_analysis_result_status.NOT_FOUND_INCORRECT and
wf_analysis_result_status.NOT_FOUND_CORRECT to
wf_analysis_result_status.NOT_FOUND_UNTRIAGED if the try-job find nothing?

[lijeffrey, 2016/04/12 21:10:54]

Done.

+
+  return analysis_result_status
+
+
+def _GetSuspectedCLs(analysis, result):
+  """Returns a list of suspected CLs.
+
+  Args:
+    analysis: The WfAnalysis entity corresponding to this try job.
+    result: A result dict containing the culprit from the results of
+      this tryjob.
+
+  Returns:
+    A combined list of suspected CLs from those already in analysis and those
+    found by this try job.
+  """
+  suspected_cls = analysis.suspected_cls or []

[stgao, 2016/04/11 19:50:13]

Should we do a copy of analysis.suspected_cls instead of updating it directly
through a reference?

[lijeffrey, 2016/04/12 21:10:54]

Done.

+
+  if not result:
+    return suspected_cls
+
+  culprit = result.get('culprit')
+  if not culprit:
+    return suspected_cls
+
+  compile_cl_info = culprit.get('compile')
+  if compile_cl_info:
+    # Suspected CL is from compile failure.
+    if compile_cl_info not in suspected_cls:
+      suspected_cls.append(compile_cl_info)
+      return suspected_cls

[stgao, 2016/04/11 19:50:12]

How about the "else" case of line #70?

[lijeffrey, 2016/04/12 21:10:54]

Done.

+
+  # Suspected CLs are from test failures.
+  for results in culprit.itervalues():
+    if results.get('revision'):
+      # Non swarming test failures, only have step level failure info.
+      cl_info = {
+          'review_url': results.get('review_url'),
+          'repo_name': results.get('repo_name'),
+          'revision': results.get('revision'),
+          'commit_position': results.get('commit_position')
+      }
+      if cl_info not in suspected_cls:
+        suspected_cls.append(cl_info)
+    else:
+      for test_cl_info in results['tests'].values():
+        if test_cl_info not in suspected_cls:
+          suspected_cls.append(test_cl_info)
+
+  return suspected_cls
+
+
 class IdentifyTryJobCulpritPipeline(BasePipeline):
   """A pipeline to identify culprit CL info based on try job compile results."""
 
   def _GetCulpritInfo(self, failed_revisions):
     """Gets commit_positions and review_urls for revisions."""
     culprits = {}
+    # TODO(lijeffrey): remove hard-coded 'chromium' when DEPS file parsing is
+    # supported.
     for failed_revision in failed_revisions:
       culprits[failed_revision] = {
-          'revision': failed_revision
+          'revision': failed_revision,
+          'repo_name': 'chromium'
       }
       change_log = GIT_REPO.GetChangeLog(failed_revision)
       if change_log:
         culprits[failed_revision]['commit_position'] = (
             change_log.commit_position)
         culprits[failed_revision]['review_url'] = change_log.code_review_url
 
     return culprits
 
   @staticmethod
@@ skipping 62 matching lines @@
 
         if step not in culprit_map:
           culprit_map[step] = {
               'tests': {}
           }
 
         if (not test_result['failures'] and
             not culprit_map[step].get('revision')):
           # Non swarming test failures, only have step level failure info.
           culprit_map[step]['revision'] = revision
+          culprit_map[step]['repo_name'] = 'chromium'
 
         for failed_test in test_result['failures']:
           # Swarming tests, gets first failed revision for each test.
           if failed_test not in culprit_map[step]['tests']:
             culprit_map[step]['tests'][failed_test] = {
                 'revision': revision
             }
 
     return culprit_map, failed_revisions
 
@@ skipping 62 matching lines @@
           try_job_result.test_results)
       if (result_to_update and
           result_to_update[-1]['try_job_id'] == try_job_id):
         result_to_update[-1].update(result)
       else:  # pragma: no cover
         result_to_update.append(result)
 
     try_job_result.status = wf_analysis_status.ANALYZED
     try_job_result.put()
 
+    # Update analysis result and suspected CLs with results of this try job.
+    analysis = WfAnalysis.Get(master_name, builder_name, build_number)
+    analysis.result_status = _GetResultAnalysisStatus(analysis, result)

[stgao, 2016/04/11 19:50:13]

If result is empty, no checking is needed in which case we could avoid a
datastore read&write.

[lijeffrey, 2016/04/12 21:10:54]

Done.

+    analysis.suspected_cls = _GetSuspectedCLs(analysis, result)
+    analysis.put()
+
     return result.get('culprit') if result else None