[Findit] Fix urls in dashboard and result page and prevent duplicated culprits in dashboard.

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 common.pipeline_wrapper import BasePipeline
 from model import analysis_status
 from model import result_status
 from model.wf_analysis import WfAnalysis
@@ skipping 39 matching lines @@
   Args:
     analysis: The WfAnalysis entity corresponding to this try job.
     result: A result dict containing the culprit from the results of
       this try job.
 
   Returns:
     A combined list of suspected CLs from those already in analysis and those
     found by this try job.
   """
   suspected_cls = analysis.suspected_cls[:] if analysis.suspected_cls else []
+  suspected_cl_revisions = [cl['revision'] for cl in suspected_cls]
   culprit = result.get('culprit')
   compile_cl_info = culprit.get('compile')
 
   if compile_cl_info:
     # Suspected CL is from compile failure.
-    if compile_cl_info not in suspected_cls:
+    revision = compile_cl_info.get('revision')
+    if revision not in suspected_cl_revisions:
+      suspected_cl_revisions.append(revision)
       suspected_cls.append(compile_cl_info)
     return suspected_cls
 
   # 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.
+      revision = results.get('revision')
       cl_info = {
-          'review_url': results.get('review_url'),
+          'url': results.get('url'),
           'repo_name': results.get('repo_name'),
           'revision': results.get('revision'),
           'commit_position': results.get('commit_position')
       }
-      if cl_info not in suspected_cls:
+      if revision not in suspected_cl_revisions:
+        suspected_cl_revisions.append(revision)
         suspected_cls.append(cl_info)
     else:
       for test_cl_info in results['tests'].values():
-        if test_cl_info not in suspected_cls:
+        revision = test_cl_info.get('revision')
+        if revision not in suspected_cl_revisions:
+          suspected_cl_revisions.append(revision)
           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."""
+    """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,
           '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
+        culprits[failed_revision]['url'] = (
+            change_log.code_review_url or change_log.commit_url)
 
     return culprits
 
   @staticmethod
   def _GetFailedRevisionFromResultsDict(results_dict):
     """Finds the failed revision from the given dict of revisions.
 
     Args:
       results_dict: (dict) A dict that maps revisions to their results. For
       example:
@@ skipping 71 matching lines @@
         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
 
   def _UpdateCulpritMapWithCulpritInfo(self, culprit_map, culprits):
-    """Fills in commit_position and review_url for each failed rev in map."""
+    """Fills in commit_position and review url for each failed rev in map."""
     for step_culprit in culprit_map.values():
       if step_culprit.get('revision'):
         culprit = culprits[step_culprit['revision']]
         step_culprit['commit_position'] = culprit['commit_position']
-        step_culprit['review_url'] = culprit['review_url']
+        step_culprit['url'] = culprit['url']
       for test_culprit in step_culprit.get('tests', {}).values():
         test_revision = test_culprit['revision']
         test_culprit.update(culprits[test_revision])
 
   def _GetCulpritDataForTest(self, culprit_map):
     """Gets culprit revision for each failure for try job metadata."""
     culprit_data = {}
     for step, step_culprit in culprit_map.iteritems():
       if step_culprit['tests']:
         culprit_data[step] = {}
@@ skipping 59 matching lines @@
       if (analysis.result_status != updated_result_status or
           analysis.suspected_cls != updated_suspected_cls):
         analysis.result_status = updated_result_status
         analysis.suspected_cls = updated_suspected_cls
         analysis.put()
 
     try_job_result.status = analysis_status.COMPLETED
     try_job_result.put()
 
     return result.get('culprit') if result else None