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 b6f3a4632a6f3a51111ec35fa18572a6a63a994d..4014cae8f12a571dff1607fdea1462f176114f14 100644 (file)
@@ -1,3 +1,47 @@
+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
index 1174f7681ed932790f3a8e1a374d41e999d18e8f..cf032e0b7004f62c40474c9f6532abfa31a71bd6 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 87c8455c55be3dd4af72928c07f7fa882d1d85af..906db77b545b338042f4983a88e80170db3ea4ee 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 9f492201c525acf3ccfdea14ea28340f82cc0b59..872cfe4548583fa04c97d14e44c9ef74eff7908a 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 64051e0b7d709d222c52254d20ad4d8f24fb5087..0b7e62f19b33fc3e1ece8b5fbc2624c5f242f161 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 c2f7a2948f8dd7c9b82bb970688b9de63ab61462..3eae72742e56aab3f8ed272f83208d11497a1f95 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 5ecf7b77bf8abf3af0d7bb0850cff508a6ce2d5f..2e0705d67dc5e7581e541bf02186a958da94a471 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)
     {