Perf Dashboard v3 confuses better and worse on A/B task page
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 19 Mar 2016 03:29:44 +0000 (03:29 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 19 Mar 2016 03:29:44 +0000 (03:29 +0000)
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

Websites/perf.webkit.org/ChangeLog
Websites/perf.webkit.org/public/v3/components/analysis-results-viewer.js
Websites/perf.webkit.org/public/v3/components/time-series-chart.js

index 4a002de..ec0e234 100644 (file)
@@ -1,5 +1,34 @@
 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
         https://bugs.webkit.org/show_bug.cgi?id=155673
 
index bbca73d..cdfeee7 100644 (file)
@@ -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 {
index de66c28..a90283c 100644 (file)
@@ -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;
         }