[Findit] Flake Checker: Extract commit position and git hash and display to UI for each analyzed bu…

appengine/findit/handlers/flake/check_flake.py

 # Copyright 2016 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 json
 import logging
 
 from google.appengine.api import users
 from google.appengine.ext import ndb
 
 from common import auth_util
 from common.base_handler import BaseHandler
 from common.base_handler import Permission
 from libs import time_util
+from libs.gitiles import commit_util
 from model import analysis_status
 from model import triage_status
 from model.flake.flake_analysis_request import FlakeAnalysisRequest
 from model.flake.master_flake_analysis import MasterFlakeAnalysis
+from waterfall import buildbot
+from waterfall import build_util
 from waterfall.flake import flake_analysis_service
 from waterfall.flake import triggering_sources
 
 
 SWARMING_TASK_BASE_URL = 'https://chromium-swarm.appspot.com'
 
 
-def _GetSuspectedFlakeAnalysisAndTriageResult(analysis):
+def _GetCommitPositionAndGitHash(master_name, builder_name, build_number):
+  """Gets the commit position and git hash of a build.
+
+  Args:
+    master_name (str): The name of the master.
+    builder_name (str): The name of the builder.
+    build_number (int): The build number.
+
+  Returns:
+    commit_position (int), git_hash (str): The git commit position corresponding
+      to the last commit in the build, and the git hash itself.
+  """
+  build = build_util.DownloadBuildData(master_name, builder_name, build_number)

[stgao, 2016/12/12 20:05:18]

This seems not good. What if there are 100s of data points?
In that case, too many HTTP requests have to be sent.

Instead of retrieving info during page rendering, we'd better retrieve at some
stage of the Swarming rerun pipeline, save the data in NDB together with the
DataPoint, and then here we just read them directly.

[lijeffrey, 2016/12/14 02:37:13]

Done.

+
+  if not build.data:
+    return None, None
+
+  build_data = json.loads(build.data)
+  git_hash = buildbot.GetBuildProperty(
+      build_data.get('properties', []), 'got_revision')
+  commit_position = None
+  changes = build_data.get('sourceStamp', {}).get('changes', [])
+
+  if changes:  # pragma: no branch
+    last_commit_message = changes[-1].get('comments')
+    commit_position, _ = commit_util.ExtractCommitPositionAndCodeReviewUrl(
+        last_commit_message)
+
+  return commit_position, git_hash
+
+
+def _GetSuspectedFlakeInfo(analysis):
+  """Returns a dict with information about the suspected flake build.
+
+  Args:
+    analysis (MasterFlakeAnalysis): The master flake analysis the suspected
+      flake build is associated with.
+
+  Returns:
+    A dict in the format:
+      {
+          'build_number': int,
+          'triage_result': int (correct, incorrect, etc.)
+          'commit_position': The human-readable commit position associated with
+            the suspected flaky build number.
+          'git_hash': The git hash associated with the suspected flaky build.
+      }
+  """
+
   if analysis.suspected_flake_build_number is not None:
+    commit_position, git_hash = _GetCommitPositionAndGitHash(
+        analysis.master_name, analysis.builder_name, analysis.build_number)
+
     return {
         'build_number': analysis.suspected_flake_build_number,
+        'commit_position': commit_position,
+        'git_hash': git_hash,
         'triage_result': (
             analysis.triage_history[-1].triage_result if analysis.triage_history
             else triage_status.UNTRIAGED)
     }
 
   return {}
 
 
+def _GetCoordinatesData(analysis):
+  if not analysis or not analysis.data_points:
+    return []
+
+  coordinates = []
+
+  for data_point in analysis.data_points:
+    # Get commit position and git hash
+    commit_position, git_hash = _GetCommitPositionAndGitHash(
+        analysis.master_name, analysis.builder_name, data_point.build_number)
+    task_url = ('%s/task?id=%s' % (
+        SWARMING_TASK_BASE_URL, data_point.task_id) if data_point.task_id
+                else None)

[chanli, 2016/12/12 14:19:05]

Nit: indent

[lijeffrey, 2016/12/14 02:37:13]

Done.

+    coordinates.append([
+        data_point.build_number, data_point.pass_rate, task_url,
+        commit_position, git_hash])
+
+  # Order by build number from earliest to latest.
+  coordinates.sort(key=lambda x: x[0])
+
+  return coordinates
+
+
 class CheckFlake(BaseHandler):
   PERMISSION_LEVEL = Permission.ANYONE
 
   def _ValidateInput(self, master_name, builder_name, build_number, step_name,
                      test_name, bug_id):
     """Ensures the input is valid and generates an error otherwise.
 
     Args:
       master_name (str): The name of the master the flaky test was found on.
       builder_name (str): The name of the builder the flaky test was found on.
@@ skipping 98 matching lines @@
         if not analysis:  # pragma: no cover
           logging.error('Flake analysis was deleted unexpectedly!')
           return {
               'template': 'error.html',
               'data': {
                   'error_message': 'Flake analysis was deleted unexpectedly!',
               },
               'return_code': 400
           }
 
-    suspected_flake = _GetSuspectedFlakeAnalysisAndTriageResult(analysis)
+    suspected_flake = _GetSuspectedFlakeInfo(analysis)
 
     data = {
         'key': analysis.key.urlsafe(),
         'master_name': analysis.master_name,
         'builder_name': analysis.builder_name,
         'build_number': analysis.build_number,
         'step_name': analysis.step_name,
         'test_name': analysis.test_name,
         'pass_rates': [],
         'analysis_status': analysis.status_description,
@@ skipping 12 matching lines @@
       data['triage_history'] = analysis.GetTriageHistory()
 
     data['pending_time'] = time_util.FormatDuration(
         analysis.request_time,
         analysis.start_time or time_util.GetUTCNow())
     if analysis.status != analysis_status.PENDING:
       data['duration'] = time_util.FormatDuration(
           analysis.start_time,
           analysis.end_time or time_util.GetUTCNow())
 
-    coordinates = []
-    for data_point in analysis.data_points:
-      if data_point.task_id:
-        task_url = '%s/task?id=%s' % (
-            SWARMING_TASK_BASE_URL, data_point.task_id)
-        coordinates.append([
-            data_point.build_number, data_point.pass_rate, task_url])
-      else:
-        coordinates.append([
-            data_point.build_number, data_point.pass_rate, None])
+    data['pass_rates'] = _GetCoordinatesData(analysis)
 
-    # Order by build number from earliest to latest.
-    coordinates.sort(key=lambda x: x[0])
-
-    data['pass_rates'] = coordinates
     return {
         'template': 'flake/result.html',
         'data': data
     }