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

appengine/findit/waterfall/process_flake_swarming_task_result_pipeline.py

Index: appengine/findit/waterfall/process_flake_swarming_task_result_pipeline.py
diff --git a/appengine/findit/waterfall/process_flake_swarming_task_result_pipeline.py b/appengine/findit/waterfall/process_flake_swarming_task_result_pipeline.py
index f9e7925cb5a89e59f81669cab5c4b397dc1e26db..502a2b814f61a5515f2fe83730f308fecb6de791 100644
--- a/appengine/findit/waterfall/process_flake_swarming_task_result_pipeline.py
+++ b/appengine/findit/waterfall/process_flake_swarming_task_result_pipeline.py
@@ -2,15 +2,53 @@
 # 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 libs.gitiles import commit_util
 from model.flake.flake_swarming_task import FlakeSwarmingTask
 from model.flake.master_flake_analysis import DataPoint
 from model.flake.master_flake_analysis import MasterFlakeAnalysis
+from waterfall import buildbot
+from waterfall import build_util
 from waterfall.process_base_swarming_task_result_pipeline import (
     ProcessBaseSwarmingTaskResultPipeline)
 
 
+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.
+  """
+  if build_number < 1:

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

Since build_number can be 0, shouldn't this be if build_number < 0?

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

You are absolutely right, good catch!

+    return None, None
+
+  build = build_util.DownloadBuildData(master_name, builder_name, build_number)
+
+  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)

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

Could we reuse
https://cs.chromium.org/chromium/infra/appengine/findit/waterfall/buildbot.py...
instead for #38-47?

I think  the commit position is in property "got_revision_cp".

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

Done.

+
+  return commit_position, git_hash
+
+
 class ProcessFlakeSwarmingTaskResultPipeline(
     ProcessBaseSwarmingTaskResultPipeline):
   """A pipeline for monitoring swarming task and processing task result.
@@ -75,6 +113,18 @@ class ProcessFlakeSwarmingTaskResultPipeline(
     data_point.build_number = build_number
     data_point.pass_rate = pass_rate
     data_point.task_id = flake_swarming_task.task_id
+
+    # Include git commit position information about each build that was run.
+    previous_commit, previous_git_hash = _GetCommitPositionAndGitHash(
+        master_name, builder_name, build_number - 1)
+    data_point.previous_commit_position = previous_commit
+    data_point.previous_git_hash = previous_git_hash
+
+    commit_position, git_hash = _GetCommitPositionAndGitHash(
+        master_name, builder_name, build_number)
+    data_point.commit_position = commit_position
+    data_point.git_hash = git_hash
+
     master_flake_analysis.data_points.append(data_point)
 
     results = flake_swarming_task.GetFlakeSwarmingTaskData()
@@ -100,4 +150,4 @@ class ProcessFlakeSwarmingTaskResultPipeline(
                        step_name, master_build_number, test_name, _):
     # Gets the appropriate kind of swarming task (FlakeSwarmingTask).
     return FlakeSwarmingTask.Get(master_name, builder_name, build_number,
-                                 step_name, test_name)
+                                 step_name, test_name)