Attachment 339359 Details for Bug 185222 – Patch

[patch] Patch

bug-185222-20180502153307.patch (text/plain), 12.73 KB, created by dewei_zhu on 2018-05-02 15:33:08 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: dewei_zhu

Created: 2018-05-02 15:33:08 PDT

Size: 12.73 KB

patch

obsolete

>Subversion Revision: 231271
>diff --git a/Websites/perf.webkit.org/ChangeLog b/Websites/perf.webkit.org/ChangeLog
>index 81b237eafd54da82f2d92b1a872510ca5d15c936..c6a3f643f08458d60adc041f6b1aeb4190816eb5 100644
>--- a/Websites/perf.webkit.org/ChangeLog
>+++ b/Websites/perf.webkit.org/ChangeLog
>@@ -1,3 +1,23 @@
>+2018-05-02  Dewei Zhu  <dewei_zhu@apple.com>
>+
>+        Create analysis task should sync analysis task status after creation.
>+        https://bugs.webkit.org/show_bug.cgi?id=185222
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * public/v3/models/analysis-task.js:
>+        (AnalysisTask.async.create): Fetch newly-created analysis task right about its creation.
>+        (AnalysisTask):
>+        * tools/js/measurement-set-analyzer.js:
>+        (MeasurementSetAnalyzer.prototype.async._analyzeMeasurementSet): Adjust as 'AnalysisTask.create' now
>+        returns an anlysis task instance.
>+        (MeasurementSetAnalyzer):
>+        * unit-tests/analysis-task-tests.js: Added a unit test for this change.
>+        (async):
>+        * unit-tests/measurement-set-analyzer-tests.js: Update unit tests accordingly.
>+        Fix a typo in one previous unit test.
>+        (async):
>+
> 2018-05-01  Dewei Zhu  <dewei_zhu@apple.com>
> 
>         Write a script that detects chart changes by using v3 API.
>diff --git a/Websites/perf.webkit.org/public/v3/models/analysis-task.js b/Websites/perf.webkit.org/public/v3/models/analysis-task.js
>index 159a25e7c056cc95975740f34298c8edc7026446..5db45242895e2c4a1e144b9fb34153b4a83319e9 100644
>--- a/Websites/perf.webkit.org/public/v3/models/analysis-task.js
>+++ b/Websites/perf.webkit.org/public/v3/models/analysis-task.js
>@@ -303,7 +303,7 @@ class AnalysisTask extends LabeledObject {
>         return results;
>     }
> 
>-    static create(name, startPoint, endPoint, testGroupName=null, repetitionCount=0)
>+    static async create(name, startPoint, endPoint, testGroupName=null, repetitionCount=0)
>     {
>         const parameters = {name, startRun: startPoint.id, endRun: endPoint.id};
>         if (testGroupName) {
>@@ -312,7 +312,8 @@ class AnalysisTask extends LabeledObject {
>             parameters['repetitionCount'] = repetitionCount;
>             parameters['testGroupName'] = testGroupName;
>         }
>-        return PrivilegedAPI.sendRequest('create-analysis-task', parameters);
>+        const response = await PrivilegedAPI.sendRequest('create-analysis-task', parameters);
>+        return AnalysisTask.fetchById(response.taskId, true);
>     }
> }
> 
>diff --git a/Websites/perf.webkit.org/tools/js/measurement-set-analyzer.js b/Websites/perf.webkit.org/tools/js/measurement-set-analyzer.js
>index b329aa7d8c37d7f0b97cb6bcd7a0c63d45169cc4..2db047a4bd148a13884dd4dffa6a4d78074de469 100644
>--- a/Websites/perf.webkit.org/tools/js/measurement-set-analyzer.js
>+++ b/Websites/perf.webkit.org/tools/js/measurement-set-analyzer.js
>@@ -103,9 +103,10 @@ class MeasurementSetAnalyzer {
>         const summary = `Potential ${rangeWithMostSignificantChange.valueChangeSummary.changeLabel} on ${platform.name()} between ${CommitSet.diff(startCommitSet, endCommitSet)}`;
> 
>         // FIXME: The iteration count should be smarter than hard-coding.
>-        const response = await AnalysisTask.create(summary, rangeWithMostSignificantChange.startPoint,
>+        const analysisTask = await AnalysisTask.create(summary, rangeWithMostSignificantChange.startPoint,
>             rangeWithMostSignificantChange.endPoint, 'Confirm', 4);
>-        this._logger.info(`Created analysis task with id "${response.taskId}" to confirm: "${summary}".`);
>+
>+        this._logger.info(`Created analysis task with id "${analysisTask.id()}" to confirm: "${summary}".`);
>     }
> }
> 
>diff --git a/Websites/perf.webkit.org/unit-tests/analysis-task-tests.js b/Websites/perf.webkit.org/unit-tests/analysis-task-tests.js
>index 109aee89a279bb554ce268827d24f8181e7b3b73..b2f0a340c5ff698f3bd7119fdab21fef4a97308a 100644
>--- a/Websites/perf.webkit.org/unit-tests/analysis-task-tests.js
>+++ b/Websites/perf.webkit.org/unit-tests/analysis-task-tests.js
>@@ -323,6 +323,67 @@ describe('AnalysisTask', () => {
>                         '22': { revision: 'ios-revision-2', ownerRevision: null, patch: null}}]}
>             );
>         });
>+
>+        it('should sync the new analysis task status onceit is created', async () => {
>+            const [startPoint, endPoint] = mockStartAndEndPoints();
>+            const creatingPromise = AnalysisTask.create('confirm', startPoint, endPoint, 'Confirm', 4);
>+            assert.equal(requests.length, 1);
>+            assert.equal(requests[0].url, '/privileged-api/generate-csrf-token');
>+            requests[0].resolve({
>+                token: 'abc',
>+                expiration: Date.now() + 3600 * 1000,
>+            });
>+
>+            await MockRemoteAPI.waitForRequest();
>+            assert.equal(requests[1].url, '/privileged-api/create-analysis-task');
>+            assert.equal(requests.length, 2);
>+            assert.deepEqual(requests[1].data, {name: 'confirm', repetitionCount: 4,
>+                startRun: 1, endRun: 2, testGroupName: 'Confirm', token: 'abc', revisionSets: [
>+                    {'11': {revision: 'webkit-revision-1', ownerRevision: null, patch: null},
>+                        '22': {revision: 'ios-revision-1', ownerRevision: null, patch: null}},
>+                    {'11': {revision: 'webkit-revision-2', ownerRevision: null, patch: null},
>+                        '22': { revision: 'ios-revision-2', ownerRevision: null, patch: null}}]}
>+            );
>+
>+            requests[1].resolve({taskId: '5255', status: 'OK'});
>+
>+            await MockRemoteAPI.waitForRequest();
>+            assert.equal(requests.length, 3);
>+            assert.equal(requests[2].url, '/api/analysis-tasks?id=5255');
>+            requests[2].resolve({
>+                analysisTasks: [{
>+                    author: null,
>+                    bugs: [],
>+                    buildRequestCount: 8,
>+                    finishedBuildRequestCount: 0,
>+                    category: 'identified',
>+                    causes: [],
>+                    createdAt: 4500,
>+                    endRun: 2,
>+                    endRunTime:  5000,
>+                    fixes: [],
>+                    id: 5255,
>+                    metric: MockModels.someMetric.id(),
>+                    name: 'confirm',
>+                    needed: null,
>+                    platform: MockModels.somePlatform.id(),
>+                    result: 'progression',
>+                    segmentationStrategy: 1,
>+                    startRun: 1,
>+                    startRunTime: 4000,
>+                    testRangeStrategy: 2
>+                }],
>+                bugs: [],
>+                commits: [],
>+                status: 'OK'
>+            });
>+            const analysisTask = await creatingPromise;
>+            assert.equal(analysisTask.id(), 5255);
>+            assert.deepEqual(analysisTask.bugs(), []);
>+            assert.equal(analysisTask.author(), '');
>+            assert.equal(analysisTask.platform(), MockModels.somePlatform);
>+            assert.equal(analysisTask.metric(), MockModels.someMetric);
>+        });
>     });
> 
>     describe('create with node privilege api', () => {
>diff --git a/Websites/perf.webkit.org/unit-tests/measurement-set-analyzer-tests.js b/Websites/perf.webkit.org/unit-tests/measurement-set-analyzer-tests.js
>index 1eb9e41953efbb46a8b4ffc8139a1a847d36a655..6a89f3532440d3269378a27b80a0c3cc4c5df13c 100644
>--- a/Websites/perf.webkit.org/unit-tests/measurement-set-analyzer-tests.js
>+++ b/Websites/perf.webkit.org/unit-tests/measurement-set-analyzer-tests.js
>@@ -247,6 +247,38 @@ describe('MeasurementSetAnalyzer', () => {
>             });
>             requests[2].resolve({taskId: '5255', status: 'OK'});
> 
>+            await MockRemoteAPI.waitForRequest();
>+            assert.equal(requests.length, 4);
>+            assert.equal(requests[3].url, '/api/analysis-tasks?id=5255');
>+
>+            requests[3].resolve({
>+                analysisTasks: [{
>+                    author: null,
>+                    bugs: [],
>+                    buildRequestCount: 8,
>+                    finishedBuildRequestCount: 0,
>+                    category: 'identified',
>+                    causes: [],
>+                    createdAt: 4500,
>+                    endRun: 6448,
>+                    endRunTime:  5000,
>+                    fixes: [],
>+                    id: 5255,
>+                    metric: MockModels.someMetric.id(),
>+                    name: 'Potential 2.38% regression on Some platform between WebKit: r40-r49',
>+                    needed: null,
>+                    platform: MockModels.somePlatform.id(),
>+                    result: 'regression',
>+                    segmentationStrategy: 1,
>+                    startRun: 6439,
>+                    startRunTime: 4000,
>+                    testRangeStrategy: 2
>+                }],
>+                bugs: [],
>+                commits: [],
>+                status: 'OK'
>+            });
>+
>             await analysisPromise;
>             assert.deepEqual(logger.info_logs, ['==== "Some test : Some metric" on "Some platform" ====',
>                 'Created analysis task with id "5255" to confirm: "Potential 2.38% regression on Some platform between WebKit: r35-r44".']);
>@@ -297,7 +329,7 @@ describe('MeasurementSetAnalyzer', () => {
>                     result: 'regression',
>                     segmentationStrategy: 1,
>                     startRun: 6434,
>-                    statrRunTime: 4000,
>+                    startRunTime: 4000,
>                     testRangeStrategy: 2
>                 }],
>                 bugs: [],
>@@ -359,6 +391,38 @@ describe('MeasurementSetAnalyzer', () => {
>             });
>             requests[2].resolve({taskId: '5255', status: 'OK'});
> 
>+            await MockRemoteAPI.waitForRequest();
>+            assert.equal(requests.length, 4);
>+            assert.equal(requests[3].url, '/api/analysis-tasks?id=5255');
>+
>+            requests[3].resolve({
>+                analysisTasks: [{
>+                    author: null,
>+                    bugs: [],
>+                    buildRequestCount: 8,
>+                    finishedBuildRequestCount: 0,
>+                    category: 'identified',
>+                    causes: [],
>+                    createdAt: 4500,
>+                    endRun: 6448,
>+                    endRunTime:  5000,
>+                    fixes: [],
>+                    id: 5255,
>+                    metric: MockModels.someMetric.id(),
>+                    name: 'Potential 2.38% regression on Some platform between WebKit: r40-r49',
>+                    needed: null,
>+                    platform: MockModels.somePlatform.id(),
>+                    result: 'regression',
>+                    segmentationStrategy: 1,
>+                    startRun: 6439,
>+                    startRunTime: 4000,
>+                    testRangeStrategy: 2
>+                }],
>+                bugs: [],
>+                commits: [],
>+                status: 'OK'
>+            });
>+
>             await analysisPromise;
>             assert.deepEqual(logger.info_logs, ['==== "Some test : Some metric" on "Some platform" ====',
>                 'Created analysis task with id "5255" to confirm: "Potential 2.38% regression on Some platform between WebKit: r40-r49".']);
>@@ -413,6 +477,38 @@ describe('MeasurementSetAnalyzer', () => {
>             });
>             requests[2].resolve({taskId: '5255', status: 'OK'});
> 
>+            await MockRemoteAPI.waitForRequest();
>+            assert.equal(requests.length, 4);
>+            assert.equal(requests[3].url, '/api/analysis-tasks?id=5255');
>+
>+            requests[3].resolve({
>+                analysisTasks: [{
>+                    author: null,
>+                    bugs: [],
>+                    buildRequestCount: 8,
>+                    finishedBuildRequestCount: 0,
>+                    category: 'identified',
>+                    causes: [],
>+                    createdAt: 4500,
>+                    endRun: 6407,
>+                    endRunTime:  5000,
>+                    fixes: [],
>+                    id: 5255,
>+                    metric: MockModels.someMetric.id(),
>+                    name: 'Potential 9.15% progression on Some platform between WebKit: r3-r8',
>+                    needed: null,
>+                    platform: MockModels.somePlatform.id(),
>+                    result: 'progression',
>+                    segmentationStrategy: 1,
>+                    startRun: 6402,
>+                    startRunTime: 4000,
>+                    testRangeStrategy: 2
>+                }],
>+                bugs: [],
>+                commits: [],
>+                status: 'OK'
>+            });
>+
>             await analysisPromise;
>             assert.deepEqual(logger.info_logs, ['==== "Some test : Some metric" on "Some platform" ====',
>                 'Created analysis task with id "5255" to confirm: "Potential 9.15% progression on Some platform between WebKit: r3-r8".']);

Flags:

rniwa: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 185222: 339359