Title: [231180] trunk/Websites/perf.webkit.org
Revision
231180
Author
rn...@webkit.org
Date
2018-04-30 16:43:45 -0700 (Mon, 30 Apr 2018)

Log Message

REGRESSION(r230960): Browser tests under TimeSeriesChart fetchMeasurementSets all fail
https://bugs.webkit.org/show_bug.cgi?id=185125

Reviewed by Saam Barati.

The bug was caused by mock-remote-api.js always loading PrivilegedAPI using require, which doesn't work in a browser.
Fixed the bug by explicitly requiring the right kind of PrivilegedAPI in each unit test instead.

* unit-tests/analysis-task-tests.js:
* unit-tests/buildbot-syncer-tests.js:
* unit-tests/commit-log-tests.js:
* unit-tests/commit-set-range-bisector-tests.js:
* unit-tests/commit-set-tests.js:
* unit-tests/measurement-set-tests.js:
* unit-tests/privileged-api-tests.js:
* unit-tests/resources/mock-remote-api.js:
(MockRemoteAPI.inject): Take PrivilegedAPI instead of the type string. Also fixed a bug that _token wasn't unset
after each unit test, and superfluous initializations of originalRemoteAPI and originalPrivilegedAPI.
* unit-tests/test-groups-tests.js:

Modified Paths

Diff

Modified: trunk/Websites/perf.webkit.org/ChangeLog (231179 => 231180)


--- trunk/Websites/perf.webkit.org/ChangeLog	2018-04-30 23:42:22 UTC (rev 231179)
+++ trunk/Websites/perf.webkit.org/ChangeLog	2018-04-30 23:43:45 UTC (rev 231180)
@@ -1,3 +1,25 @@
+2018-04-30  Ryosuke Niwa  <rn...@webkit.org>
+
+        REGRESSION(r230960): Browser tests under TimeSeriesChart fetchMeasurementSets all fail
+        https://bugs.webkit.org/show_bug.cgi?id=185125
+
+        Reviewed by Saam Barati.
+
+        The bug was caused by mock-remote-api.js always loading PrivilegedAPI using require, which doesn't work in a browser.
+        Fixed the bug by explicitly requiring the right kind of PrivilegedAPI in each unit test instead.
+
+        * unit-tests/analysis-task-tests.js:
+        * unit-tests/buildbot-syncer-tests.js:
+        * unit-tests/commit-log-tests.js:
+        * unit-tests/commit-set-range-bisector-tests.js:
+        * unit-tests/commit-set-tests.js:
+        * unit-tests/measurement-set-tests.js:
+        * unit-tests/privileged-api-tests.js:
+        * unit-tests/resources/mock-remote-api.js:
+        (MockRemoteAPI.inject): Take PrivilegedAPI instead of the type string. Also fixed a bug that _token wasn't unset
+        after each unit test, and superfluous initializations of originalRemoteAPI and originalPrivilegedAPI.
+        * unit-tests/test-groups-tests.js:
+
 2018-04-30  Dewei Zhu  <dewei_...@apple.com>
 
         MeasurementSet._constructUrl should construct absolute url.

Modified: trunk/Websites/perf.webkit.org/unit-tests/analysis-task-tests.js (231179 => 231180)


--- trunk/Websites/perf.webkit.org/unit-tests/analysis-task-tests.js	2018-04-30 23:42:22 UTC (rev 231179)
+++ trunk/Websites/perf.webkit.org/unit-tests/analysis-task-tests.js	2018-04-30 23:43:45 UTC (rev 231180)
@@ -3,8 +3,10 @@
 const assert = require('assert');
 
 require('../tools/js/v3-models.js');
-let MockModels = require('./resources/mock-v3-models.js').MockModels;
-let MockRemoteAPI = require('./resources/mock-remote-api.js').MockRemoteAPI;
+const MockModels = require('./resources/mock-v3-models.js').MockModels;
+const MockRemoteAPI = require('./resources/mock-remote-api.js').MockRemoteAPI;
+const BrowserPrivilegedAPI = require('../public/v3/privileged-api.js').PrivilegedAPI;
+const NodePrivilegedAPI = require('../tools/js/privileged-api').PrivilegedAPI;
 
 function sampleAnalysisTask()
 {
@@ -129,7 +131,7 @@
     }
 
     describe('fetchAll', () => {
-        const requests = MockRemoteAPI.inject();
+        const requests = MockRemoteAPI.inject(null, BrowserPrivilegedAPI);
         it('should request all analysis tasks', () => {
             let callCount = 0;
             AnalysisTask.fetchAll().then(() => { callCount++; });
@@ -245,7 +247,7 @@
     }
 
     describe('create with browser privilege api', () => {
-        const requests = MockRemoteAPI.inject();
+        const requests = MockRemoteAPI.inject(null, BrowserPrivilegedAPI);
 
         it('should create analysis task with confirming repetition count zero as default with browser privilege api', async () => {
             const [startPoint, endPoint] = mockStartAndEndPoints();
@@ -287,7 +289,7 @@
     });
 
     describe('create with node privilege api', () => {
-        const requests = MockRemoteAPI.inject(null, 'node');
+        const requests = MockRemoteAPI.inject(null, NodePrivilegedAPI);
         beforeEach(() => {
             PrivilegedAPI.configure('worker', 'password');
         });

Modified: trunk/Websites/perf.webkit.org/unit-tests/buildbot-syncer-tests.js (231179 => 231180)


--- trunk/Websites/perf.webkit.org/unit-tests/buildbot-syncer-tests.js	2018-04-30 23:42:22 UTC (rev 231179)
+++ trunk/Websites/perf.webkit.org/unit-tests/buildbot-syncer-tests.js	2018-04-30 23:43:45 UTC (rev 231180)
@@ -3,12 +3,14 @@
 let assert = require('assert');
 
 require('../tools/js/v3-models.js');
-let MockRemoteAPI = require('./resources/mock-remote-api.js').MockRemoteAPI;
-let MockModels = require('./resources/mock-v3-models.js').MockModels;
+const BrowserPrivilegedAPI = require('../public/v3/privileged-api.js').PrivilegedAPI;
 
-let BuildbotBuildEntry = require('../tools/js/buildbot-syncer.js').BuildbotBuildEntry;
-let BuildbotSyncer = require('../tools/js/buildbot-syncer.js').BuildbotSyncer;
+const MockRemoteAPI = require('./resources/mock-remote-api.js').MockRemoteAPI;
+const MockModels = require('./resources/mock-v3-models.js').MockModels;
 
+const BuildbotBuildEntry = require('../tools/js/buildbot-syncer.js').BuildbotBuildEntry;
+const BuildbotSyncer = require('../tools/js/buildbot-syncer.js').BuildbotSyncer;
+
 function sampleiOSConfig()
 {
     return {
@@ -331,7 +333,7 @@
 
 describe('BuildbotSyncer', () => {
     MockModels.inject();
-    let requests = MockRemoteAPI.inject('http://build.webkit.org');
+    const requests = MockRemoteAPI.inject('http://build.webkit.org', BrowserPrivilegedAPI);
 
     describe('_loadConfig', () => {
 

Modified: trunk/Websites/perf.webkit.org/unit-tests/commit-log-tests.js (231179 => 231180)


--- trunk/Websites/perf.webkit.org/unit-tests/commit-log-tests.js	2018-04-30 23:42:22 UTC (rev 231179)
+++ trunk/Websites/perf.webkit.org/unit-tests/commit-log-tests.js	2018-04-30 23:43:45 UTC (rev 231180)
@@ -3,6 +3,7 @@
 const assert = require('assert');
 
 require('../tools/js/v3-models.js');
+const BrowserPrivilegedAPI = require('../public/v3/privileged-api.js').PrivilegedAPI;
 const MockModels = require('./resources/mock-v3-models.js').MockModels;
 const MockRemoteAPI = require('../unit-tests/resources/mock-remote-api.js').MockRemoteAPI;
 
@@ -257,7 +258,7 @@
 
     describe('fetchOwnedCommits', () => {
         beforeEach(() => {
-            MockRemoteAPI.inject();
+            MockRemoteAPI.inject(null, BrowserPrivilegedAPI);
         });
 
         it('should reject if repository of the commit does not own other repositories', () => {

Modified: trunk/Websites/perf.webkit.org/unit-tests/commit-set-range-bisector-tests.js (231179 => 231180)


--- trunk/Websites/perf.webkit.org/unit-tests/commit-set-range-bisector-tests.js	2018-04-30 23:42:22 UTC (rev 231179)
+++ trunk/Websites/perf.webkit.org/unit-tests/commit-set-range-bisector-tests.js	2018-04-30 23:43:45 UTC (rev 231180)
@@ -3,6 +3,7 @@
 const assert = require('assert');
 
 require('../tools/js/v3-models.js');
+const BrowserPrivilegedAPI = require('../public/v3/privileged-api.js').PrivilegedAPI;
 const MockModels = require('./resources/mock-v3-models.js').MockModels;
 const MockRemoteAPI = require('./resources/mock-remote-api.js').MockRemoteAPI;
 
@@ -271,7 +272,7 @@
 
     describe('commitSetClosestToMiddleOfAllCommits', () => {
         MockModels.inject();
-        const requests = MockRemoteAPI.inject();
+        const requests = MockRemoteAPI.inject(null, BrowserPrivilegedAPI);
 
         it('should return "null" if no common repository found', async () => {
             const middleCommitSet = await CommitSetRangeBisector.commitSetClosestToMiddleOfAllCommits(commitSetsWithNoCommonRepository().splice(0, 2));

Modified: trunk/Websites/perf.webkit.org/unit-tests/commit-set-tests.js (231179 => 231180)


--- trunk/Websites/perf.webkit.org/unit-tests/commit-set-tests.js	2018-04-30 23:42:22 UTC (rev 231179)
+++ trunk/Websites/perf.webkit.org/unit-tests/commit-set-tests.js	2018-04-30 23:43:45 UTC (rev 231180)
@@ -2,6 +2,7 @@
 
 const assert = require('assert');
 require('../tools/js/v3-models.js');
+const BrowserPrivilegedAPI = require('../public/v3/privileged-api.js').PrivilegedAPI;
 const MockModels = require('./resources/mock-v3-models.js').MockModels;
 const MockRemoteAPI = require('../unit-tests/resources/mock-remote-api.js').MockRemoteAPI;
 
@@ -161,7 +162,7 @@
 }
 
 describe('CommitSet', () => {
-    MockRemoteAPI.inject();
+    MockRemoteAPI.inject(null, BrowserPrivilegedAPI);
     MockModels.inject();
 
     function oneCommitSet()
@@ -399,7 +400,7 @@
 });
 
 describe('IntermediateCommitSet', () => {
-    MockRemoteAPI.inject();
+    MockRemoteAPI.inject(null, BrowserPrivilegedAPI);
     MockModels.inject();
 
     describe('setCommitForRepository', () => {

Modified: trunk/Websites/perf.webkit.org/unit-tests/measurement-set-tests.js (231179 => 231180)


--- trunk/Websites/perf.webkit.org/unit-tests/measurement-set-tests.js	2018-04-30 23:42:22 UTC (rev 231179)
+++ trunk/Websites/perf.webkit.org/unit-tests/measurement-set-tests.js	2018-04-30 23:43:45 UTC (rev 231180)
@@ -3,14 +3,14 @@
 const assert = require('assert');
 if (!assert.almostEqual)
     assert.almostEqual = require('./resources/almost-equal.js');
-
+require('../tools/js/v3-models.js');
+const BrowserPrivilegedAPI = require('../public/v3/privileged-api.js').PrivilegedAPI;
 const MockRemoteAPI = require('./resources/mock-remote-api.js').MockRemoteAPI;
-require('../tools/js/v3-models.js');
 const MockModels = require('./resources/mock-v3-models.js').MockModels;
 
 describe('MeasurementSet', () => {
     MockModels.inject();
-    let requests = MockRemoteAPI.inject();
+    const requests = MockRemoteAPI.inject(null, BrowserPrivilegedAPI);
 
     beforeEach(() => {
         MeasurementSet._set = null;

Modified: trunk/Websites/perf.webkit.org/unit-tests/privileged-api-tests.js (231179 => 231180)


--- trunk/Websites/perf.webkit.org/unit-tests/privileged-api-tests.js	2018-04-30 23:42:22 UTC (rev 231179)
+++ trunk/Websites/perf.webkit.org/unit-tests/privileged-api-tests.js	2018-04-30 23:43:45 UTC (rev 231180)
@@ -1,12 +1,13 @@
 'use strict';
 
 const assert = require('assert');
-
+require('../tools/js/v3-models.js');
+const BrowserPrivilegedAPI = require('../public/v3/privileged-api.js').PrivilegedAPI;
+const NodePrivilegedAPI = require('../tools/js/privileged-api').PrivilegedAPI;
 const MockRemoteAPI = require('./resources/mock-remote-api.js').MockRemoteAPI;
-require('../tools/js/v3-models.js');
 
 describe('BrowserPrivilegedAPI', () => {
-    let requests = MockRemoteAPI.inject();
+    const requests = MockRemoteAPI.inject(null, BrowserPrivilegedAPI);
 
     beforeEach(() => {
         PrivilegedAPI._token = null;
@@ -169,7 +170,7 @@
 });
 
 describe('NodePrivilegedAPI', () => {
-    let requests = MockRemoteAPI.inject(null, 'node');
+    let requests = MockRemoteAPI.inject(null, NodePrivilegedAPI);
     beforeEach(() => {
         PrivilegedAPI.configure('slave_name', 'password');
     });

Modified: trunk/Websites/perf.webkit.org/unit-tests/resources/mock-remote-api.js (231179 => 231180)


--- trunk/Websites/perf.webkit.org/unit-tests/resources/mock-remote-api.js	2018-04-30 23:42:22 UTC (rev 231179)
+++ trunk/Websites/perf.webkit.org/unit-tests/resources/mock-remote-api.js	2018-04-30 23:43:45 UTC (rev 231180)
@@ -1,5 +1,3 @@
-const BrowserPrivilegedAPI = require('../../public/v3/privileged-api.js').PrivilegedAPI;
-const NodePrivilegedAPI = require('../../tools/js/privileged-api').PrivilegedAPI;
 
 var MockRemoteAPI = {
     url: function (path)
@@ -60,13 +58,10 @@
         }
         return this._waitingPromise;
     },
-    inject: function (urlPrefix, privilegedAPIType='browser')
+    inject: function (urlPrefix, privilegedAPI)
     {
-        console.assert(privilegedAPIType === 'browser' || privilegedAPIType === 'node');
-        let originalRemoteAPI = global.RemoteAPI;
-        let originalPrivilegedAPI = global.PrivilegedAPI;
-        const useNodePrivilegedAPI = privilegedAPIType === 'node';
-        const PrivilegedAPI = useNodePrivilegedAPI ? NodePrivilegedAPI: BrowserPrivilegedAPI;
+        let originalRemoteAPI;
+        let originalPrivilegedAPI;
 
         beforeEach(() => {
             MockRemoteAPI.reset(urlPrefix);
@@ -73,14 +68,16 @@
             originalRemoteAPI = global.RemoteAPI;
             global.RemoteAPI = MockRemoteAPI;
             originalPrivilegedAPI = global.PrivilegedAPI;
-            global.PrivilegedAPI = PrivilegedAPI;
-            if (!useNodePrivilegedAPI)
-                PrivilegedAPI._token = null;
+            global.PrivilegedAPI = privilegedAPI;
+            if (privilegedAPI._token)
+                privilegedAPI._token = null;
         });
 
         afterEach(() => {
             global.RemoteAPI = originalRemoteAPI;
             global.PrivilegedAPI = originalPrivilegedAPI;
+            if (privilegedAPI._token)
+                privilegedAPI._token = null;
         });
 
         return MockRemoteAPI.requests;

Modified: trunk/Websites/perf.webkit.org/unit-tests/test-groups-tests.js (231179 => 231180)


--- trunk/Websites/perf.webkit.org/unit-tests/test-groups-tests.js	2018-04-30 23:42:22 UTC (rev 231179)
+++ trunk/Websites/perf.webkit.org/unit-tests/test-groups-tests.js	2018-04-30 23:43:45 UTC (rev 231180)
@@ -1,8 +1,8 @@
 'use strict';
 
 const assert = require('assert');
-
 require('../tools/js/v3-models.js');
+const BrowserPrivilegedAPI = require('../public/v3/privileged-api.js').PrivilegedAPI;
 const MockModels = require('./resources/mock-v3-models.js').MockModels;
 const MockRemoteAPI = require('./resources/mock-remote-api.js').MockRemoteAPI;
 
@@ -108,7 +108,7 @@
     MockModels.inject();
 
     describe('fetchForTask', () => {
-        const requests = MockRemoteAPI.inject('https://perf.webkit.org');
+        const requests = MockRemoteAPI.inject('https://perf.webkit.org', BrowserPrivilegedAPI);
 
         it('should be able to fetch the list of test groups for the same task twice using cache', () => {
             const fetchPromise = TestGroup.fetchForTask(1376);
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to