Refine v3 UI's analysis task page
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 12 Feb 2016 05:14:32 +0000 (05:14 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 12 Feb 2016 05:14:32 +0000 (05:14 +0000)
https://bugs.webkit.org/show_bug.cgi?id=154152

Reviewed by Chris Dumez.

This patch makes the following refinements to the analysis task page:
 - Always show the relative different of in-progress A/B testing.
 - Make the annotations (colored bars) in the chart open other analysis tasks.
 - Order the A/B testing groups in the reverse chronological order.
 - Select the time range corresponding to the current test group.

* public/v3/components/analysis-results-viewer.js:
(AnalysisResultsViewer.cssTemplate): Fixed the bug that pending and running testing groups are no longer
colored after r196440. Use a slightly more opaque color for currently running groups compared to pending ones.

* public/v3/components/chart-pane-base.js:
(ChartPaneBase.prototype.setMainSelection): Added.
(ChartPaneBase.prototype._openAnalysisTask): Moved the code from ChartPane._openAnalysisTask so that it can be
reused in both AnalysisTaskChartPane and ChartPane (in charts page).
(ChartPaneBase.prototype.router): Added. Overridden by each subclass.

* public/v3/components/test-group-results-table.js:
(TestGroupResultsTable.prototype.buildRowGroups): Always show the summary (relative difference of A to B) as
long as there are some results in each set.

* public/v3/models/test-group.js:
(TestGroup.prototype.compareTestResults): Always set .label and .fullLabel with the relative change as long as
there are some values. Keep using "pending" and "running" in .status since that would determine the color of
stacking blocks representing those A/B testing groups.

* public/v3/pages/analysis-task-page.js:
(AnalysisTaskChartPane):
(AnalysisTaskChartPane.prototype.setPage): Added.
(AnalysisTaskChartPane.prototype.router): Added.
(AnalysisTaskPage):
(AnalysisTaskPage.prototype.render): Show the list of A/B testing groups in the reverse chronological order.
Also set the main chart's selection to the time range of the current test group.

* public/v3/pages/chart-pane.js:
(ChartPane.prototype.router): Added.
(ChartPane.prototype._openAnalysisTask): Moved to ChartPaneBase.prototype._openAnalysisTask.

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@196463 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/chart-pane-base.js
Websites/perf.webkit.org/public/v3/components/test-group-results-table.js
Websites/perf.webkit.org/public/v3/models/test-group.js
Websites/perf.webkit.org/public/v3/pages/analysis-task-page.js
Websites/perf.webkit.org/public/v3/pages/chart-pane.js

index b6f3a46..4014cae 100644 (file)
@@ -1,5 +1,49 @@
 2016-02-11  Ryosuke Niwa  <rniwa@webkit.org>
 
+        Refine v3 UI's analysis task page
+        https://bugs.webkit.org/show_bug.cgi?id=154152
+
+        Reviewed by Chris Dumez.
+
+        This patch makes the following refinements to the analysis task page:
+         - Always show the relative different of in-progress A/B testing.
+         - Make the annotations (colored bars) in the chart open other analysis tasks.
+         - Order the A/B testing groups in the reverse chronological order.
+         - Select the time range corresponding to the current test group.
+
+        * public/v3/components/analysis-results-viewer.js:
+        (AnalysisResultsViewer.cssTemplate): Fixed the bug that pending and running testing groups are no longer
+        colored after r196440. Use a slightly more opaque color for currently running groups compared to pending ones.
+
+        * public/v3/components/chart-pane-base.js:
+        (ChartPaneBase.prototype.setMainSelection): Added.
+        (ChartPaneBase.prototype._openAnalysisTask): Moved the code from ChartPane._openAnalysisTask so that it can be
+        reused in both AnalysisTaskChartPane and ChartPane (in charts page).
+        (ChartPaneBase.prototype.router): Added. Overridden by each subclass.
+
+        * public/v3/components/test-group-results-table.js:
+        (TestGroupResultsTable.prototype.buildRowGroups): Always show the summary (relative difference of A to B) as
+        long as there are some results in each set.
+
+        * public/v3/models/test-group.js:
+        (TestGroup.prototype.compareTestResults): Always set .label and .fullLabel with the relative change as long as
+        there are some values. Keep using "pending" and "running" in .status since that would determine the color of
+        stacking blocks representing those A/B testing groups. 
+
+        * public/v3/pages/analysis-task-page.js:
+        (AnalysisTaskChartPane):
+        (AnalysisTaskChartPane.prototype.setPage): Added.
+        (AnalysisTaskChartPane.prototype.router): Added.
+        (AnalysisTaskPage):
+        (AnalysisTaskPage.prototype.render): Show the list of A/B testing groups in the reverse chronological order.
+        Also set the main chart's selection to the time range of the current test group.
+
+        * public/v3/pages/chart-pane.js:
+        (ChartPane.prototype.router): Added.
+        (ChartPane.prototype._openAnalysisTask): Moved to ChartPaneBase.prototype._openAnalysisTask.
+
+2016-02-11  Ryosuke Niwa  <rniwa@webkit.org>
+
         Add a script to process backlogs created while perf dashboard was in the maintenance mode
         https://bugs.webkit.org/show_bug.cgi?id=154140
 
index 1174f76..cf032e0 100644 (file)
@@ -272,7 +272,10 @@ class AnalysisResultsViewer extends ResultsTable {
             .analysis-view .stacking-block.unchanged {
                 background: rgba(128, 128, 128, 0.5);
             }
-            .analysis-view .stacking-block.incomplete {
+            .analysis-view .stacking-block.pending {
+                background: rgba(204, 204, 51, 0.2);
+            }
+            .analysis-view .stacking-block.running {
                 background: rgba(204, 204, 51, 0.5);
             }
             .analysis-view .stacking-block.worse {
index 87c8455..906db77 100644 (file)
@@ -107,6 +107,12 @@ class ChartPaneBase extends ComponentBase {
             this._mainChart.setDomain(startTime, endTime);
     }
 
+    setMainSelection(selection)
+    {
+        if (this._mainChart)
+            this._mainChart.setSelection(selection);
+    }
+
     _overviewSelectionDidChange(domain, didEndDrag) { }
 
     _mainSelectionDidChange(selection, didEndDrag)
@@ -133,8 +139,14 @@ class ChartPaneBase extends ComponentBase {
         this.render();
     }
 
-    _openAnalysisTask()
-    { }
+    _openAnalysisTask(annotation)
+    {
+        var router = this.router();
+        console.assert(router);
+        window.open(router.url(`analysis/task/${annotation.task.id()}`), '_blank');
+    }
+
+    router() { return null; }
 
     _openCommitViewer(repository, from, to)
     {
index 9f49220..872cfe4 100644 (file)
@@ -61,7 +61,7 @@ class TestGroupResultsTable extends ResultsTable {
                 var endConfig = testGroup.labelForRootSet(rootSets[j]);
 
                 var result = this._testGroup.compareTestResults(rootSets[i], rootSets[j]);
-                if (result.status == 'pending' || result.status == 'running' || result.status == 'failed')
+                if (result.changeType == null)
                     continue;
 
                 var row = new ResultsTableRow(`${startConfig} to ${endConfig}`, null);
index 64051e0..0b7e62f 100644 (file)
@@ -107,28 +107,35 @@ class TestGroup extends LabeledObject {
         console.assert(metric);
 
         var result = {changeType: null, status: 'failed', label: 'Failed', fullLabel: 'Failed', isStatisticallySignificant: false};
-        if (beforeValues.length && afterValues.length) {
-            var diff = afterMean - beforeMean;
-            var smallerIsBetter = metric.isSmallerBetter();
-            result.changeType = diff < 0 == smallerIsBetter ? 'better' : 'worse';
-            result.label = Math.abs(diff / beforeMean * 100).toFixed(2) + '% ' + result.changeType;
-            result.isStatisticallySignificant = Statistics.testWelchsT(beforeValues, afterValues);
-            result.status = result.isStatisticallySignificant ? result.changeType : 'unchanged';
-        }
 
-        if (!this.hasCompleted()) {
+        var hasCompleted = this.hasCompleted();
+        if (!hasCompleted) {
             if (this.hasStarted()) {
                 result.status = 'running';
                 result.label = 'Running';
                 result.fullLabel = 'Running';
             } else {
+                console.assert(result.changeType === null);
                 result.status = 'pending';
                 result.label = 'Pending';
                 result.fullLabel = 'Pending';
             }
-        } else if (result.changeType) {
-            var significance = result.isStatisticallySignificant ? 'significant' : 'insignificant';
-            result.fullLabel = `${result.label} (statistically ${significance})`;
+        }
+
+        if (beforeValues.length && afterValues.length) {
+            var diff = afterMean - beforeMean;
+            var smallerIsBetter = metric.isSmallerBetter();
+            var changeType = diff < 0 == smallerIsBetter ? 'better' : 'worse';
+            var changeLabel = Math.abs(diff / beforeMean * 100).toFixed(2) + '% ' + changeType;
+            var isSignificant = Statistics.testWelchsT(beforeValues, afterValues);
+            var significanceLabel = isSignificant ? 'significant' : 'insignificant';
+
+            result.changeType = changeType;
+            result.label = changeLabel;
+            if (hasCompleted)
+                result.status = isSignificant ? result.changeType : 'unchanged';
+            result.fullLabel = `${result.label} (statistically ${significanceLabel})`;
+            result.isStatisticallySignificant = isSignificant;
         }
 
         return result;
index c2f7a29..3eae727 100644 (file)
@@ -1,6 +1,13 @@
 
 class AnalysisTaskChartPane extends ChartPaneBase {
-    constructor() { super('analysis-task-chart-pane'); }
+    constructor()
+    {
+        super('analysis-task-chart-pane');
+        this._page = null;
+    }
+
+    setPage(page) { this._page = page; }
+    router() { return this._page.router(); }
 }
 
 ComponentBase.defineElement('analysis-task-chart-pane', AnalysisTaskChartPane);
@@ -21,6 +28,7 @@ class AnalysisTaskPage extends PageWithHeading {
         this._errorMessage = null;
         this._currentTestGroup = null;
         this._chartPane = this.content().querySelector('analysis-task-chart-pane').component();
+        this._chartPane.setPage(this);
         this._analysisResultsViewer = this.content().querySelector('analysis-results-viewer').component();
         this._analysisResultsViewer.setTestGroupCallback(this._showTestGroup.bind(this));
         this._testGroupResultsTable = this.content().querySelector('test-group-results-table').component();
@@ -166,7 +174,7 @@ class AnalysisTaskPage extends PageWithHeading {
                     return element('li', {class: 'test-group-list-' + group.id()}, link(group.label(), function () {
                         self._showTestGroup(group);
                     }));
-                }));
+                }).reverse());
             this._renderedCurrentTestGroup = null;
         }
 
@@ -182,6 +190,15 @@ class AnalysisTaskPage extends PageWithHeading {
                     element.classList.add('selected');
             }
 
+            this._chartPane.setMainSelection(null);
+            if (this._currentTestGroup) {
+                var rootSetsInTestGroup = this._currentTestGroup.requestedRootSets();
+                var startTime = rootSetsInTestGroup[0].latestCommitTime();
+                var endTime = rootSetsInTestGroup[rootSetsInTestGroup.length - 1].latestCommitTime();
+                if (startTime != endTime)
+                    this._chartPane.setMainSelection([startTime, endTime]);
+            }
+
             this.content().querySelector('.test-group-retry-button').textContent = this._currentTestGroup ? 'Retry' : 'Confirm the change';
 
             var repetitionCount = this._currentTestGroup ? this._currentTestGroup.repetitionCount() : 4;
index 5ecf7b7..2e0705d 100644 (file)
@@ -63,10 +63,7 @@ class ChartPane extends ChartPaneBase {
         this._chartsPage.setMainDomainFromZoom(selection, this);
     }
 
-    _openAnalysisTask(annotation)
-    {
-        window.open(this._chartsPage.router().url(`analysis/task/${annotation.task.id()}`), '_blank');
-    }
+    router() { return this._chartsPage.router(); }
 
     _indicatorDidChange(indicatorID, isLocked)
     {