Title: [219321] trunk/Websites/perf.webkit.org
- Revision
- 219321
- Author
- [email protected]
- Date
- 2017-07-10 20:45:00 -0700 (Mon, 10 Jul 2017)
Log Message
A/B testing results page show results for the top-level tests instead of the one being analyzed
https://bugs.webkit.org/show_bug.cgi?id=174304
Reviewed by Antti Koivisto.
When a specific subtest is analyzed (e.g. Images subtest of MotionMark), then TestGroupResultsViewer
should expand and highlight that specific subtest instead of simply showing the top-level test's score.
This is especially misleading since AnalysisResultsViewer (stacking bars for each test group) uses
the score of the specific subtest being analyzed.
Fixed the bug by passing in the metric associated with the analysis task from AnalysisTaskPage to
TestGroupResultsViewer via AnalysisTaskTestGroupPane. Also made TestGroupResultsViewer.setAnalysisResults
auto-expand the tests that are ancestors of the specified metric. Without that, the test won't be shown
to the user until the ancestor tests are expanded by the user.
Also fixed the bug that we were always listing sub-tests regardless of whether they have results or not.
Since tests tend to change over time, we shouldn't show a test if it doesn't have any results associated.
* public/v3/components/test-group-results-viewer.js:
(TestGroupResultsViewer.prototype.setAnalysisResults): Expand the ancestor tests of the metric.
(TestGroupResultsViewer.prototype._buildRowsForTest): Exit early if this test doesn't have any results.
* public/v3/models/analysis-results.js:
(AnalysisResults.prototype.containsTest): Added.
* public/v3/pages/analysis-task-page.js:
(AnalysisTaskTestGroupPane.prototype.setAnalysisResults): Takes a metric to pass it to the results viewer.
(AnalysisTaskPage.prototype._assignTestResultsIfPossible):
Modified Paths
Diff
Modified: trunk/Websites/perf.webkit.org/ChangeLog (219320 => 219321)
--- trunk/Websites/perf.webkit.org/ChangeLog 2017-07-11 03:40:59 UTC (rev 219320)
+++ trunk/Websites/perf.webkit.org/ChangeLog 2017-07-11 03:45:00 UTC (rev 219321)
@@ -1,3 +1,32 @@
+2017-07-10 Ryosuke Niwa <[email protected]>
+
+ A/B testing results page show results for the top-level tests instead of the one being analyzed
+ https://bugs.webkit.org/show_bug.cgi?id=174304
+
+ Reviewed by Antti Koivisto.
+
+ When a specific subtest is analyzed (e.g. Images subtest of MotionMark), then TestGroupResultsViewer
+ should expand and highlight that specific subtest instead of simply showing the top-level test's score.
+ This is especially misleading since AnalysisResultsViewer (stacking bars for each test group) uses
+ the score of the specific subtest being analyzed.
+
+ Fixed the bug by passing in the metric associated with the analysis task from AnalysisTaskPage to
+ TestGroupResultsViewer via AnalysisTaskTestGroupPane. Also made TestGroupResultsViewer.setAnalysisResults
+ auto-expand the tests that are ancestors of the specified metric. Without that, the test won't be shown
+ to the user until the ancestor tests are expanded by the user.
+
+ Also fixed the bug that we were always listing sub-tests regardless of whether they have results or not.
+ Since tests tend to change over time, we shouldn't show a test if it doesn't have any results associated.
+
+ * public/v3/components/test-group-results-viewer.js:
+ (TestGroupResultsViewer.prototype.setAnalysisResults): Expand the ancestor tests of the metric.
+ (TestGroupResultsViewer.prototype._buildRowsForTest): Exit early if this test doesn't have any results.
+ * public/v3/models/analysis-results.js:
+ (AnalysisResults.prototype.containsTest): Added.
+ * public/v3/pages/analysis-task-page.js:
+ (AnalysisTaskTestGroupPane.prototype.setAnalysisResults): Takes a metric to pass it to the results viewer.
+ (AnalysisTaskPage.prototype._assignTestResultsIfPossible):
+
2017-07-06 Ryosuke Niwa <[email protected]>
Safari 10.1 fails to upload a patch on perf try bots page
Modified: trunk/Websites/perf.webkit.org/public/v3/components/test-group-results-viewer.js (219320 => 219321)
--- trunk/Websites/perf.webkit.org/public/v3/components/test-group-results-viewer.js 2017-07-11 03:40:59 UTC (rev 219320)
+++ trunk/Websites/perf.webkit.org/public/v3/components/test-group-results-viewer.js 2017-07-11 03:45:00 UTC (rev 219321)
@@ -24,6 +24,12 @@
{
this._analysisResults = analysisResults;
this._currentMetric = metric;
+ if (metric) {
+ const path = metric.test().path();
+ for (let i = path.length - 2; i >= 0; i--)
+ this._expandedTests.add(path[i]);
+ }
+
this.enqueueToRender();
}
@@ -58,6 +64,9 @@
_buildRowsForTest(testGroup, expandedTests, test, sharedPath, maxDepth, depth)
{
+ if (!this._analysisResults.containsTest(test))
+ return [];
+
const element = ComponentBase.createElement;
const rows = element('tbody', test.metrics().map((metric) => this._buildRowForMetric(testGroup, metric, sharedPath, maxDepth, depth)));
Modified: trunk/Websites/perf.webkit.org/public/v3/models/analysis-results.js (219320 => 219321)
--- trunk/Websites/perf.webkit.org/public/v3/models/analysis-results.js 2017-07-11 03:40:59 UTC (rev 219320)
+++ trunk/Websites/perf.webkit.org/public/v3/models/analysis-results.js 2017-07-11 03:45:00 UTC (rev 219321)
@@ -17,6 +17,16 @@
highestTests() { return this._lazilyComputedHighestTests.evaluate(this._metricIds); }
+ containsTest(test)
+ {
+ console.assert(test instanceof Test);
+ for (let metric of test.metrics()) {
+ if (metric.id() in this._metricToBuildMap)
+ return true;
+ }
+ return false;
+ }
+
_computeHighestTests(metricIds)
{
const testsInResults = new Set(metricIds.map((metricId) => Metric.findById(metricId).test()));
Modified: trunk/Websites/perf.webkit.org/public/v3/pages/analysis-task-page.js (219320 => 219321)
--- trunk/Websites/perf.webkit.org/public/v3/pages/analysis-task-page.js 2017-07-11 03:40:59 UTC (rev 219320)
+++ trunk/Websites/perf.webkit.org/public/v3/pages/analysis-task-page.js 2017-07-11 03:45:00 UTC (rev 219321)
@@ -280,10 +280,10 @@
this.enqueueToRender();
}
- setAnalysisResults(analysisResults)
+ setAnalysisResults(analysisResults, metric)
{
this.part('revision-table').setAnalysisResults(analysisResults);
- this.part('results-viewer').setAnalysisResults(analysisResults, null);
+ this.part('results-viewer').setAnalysisResults(analysisResults, metric);
this.enqueueToRender();
}
@@ -613,8 +613,8 @@
if (!this._task || !this._testGroups || !this._analysisResults)
return false;
- this.part('group-pane').setAnalysisResults(this._analysisResults);
let metric = this._metric;
+ this.part('group-pane').setAnalysisResults(this._analysisResults, metric);
if (metric) {
const view = this._analysisResults.viewForMetric(metric);
this.part('results-pane').setAnalysisResultsView(view);
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
