Perf Dashboard v3 confuses better and worse on A/B task page

https://bugs.webkit.org/show_bug.cgi?id=155675
<rdar://problem/25208723>

Reviewed by Joseph Pecoraro.

The analysis results viewer on v3 UI sometimes treats regressions as progressions and vice versa when
the first set (i.e. set A) of the revisions used in an A/B testing never appears in the original graph,
and its latest commit time matches that of the second set, which appears in the original graph.

Because the analysis results viewer compares results in the increasing row number, this results in
B to be compared to A instead of A to be compared to B. Fixed the bug by preventing the wrong ordering
to occur in _buildRowsForPointsAndTestGroups by always inserting a root set A before B when B appears
and A doesn't appear in the original graph.

* public/v3/components/analysis-results-viewer.js:
(AnalysisResultsViewer.prototype._collectRootSetsInTestGroups): Remember the succeeding root set B
when creating an entry for root set A.
(AnalysisResultsViewer.prototype._buildRowsForPointsAndTestGroups): Fixed the bug. Also un-duplicated
the code to create a new row.
(AnalysisResultsViewer.RootSetInTestGroup): Now takes a succeeding root set. e.g. it's B for A and
undefined for B in A/B testing.
(AnalysisResultsViewer.RootSetInTestGroup.prototype.succeedingRootSet): Added.
* public/v3/components/time-series-chart.js:
(TimeSeriesChart.computeTimeGrid): Fixed the bug that we would end up showing 0 AM instead of dates
when both dates and months change.

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@198464 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/Websites/perf.webkit.org/ChangeLog b/Websites/perf.webkit.org/ChangeLog
index 4a002de9a755582bd95dc7627bb07c8bb24c823b..ec0e234c1ce2d4667c4cf59b0b5be031941a2b80 100644 (file)
--- a/Websites/perf.webkit.org/ChangeLog
+++ b/Websites/perf.webkit.org/ChangeLog
@@ -1,3 +1,32 @@
+2016-03-18  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Perf Dashboard v3 confuses better and worse on A/B task page
+        https://bugs.webkit.org/show_bug.cgi?id=155675
+        <rdar://problem/25208723>
+
+        Reviewed by Joseph Pecoraro.
+
+        The analysis results viewer on v3 UI sometimes treats regressions as progressions and vice versa when
+        the first set (i.e. set A) of the revisions used in an A/B testing never appears in the original graph,
+        and its latest commit time matches that of the second set, which appears in the original graph.
+
+        Because the analysis results viewer compares results in the increasing row number, this results in
+        B to be compared to A instead of A to be compared to B. Fixed the bug by preventing the wrong ordering
+        to occur in _buildRowsForPointsAndTestGroups by always inserting a root set A before B when B appears
+        and A doesn't appear in the original graph.
+
+        * public/v3/components/analysis-results-viewer.js:
+        (AnalysisResultsViewer.prototype._collectRootSetsInTestGroups): Remember the succeeding root set B
+        when creating an entry for root set A.
+        (AnalysisResultsViewer.prototype._buildRowsForPointsAndTestGroups): Fixed the bug. Also un-duplicated
+        the code to create a new row.
+        (AnalysisResultsViewer.RootSetInTestGroup): Now takes a succeeding root set. e.g. it's B for A and
+        undefined for B in A/B testing.
+        (AnalysisResultsViewer.RootSetInTestGroup.prototype.succeedingRootSet): Added.
+        * public/v3/components/time-series-chart.js:
+        (TimeSeriesChart.computeTimeGrid): Fixed the bug that we would end up showing 0 AM instead of dates
+        when both dates and months change.
+
 2016-03-18  Ryosuke Niwa  <rniwa@webkit.org>
 
         Add unit tests for measurement-set.js and measurement-adapter.js

diff --git a/Websites/perf.webkit.org/public/v3/components/analysis-results-viewer.js b/Websites/perf.webkit.org/public/v3/components/analysis-results-viewer.js
index bbca73dbd7df54002f21024f7b689a4a9c07d2dc..cdfeee7ccefd5c674cab861e09447ef2e056299d 100644 (file)
--- a/Websites/perf.webkit.org/public/v3/components/analysis-results-viewer.js
+++ b/Websites/perf.webkit.org/public/v3/components/analysis-results-viewer.js
@@ -139,7 +139,7 @@ class AnalysisResultsViewer extends ResultsTable {
         for (var group of this._testGroups) {
             var sortedSets = group.requestedRootSets();
             for (var i = 0; i < sortedSets.length; i++)
-                rootSetsInTestGroups.push(new AnalysisResultsViewer.RootSetInTestGroup(group, sortedSets[i]));
+                rootSetsInTestGroups.push(new AnalysisResultsViewer.RootSetInTestGroup(group, sortedSets[i], sortedSets[i + 1]));
         }
 
         return rootSetsInTestGroups;
@@ -191,14 +191,20 @@ class AnalysisResultsViewer extends ResultsTable {
             }
 
             var groupTime = entry.rootSet().latestCommitTime();
+            var newRow = new ResultsTableRow(null, entry.rootSet());
+            rowToMatchingRootSets.set(newRow, [entry]);
+
             for (var i = 0; i < rowList.length; i++) {
                 if (rowList[i] instanceof AnalysisResultsViewer.ExpandableRow)
                     continue;
 
+                if (rowList[i].rootSet().equals(entry.succeedingRootSet())) {
+                    rowList.splice(i, 0, newRow);
+                    return;
+                }
+
                 var rowTime = rowList[i].rootSet().latestCommitTime();
                 if (rowTime > groupTime) {
-                    var newRow = new ResultsTableRow(null, entry.rootSet());
-                    rowToMatchingRootSets.set(newRow, [entry]);
                     rowList.splice(i, 0, newRow);
                     return;
                 }
@@ -213,9 +219,7 @@ class AnalysisResultsViewer extends ResultsTable {
                             var newCommit = entry.rootSet().commitForRepository(repository);
                             var rowCommit = rowList[j].rootSet().commitForRepository(repository);
                             if (!rowCommit || newCommit.time() < rowCommit.time()) {
-                                var row = new ResultsTableRow(null, entry.rootSet());
-                                rowToMatchingRootSets.set(row, [entry]);
-                                rowList.splice(j, 0, row);
+                                rowList.splice(j, 0, newRow);
                                 return;
                             }
                         }
@@ -352,16 +356,18 @@ AnalysisResultsViewer.ExpandableRow = class extends ResultsTableRow {
 }
 
 AnalysisResultsViewer.RootSetInTestGroup = class {
-    constructor(testGroup, rootSet)
+    constructor(testGroup, rootSet, succeedingRootSet)
     {
         console.assert(testGroup instanceof TestGroup);
         console.assert(rootSet instanceof RootSet);
         this._testGroup = testGroup;
         this._rootSet = rootSet;
+        this._succeedingRootSet = succeedingRootSet;
     }
 
     testGroup() { return this._testGroup; }
     rootSet() { return this._rootSet; }
+    succeedingRootSet() { return this._succeedingRootSet; }
 }
 
 AnalysisResultsViewer.TestGroupStackingBlock = class {

diff --git a/Websites/perf.webkit.org/public/v3/components/time-series-chart.js b/Websites/perf.webkit.org/public/v3/components/time-series-chart.js
index de66c28bc59bf2532e63085bcdf34d9eb68ec648..a90283c079fe996ca08b0cad9860286d885e38f1 100644 (file)
--- a/Websites/perf.webkit.org/public/v3/components/time-series-chart.js
+++ b/Websites/perf.webkit.org/public/v3/components/time-series-chart.js
@@ -579,6 +579,7 @@ class TimeSeriesChart extends ComponentBase {
         var result = [];
 
         var previousDate = null;
+        var previousMonth = null;
         var previousHour = null;
         while (currentTime <= max) {
             var time = new Date(currentTime);
@@ -590,7 +591,7 @@ class TimeSeriesChart extends ComponentBase {
             iterator.next(currentTime);
 
             var label;
-            if (date == previousDate)
+            if (date == previousDate && month == previousMonth)
                 label = hourLabel;
             else {
                 label = `${month}/${date}`;
@@ -601,6 +602,7 @@ class TimeSeriesChart extends ComponentBase {
             result.push({time: time, label: label});
 
             previousDate = date;
+            previousMonth = month;
             previousHour = hour;
         }