[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 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 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.flake import flake_analysis_service
 from waterfall.flake import triggering_sources
 
 
-def _GetSuspectedFlakeAnalysisAndTriageResult(analysis):
-  if analysis.suspected_flake_build_number is not None:
-    return {
-        'build_number': analysis.suspected_flake_build_number,
-        'triage_result': (
-            analysis.triage_history[-1].triage_result if analysis.triage_history
-            else triage_status.UNTRIAGED)
-    }
+SWARMING_TASK_BASE_URL = 'https://chromium-swarm.appspot.com'

[stgao, 2016/12/14 07:58:28]

You may want to pull in my change and rebase.
This was gone.

[lijeffrey, 2016/12/14 22:15:11]

Done.

 
-  return {}
+
+def _FindSuspectedFlakeBuildDataPoint(analysis):
+  for data_point in analysis.data_points:
+    if data_point.build_number == analysis.suspected_flake_build_number:
+      return data_point
+
+  return None  # pragma: no cover. This should not happen.

[chanli, 2016/12/14 03:41:05]

Would you mind add a test for this? Should be very easy to test

[lijeffrey, 2016/12/14 22:15:11]

Done.

+
+
+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,
+          'commit_position': int,
+          'git_hash': str,
+          'previous_commit_position': int,
+          'previous_git_hash': str,
+          'triage_result': int (correct, incorrect, etc.)
+      }
+  """
+  if analysis.suspected_flake_build_number is None:
+    return {}
+
+  data_point = _FindSuspectedFlakeBuildDataPoint(analysis)
+  assert data_point
+
+  return {
+      'build_number': analysis.suspected_flake_build_number,
+      'commit_position': data_point.commit_position,
+      'git_hash': data_point.git_hash,
+      'previous_commit_position': data_point.previous_commit_position,
+      'previous_git_hash': data_point.previous_git_hash,
+      'triage_result': (
+          analysis.triage_history[-1].triage_result if analysis.triage_history
+          else triage_status.UNTRIAGED)
+  }
+
+
+def _GetCoordinatesData(analysis):
+  if not analysis or not analysis.data_points:
+    return []
+
+  coordinates = []
+
+  for data_point in analysis.data_points:
+    coordinates.append([
+        data_point.build_number, data_point.pass_rate, data_point.task_id,
+        data_point.previous_commit_position, data_point.commit_position,
+        data_point.previous_git_hash, data_point.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:
@@ skipping 100 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:
-      coordinates.append([
-          data_point.build_number, data_point.pass_rate, data_point.task_id])
+    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
     }