REGRESSION(r230960): Browser tests under TimeSeriesChart fetchMeasurementSets all...
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 30 Apr 2018 23:43:45 +0000 (23:43 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 30 Apr 2018 23:43:45 +0000 (23:43 +0000)
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:

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

Websites/perf.webkit.org/ChangeLog
Websites/perf.webkit.org/unit-tests/analysis-task-tests.js
Websites/perf.webkit.org/unit-tests/buildbot-syncer-tests.js
Websites/perf.webkit.org/unit-tests/commit-log-tests.js
Websites/perf.webkit.org/unit-tests/commit-set-range-bisector-tests.js
Websites/perf.webkit.org/unit-tests/commit-set-tests.js
Websites/perf.webkit.org/unit-tests/measurement-set-tests.js
Websites/perf.webkit.org/unit-tests/privileged-api-tests.js
Websites/perf.webkit.org/unit-tests/resources/mock-remote-api.js
Websites/perf.webkit.org/unit-tests/test-groups-tests.js

index 5a5831d..f6a8e66 100644 (file)
@@ -1,3 +1,25 @@
+2018-04-30  Ryosuke Niwa  <rniwa@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_zhu@apple.com>
 
         MeasurementSet._constructUrl should construct absolute url.
index 5d499ab..7fc1542 100644 (file)
@@ -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('AnalysisTask', () => {
     }
 
     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('AnalysisTask', () => {
     }
 
     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('AnalysisTask', () => {
     });
 
     describe('create with node privilege api', () => {
-        const requests = MockRemoteAPI.inject(null, 'node');
+        const requests = MockRemoteAPI.inject(null, NodePrivilegedAPI);
         beforeEach(() => {
             PrivilegedAPI.configure('worker', 'password');
         });
index 2e2c3b4..66cafc6 100644 (file)
@@ -3,11 +3,13 @@
 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()
 {
@@ -331,7 +333,7 @@ function sampleFinishedBuild(buildRequestId, workerName, builderName)
 
 describe('BuildbotSyncer', () => {
     MockModels.inject();
-    let requests = MockRemoteAPI.inject('http://build.webkit.org');
+    const requests = MockRemoteAPI.inject('http://build.webkit.org', BrowserPrivilegedAPI);
 
     describe('_loadConfig', () => {
 
index cc2dbbf..2ed1d80 100644 (file)
@@ -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('CommitLog', function () {
 
     describe('fetchOwnedCommits', () => {
         beforeEach(() => {
-            MockRemoteAPI.inject();
+            MockRemoteAPI.inject(null, BrowserPrivilegedAPI);
         });
 
         it('should reject if repository of the commit does not own other repositories', () => {
index f10cc1c..b5ae202 100644 (file)
@@ -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('CommitSetRangeBisector', () => {
 
     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));
index a8f345f..7e39ab7 100644 (file)
@@ -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 @@ function anotherCommitWithGitRevision()
 }
 
 describe('CommitSet', () => {
-    MockRemoteAPI.inject();
+    MockRemoteAPI.inject(null, BrowserPrivilegedAPI);
     MockModels.inject();
 
     function oneCommitSet()
@@ -399,7 +400,7 @@ describe('CommitSet', () => {
 });
 
 describe('IntermediateCommitSet', () => {
-    MockRemoteAPI.inject();
+    MockRemoteAPI.inject(null, BrowserPrivilegedAPI);
     MockModels.inject();
 
     describe('setCommitForRepository', () => {
index cf05e08..383d753 100644 (file)
@@ -3,14 +3,14 @@
 const assert = require('assert');
 if (!assert.almostEqual)
     assert.almostEqual = require('./resources/almost-equal.js');
-
-const MockRemoteAPI = require('./resources/mock-remote-api.js').MockRemoteAPI;
 require('../tools/js/v3-models.js');
+const BrowserPrivilegedAPI = require('../public/v3/privileged-api.js').PrivilegedAPI;
+const MockRemoteAPI = require('./resources/mock-remote-api.js').MockRemoteAPI;
 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;
index 9da4b89..eb425df 100644 (file)
@@ -1,12 +1,13 @@
 'use strict';
 
 const assert = require('assert');
-
-const MockRemoteAPI = require('./resources/mock-remote-api.js').MockRemoteAPI;
 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;
 
 describe('BrowserPrivilegedAPI', () => {
-    let requests = MockRemoteAPI.inject();
+    const requests = MockRemoteAPI.inject(null, BrowserPrivilegedAPI);
 
     beforeEach(() => {
         PrivilegedAPI._token = null;
@@ -169,7 +170,7 @@ describe('BrowserPrivilegedAPI', () => {
 });
 
 describe('NodePrivilegedAPI', () => {
-    let requests = MockRemoteAPI.inject(null, 'node');
+    let requests = MockRemoteAPI.inject(null, NodePrivilegedAPI);
     beforeEach(() => {
         PrivilegedAPI.configure('slave_name', 'password');
     });
index 6bc10fb..be43fee 100644 (file)
@@ -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,27 +58,26 @@ var MockRemoteAPI = {
         }
         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);
             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;
index f3e833c..4891e3d 100644 (file)
@@ -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 @@ describe('TestGroup', function () {
     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);