Perf dashboard should let user cancel pending A/B testing and hide failed ones
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 19 Feb 2016 04:18:27 +0000 (04:18 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 19 Feb 2016 04:18:27 +0000 (04:18 +0000)
https://bugs.webkit.org/show_bug.cgi?id=154433

Reviewed by Chris Dumez.

Added a button to hide a test group in the details view (the bottom table) in the analysis task page, and
"Show hidden tests" link to show the hidden test groups on demand. When a test group is hidden, all pending
requests in the group will also be canceled since a common scenario of using this feature is that the user
had triggered an useless A/B testing; e.g. all builds will fail, wrong, etc... We can revisit and add the
capability to just cancel the pending requests and leaving the group visible later if necessary.

Run `ALTER TYPE build_request_status_type ADD VALUE 'canceled';` to add the new type.

* init-database.sql: Added testgroup_hidden column to analysis_test_groups table and added 'canceled'
as a value to build_request_status_type table.
* public/api/test-groups.php:
(format_test_group): Added 'hidden' field in the JSON result.
* public/privileged-api/update-test-group.php:
(main): Added the support for updating testgroup_hidden column. When this column is set to true, also
cancel all pending build requests (by setting its request_status to 'canceled' which will be ignore by
the syncing script).
* public/v3/components/test-group-results-table.js:
(TestGroupResultsTable.prototype.setTestGroup): Reset _renderedTestGroup here so that the next call to
render() will update the table; e.g. when build requests' status change from 'Pending' to 'Canceled'.
* public/v3/models/build-request.js:
(BuildRequest.prototype.hasCompleted): A build request is considered complete/finished if it's canceled.
(BuildRequest.prototype.hasPending): Added.
(BuildRequest.prototype.statusLabel): Handle 'canceled' status.
* public/v3/models/test-group.js:
(TestGroup):
(TestGroup.prototype.updateSingleton): Added to update 'hidden' field.
(TestGroup.prototype.isHidden): Added.
(TestGroup.prototype.hasPending): Added.
(TestGroup.prototype.hasPending): Added.
(TestGroup.prototype.updateHiddenFlag): Added. Uses the privileged API to update testgroup_hidden column.
The JSON API also updates the status of the 'pending' build requests in the group to 'canceled'.
* public/v3/pages/analysis-task-page.js:
(AnalysisTaskPage): Added _showHiddenTestGroups and _filteredTestGroups as instance variables.
(AnalysisTaskPage.prototype._didFetchTestGroups):
(AnalysisTaskPage.prototype._showAllTestGroups): Added.
(AnalysisTaskPage.prototype._didUpdateTestGroupHiddenState): Extracted from _didFetchTestGroups.
(AnalysisTaskPage.prototype._renderTestGroupList): Use the filtered list of test groups to show the list
of test groups. When all test groups are shown, we would first show the hidden ones after the regular ones.
(AnalysisTaskPage.prototype._createTestGroupListItem): Extracted from _renderTestGroupList.
(AnalysisTaskPage.prototype._renderTestGroupDetails): Update the text inside the button to hide the test
group. Also show a warning text that the pending requests will be canceled if there are any.
(AnalysisTaskPage.prototype._hideCurrentTestGroup): Added.
(AnalysisTaskPage.cssTemplate): Updated the style.

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

Websites/perf.webkit.org/ChangeLog
Websites/perf.webkit.org/init-database.sql
Websites/perf.webkit.org/public/api/test-groups.php
Websites/perf.webkit.org/public/privileged-api/update-test-group.php
Websites/perf.webkit.org/public/v3/components/test-group-results-table.js
Websites/perf.webkit.org/public/v3/models/build-request.js
Websites/perf.webkit.org/public/v3/models/test-group.js
Websites/perf.webkit.org/public/v3/pages/analysis-task-page.js

index 3f1468b..bc86011 100644 (file)
@@ -1,5 +1,56 @@
 2016-02-18  Ryosuke Niwa  <rniwa@webkit.org>
 
+        Perf dashboard should let user cancel pending A/B testing and hide failed ones
+        https://bugs.webkit.org/show_bug.cgi?id=154433
+
+        Reviewed by Chris Dumez.
+
+        Added a button to hide a test group in the details view (the bottom table) in the analysis task page, and
+        "Show hidden tests" link to show the hidden test groups on demand. When a test group is hidden, all pending
+        requests in the group will also be canceled since a common scenario of using this feature is that the user
+        had triggered an useless A/B testing; e.g. all builds will fail, wrong, etc... We can revisit and add the
+        capability to just cancel the pending requests and leaving the group visible later if necessary.
+
+        Run `ALTER TYPE build_request_status_type ADD VALUE 'canceled';` to add the new type.
+
+        * init-database.sql: Added testgroup_hidden column to analysis_test_groups table and added 'canceled'
+        as a value to build_request_status_type table.
+        * public/api/test-groups.php:
+        (format_test_group): Added 'hidden' field in the JSON result.
+        * public/privileged-api/update-test-group.php:
+        (main): Added the support for updating testgroup_hidden column. When this column is set to true, also
+        cancel all pending build requests (by setting its request_status to 'canceled' which will be ignore by
+        the syncing script).
+        * public/v3/components/test-group-results-table.js:
+        (TestGroupResultsTable.prototype.setTestGroup): Reset _renderedTestGroup here so that the next call to
+        render() will update the table; e.g. when build requests' status change from 'Pending' to 'Canceled'.
+        * public/v3/models/build-request.js:
+        (BuildRequest.prototype.hasCompleted): A build request is considered complete/finished if it's canceled.
+        (BuildRequest.prototype.hasPending): Added.
+        (BuildRequest.prototype.statusLabel): Handle 'canceled' status.
+        * public/v3/models/test-group.js:
+        (TestGroup):
+        (TestGroup.prototype.updateSingleton): Added to update 'hidden' field.
+        (TestGroup.prototype.isHidden): Added.
+        (TestGroup.prototype.hasPending): Added.
+        (TestGroup.prototype.hasPending): Added.
+        (TestGroup.prototype.updateHiddenFlag): Added. Uses the privileged API to update testgroup_hidden column.
+        The JSON API also updates the status of the 'pending' build requests in the group to 'canceled'.
+        * public/v3/pages/analysis-task-page.js:
+        (AnalysisTaskPage): Added _showHiddenTestGroups and _filteredTestGroups as instance variables.
+        (AnalysisTaskPage.prototype._didFetchTestGroups):
+        (AnalysisTaskPage.prototype._showAllTestGroups): Added.
+        (AnalysisTaskPage.prototype._didUpdateTestGroupHiddenState): Extracted from _didFetchTestGroups.
+        (AnalysisTaskPage.prototype._renderTestGroupList): Use the filtered list of test groups to show the list
+        of test groups. When all test groups are shown, we would first show the hidden ones after the regular ones.
+        (AnalysisTaskPage.prototype._createTestGroupListItem): Extracted from _renderTestGroupList.
+        (AnalysisTaskPage.prototype._renderTestGroupDetails): Update the text inside the button to hide the test
+        group. Also show a warning text that the pending requests will be canceled if there are any.
+        (AnalysisTaskPage.prototype._hideCurrentTestGroup): Added.
+        (AnalysisTaskPage.cssTemplate): Updated the style.
+
+2016-02-18  Ryosuke Niwa  <rniwa@webkit.org>
+
         The rows in the analysis results table should be expandable
         https://bugs.webkit.org/show_bug.cgi?id=154427
 
index 22858f1..4ba437e 100644 (file)
@@ -225,10 +225,11 @@ CREATE TABLE triggerable_configurations (
 
 CREATE TABLE analysis_test_groups (
     testgroup_id serial PRIMARY KEY,
-    testgroup_task integer REFERENCES analysis_tasks NOT NULL,
+    testgroup_task integer NOT NULL REFERENCES analysis_tasks ON DELETE CASCADE,
     testgroup_name varchar(256),
     testgroup_author varchar(256),
     testgroup_created_at timestamp NOT NULL DEFAULT (CURRENT_TIMESTAMP AT TIME ZONE 'UTC'),
+    testgroup_hidden boolean NOT NULL DEFAULT FALSE,
     CONSTRAINT testgroup_name_must_be_unique_for_each_task UNIQUE(testgroup_task, testgroup_name));
 CREATE INDEX testgroup_task_index ON analysis_test_groups(testgroup_task);
 
@@ -239,13 +240,13 @@ CREATE TABLE roots (
     root_set integer REFERENCES root_sets NOT NULL,
     root_commit integer REFERENCES commits NOT NULL);
 
-CREATE TYPE build_request_status_type as ENUM ('pending', 'scheduled', 'running', 'failed', 'completed');
+CREATE TYPE build_request_status_type as ENUM ('pending', 'scheduled', 'running', 'failed', 'completed', 'canceled');
 CREATE TABLE build_requests (
     request_id serial PRIMARY KEY,
     request_triggerable integer REFERENCES build_triggerables NOT NULL,
     request_platform integer REFERENCES platforms NOT NULL,
     request_test integer REFERENCES tests NOT NULL,
-    request_group integer REFERENCES analysis_test_groups NOT NULL,
+    request_group integer NOT NULL REFERENCES analysis_test_groups ON DELETE CASCADE,
     request_order integer NOT NULL,
     request_root_set integer REFERENCES root_sets NOT NULL,
     request_status build_request_status_type NOT NULL DEFAULT 'pending',
index 752e629..7dcabc0 100644 (file)
@@ -65,6 +65,7 @@ function format_test_group($group_row) {
         'name' => $group_row['testgroup_name'],
         'author' => $group_row['testgroup_author'],
         'createdAt' => strtotime($group_row['testgroup_created_at']) * 1000,
+        'hidden' => Database::is_true($group_row['testgroup_hidden']),
         'buildRequests' => array(),
         'rootSets' => array(),
     );
index 19a9928..db1ed38 100644 (file)
@@ -14,6 +14,9 @@ function main() {
     if (array_key_exists('name', $data))
         $values['name'] = $data['name'];
 
+    if (array_key_exists('hidden', $data))
+        $values['hidden'] = Database::to_database_boolean($data['hidden']);
+
     if (!$values)
         exit_with_error('NothingToUpdate');
 
@@ -25,6 +28,11 @@ function main() {
         exit_with_error('FailedToUpdateTestGroup', array('id' => $test_group_id, 'values' => $values));
     }
 
+    if (array_get($data, 'hidden')) {
+        $db->query_and_get_affected_rows('UPDATE build_requests SET request_status = $1
+            WHERE request_group = $2 AND request_status = $3', array('canceled', $test_group_id, 'pending'));
+    }
+
     $db->commit_transaction();
 
     exit_with_success();
index dfbf3fa..da62195 100644 (file)
@@ -8,7 +8,11 @@ class TestGroupResultsTable extends ResultsTable {
     }
 
     didUpdateResults() { this._renderedTestGroup = null; }
-    setTestGroup(testGroup) { this._testGroup = testGroup; }
+    setTestGroup(testGroup)
+    {
+        this._testGroup = testGroup;
+        this._renderedTestGroup = null;
+    }
 
     heading()
     {
index a374b1d..6f5692b 100644 (file)
@@ -30,8 +30,9 @@ class BuildRequest extends DataModelObject {
     order() { return this._order; }
     rootSet() { return this._rootSet; }
 
-    hasCompleted() { return this._status == 'failed' || this._status == 'completed'; }
+    hasCompleted() { return this._status == 'failed' || this._status == 'completed' || this._status == 'canceled'; }
     hasStarted() { return this._status != 'pending'; }
+    hasPending() { return this._status == 'pending'; }
     statusLabel()
     {
         switch (this._status) {
@@ -45,6 +46,8 @@ class BuildRequest extends DataModelObject {
             return 'Failed';
         case 'completed':
             return 'Completed';
+        case 'canceled':
+            return 'Canceled';
         }
     }
     statusUrl() { return this._statusUrl; }
index 8773d72..f32143a 100644 (file)
@@ -7,6 +7,7 @@ class TestGroup extends LabeledObject {
         this._taskId = object.task;
         this._authorName = object.author;
         this._createdAt = new Date(object.createdAt);
+        this._isHidden = object.hidden;
         this._buildRequests = [];
         this._requestsAreInOrder = false;
         this._repositories = null;
@@ -17,7 +18,19 @@ class TestGroup extends LabeledObject {
         this._platform = object.platform;
     }
 
+    updateSingleton(object)
+    {
+        super.updateSingleton(object);
+
+        console.assert(this._taskId == object.task);
+        console.assert(+this._createdAt == +object.createdAt);
+        console.assert(this._platform == object.platform);
+
+        this._isHidden = object.hidden;
+    }
+
     createdAt() { return this._createdAt; }
+    isHidden() { return this._isHidden; }
     buildRequests() { return this._buildRequests; }
     addBuildRequest(request)
     {
@@ -96,6 +109,11 @@ class TestGroup extends LabeledObject {
         return this._buildRequests.some(function (request) { return request.hasStarted(); });
     }
 
+    hasPending()
+    {
+        return this._buildRequests.some(function (request) { return request.hasPending(); });
+    }
+
     compareTestResults(rootSetA, rootSetB)
     {
         var beforeValues = this._valuesForRootSet(rootSetA);
@@ -165,6 +183,19 @@ class TestGroup extends LabeledObject {
         });
     }
 
+    updateHiddenFlag(hidden)
+    {
+        var self = this;
+        var id = this.id();
+        return PrivilegedAPI.sendRequest('update-test-group', {
+            group: id,
+            hidden: !!hidden,
+        }).then(function (data) {
+            return TestGroup.cachedFetch(`../api/test-groups/${id}`, {}, true)
+                .then(TestGroup._createModelsFromFetchedTestGroups.bind(TestGroup));
+        });
+    }
+
     static createAndRefetchTestGroups(task, name, repetitionCount, rootSets)
     {
         var self = this;
index e457f72..136c4d6 100644 (file)
@@ -44,6 +44,8 @@ class AnalysisTaskPage extends PageWithHeading {
         this._endPoint = null;
         this._errorMessage = null;
         this._currentTestGroup = null;
+        this._filteredTestGroups = null;
+        this._showHiddenTestGroups = false;
 
         this._chartPane = this.content().querySelector('analysis-task-chart-pane').component();
         this._chartPane.setPage(this);
@@ -69,8 +71,10 @@ class AnalysisTaskPage extends PageWithHeading {
         this._newTestGroupFormForViewer = this.content().querySelector('.analysis-results-view customizable-test-group-form').component();
         this._newTestGroupFormForViewer.setStartCallback(this._createNewTestGroupFromViewer.bind(this));
 
-        this._retryForm = this.content().querySelector('.test-group-retry-form').firstChild.component();
+        this._retryForm = this.content().querySelector('.test-group-retry-form test-group-form').component();
         this._retryForm.setStartCallback(this._retryCurrentTestGroup.bind(this));
+        this._hideButton = this.content().querySelector('.test-group-hide-button');
+        this._hideButton.onclick = this._hideCurrentTestGroup.bind(this);
     }
 
     title() { return this._task ? this._task.label() : 'Analysis Task'; }
@@ -157,14 +161,31 @@ class AnalysisTaskPage extends PageWithHeading {
     _didFetchTestGroups(testGroups)
     {
         this._testGroups = testGroups.sort(function (a, b) { return +a.createdAt() - b.createdAt(); });
-        this._currentTestGroup = testGroups.length ? testGroups[testGroups.length - 1] : null;
-
-        this._analysisResultsViewer.setTestGroups(testGroups);
-        this._testGroupResultsTable.setTestGroup(this._currentTestGroup);
+        this._didUpdateTestGroupHiddenState();
         this._assignTestResultsIfPossible();
         this.render();
     }
 
+    _showAllTestGroups()
+    {
+        this._showHiddenTestGroups = true;
+        this._didUpdateTestGroupHiddenState();
+        this.render();
+    }
+
+    _didUpdateTestGroupHiddenState()
+    {
+        this._renderedCurrentTestGroup = null;
+        this._renderedTestGroups = null;
+        if (!this._showHiddenTestGroups)
+            this._filteredTestGroups = this._testGroups.filter(function (group) { return !group.isHidden(); });
+        else
+            this._filteredTestGroups = this._testGroups;
+        this._currentTestGroup = this._filteredTestGroups ? this._filteredTestGroups[this._filteredTestGroups.length - 1] : null;
+        this._analysisResultsViewer.setTestGroups(this._filteredTestGroups);
+        this._testGroupResultsTable.setTestGroup(this._currentTestGroup);
+    }
+
     _didFetchAnalysisResults(results)
     {
         this._analysisResults = results;
@@ -277,26 +298,27 @@ class AnalysisTaskPage extends PageWithHeading {
             this._renderedTestGroups = this._testGroups;
             this._testGroupLabelMap.clear();
 
-            var self = this;
-            var updateTestGroupName = this._updateTestGroupName.bind(this);
-            var showTestGroup = this._showTestGroup.bind(this);
+            var unhiddenTestGroups = this._filteredTestGroups.filter(function (group) { return !group.isHidden(); });
+            var hiddenTestGroups = this._filteredTestGroups.filter(function (group) { return group.isHidden(); });
+
+            var listItems = [];
+            for (var group of hiddenTestGroups)
+                listItems.unshift(this._createTestGroupListItem(group));
+            for (var group of unhiddenTestGroups)
+                listItems.unshift(this._createTestGroupListItem(group));
 
-            this.renderReplace(this.content().querySelector('.test-group-list'),
-                this._testGroups.map(function (group) {
-                    var text = new EditableText(group.label());
-                    text.setStartedEditingCallback(function () { return text.render(); });
-                    text.setUpdateCallback(function () { return updateTestGroupName(group); });
+            if (this._testGroups.length != this._filteredTestGroups.length) {
+                listItems.push(element('li', {class: 'test-group-list-show-all'},
+                    link('Show hidden tests', this._showAllTestGroups.bind(this))));
+            }
 
-                    self._testGroupLabelMap.set(group, text);
-                    return element('li', {class: 'test-group-list-' + group.id()},
-                        link(text, group.label(), function () { showTestGroup(group); }));
-                }).reverse());
+            this.renderReplace(this.content().querySelector('.test-group-list'), listItems);
 
             this._renderedCurrentTestGroup = null;
         }
 
         if (this._testGroups) {
-            for (var testGroup of this._testGroups) {
+            for (var testGroup of this._filteredTestGroups) {
                 var label = this._testGroupLabelMap.get(testGroup);
                 label.setText(testGroup.label());
                 label.render();
@@ -304,6 +326,17 @@ class AnalysisTaskPage extends PageWithHeading {
         }
     }
 
+    _createTestGroupListItem(group)
+    {
+        var text = new EditableText(group.label());
+        text.setStartedEditingCallback(function () { return text.render(); });
+        text.setUpdateCallback(this._updateTestGroupName.bind(this, group));
+
+        this._testGroupLabelMap.set(group, text);
+        return ComponentBase.createElement('li', {class: 'test-group-list-' + group.id()},
+            ComponentBase.createLink(text, group.label(), this._showTestGroup.bind(this, group)));
+    }
+
     _renderTestGroupDetails()
     {
         if (this._renderedCurrentTestGroup !== this._currentTestGroup) {
@@ -332,6 +365,12 @@ class AnalysisTaskPage extends PageWithHeading {
                 this._retryForm.setRepetitionCount(this._currentTestGroup.repetitionCount());
             this._retryForm.element().style.display = this._currentTestGroup ? null : 'none';
 
+            this.content().querySelector('.test-group-hide-button').textContent
+                = this._currentTestGroup && this._currentTestGroup.isHidden() ? 'Unhide' : 'Hide';
+
+            this.content().querySelector('.pending-request-cancel-warning').style.display
+                = this._currentTestGroup && this._currentTestGroup.hasPending() ? null : 'none';
+
             this._renderedCurrentTestGroup = this._currentTestGroup;
         }
         this._retryForm.render();
@@ -373,7 +412,21 @@ class AnalysisTaskPage extends PageWithHeading {
             self.render();
         }, function (error) {
             self.render();
-            alert('Failed to update the name: ' + error);
+            alert('Failed to hide the test name: ' + error);
+        });
+    }
+
+    _hideCurrentTestGroup()
+    {
+        var self = this;
+        console.assert(this._currentTestGroup);
+        return this._currentTestGroup.updateHiddenFlag(!this._currentTestGroup.isHidden()).then(function () {
+            self._didUpdateTestGroupHiddenState();
+            self.render();
+        }, function (error) {
+            self._mayHaveMutatedTestGroupHiddenState();
+            self.render();
+            alert('Failed to update the group: ' + error);
         });
     }
 
@@ -569,6 +622,8 @@ class AnalysisTaskPage extends PageWithHeading {
                     <div class="test-group-details">
                         <test-group-results-table></test-group-results-table>
                         <div class="test-group-retry-form"><test-group-form></test-group-form></div>
+                        <button class="test-group-hide-button">Hide</button>
+                        <span class="pending-request-cancel-warning">(cancels pending requests)</span>
                     </div>
                 </section>
             </div>
@@ -690,6 +745,10 @@ class AnalysisTaskPage extends PageWithHeading {
                 margin: 0.5rem;
             }
 
+            .test-group-hide-button {
+                margin: 0.5rem;
+            }
+
             .test-group-list {
                 display: table-cell;
                 margin: 0;
@@ -697,6 +756,7 @@ class AnalysisTaskPage extends PageWithHeading {
                 list-style: none;
                 border-right: solid 1px #ccc;
                 white-space: nowrap;
+                min-width: 8rem;
             }
 
             .test-group-list:empty {
@@ -707,16 +767,28 @@ class AnalysisTaskPage extends PageWithHeading {
 
             .test-group-list > li {
                 display: block;
+                font-size: 0.9rem;
             }
 
             .test-group-list > li > a {
                 display: block;
                 color: inherit;
                 text-decoration: none;
-                font-size: 0.9rem;
                 margin: 0;
                 padding: 0.2rem;
             }
+            
+            .test-group-list > li.test-group-list-show-all {
+                font-size: 0.8rem;
+                margin-top: 0.5rem;
+                padding-right: 1rem;
+                text-align: center;
+                color: #999;
+            }
+
+            .test-group-list > li.test-group-list-show-all:not(.selected) a:hover {
+                background: inherit;
+            }
 
             .test-group-list > li.selected > a {
                 background: rgba(204, 153, 51, 0.1);