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

appengine/findit/templates/flake/result.html

Index: appengine/findit/templates/flake/result.html
diff --git a/appengine/findit/templates/flake/result.html b/appengine/findit/templates/flake/result.html
index 3739519c1f190132bd8451403ecfc5253f753745..815e7f85ba63fa3dcf12f197c10d334f8f2314b7 100644
--- a/appengine/findit/templates/flake/result.html
+++ b/appengine/findit/templates/flake/result.html
@@ -86,13 +86,18 @@
       var not_run_data = [];
       var run_data = [];
       $.each(findit.passRates, function(index, value) {
-        // Convert pass rate from [0, 1] to a percentage.
-        var rate = (value[1] * 100).toFixed(0);
-        // -1 means that the test doesn't exist or is disabled at the build.
-        if (rate < 0) {
-          not_run_data.push([value[0], 100, value[2]]);
+        // Convert pass rate from [0, 1] to a percentage. Include information about each point in the format:
+        // [build number, pass rate, swarming task url,
+        //  previous build's commit position, this build's commit position,
+        //  previous build's git hash, this build's git hash].
+        var pass_rate = (value[1] * 100).toFixed(0);
+
+        if (pass_rate < 0) {
+          // -1 means that the test doesn't exist or is disabled at the build.
+          // We consider that a 100% pass rate.
+          not_run_data.push([value[0], 100, value[2], value[3], value[4], value[5], value[6]]);

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

Could we use commit position as X axis now? Later when data point from try-job
is added, we need commit position instead of build number. Better to get
prepared now.

BTW, we might need a script to update legacy data for those commit position and
git hash.

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

Done.

         } else {
-          run_data.push([value[0], rate, value[2]]);
+          run_data.push([value[0], pass_rate, value[2], value[3], value[4], value[5], value[6]]);
         }
       });
 
@@ -169,19 +174,32 @@
         var buildNumber = itemData[0];
         var passRate = itemData[1];
         var swarmingTaskId = itemData[2];
+        var previousBuildCommitPosition = itemData[3];
+        var thisBuildCommitPosition = itemData[4];
+        var previousBuildGitHash = itemData[5];
+        var thisBuildGitHash = itemData[6];
+
+        var tooltipStr = 'Build #: <a href="https://build.chromium.org/p/' + findit.masterName + '/builders/' + findit.builderName + '/builds/' + buildNumber + '" target="_blank">' + buildNumber + '</a>';

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

For the order of info entries, how about below?

Pass rate: XX%
Range: last:current
Build #: ...
Swarming task: ...

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

Done.

+
+        if (previousBuildCommitPosition && previousBuildGitHash && thisBuildCommitPosition && thisBuildGitHash) {

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

I think if we have git hashes but not commit positions, we should still display
the ranges, but with shortened git hashes. WDYT?

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

They should come together as far as I can tell. This branch is for the case
build_number is 0 where there is no previous commit position/git hash

+          tooltipStr += '<br>Range: <a href="https://crrev.com/' + previousBuildGitHash + '..' + thisBuildGitHash + '"target="_blank">' + '(' + previousBuildCommitPosition + ':' + thisBuildCommitPosition + ']</a>';

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

Nit: Range: (rev1:rev2]

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

I think this should already do that?

+        } else if (thisBuildCommitPosition && thisBuildGitHash) {
+          // Previous information not available, so only show this build.
+          tooltipStr += '<br>Range: <a href="https://crrev.com/' + thisBuildGitHash + '" "target="_blank">' + thisBuildCommitPosition + '</a>';
+        }
+        
+        tooltipStr += '<br>Pass rate: ' + passRate + '%';
 
-        var tooltipStr = 'Build #: <a href="https://build.chromium.org/p/' + findit.masterName + '/builders/' + findit.builderName + '/builds/' + buildNumber + '" target="_blank">' + buildNumber + '</a><br> Pass rate: ' + passRate + '%';
         if (swarmingTaskId) {
           tooltipStr += '<br>Swarming Task: <a href="https://chromium-swarm.appspot.com/task?id=' + swarmingTaskId + '" target="_blank">' + swarmingTaskId +'</a>';
         } else {
           tooltipStr += '<br>Swarming Task: N/A';
         }
-
         $("#tooltip").html(tooltipStr).css({top: item.pageY + 5, left: item.pageX + 5}).show();
       }
 
       var dataPointSelected = false;
-      $("#flake-data").bind("plothover", function (event, pos, item) {
+      $("#flake-data").bind("plothover", function(event, pos, item) {
         if (dataPointSelected)
           return;  // A data point is selected due to a click.
 
@@ -191,7 +209,7 @@
           $("#tooltip").hide();
         }
       });
-      $("#flake-data").bind("plotclick", function (event, pos, item) {
+      $("#flake-data").bind("plotclick", function(event, pos, item) {
         if (item) {
           dataPointSelected = true;  // Set selected data point upon click.
           showTooltipForDataPoint(item);
@@ -239,8 +257,14 @@
     Error info: {{ error }}<br>
     {% endif %}
     {% if suspected_flake.build_number %}
-    Flakiness <b>started</b> in Build: <a href="https://build.chromium.org/p/{{ master_name }}/builders/{{ builder_name }}/builds/{{ suspected_flake.build_number }}" target="_blank" style="color:red;font-weight:bold">{{ suspected_flake.build_number }}</a>
+      Flakiness <b>started</b> in Build: <a href="https://build.chromium.org/p/{{ master_name }}/builders/{{ builder_name }}/builds/{{ suspected_flake.build_number }}" target="_blank" style="color:red;font-weight:bold">{{ suspected_flake.build_number }}</a>
+    {% endif %}
+    {% if suspected_flake.commit_position and suspected_flake.git_hash %}
+      <br>
+      Regression range: <a href="https://crrev.com/{{ suspected_flake.git_hash }}..{{ suspected_flake.previous_git_hash }}" target="_blank">({{ suspected_flake.commit_position }}:{{ suspected_flake.previous_commit_position }}]</a>

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

Nit: Range: (rev1:rev2]

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

Should the commit position and git hash switch in order? The link seems not
working in the demo UI.

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

nit: "(" and "]" could be removed.

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

Done.

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

Done.

+      <br>
     {% endif %}
+
   </div>
   <br>
   <div>