Fix os-build-fetcher.js and subprocess.js to make them work
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 20 Mar 2017 22:36:40 +0000 (22:36 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 20 Mar 2017 22:36:40 +0000 (22:36 +0000)
https://bugs.webkit.org/show_bug.cgi?id=169844

Reviewed by Antti Koivisto.

The script added in r213976 has a bug that it can execute commands to fetch subcommits in parallel.
Some commands to poll the lsit of system components is not desirable to be ran in parallel.

* server-tests/resources/mock-subprocess.js:
(MockSubprocess): Use const declaration.
(MockSubprocess.resetAndWaitForInvocation): Added.
(MockSubprocess.waitForInvocation): Renamed from waitingForInvocation. A function name must be a verb.
See https://webkit.org/code-style-guidelines/#names-verb
(MockSubprocess.reset): Set invocations.length to 0 so that tests can store a reference to the array
regardless of whether reset is called or when it's called.

* server-tests/tools-os-build-fetcher-tests.js: Updated tests per the code change. Most of codes now
expect each command to be ran seprately. e.g. if there were two commands to run, instead of expecting
them to be both ran, and resolving invocation promises, we'd wait for one command to run, resolve,
its subcommand to run, and then move onto the second top-level command. Also use a local reference
to MockSubprocess.invocations instead of using the fully qualified name.

* tools/js/os-build-fetcher.js:
(mapInSerialPromiseChain): Added. Calling a closure that returns a promise on each item in an array
in serial (not asynchronous) is a very common pattern in this class.
(OSBuildFetcher.fetchAndReportAllInOrder): Added.
(OSBuildFetcher.prototype.fetchAndReportNewBuilds): Log what the number of builds being submitted.
(OSBuildFetcher.prototype._fetchAvailableBuilds): Fixed the main bug. Using Promise.all would result
in each top-level command to be execued in parallel. Since each subcommand is executed as soon as
its parent command is executed, this results in commands to be executed in parallel.
Added a whole bunch of logging so that we can at least detect a bug like this in the future.
(OSBuildFetcher.prototype._commitsForAvailableBuilds): Cleanup the coding style.
(OSBuildFetcher.prototype._addSubCommitsForBuild): Use mapInSerialPromiseChain. Tightened the assertion
about the content returned by a subcommand.

* tools/js/subprocess.js: Fixed the bug that we were importing require('child_process').ChildProcess.
execFile is defined on require('child_process') itself.
(Subprocess.prototype.execute): Fixed a typo. this._childProcess doesn't exist.
(Subprocess):

* tools/sync-os-versions.js: Renamed from tools/pull-os-versions.js.
(syncLoop): Cleaned up the coding style a little. Also added logging about how long we're about to sleep.

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

Websites/perf.webkit.org/ChangeLog
Websites/perf.webkit.org/server-tests/resources/mock-subprocess.js
Websites/perf.webkit.org/server-tests/tools-os-build-fetcher-tests.js
Websites/perf.webkit.org/tools/js/os-build-fetcher.js
Websites/perf.webkit.org/tools/js/subprocess.js
Websites/perf.webkit.org/tools/sync-os-versions.js [moved from Websites/perf.webkit.org/tools/pull-os-versions.js with 68% similarity]

index fdba6fd1a3390720b8fba068a9c2451b1b4586e8..fc12eb1349edb381311b77014bd45221593a814f 100644 (file)
@@ -1,3 +1,48 @@
+2017-03-20  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Fix os-build-fetcher.js and subprocess.js to make them work
+        https://bugs.webkit.org/show_bug.cgi?id=169844
+
+        Reviewed by Antti Koivisto.
+
+        The script added in r213976 has a bug that it can execute commands to fetch subcommits in parallel.
+        Some commands to poll the lsit of system components is not desirable to be ran in parallel.
+
+        * server-tests/resources/mock-subprocess.js:
+        (MockSubprocess): Use const declaration.
+        (MockSubprocess.resetAndWaitForInvocation): Added.
+        (MockSubprocess.waitForInvocation): Renamed from waitingForInvocation. A function name must be a verb.
+        See https://webkit.org/code-style-guidelines/#names-verb
+        (MockSubprocess.reset): Set invocations.length to 0 so that tests can store a reference to the array
+        regardless of whether reset is called or when it's called.
+
+        * server-tests/tools-os-build-fetcher-tests.js: Updated tests per the code change. Most of codes now
+        expect each command to be ran seprately. e.g. if there were two commands to run, instead of expecting
+        them to be both ran, and resolving invocation promises, we'd wait for one command to run, resolve,
+        its subcommand to run, and then move onto the second top-level command. Also use a local reference
+        to MockSubprocess.invocations instead of using the fully qualified name.
+
+        * tools/js/os-build-fetcher.js:
+        (mapInSerialPromiseChain): Added. Calling a closure that returns a promise on each item in an array
+        in serial (not asynchronous) is a very common pattern in this class.
+        (OSBuildFetcher.fetchAndReportAllInOrder): Added.
+        (OSBuildFetcher.prototype.fetchAndReportNewBuilds): Log what the number of builds being submitted.
+        (OSBuildFetcher.prototype._fetchAvailableBuilds): Fixed the main bug. Using Promise.all would result
+        in each top-level command to be execued in parallel. Since each subcommand is executed as soon as
+        its parent command is executed, this results in commands to be executed in parallel.
+        Added a whole bunch of logging so that we can at least detect a bug like this in the future.
+        (OSBuildFetcher.prototype._commitsForAvailableBuilds): Cleanup the coding style.
+        (OSBuildFetcher.prototype._addSubCommitsForBuild): Use mapInSerialPromiseChain. Tightened the assertion
+        about the content returned by a subcommand.
+
+        * tools/js/subprocess.js: Fixed the bug that we were importing require('child_process').ChildProcess.
+        execFile is defined on require('child_process') itself.
+        (Subprocess.prototype.execute): Fixed a typo. this._childProcess doesn't exist.
+        (Subprocess):
+
+        * tools/sync-os-versions.js: Renamed from tools/pull-os-versions.js.
+        (syncLoop): Cleaned up the coding style a little. Also added logging about how long we're about to sleep.
+
 2017-03-16  Ryosuke Niwa  <rniwa@webkit.org>
 
         Add the file uploading capability to the perf dashboard.
index 975b93a33da8befbea2b2e8befad08af09e43558..f8e2a9777641c0db88e0e777c607deb40f3b1636 100644 (file)
@@ -1,4 +1,4 @@
-var MockSubprocess = {
+const MockSubprocess = {
     execute: function (command)
     {
         const invocation = {command};
@@ -16,7 +16,12 @@ var MockSubprocess = {
         this.invocations.push(invocation);
         return invocation.promise;
     },
-    waitingForInvocation: function ()
+    resetAndWaitForInvocation: function ()
+    {
+        this.reset();
+        return this.waitForInvocation();
+    },
+    waitForInvocation: function ()
     {
         if (!this._waitingInvocation) {
             this._waitingInvocation = new Promise(function (resolve, reject) {
@@ -43,7 +48,7 @@ var MockSubprocess = {
     },
     reset: function ()
     {
-        MockSubprocess.invocations = [];
+        MockSubprocess.invocations.length = 0;
         MockSubprocess._waitingInvocation = null;
         MockSubprocess._waitingInvocationResolver = null;
     },
index 759dc1dbf0d1d496c24698be27fc80f514eaba9d..9daabd0f842a28f46f7d83f730cca107adeb828d 100644 (file)
@@ -125,10 +125,10 @@ describe('OSBuildFetcher', function() {
         it('should only return commits whose orders are higher than specified order', () => {
             const logger = new MockLogger;
             const fetchter = new OSBuildFetcher(null, null, null, MockSubprocess, logger);
-            const waitingForInvocationPromise = MockSubprocess.waitingForInvocation();
+            const waitForInvocationPromise = MockSubprocess.waitForInvocation();
             const fetchCommitsPromise = fetchter._commitsForAvailableBuilds('OSX', ['list', 'build1'], '^\\.*$', 1604000000);
 
-            return waitingForInvocationPromise.then(() => {
+            return waitForInvocationPromise.then(() => {
                 assert.equal(MockSubprocess.invocations.length, 1);
                 assert.deepEqual(MockSubprocess.invocations[0].command, ['list', 'build1']);
                 MockSubprocess.invocations[0].resolve('16D321\n16E321z\n\n16F321');
@@ -145,15 +145,15 @@ describe('OSBuildFetcher', function() {
         it('should add sub-commit info for commits', () => {
             const logger = new MockLogger;
             const fetchter = new OSBuildFetcher(null, null, null, MockSubprocess, logger);
-            const waitingForInvocationPromise = MockSubprocess.waitingForInvocation();
+            const waitForInvocationPromise = MockSubprocess.waitForInvocation();
             const addSubCommitPromise = fetchter._addSubCommitsForBuild([osxCommit, anotherOSXCommit], ['subCommit', 'for', 'revision']);
 
-            return waitingForInvocationPromise.then(() => {
+            return waitForInvocationPromise.then(() => {
                 assert.equal(MockSubprocess.invocations.length, 1);
                 assert.deepEqual(MockSubprocess.invocations[0].command, ['subCommit', 'for', 'revision', 'Sierra16D32']);
                 MockSubprocess.invocations[0].resolve(JSON.stringify(subCommitWithWebKit));
                 MockSubprocess.reset();
-                return MockSubprocess.waitingForInvocation();
+                return MockSubprocess.waitForInvocation();
             }).then(() => {
                 assert.equal(MockSubprocess.invocations.length, 1);
                 assert.deepEqual(MockSubprocess.invocations[0].command, ['subCommit', 'for', 'revision', 'Sierra16E32']);
@@ -173,10 +173,10 @@ describe('OSBuildFetcher', function() {
         it('should fail if the command to get sub-commit info fails', () => {
             const logger = new MockLogger;
             const fetchter = new OSBuildFetcher(null, null, null, MockSubprocess, logger);
-            const waitingForInvocationPromise = MockSubprocess.waitingForInvocation();
+            const waitForInvocationPromise = MockSubprocess.waitForInvocation();
             const addSubCommitPromise = fetchter._addSubCommitsForBuild([osxCommit], ['subCommit', 'for', 'revision'])
 
-            return waitingForInvocationPromise.then(() => {
+            return waitForInvocationPromise.then(() => {
                 assert.equal(MockSubprocess.invocations.length, 1);
                 assert.deepEqual(MockSubprocess.invocations[0].command, ['subCommit', 'for', 'revision', 'Sierra16D32']);
                 MockSubprocess.invocations[0].reject('Failed getting sub-commit');
@@ -194,10 +194,10 @@ describe('OSBuildFetcher', function() {
         it('should fail if entries in sub-commits does not contain revision', () => {
             const logger = new MockLogger;
             const fetchter = new OSBuildFetcher(null, null, null, MockSubprocess, logger);
-            const waitingForInvocationPromise = MockSubprocess.waitingForInvocation();
+            const waitForInvocationPromise = MockSubprocess.waitForInvocation();
             const addSubCommitPromise = fetchter._addSubCommitsForBuild([osxCommit], ['subCommit', 'for', 'revision'])
 
-            return waitingForInvocationPromise.then(() => {
+            return waitForInvocationPromise.then(() => {
                 assert.equal(MockSubprocess.invocations.length, 1);
                 assert.deepEqual(MockSubprocess.invocations[0].command, ['subCommit', 'for', 'revision', 'Sierra16D32']);
                 MockSubprocess.invocations[0].resolve('{"WebKit":{"RandomKey": "RandomValue"}}');
@@ -213,6 +213,7 @@ describe('OSBuildFetcher', function() {
     })
 
     describe('OSBuildFetcher.fetchAndReportNewBuilds', () => {
+        const invocations = MockSubprocess.invocations;
 
         beforeEach(function () {
             TestServer.database().connect({keepAlive: true});
@@ -222,7 +223,6 @@ describe('OSBuildFetcher', function() {
             TestServer.database().disconnect();
         });
 
-
         it('should report all build commits with sub-commits', () => {
             const logger = new MockLogger;
             const fetchter = new OSBuildFetcher(config, TestServer.remoteAPI(), slaveAuth, MockSubprocess, logger);
@@ -250,66 +250,63 @@ describe('OSBuildFetcher', function() {
                 assert.equal(result['commits'].length, 1);
                 assert.equal(result['commits'][0]['revision'], 'Sierra16E33g');
                 assert.equal(result['commits'][0]['order'], 1604003307);
-                const waitingForInvocationPromise = MockSubprocess.waitingForInvocation();
+                const waitForInvocationPromise = MockSubprocess.waitForInvocation();
                 fetchAvailableBuildsPromise = fetchter._fetchAvailableBuilds();
-                return waitingForInvocationPromise;
+                return waitForInvocationPromise;
             }).then(() => {
-                return MockSubprocess.waitingForInvocation();
+                assert.equal(invocations.length, 1);
+                assert.deepEqual(invocations[0].command, ['list', 'all osx 16Dxx builds']);
+                invocations[0].resolve('\n\nSierra16D68\nSierra16D69\n');
+                return MockSubprocess.resetAndWaitForInvocation();
             }).then(() => {
-                MockSubprocess.invocations.sort((invocation, antoherInvocation) => invocation['command'] > antoherInvocation['command']);
-                assert.equal(MockSubprocess.invocations.length, 2);
-                assert.deepEqual(MockSubprocess.invocations[0].command, ['list', 'all osx 16Dxx builds']);
-                assert.deepEqual(MockSubprocess.invocations[1].command, ['list', 'all osx 16Exx builds']);
-                MockSubprocess.invocations[0].resolve('\n\nSierra16D68\nSierra16D69\n');
-                MockSubprocess.invocations[1].resolve('\n\nSierra16E32\nSierra16E33\nSierra16E33h\nSierra16E34');
-                MockSubprocess.reset();
-                return MockSubprocess.waitingForInvocation();
+                assert.equal(invocations.length, 1);
+                assert.deepEqual(invocations[0].command, ['list', 'subCommit', 'for', 'revision', 'Sierra16D69']);
+                invocations[0].resolve(JSON.stringify(subCommitWithWebKit));
+                return MockSubprocess.resetAndWaitForInvocation();
             }).then(() => {
-                MockSubprocess.invocations.sort((invocation, antoherInvocation) => invocation['command'] > antoherInvocation['command']);
-                assert.equal(MockSubprocess.invocations.length, 2);
-                assert.deepEqual(MockSubprocess.invocations[0].command, ['list', 'subCommit', 'for', 'revision', 'Sierra16D69']);
-                assert.deepEqual(MockSubprocess.invocations[1].command, ['list', 'subCommit', 'for', 'revision', 'Sierra16E33h']);
-
-                MockSubprocess.invocations[0].resolve(JSON.stringify(subCommitWithWebKit));
-                MockSubprocess.invocations[1].resolve(JSON.stringify(anotherSubCommitWithWebKit));
-                MockSubprocess.reset();
-                return MockSubprocess.waitingForInvocation();
+                assert.equal(invocations.length, 1);
+                assert.deepEqual(invocations[0].command, ['list', 'all osx 16Exx builds']);
+                invocations[0].resolve('\n\nSierra16E32\nSierra16E33\nSierra16E33h\nSierra16E34');
+                return MockSubprocess.resetAndWaitForInvocation();
             }).then(() => {
-                assert.equal(MockSubprocess.invocations.length, 1);
-                assert.deepEqual(MockSubprocess.invocations[0].command, ['list', 'subCommit', 'for', 'revision', 'Sierra16E34']);
-                MockSubprocess.invocations[0].resolve(JSON.stringify(anotherSubCommitWithWebKitAndJavaScriptCore));
+                assert.equal(invocations.length, 1);
+                assert.deepEqual(invocations[0].command, ['list', 'subCommit', 'for', 'revision', 'Sierra16E33h']);
+                invocations[0].resolve(JSON.stringify(anotherSubCommitWithWebKit));
+                return MockSubprocess.resetAndWaitForInvocation();
+            }).then(() => {
+                assert.equal(invocations.length, 1);
+                assert.deepEqual(invocations[0].command, ['list', 'subCommit', 'for', 'revision', 'Sierra16E34']);
+                invocations[0].resolve(JSON.stringify(anotherSubCommitWithWebKitAndJavaScriptCore));
                 return fetchAvailableBuildsPromise;
             }).then((results) => {
                 assert.equal(results.length, 3);
                 MockSubprocess.reset();
                 fetchAndReportPromise = fetchter.fetchAndReportNewBuilds();
-                return MockSubprocess.waitingForInvocation();
+                return MockSubprocess.waitForInvocation();
             }).then(() => {
-                return MockSubprocess.waitingForInvocation();
+                assert.equal(invocations.length, 1);
+                assert.deepEqual(invocations[0].command, ['list', 'all osx 16Dxx builds']);
+                invocations[0].resolve('\n\nSierra16D68\nSierra16D69\n');
+                return MockSubprocess.resetAndWaitForInvocation();
             }).then(() => {
-                MockSubprocess.invocations.sort((invocation, antoherInvocation) => invocation['command'] > antoherInvocation['command']);
-                assert.equal(MockSubprocess.invocations.length, 2);
-                assert.deepEqual(MockSubprocess.invocations[0].command, ['list', 'all osx 16Dxx builds']);
-                assert.deepEqual(MockSubprocess.invocations[1].command, ['list', 'all osx 16Exx builds']);
-                MockSubprocess.invocations[0].resolve('\n\nSierra16D68\nSierra16D69\n');
-                MockSubprocess.invocations[1].resolve('\n\nSierra16E32\nSierra16E33\nSierra16E33h\nSierra16E34');
-                MockSubprocess.reset();
-                return MockSubprocess.waitingForInvocation();
+                assert.equal(invocations.length, 1);
+                assert.deepEqual(invocations[0].command, ['list', 'subCommit', 'for', 'revision', 'Sierra16D69']);
+                invocations[0].resolve(JSON.stringify(subCommitWithWebKit));
+                return MockSubprocess.resetAndWaitForInvocation();
             }).then(() => {
-                MockSubprocess.invocations.sort((invocation, antoherInvocation) => invocation['command'] > antoherInvocation['command']);
-                assert.equal(MockSubprocess.invocations.length, 2);
-                assert.deepEqual(MockSubprocess.invocations[0].command, ['list', 'subCommit', 'for', 'revision', 'Sierra16D69']);
-                assert.deepEqual(MockSubprocess.invocations[1].command, ['list', 'subCommit', 'for', 'revision', 'Sierra16E33h']);
-
-                MockSubprocess.invocations[0].resolve(JSON.stringify(subCommitWithWebKit));
-                MockSubprocess.invocations[1].resolve(JSON.stringify(anotherSubCommitWithWebKit));
-
-                MockSubprocess.reset();
-                return MockSubprocess.waitingForInvocation();
+                assert.equal(invocations.length, 1);
+                assert.deepEqual(invocations[0].command, ['list', 'all osx 16Exx builds']);
+                invocations[0].resolve('\n\nSierra16E32\nSierra16E33\nSierra16E33h\nSierra16E34');
+                return MockSubprocess.resetAndWaitForInvocation();
             }).then(() => {
-                assert.equal(MockSubprocess.invocations.length, 1);
-                MockSubprocess.invocations[0].resolve(JSON.stringify(anotherSubCommitWithWebKitAndJavaScriptCore));
-                assert.deepEqual(MockSubprocess.invocations[0].command, ['list', 'subCommit', 'for', 'revision', 'Sierra16E34']);
+                assert.equal(invocations.length, 1);
+                assert.deepEqual(invocations[0].command, ['list', 'subCommit', 'for', 'revision', 'Sierra16E33h']);
+                invocations[0].resolve(JSON.stringify(anotherSubCommitWithWebKit));
+                return MockSubprocess.resetAndWaitForInvocation();
+            }).then(() => {
+                assert.equal(invocations.length, 1);
+                invocations[0].resolve(JSON.stringify(anotherSubCommitWithWebKitAndJavaScriptCore));
+                assert.deepEqual(invocations[0].command, ['list', 'subCommit', 'for', 'revision', 'Sierra16E34']);
 
                 return fetchAndReportPromise;
             }).then((result) => {
@@ -386,18 +383,18 @@ describe('OSBuildFetcher', function() {
                 assert.equal(result['commits'].length, 1);
                 assert.equal(result['commits'][0]['revision'], 'Sierra16E33g');
                 assert.equal(result['commits'][0]['order'], 1604003307);
-                const waitingForInvocationPromise = MockSubprocess.waitingForInvocation();
+                const waitForInvocationPromise = MockSubprocess.waitForInvocation();
                 fetchAndReportPromise = fetchter.fetchAndReportNewBuilds();
-                return waitingForInvocationPromise;
+                return waitForInvocationPromise;
             }).then(() => {
-                return MockSubprocess.waitingForInvocation();
+                assert.equal(invocations.length, 1);
+                assert.deepEqual(invocations[0].command, ['list', 'all osx 16Dxx builds']);
+                invocations[0].resolve('\n\nSierra16D68\nSierra16D69\n');
+                return MockSubprocess.resetAndWaitForInvocation();
             }).then(() => {
-                MockSubprocess.invocations.sort((invocation, antoherInvocation) => invocation['command'] > antoherInvocation['command']);
-                assert.equal(MockSubprocess.invocations.length, 2);
-                assert.deepEqual(MockSubprocess.invocations[0].command, ['list', 'all osx 16Dxx builds']);
-                assert.deepEqual(MockSubprocess.invocations[1].command, ['list', 'all osx 16Exx builds']);
-                MockSubprocess.invocations[0].resolve('\n\nSierra16D68\nSierra16D69\n');
-                MockSubprocess.invocations[1].resolve('\n\nSierra16E32\nSierra16E33\nSierra16E33h\nSierra16E34');
+                assert.equal(invocations.length, 1);
+                assert.deepEqual(invocations[0].command, ['list', 'all osx 16Exx builds']);
+                invocations[0].resolve('\n\nSierra16E32\nSierra16E33\nSierra16E33h\nSierra16E34');
                 return fetchAndReportPromise;
             }).then((result) => {
                 assert.equal(result['status'], 'OK');
@@ -471,35 +468,32 @@ describe('OSBuildFetcher', function() {
                 assert.equal(result['commits'][0]['revision'], 'Sierra16E33g');
                 assert.equal(result['commits'][0]['order'], 1604003307);
 
-                const waitingForInvocationPromise = MockSubprocess.waitingForInvocation();
+                const waitForInvocationPromise = MockSubprocess.waitForInvocation();
                 fetchAndReportPromise = fetchter.fetchAndReportNewBuilds();
-                return waitingForInvocationPromise;
+                return waitForInvocationPromise;
             }).then(() => {
-                return MockSubprocess.waitingForInvocation();
+                assert.equal(invocations.length, 1);
+                assert.deepEqual(invocations[0].command, ['list', 'all osx 16Dxx builds']);
+                invocations[0].resolve('\n\nSierra16D68\nSierra16D69\n');
+                return MockSubprocess.resetAndWaitForInvocation();
             }).then(() => {
-                MockSubprocess.invocations.sort((invocation, antoherInvocation) => invocation['command'] > antoherInvocation['command']);
-                assert.equal(MockSubprocess.invocations.length, 2);
-                assert.deepEqual(MockSubprocess.invocations[0].command, ['list', 'all osx 16Dxx builds']);
-                assert.deepEqual(MockSubprocess.invocations[1].command, ['list', 'all osx 16Exx builds']);
-                MockSubprocess.invocations[0].resolve('\n\nSierra16D68\nSierra16D69\n');
-                MockSubprocess.invocations[1].resolve('\n\nSierra16E32\nSierra16E33\nSierra16E33h\nSierra16E34');
-                MockSubprocess.reset();
-                return MockSubprocess.waitingForInvocation();
-            }).then(() => {
-                MockSubprocess.invocations.sort((invocation, antoherInvocation) => invocation['command'] > antoherInvocation['command']);
-                assert.equal(MockSubprocess.invocations.length, 2);
-                assert.deepEqual(MockSubprocess.invocations[0].command, ['list', 'subCommit', 'for', 'revision', 'Sierra16D69']);
-                assert.deepEqual(MockSubprocess.invocations[1].command, ['list', 'subCommit', 'for', 'revision', 'Sierra16E33h']);
-
+                assert.equal(invocations.length, 1);
+                assert.deepEqual(invocations[0].command, ['list', 'subCommit', 'for', 'revision', 'Sierra16D69']);
                 MockSubprocess.invocations[0].resolve(JSON.stringify(subCommitWithWebKit));
-                MockSubprocess.invocations[1].resolve(JSON.stringify(anotherSubCommitWithWebKit));
-                MockSubprocess.reset();
-                return MockSubprocess.waitingForInvocation();
+                return MockSubprocess.resetAndWaitForInvocation();
             }).then(() => {
-                assert.equal(MockSubprocess.invocations.length, 1);
-                assert.deepEqual(MockSubprocess.invocations[0].command, ['list', 'subCommit', 'for', 'revision', 'Sierra16E34']);
-                MockSubprocess.invocations[0].reject('Command failed');
-
+                assert.equal(invocations.length, 1);
+                assert.deepEqual(invocations[0].command, ['list', 'all osx 16Exx builds']);
+                invocations[0].resolve('\n\nSierra16E32\nSierra16E33\nSierra16E33h\nSierra16E34');
+                return MockSubprocess.resetAndWaitForInvocation();
+            }).then(() => {
+                assert.deepEqual(invocations[0].command, ['list', 'subCommit', 'for', 'revision', 'Sierra16E33h']);
+                invocations[0].resolve(JSON.stringify(anotherSubCommitWithWebKit));
+                return MockSubprocess.resetAndWaitForInvocation();
+            }).then(() => {
+                assert.equal(invocations.length, 1);
+                assert.deepEqual(invocations[0].command, ['list', 'subCommit', 'for', 'revision', 'Sierra16E34']);
+                invocations[0].reject('Command failed');
                 return fetchAndReportPromise.then(() => {
                     assert(false, 'should never be reached');
                 }, (error_output) => {
index 2256e2e28b431e1edb4b3541123d88743d1894c3..0eb2dc3d2b9ccebbb8e227e0341fa5826780f6e1 100644 (file)
@@ -2,6 +2,14 @@
 
 let assert = require('assert');
 
+function mapInSerialPromiseChain(list, callback)
+{
+    const results = [];
+    return list.reduce((chainedPromise, item) => {
+        return chainedPromise.then(() => callback(item)).then((result) => results.push(result));
+    }, Promise.resolve()).then(() => results);
+}
+
 class OSBuildFetcher {
 
     constructor(osConfig, remoteAPI, slaveAuth, subprocess, logger)
@@ -14,9 +22,15 @@ class OSBuildFetcher {
         this._subprocess = subprocess;
     }
 
+    static fetchAndReportAllInOrder(fetcherList)
+    {
+        return mapInSerialPromiseChain(fetcherList, (fetcher) => fetcher.fetchAndReportNewBuilds());
+    }
+
     fetchAndReportNewBuilds()
     {
-        return this._fetchAvailableBuilds().then((results) =>{
+        return this._fetchAvailableBuilds().then((results) => {
+            this._logger.log(`Submitting ${results.length} builds for ${this._osConfig['name']}`);
             return this._submitCommits(results);
         });
     }
@@ -27,24 +41,29 @@ class OSBuildFetcher {
         const repositoryName = config['name'];
         let customCommands = config['customCommands'];
 
-        return Promise.all(customCommands.map((command) => {
+        return mapInSerialPromiseChain(customCommands, (command) => {
             assert(command['minRevision']);
             assert(command['maxRevision']);
             const minRevisionOrder = this._computeOrder(command['minRevision']);
             const maxRevisionOrder = this._computeOrder(command['maxRevision']);
 
-            let fetchCommitsPromise = this._remoteAPI.getJSONWithStatus(`/api/commits/${escape(repositoryName)}/last-reported?from=${minRevisionOrder}&to=${maxRevisionOrder}`).then((result) => {
+            const url = `/api/commits/${escape(repositoryName)}/last-reported?from=${minRevisionOrder}&to=${maxRevisionOrder}`;
+
+            return this._remoteAPI.getJSONWithStatus(url).then((result) => {
                 const minOrder = result['commits'].length == 1 ? parseInt(result['commits'][0]['order']) : 0;
                 return this._commitsForAvailableBuilds(repositoryName, command['command'], command['linesToIgnore'], minOrder);
-            })
+            }).then((commits) => {
+                const label = 'name' in command ? `"${command['name']}"` : `"command['minRevision']" to "command['maxRevision']"`;
+                this._logger.log(`Found ${commits.length} builds for ${label}`);
 
-            if ('subCommitCommand' in command)
-                fetchCommitsPromise = fetchCommitsPromise.then((commits) => this._addSubCommitsForBuild(commits, command['subCommitCommand']));
+                if ('subCommitCommand' in command) {
+                    this._logger.log(`Resolving subcommits for "${label}"`);
+                    return this._addSubCommitsForBuild(commits, command['subCommitCommand']);
+                }
 
-            return fetchCommitsPromise;
-        })).then(results => {
-            return Array.prototype.concat.apply([], results);
-        });
+                return commits;
+            });
+        }).then((results) => [].concat(...results));
     }
 
     _computeOrder(revision)
@@ -65,27 +84,27 @@ class OSBuildFetcher {
             let lines = output.split('\n');
             if (linesToIgnore){
                 const regex = new RegExp(linesToIgnore);
-                lines = lines.filter(function(line) {return !regex.exec(line);});
+                lines = lines.filter((line) => !regex.exec(line));
             }
-            return lines.map(revision => ({repository, revision, 'order': this._computeOrder(revision)}))
-                .filter(commit => commit['order'] > minOrder);
+            return lines.map((revision) => ({repository, revision, 'order': this._computeOrder(revision)}))
+                .filter((commit) => commit['order'] > minOrder);
         });
     }
 
     _addSubCommitsForBuild(commits, command)
     {
-        return commits.reduce((promise, commit) => {
-            return promise.then(() => {
-                return this._subprocess.execute(command.concat(commit['revision']));
-            }).then((subCommitOutput) => {
+        return mapInSerialPromiseChain(commits, (commit) => {
+            return this._subprocess.execute(command.concat(commit['revision'])).then((subCommitOutput) => {
                 const subCommits = JSON.parse(subCommitOutput);
                 for (let repositoryName in subCommits) {
                     const subCommit = subCommits[repositoryName];
-                    assert(subCommit['revision']);
+                    assert.deepEqual(Object.keys(subCommit), ['revision']);
+                    assert(typeof(subCommit['revision']) == 'string');
                 }
                 commit['subCommits'] = subCommits;
+                return commit;
             });
-        }, Promise.resolve()).then(() => commits);
+        });
     }
 
     _submitCommits(commits)
@@ -94,5 +113,6 @@ class OSBuildFetcher {
         return this._remoteAPI.postJSONWithStatus('/api/report-commits/', commitsToReport);
     }
 }
+
 if (typeof module != 'undefined')
     module.exports.OSBuildFetcher = OSBuildFetcher;
index 8e5ab9c861542b2b6055fb1f5b2a4fbecff89a22..4edf0a6cdf8bb01279be885c7aa279c213afe6c4 100644 (file)
@@ -1,10 +1,10 @@
 'use strict';
-const childProcess = require('child_process').ChildProcess;
+const childProcess = require('child_process');
 
 class Subprocess {
     execute(command) {
         return new Promise((resolve, reject) => {
-            this._childProcess.execFile(command[0], command.slice(1), (error, stdout, stderr) => {
+            childProcess.execFile(command[0], command.slice(1), (error, stdout, stderr) => {
                 if (error)
                     reject(stderr);
                 else
similarity index 68%
rename from Websites/perf.webkit.org/tools/pull-os-versions.js
rename to Websites/perf.webkit.org/tools/sync-os-versions.js
index d99910a67b986bc72615ba5fbe43827cadba4955..77f4a0a7d34381943076b83ec48d5bc90b483586 100644 (file)
@@ -28,18 +28,17 @@ function syncLoop(options)
 
     const remoteAPI = new RemoteAPI(serverConfig.server);
 
-    Promise.all(osConfigList.map(osConfig => new OSBuildFetcher(osConfig, remoteAPI, serverConfig.slave, new Subprocess, console))).then((fetchers) => {
-        return fetchers.reduce((promise, fetcher) => {
-            return promise.then(() => fetcher.fetchAndReportNewBuilds());
-        }, Promise.resolve());
-    }).catch((error) => {
+    const fetchers = osConfigList.map((osConfig) => new OSBuildFetcher(osConfig, remoteAPI, serverConfig.slave, new Subprocess, console));
+    OSBuildFetcher.fetchAndReportAllInOrder(fetchers).catch((error) => {
         console.error(error);
         if (typeof(error.stack) == 'string') {
             for (let line of error.stack.split('\n'))
                 console.error(line);
         }
-    }).then(function () {
-        setTimeout(syncLoop.bind(global, options), options['--seconds-to-sleep'] * 1000);
+    }).then(() => {
+        const secondsToSleep = options['--seconds-to-sleep'];
+        console.log(`Sleeping for ${Math.floor(secondsToSleep / 3600)}h ${Math.floor(secondsToSleep / 60) % 60}m ${secondsToSleep % 60}s`);
+        setTimeout(() => syncLoop(options), secondsToSleep * 1000);
     });
 }