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

appengine/findit/handlers/flake/test/check_flake_test.py

Index: appengine/findit/handlers/flake/test/check_flake_test.py
diff --git a/appengine/findit/handlers/flake/test/check_flake_test.py b/appengine/findit/handlers/flake/test/check_flake_test.py
index 8dbdf5aa4837e9a83e525e5d335ca0f8b79d39cc..9a11a343ff4ca80d14376ab2c0bf5ac2e347513d 100644
--- a/appengine/findit/handlers/flake/test/check_flake_test.py
+++ b/appengine/findit/handlers/flake/test/check_flake_test.py
@@ -3,6 +3,7 @@
 # found in the LICENSE file.
 
 import datetime
+import json
 import mock
 import re
 
@@ -19,6 +20,8 @@ from model.flake.flake_analysis_request import BuildStep
 from model.flake.flake_analysis_request import FlakeAnalysisRequest
 from model.flake.master_flake_analysis import DataPoint
 from model.flake.master_flake_analysis import MasterFlakeAnalysis
+from model.wf_build import WfBuild
+from waterfall import build_util
 from waterfall.flake import flake_analysis_service
 from waterfall.test import wf_testcase
 
@@ -71,7 +74,15 @@ class CheckFlakeTest(wf_testcase.WaterfallTestCase):
             'test_name': test_name
         })
 
-  def testAnyoneCanViewScheduledAnalysis(self):
+  @mock.patch.object(check_flake, '_GetSuspectedFlakeInfo',
+                     return_value={
+                         'build_number': 100,
+                         'commit_position': 12345,
+                         'git_hash': 'a_git_hash',
+                         'triage_result': 0})
+  @mock.patch.object(check_flake, '_GetCoordinatesData',
+                     return_value=[[100, 0.9, 'url', 12345, 'a_git_hash']])
+  def testAnyoneCanViewScheduledAnalysis(self, *_):
     master_name = 'm'
     builder_name = 'b'
     build_number = '123'
@@ -98,11 +109,9 @@ class CheckFlakeTest(wf_testcase.WaterfallTestCase):
         'key': analysis.key.urlsafe(),
         'format': 'json'})
 
-    task_url = '%s/task?id=%s' % (
-      check_flake.SWARMING_TASK_BASE_URL, data_point.task_id)
     expected_check_flake_result = {
         'key': analysis.key.urlsafe(),
-        'pass_rates': [[int(build_number), success_rate, task_url]],
+        'pass_rates': [[100, 0.9, 'url', 12345, 'a_git_hash']],
         'analysis_status': STATUS_TO_DESCRIPTION.get(analysis.status),
         'master_name': master_name,
         'builder_name': builder_name,
@@ -117,6 +126,8 @@ class CheckFlakeTest(wf_testcase.WaterfallTestCase):
         'duration': '00:59:55',
         'suspected_flake': {
             'build_number': 100,
+            'commit_position': 12345,
+            'git_hash': 'a_git_hash',
             'triage_result': 0
         },
         'version_number': 1,
@@ -148,7 +159,15 @@ class CheckFlakeTest(wf_testcase.WaterfallTestCase):
 
   @mock.patch.object(flake_analysis_service, 'ScheduleAnalysisForFlake',
                      return_value=False)
-  def testRequestExistingAnalysis(self, _):
+  @mock.patch.object(check_flake, '_GetSuspectedFlakeInfo',
+                     return_value={
+                         'build_number': 100,
+                         'commit_position': 12345,
+                         'git_hash': 'a_git_hash',
+                         'triage_result': 0})
+  @mock.patch.object(check_flake, '_GetCoordinatesData',
+                     return_value=[[100, 0.9, 'url', 12345, 'a_git_hash']])
+  def testRequestExistingAnalysis(self, *_):
     master_name = 'm'
     builder_name = 'b'
     build_number = 123
@@ -191,7 +210,7 @@ class CheckFlakeTest(wf_testcase.WaterfallTestCase):
 
     expected_check_flake_result = {
         'key': previous_analysis.key.urlsafe(),
-        'pass_rates': [[build_number - 1, success_rate, None]],
+        'pass_rates': [[100, 0.9, 'url', 12345, 'a_git_hash']],
         'analysis_status': STATUS_TO_DESCRIPTION.get(previous_analysis.status),
         'master_name': master_name,
         'builder_name': builder_name,
@@ -206,6 +225,8 @@ class CheckFlakeTest(wf_testcase.WaterfallTestCase):
         'duration': '00:59:55',
         'suspected_flake': {
             'build_number': 100,
+            'commit_position': 12345,
+            'git_hash': 'a_git_hash',
             'triage_result': 0
         },
         'version_number': 1,
@@ -243,8 +264,16 @@ class CheckFlakeTest(wf_testcase.WaterfallTestCase):
             'test_name': test_name,
             'format': 'json'})
 
+  @mock.patch.object(check_flake, '_GetSuspectedFlakeInfo',
+                     return_value={
+                         'build_number': 100,
+                         'commit_position': 12345,
+                         'git_hash': 'a_git_hash',
+                         'triage_result': 0})
+  @mock.patch.object(check_flake, '_GetCoordinatesData',
+                     return_value=[[100, 0.9, 'url', 12345, 'a_git_hash']])
   @mock.patch.object(users, 'is_current_user_admin', return_value=True)
-  def testGetTriageHistory(self, _):
+  def testGetTriageHistory(self, *_):
     master_name = 'm'
     builder_name = 'b'
     build_number = '123'
@@ -313,3 +342,68 @@ class CheckFlakeTest(wf_testcase.WaterfallTestCase):
         CheckFlake()._ValidateInput(
             'm', 'b', '1', 's', 't', 'a').get('data', {}).get('error_message'),
         'Bug id (optional) must be an int')
+
+  @mock.patch.object(build_util, '_BuildDataNeedUpdating', return_value=False)
+  def testGetCommitPositionAndGitHash(self, _):
+    build = WfBuild.Create('m', 'b', 123)
+    build.data = json.dumps({
+        'properties': [
+            ['got_revision', 'a_git_hash']
+        ],
+        'sourceStamp': {
+            'changes': [
+                {
+                    'comments': ('Cr-Commit-Position: '
+                                 'refs/heads/master@{#12345}')
+                }
+            ]
+        }
+    })
+    build.put()
+
+    self.assertEqual((12345, 'a_git_hash'),
+                     check_flake._GetCommitPositionAndGitHash('m', 'b', 123))
+
+  @mock.patch.object(build_util, '_BuildDataNeedUpdating', return_value=False)
+  def testGetCommitPositionAndGitHashNoBuildDataAvailable(self, _):
+    build = WfBuild.Create('m', 'b', 123)
+    build.data = {}
+    build.put()
+
+    self.assertEqual((None, None),
+                     check_flake._GetCommitPositionAndGitHash('m', 'b', 123))
+
+  @mock.patch.object(check_flake, '_GetCommitPositionAndGitHash',
+                     return_value=(12345, 'a_git_hash'))
+  def testGetSuspectedFlakeInfo(self, _):
+    analysis = MasterFlakeAnalysis.Create('m', 'b', 123, 's', 't')
+    analysis.suspected_flake_build_number = 123
+    analysis.put()
+    expected_result = {
+        'build_number': analysis.suspected_flake_build_number,
+        'commit_position': 12345,
+        'git_hash': 'a_git_hash',
+        'triage_result': 0
+    }
+    self.assertEqual(expected_result,
+                     check_flake._GetSuspectedFlakeInfo(analysis))
+
+  @mock.patch.object(check_flake, '_GetCommitPositionAndGitHash',
+                     return_value=(12345, 'a_git_hash'))
+  def testGetCoordinatesData(self, _):
+    master_name = 'm'
+    builder_name = 'b'
+    build_number = 123
+    step_name = 's'
+    test_name = 't'
+    success_rate = .9
+    analysis = MasterFlakeAnalysis.Create(
+        master_name, builder_name, build_number, step_name, test_name)
+    data_point = DataPoint()
+    data_point.build_number = build_number
+    data_point.pass_rate = success_rate
+    analysis.data_points.append(data_point)
+    analysis.Save()
+
+    self.assertEqual([[build_number, success_rate, None, 12345, 'a_git_hash']],
+                     check_flake._GetCoordinatesData(analysis))