Attachment 358119 Details for Bug 193047 – Patch

[patch] Patch

bug-193047-20181228032042.patch (text/plain), 18.87 KB, created by dewei_zhu on 2018-12-28 03:20:43 PST

(hide)

Description:

Filename:

MIME Type:

Creator: dewei_zhu

Created: 2018-12-28 03:20:43 PST

Size: 18.87 KB

patch

obsolete

>Subversion Revision: 239556
>diff --git a/Websites/perf.webkit.org/ChangeLog b/Websites/perf.webkit.org/ChangeLog
>index 0936909761a24f17dcf3aa9b7a42ee14d24be505..de38096c6bd6b0e7bef5de084c829f2d48f20f60 100644
>--- a/Websites/perf.webkit.org/ChangeLog
>+++ b/Websites/perf.webkit.org/ChangeLog
>@@ -1,3 +1,28 @@
>+2018-12-28  Dewei Zhu  <dewei_zhu@apple.com>
>+
>+        User should be able to add build request manually.
>+        https://bugs.webkit.org/show_bug.cgi?id=193047
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Added UI for user to add extra build requests on analysis task page.  
>+
>+        * public/privileged-api/add-build-requests.php: Extended API to allow update 'needs_notification' flag.
>+        * public/v3/models/test-group.js:
>+        (TestGroup.prototype.async.addMoreBuildRequests): Updated this function to allow passing 'needsNotification'
>+        flag if specified.
>+        (TestGroup.prototype.async.minBuildRequestsToFulfillInitialRequest): Added this function to calculate
>+        minimum required build requests to fulfill initial requested iterations. This is used for providing sugguested
>+        count on the UI.
>+        * public/v3/pages/analysis-task-page.js: Added UI to add build requests.
>+        (AnalysisTaskTestGroupPane.prototype.didConstructShadowTree):
>+        (AnalysisTaskTestGroupPane.prototype._renderCurrentTestGroup):
>+        (AnalysisTaskTestGroupPane.cssTemplate):
>+        (AnalysisTaskPage.prototype.didConstructShadowTree):
>+        (AnalysisTaskPage.prototype.async._addBuildRequestForCurrentTestGroup):
>+        * server-tests/privileged-api-add-build-requests-tests.js: Added a unit test.
>+        * unit-tests/test-groups-tests.js: Added unit tests.
>+
> 2018-12-14  Dewei Zhu  <dewei_zhu@apple.com>
> 
>         Extend commits table to contain testability information.
>diff --git a/Websites/perf.webkit.org/public/privileged-api/add-build-requests.php b/Websites/perf.webkit.org/public/privileged-api/add-build-requests.php
>index d1c205eaf86ec18f06b34f2b7139c2356ee731fc..f69860f2981523ac9c4ac706eba6fac7eb109f43 100644
>--- a/Websites/perf.webkit.org/public/privileged-api/add-build-requests.php
>+++ b/Websites/perf.webkit.org/public/privileged-api/add-build-requests.php
>@@ -41,6 +41,12 @@ function main() {
>     }
> 
>     $db->begin_transaction();
>+
>+    if (array_key_exists('needsNotification', $data)) {
>+        $db->update_row('analysis_test_groups', 'testgroup', array('id' => $test_group_id),
>+            array('needs_notification' => Database::to_database_boolean($data['needsNotification'])));
>+    }
>+
>     for ($i = 0; $i < $additional_build_request_count; $i++) {
>         foreach ($commit_sets as $commit_set) {
>             $build_request = $build_request_by_commit_set[$commit_set];
>diff --git a/Websites/perf.webkit.org/public/v3/models/test-group.js b/Websites/perf.webkit.org/public/v3/models/test-group.js
>index 97e8bc8ee3cda9a6f641623534e67fb80039982b..54b049a97fc5734ea64a76dad1387c09bdd11cf9 100644
>--- a/Websites/perf.webkit.org/public/v3/models/test-group.js
>+++ b/Websites/perf.webkit.org/public/v3/models/test-group.js
>@@ -140,6 +140,19 @@ class TestGroup extends LabeledObject {
>         return this._buildRequests.some(function (request) { return request.isPending(); });
>     }
> 
>+    minBuildRequestsToFulfillInitialRequest()
>+    {
>+        let minRequiredBuildRequests = 0;
>+        for (const commitSet of this.requestedCommitSets()) {
>+            const buildRequests = this.requestsForCommitSet(commitSet).filter((request) => request.isTest());
>+            const completed = buildRequests.filter((request) => request.hasCompleted()).length;
>+            const unfinished = buildRequests.filter((request) => !request.hasFinished()).length;
>+            const requiredBuildRequests = this.initialRepetitionCount() - completed - unfinished;
>+            minRequiredBuildRequests = Math.max(minRequiredBuildRequests, requiredBuildRequests);
>+        }
>+        return minRequiredBuildRequests;
>+    }
>+
>     compareTestResults(metric, beforeMeasurements, afterMeasurements)
>     {
>         console.assert(metric);
>@@ -228,12 +241,17 @@ class TestGroup extends LabeledObject {
>         });
>     }
> 
>-    async addMoreBuildRequests(addCount)
>+    async addMoreBuildRequests(addCount, needsNotification)
>     {
>-        return await this._updateBuildRequest({
>+        const payload = {
>             group: this.id(),
>             addCount,
>-        }, 'add-build-requests');
>+        };
>+
>+        if (typeof(needsNotification) !== 'undefined')
>+            payload['needsNotification'] = !!needsNotification;
>+
>+        return await this._updateBuildRequest(payload, 'add-build-requests');
>     }
> 
>     async clearMayNeedMoreBuildRequests()
>diff --git a/Websites/perf.webkit.org/public/v3/pages/analysis-task-page.js b/Websites/perf.webkit.org/public/v3/pages/analysis-task-page.js
>index 8280daa4b0ccd38fb282a2472ef698ebb003c5d7..c19be9667696fed916226bf1ebcf72a45aa6c7f5 100644
>--- a/Websites/perf.webkit.org/public/v3/pages/analysis-task-page.js
>+++ b/Websites/perf.webkit.org/public/v3/pages/analysis-task-page.js
>@@ -270,6 +270,9 @@ class AnalysisTaskTestGroupPane extends ComponentBase {
>         this.part('retry-form').listenToAction('startTesting', (repetitionCount, notifyOnCompletion) => {
>             this.dispatchAction('retryTestGroup', this._currentTestGroup, repetitionCount, notifyOnCompletion);
>         });
>+        this.part('add-build-request-form').listenToAction('startTesting', (repetitionCount, notifyOnCompletion) => {
>+            this.dispatchAction('addBuildRequest', this._currentTestGroup, repetitionCount, notifyOnCompletion);
>+        });
>         this.part('bisect-form').listenToAction('startTesting', (repetitionCount, notifyOnCompletion) => {
>             const bisectingCommitSet = this._bisectingCommitSetByTestGroup.get(this._currentTestGroup);
>             const [oneCommitSet, anotherCommitSet] = this._currentTestGroup.requestedCommitSets();
>@@ -371,11 +374,13 @@ class AnalysisTaskTestGroupPane extends ComponentBase {
>         if (currentGroup) {
>             this.part('retry-form').setRepetitionCount(currentGroup.initialRepetitionCount());
>             this.part('bisect-form').setRepetitionCount(currentGroup.initialRepetitionCount());
>+            this.part('add-build-request-form').setRepetitionCount(Math.max(currentGroup.minBuildRequestsToFulfillInitialRequest(), 1));
>             const summary = `${currentGroup.initialRepetitionCount()} requested, ${currentGroup.repetitionCount() - currentGroup.initialRepetitionCount()} added due to failures.`;
>             this.content('status-summary').innerHTML = summary;
>         }
>         this.content('retry-form').style.display = currentGroup ? null : 'none';
>         this.content('bisect-form').style.display = currentGroup && this._bisectingCommitSetByTestGroup.get(currentGroup) ? null : 'none';
>+        this.content('add-build-request-form').style.display = currentGroup ? null : 'none';
>         this.content('status-summary').style.display = currentGroup && currentGroup.repetitionCount() > currentGroup.initialRepetitionCount() ? null : 'none';
> 
>         const hideButton = this.content('hide-button');
>@@ -395,6 +400,7 @@ class AnalysisTaskTestGroupPane extends ComponentBase {
>                 <div id="status-summary" class="summary"></div>
>                 <test-group-form id="retry-form">Retry</test-group-form>
>                 <test-group-form id="bisect-form">Bisect</test-group-form>
>+                <test-group-form id="add-build-request-form">Add</test-group-form>
>                 <button id="hide-button">Hide</button>
>                 <span id="pending-request-cancel-warning">(cancels pending requests)</span>
>             </div>`;
>@@ -466,7 +472,7 @@ class AnalysisTaskTestGroupPane extends ComponentBase {
>                 margin: 0;
>             }
> 
>-            #retry-form, #bisect-form {
>+            #retry-form, #bisect-form, #add-build-request-form {
>                 display: block;
>                 margin: 0.5rem;
>             }
>@@ -552,7 +558,8 @@ class AnalysisTaskPage extends PageWithHeading {
>         groupPane.listenToAction('renameTestGroup', (testGroup, newName) => this._updateTestGroupName(testGroup, newName));
>         groupPane.listenToAction('toggleTestGroupVisibility', (testGroup) => this._hideCurrentTestGroup(testGroup));
>         groupPane.listenToAction('retryTestGroup', (testGroup, repetitionCount, notifyOnCompletion) => this._retryCurrentTestGroup(testGroup, repetitionCount, notifyOnCompletion));
>-        groupPane.listenToAction('bisectTestGroup', (testGroup, commitSets, repetitionCount, notifyOnCompletion) => this._bisectCurrentTestGroup(testGroup, commitSets, repetitionCount, notifyOnCompletion));
>+        groupPane.listenToAction('bisectTestGroup', (testGroup, commitSets, repetitionCount, notifyOnCompletion) => this._bisectCurrentTestGroup(testGroup, commitSets, repetitionCount, notifyOnCompletion));groupPane.listenToAction('retryTestGroup', (testGroup, repetitionCount, notifyOnCompletion) => this._retryCurrentTestGroup(testGroup, repetitionCount, notifyOnCompletion));
>+        groupPane.listenToAction('addBuildRequest', (testGroup, repetitionCount, notifyOnCompletion) => this._addBuildRequestForCurrentTestGroup(testGroup, repetitionCount, notifyOnCompletion));
> 
>         this.part('cause-list').listenToAction('addItem', (repository, revision) => {
>             this._associateCommit('cause', repository, revision);
>@@ -858,6 +865,17 @@ class AnalysisTaskPage extends PageWithHeading {
>         }
>     }
> 
>+    async _addBuildRequestForCurrentTestGroup(testGroup, repetitionCount, notifyOnCompletion)
>+    {
>+        try {
>+            await testGroup.addMoreBuildRequests(repetitionCount, notifyOnCompletion);
>+            const analysisResults = await AnalysisResults.fetch(testGroup.task().id());
>+            await this._didFetchAnalysisResults(analysisResults);
>+        } catch(error) {
>+            alert('Failed to add build requests: ' + error);
>+        }
>+    }
>+
>     _createTestGroupAfterVerifyingCommitSetList(testGroupName, repetitionCount, commitSetMap, notifyOnCompletion)
>     {
>         if (this._hasDuplicateTestGroupName(testGroupName)) {
>diff --git a/Websites/perf.webkit.org/server-tests/privileged-api-add-build-requests-tests.js b/Websites/perf.webkit.org/server-tests/privileged-api-add-build-requests-tests.js
>index 9440477e9129bd79d5e4f06ebae7526b1e3cb5a8..70e2559b795f52b06d0e9452e39326df20dd8fe9 100644
>--- a/Websites/perf.webkit.org/server-tests/privileged-api-add-build-requests-tests.js
>+++ b/Websites/perf.webkit.org/server-tests/privileged-api-add-build-requests-tests.js
>@@ -180,4 +180,25 @@ describe('/privileged-api/add-build-requests', function() {
> 
>         await assertThrows('CannotAddToHiddenTestGroup', async () => await PrivilegedAPI.sendRequest('add-build-requests', {group: insertedGroupId, addCount: 2}))
>     });
>+
>+    it('should be able to add build request and set "needs_notification" flag', async () => {
>+        await addTriggerableAndCreateTask('some task');
>+        const webkit = Repository.all().filter((repository) => repository.name() == 'WebKit')[0];
>+        const revisionSets = [{[webkit.id()]: {revision: '191622'}}, {[webkit.id()]: {revision: '191623'}}];
>+        let result = await PrivilegedAPI.sendRequest('create-test-group',
>+            {name: 'test', taskName: 'other task', platform: MockData.somePlatformId(), test: MockData.someTestId(), needsNotification: false, repetitionCount: 2, revisionSets});
>+        const insertedGroupId = result['testGroupId'];
>+
>+        await PrivilegedAPI.sendRequest('add-build-requests', {group: insertedGroupId, addCount: 2, needsNotification: true});
>+
>+        const updatedGroups = await TestGroup.fetchForTask(result['taskId'], true);
>+        assert.equal(updatedGroups.length, 1);
>+        assert.equal(updatedGroups[0].repetitionCount(), 4);
>+        assert.equal(updatedGroups[0].initialRepetitionCount(), 2);
>+        assert.equal(updatedGroups[0].needsNotification(), true);
>+        for (const commitSet of updatedGroups[0].requestedCommitSets()) {
>+            const buildRequests = updatedGroups[0].requestsForCommitSet(commitSet);
>+            assert.equal(buildRequests.length, 4);
>+        }
>+    });
> });
>\ No newline at end of file
>diff --git a/Websites/perf.webkit.org/unit-tests/test-groups-tests.js b/Websites/perf.webkit.org/unit-tests/test-groups-tests.js
>index aa43554be8f96ab3300d5373a1489046a6917879..7d3aa35f8c0e527d0b1221b392c3c985f09a7fd5 100644
>--- a/Websites/perf.webkit.org/unit-tests/test-groups-tests.js
>+++ b/Websites/perf.webkit.org/unit-tests/test-groups-tests.js
>@@ -7,7 +7,8 @@ const NodePrivilegedAPI = require('../tools/js/privileged-api.js').PrivilegedAPI
> const MockModels = require('./resources/mock-v3-models.js').MockModels;
> const MockRemoteAPI = require('./resources/mock-remote-api.js').MockRemoteAPI;
> 
>-function sampleTestGroup(needsNotification=true, initialRepetitionCount=2, mayNeedMoreRequests=true) {
>+function sampleTestGroup(needsNotification=true, initialRepetitionCount=2, mayNeedMoreRequests=true,
>+    buildRequestsStatus=["pending", "pending", "pending", "pending"]) {
>     return {
>         "testGroups": [{
>             "id": "2128",
>@@ -32,7 +33,7 @@ function sampleTestGroup(needsNotification=true, initialRepetitionCount=2, mayNe
>             "testGroup": "2128",
>             "order": "0",
>             "commitSet": "4255",
>-            "status": "pending",
>+            "status": buildRequestsStatus[0],
>             "url": null,
>             "build": null,
>             "createdAt": 1458688514000
>@@ -44,7 +45,7 @@ function sampleTestGroup(needsNotification=true, initialRepetitionCount=2, mayNe
>             "testGroup": "2128",
>             "order": "1",
>             "commitSet": "4256",
>-            "status": "pending",
>+            "status": buildRequestsStatus[1],
>             "url": null,
>             "build": null,
>             "createdAt": 1458688514000
>@@ -57,7 +58,7 @@ function sampleTestGroup(needsNotification=true, initialRepetitionCount=2, mayNe
>             "testGroup": "2128",
>             "order": "2",
>             "commitSet": "4255",
>-            "status": "pending",
>+            "status": buildRequestsStatus[2],
>             "url": null,
>             "build": null,
>             "createdAt": 1458688514000
>@@ -69,7 +70,7 @@ function sampleTestGroup(needsNotification=true, initialRepetitionCount=2, mayNe
>             "testGroup": "2128",
>             "order": "3",
>             "commitSet": "4256",
>-            "status": "pending",
>+            "status": buildRequestsStatus[3],
>             "url": null,
>             "build": null,
>             "createdAt": 1458688514000
>@@ -237,6 +238,90 @@ describe('TestGroup', function () {
>         });
>     });
> 
>+    describe('addMoreBuildRequests', () => {
>+        const requests = MockRemoteAPI.inject('https://perf.webkit.org', NodePrivilegedAPI);
>+        beforeEach(() => {
>+            PrivilegedAPI.configure('test', 'password');
>+        });
>+
>+        it('should be able to add build request without modifying "need_notification" flag', async () => {
>+            const fetchPromise = TestGroup.fetchForTask(1376);
>+            requests[0].resolve(sampleTestGroup());
>+            const testGroups = await fetchPromise;
>+            assert(testGroups.length, 1);
>+            const testGroup = testGroups[0];
>+
>+            testGroup.addMoreBuildRequests(1);
>+            assert.equal(requests.length, 2);
>+            assert.equal(requests.length, 2);
>+            assert.equal(requests[1].method, 'POST');
>+            assert.equal(requests[1].url, '/privileged-api/add-build-requests');
>+            assert.deepEqual(requests[1].data, {group: '2128', addCount: 1, slaveName: 'test', slavePassword: 'password'});
>+        });
>+
>+        it('should be able to add build request and modify "need_notification" flag', async () => {
>+            const fetchPromise = TestGroup.fetchForTask(1376);
>+            requests[0].resolve(sampleTestGroup());
>+            const testGroups = await fetchPromise;
>+            assert(testGroups.length, 1);
>+            const testGroup = testGroups[0];
>+
>+            testGroup.addMoreBuildRequests(1, true);
>+            assert.equal(requests.length, 2);
>+            assert.equal(requests.length, 2);
>+            assert.equal(requests[1].method, 'POST');
>+            assert.equal(requests[1].url, '/privileged-api/add-build-requests');
>+            assert.deepEqual(requests[1].data, {group: '2128', addCount: 1, needsNotification: true, slaveName: 'test', slavePassword: 'password'});
>+        });
>+    });
>+
>+    describe('minBuildRequestsToFulfillInitialRequest', () => {
>+        const requests = MockRemoteAPI.inject('https://perf.webkit.org', NodePrivilegedAPI);
>+        beforeEach(() => {
>+            PrivilegedAPI.configure('test', 'password');
>+        });
>+
>+        it('should return 0 if all build requests are pending', async () => {
>+            const fetchPromise = TestGroup.fetchForTask(1376);
>+            requests[0].resolve(sampleTestGroup());
>+            const testGroups = await fetchPromise;
>+            assert(testGroups.length, 1);
>+            const testGroup = testGroups[0];
>+
>+            assert.equal(testGroup.minBuildRequestsToFulfillInitialRequest(), 0);
>+        });
>+
>+        it('should return maximum required count among commit sets', async () => {
>+            const fetchPromise = TestGroup.fetchForTask(1376);
>+            requests[0].resolve(sampleTestGroup(true, 2, true, ["failed", "failed", "completed", "failed"]));
>+            const testGroups = await fetchPromise;
>+            assert(testGroups.length, 1);
>+            const testGroup = testGroups[0];
>+
>+            assert.equal(testGroup.minBuildRequestsToFulfillInitialRequest(), 2);
>+        });
>+
>+        it('should return 0 if all finished build requests are completed and remaining are pending', async () => {
>+            const fetchPromise = TestGroup.fetchForTask(1376);
>+            requests[0].resolve(sampleTestGroup(true, 2, true, ["completed", "pending", "pending", "pending"]));
>+            const testGroups = await fetchPromise;
>+            assert(testGroups.length, 1);
>+            const testGroup = testGroups[0];
>+
>+            assert.equal(testGroup.minBuildRequestsToFulfillInitialRequest(), 0);
>+        });
>+
>+        it('should retry canceled ones', async () => {
>+            const fetchPromise = TestGroup.fetchForTask(1376);
>+            requests[0].resolve(sampleTestGroup(true, 2, true, ["completed", "canceled", "canceled", "canceled"]));
>+            const testGroups = await fetchPromise;
>+            assert(testGroups.length, 1);
>+            const testGroup = testGroups[0];
>+
>+            assert.equal(testGroup.minBuildRequestsToFulfillInitialRequest(), 2);
>+        });
>+    });
>+
>     describe('_createModelsFromFetchedTestGroups', function () {
>         it('should create test groups', function () {
>             var groups = TestGroup._createModelsFromFetchedTestGroups(sampleTestGroup());
>@@ -487,5 +572,4 @@ describe('TestGroup', function () {
>             assert.equal(requests[3].url, '/api/analysis-tasks?id=123');
>         });
>     })
>-
> });
>\ No newline at end of file

Flags:

rniwa: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 193047: 358119