Escape builder names in url* and pathFor* methods of BuildbotSyncer
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 9 Apr 2016 04:56:42 +0000 (04:56 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 9 Apr 2016 04:56:42 +0000 (04:56 +0000)
https://bugs.webkit.org/show_bug.cgi?id=156427

Reviewed by Darin Adler.

The build fix in r199251 breaks other usage of RemoteAPI. Fix it properly by escaping builder names in
various methods of BuildbotSyncer.

Also fixed a typo in the logging and a bug that the new syncing script never updated "scheduled" to "running".

* server-tests/resources/mock-data.js:
(MockData.mockTestSyncConfigWithTwoBuilders): Renamed "some-builder-2" to "some builder 2" to test the
new escaping behavior in tools-buildbot-triggerable-tests.js and buildbot-syncer-tests.js.

* server-tests/tools-buildbot-triggerable-tests.js: Added tests for status url, and added a new test case
for updating "scheduled" to "running".

* tools/js/buildbot-syncer.js:
(BuildbotBuildEntry.buildRequestStatusIfUpdateIsNeeded): Update the status to "running" when the request's
status is "scheduled" and the buildbot's build is currently in progress.
(BuildbotSyncer.prototype.pathForPendingBuildsJSON): Escape the builder name.
(BuildbotSyncer.prototype.pathForBuildJSON): Ditto.
(BuildbotSyncer.prototype.pathForForceBuild): Ditto.
(BuildbotSyncer.prototype.url): Ditto.
(BuildbotSyncer.prototype.urlForBuildNumber): Ditto.

* tools/js/buildbot-triggerable.js:
(BuildbotTriggerable.prototype._pullBuildbotOnAllSyncers):
(BuildbotTriggerable.prototype._scheduleNextRequestInGroupIfSlaveIsAvailable): Fixed a typo. We are
scheduling new build requests, not syncing them.
* tools/js/remote.js:
(RemoteAPI.sendHttpRequest): Reverted r199251.
* unit-tests/buildbot-syncer-tests.js:

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

Websites/perf.webkit.org/ChangeLog
Websites/perf.webkit.org/server-tests/resources/mock-data.js
Websites/perf.webkit.org/server-tests/tools-buildbot-triggerable-tests.js
Websites/perf.webkit.org/tools/js/buildbot-syncer.js
Websites/perf.webkit.org/tools/js/buildbot-triggerable.js
Websites/perf.webkit.org/tools/js/remote.js
Websites/perf.webkit.org/unit-tests/buildbot-syncer-tests.js

index a547105..1105fab 100644 (file)
@@ -1,5 +1,41 @@
 2016-04-08  Ryosuke Niwa  <rniwa@webkit.org>
 
+        Escape builder names in url* and pathFor* methods of BuildbotSyncer
+        https://bugs.webkit.org/show_bug.cgi?id=156427
+
+        Reviewed by Darin Adler.
+
+        The build fix in r199251 breaks other usage of RemoteAPI. Fix it properly by escaping builder names in
+        various methods of BuildbotSyncer.
+
+        Also fixed a typo in the logging and a bug that the new syncing script never updated "scheduled" to "running".
+
+        * server-tests/resources/mock-data.js:
+        (MockData.mockTestSyncConfigWithTwoBuilders): Renamed "some-builder-2" to "some builder 2" to test the
+        new escaping behavior in tools-buildbot-triggerable-tests.js and buildbot-syncer-tests.js.
+
+        * server-tests/tools-buildbot-triggerable-tests.js: Added tests for status url, and added a new test case
+        for updating "scheduled" to "running".
+
+        * tools/js/buildbot-syncer.js:
+        (BuildbotBuildEntry.buildRequestStatusIfUpdateIsNeeded): Update the status to "running" when the request's
+        status is "scheduled" and the buildbot's build is currently in progress.
+        (BuildbotSyncer.prototype.pathForPendingBuildsJSON): Escape the builder name.
+        (BuildbotSyncer.prototype.pathForBuildJSON): Ditto.
+        (BuildbotSyncer.prototype.pathForForceBuild): Ditto.
+        (BuildbotSyncer.prototype.url): Ditto.
+        (BuildbotSyncer.prototype.urlForBuildNumber): Ditto.
+
+        * tools/js/buildbot-triggerable.js:
+        (BuildbotTriggerable.prototype._pullBuildbotOnAllSyncers):
+        (BuildbotTriggerable.prototype._scheduleNextRequestInGroupIfSlaveIsAvailable): Fixed a typo. We are
+        scheduling new build requests, not syncing them.
+        * tools/js/remote.js:
+        (RemoteAPI.sendHttpRequest): Reverted r199251.
+        * unit-tests/buildbot-syncer-tests.js:
+
+2016-04-08  Ryosuke Niwa  <rniwa@webkit.org>
+
         Build fix. We need to escape the path or http.request would fail.
 
         * tools/js/remote.js:
index 6c88681..3a1e64a 100644 (file)
@@ -96,7 +96,7 @@ MockData = {
                 {
                     'platform': 'some platform',
                     'test': ['some test'],
-                    'builder': 'some-builder-2',
+                    'builder': 'some builder 2',
                     'arguments': {
                         'wk': {'root': 'WebKit'},
                         'os': {'root': 'OS X'},
index 42c5ae3..45e3647 100644 (file)
@@ -158,7 +158,7 @@ describe('BuildbotTriggerable', function () {
                 assert.equal(MockRemoteAPI.requests[0].url, '/json/builders/some-builder-1/pendingBuilds');
                 MockRemoteAPI.requests[0].resolve([MockData.pendingBuild({buildRequestId: 999})]);
                 assert.equal(MockRemoteAPI.requests[1].method, 'GET');
-                assert.equal(MockRemoteAPI.requests[1].url, '/json/builders/some-builder-2/pendingBuilds');
+                assert.equal(MockRemoteAPI.requests[1].url, '/json/builders/some%20builder%202/pendingBuilds');
                 MockRemoteAPI.requests[1].resolve([]);
                 return MockRemoteAPI.waitForRequest();
             }).then(function () {
@@ -167,13 +167,13 @@ describe('BuildbotTriggerable', function () {
                 assert.equal(MockRemoteAPI.requests[2].url, '/json/builders/some-builder-1/builds/?select=-1&select=-2');
                 MockRemoteAPI.requests[2].resolve({});
                 assert.equal(MockRemoteAPI.requests[3].method, 'GET');
-                assert.equal(MockRemoteAPI.requests[3].url, '/json/builders/some-builder-2/builds/?select=-1&select=-2');
+                assert.equal(MockRemoteAPI.requests[3].url, '/json/builders/some%20builder%202/builds/?select=-1&select=-2');
                 MockRemoteAPI.requests[3].resolve({});
                 return MockRemoteAPI.waitForRequest();
             }).then(function () {
                 assert.equal(MockRemoteAPI.requests.length, 5);
                 assert.equal(MockRemoteAPI.requests[4].method, 'POST');
-                assert.equal(MockRemoteAPI.requests[4].url, '/builders/some-builder-2/force');
+                assert.equal(MockRemoteAPI.requests[4].url, '/builders/some%20builder%202/force');
                 assert.deepEqual(MockRemoteAPI.requests[4].data, {'wk': '191622', 'os': '10.11 15A284', 'build-request-id': '700'});
                 MockRemoteAPI.requests[4].resolve('OK');
                 return MockRemoteAPI.waitForRequest();
@@ -183,8 +183,8 @@ describe('BuildbotTriggerable', function () {
                 assert.equal(MockRemoteAPI.requests[5].url, '/json/builders/some-builder-1/pendingBuilds');
                 MockRemoteAPI.requests[5].resolve([MockData.pendingBuild({buildRequestId: 999})]);
                 assert.equal(MockRemoteAPI.requests[6].method, 'GET');
-                assert.equal(MockRemoteAPI.requests[6].url, '/json/builders/some-builder-2/pendingBuilds');
-                MockRemoteAPI.requests[6].resolve([MockData.pendingBuild({builder: 'some-builder-2', buildRequestId: 700})]);
+                assert.equal(MockRemoteAPI.requests[6].url, '/json/builders/some%20builder%202/pendingBuilds');
+                MockRemoteAPI.requests[6].resolve([MockData.pendingBuild({builder: 'some builder 2', buildRequestId: 700})]);
                 return MockRemoteAPI.waitForRequest();
             }).then(function () {
                 assert.equal(MockRemoteAPI.requests.length, 9);
@@ -192,22 +192,30 @@ describe('BuildbotTriggerable', function () {
                 assert.equal(MockRemoteAPI.requests[7].url, '/json/builders/some-builder-1/builds/?select=-1&select=-2');
                 MockRemoteAPI.requests[7].resolve({});
                 assert.equal(MockRemoteAPI.requests[8].method, 'GET');
-                assert.equal(MockRemoteAPI.requests[8].url, '/json/builders/some-builder-2/builds/?select=-1&select=-2');
+                assert.equal(MockRemoteAPI.requests[8].url, '/json/builders/some%20builder%202/builds/?select=-1&select=-2');
                 MockRemoteAPI.requests[8].resolve({});
                 return syncPromise;
             }).then(function () {
                 assert.equal(BuildRequest.all().length, 4);
                 assert.equal(BuildRequest.findById(700).status(), 'pending');
+                assert.equal(BuildRequest.findById(700).statusUrl(), null);
                 assert.equal(BuildRequest.findById(701).status(), 'pending');
+                assert.equal(BuildRequest.findById(701).statusUrl(), null);
                 assert.equal(BuildRequest.findById(702).status(), 'pending');
+                assert.equal(BuildRequest.findById(702).statusUrl(), null);
                 assert.equal(BuildRequest.findById(703).status(), 'pending');
+                assert.equal(BuildRequest.findById(703).statusUrl(), null);
                 return BuildRequest.fetchForTriggerable(MockData.mockTestSyncConfigWithTwoBuilders().triggerableName);
             }).then(function () {
                 assert.equal(BuildRequest.all().length, 4);
                 assert.equal(BuildRequest.findById(700).status(), 'scheduled');
+                assert.equal(BuildRequest.findById(700).statusUrl(), 'http://build.webkit.org/builders/some%20builder%202/');
                 assert.equal(BuildRequest.findById(701).status(), 'pending');
+                assert.equal(BuildRequest.findById(701).statusUrl(), null);
                 assert.equal(BuildRequest.findById(702).status(), 'pending');
+                assert.equal(BuildRequest.findById(702).statusUrl(), null);
                 assert.equal(BuildRequest.findById(703).status(), 'pending');
+                assert.equal(BuildRequest.findById(703).statusUrl(), null);
                 done();
             }).catch(done);
         });
@@ -233,7 +241,7 @@ describe('BuildbotTriggerable', function () {
                 assert.equal(MockRemoteAPI.requests[0].url, '/json/builders/some-builder-1/pendingBuilds');
                 MockRemoteAPI.requests[0].resolve([MockData.pendingBuild({buildRequestId: 700})]);
                 assert.equal(MockRemoteAPI.requests[1].method, 'GET');
-                assert.equal(MockRemoteAPI.requests[1].url, '/json/builders/some-builder-2/pendingBuilds');
+                assert.equal(MockRemoteAPI.requests[1].url, '/json/builders/some%20builder%202/pendingBuilds');
                 MockRemoteAPI.requests[1].resolve([]);
                 return MockRemoteAPI.waitForRequest();
             }).then(function () {
@@ -242,7 +250,7 @@ describe('BuildbotTriggerable', function () {
                 assert.equal(MockRemoteAPI.requests[2].url, '/json/builders/some-builder-1/builds/?select=-1&select=-2');
                 MockRemoteAPI.requests[2].resolve({});
                 assert.equal(MockRemoteAPI.requests[3].method, 'GET');
-                assert.equal(MockRemoteAPI.requests[3].url, '/json/builders/some-builder-2/builds/?select=-1&select=-2');
+                assert.equal(MockRemoteAPI.requests[3].url, '/json/builders/some%20builder%202/builds/?select=-1&select=-2');
                 MockRemoteAPI.requests[3].resolve({});
                 return MockRemoteAPI.waitForRequest();
             }).then(function () {
@@ -251,7 +259,7 @@ describe('BuildbotTriggerable', function () {
                 assert.equal(MockRemoteAPI.requests[4].url, '/json/builders/some-builder-1/pendingBuilds');
                 MockRemoteAPI.requests[4].resolve([MockData.pendingBuild({buildRequestId: 700})]);
                 assert.equal(MockRemoteAPI.requests[5].method, 'GET');
-                assert.equal(MockRemoteAPI.requests[5].url, '/json/builders/some-builder-2/pendingBuilds');
+                assert.equal(MockRemoteAPI.requests[5].url, '/json/builders/some%20builder%202/pendingBuilds');
                 MockRemoteAPI.requests[5].resolve([]);
                 return MockRemoteAPI.waitForRequest();
             }).then(function () {
@@ -260,22 +268,30 @@ describe('BuildbotTriggerable', function () {
                 assert.equal(MockRemoteAPI.requests[6].url, '/json/builders/some-builder-1/builds/?select=-1&select=-2');
                 MockRemoteAPI.requests[6].resolve({});
                 assert.equal(MockRemoteAPI.requests[7].method, 'GET');
-                assert.equal(MockRemoteAPI.requests[7].url, '/json/builders/some-builder-2/builds/?select=-1&select=-2');
+                assert.equal(MockRemoteAPI.requests[7].url, '/json/builders/some%20builder%202/builds/?select=-1&select=-2');
                 MockRemoteAPI.requests[7].resolve({});
                 return syncPromise;
             }).then(function () {
                 assert.equal(BuildRequest.all().length, 4);
                 assert.equal(BuildRequest.findById(700).status(), 'pending');
+                assert.equal(BuildRequest.findById(700).statusUrl(), null);
                 assert.equal(BuildRequest.findById(701).status(), 'pending');
+                assert.equal(BuildRequest.findById(701).statusUrl(), null);
                 assert.equal(BuildRequest.findById(702).status(), 'pending');
+                assert.equal(BuildRequest.findById(702).statusUrl(), null);
                 assert.equal(BuildRequest.findById(703).status(), 'pending');
+                assert.equal(BuildRequest.findById(703).statusUrl(), null);
                 return BuildRequest.fetchForTriggerable(MockData.mockTestSyncConfigWithTwoBuilders().triggerableName);
             }).then(function () {
                 assert.equal(BuildRequest.all().length, 4);
                 assert.equal(BuildRequest.findById(700).status(), 'scheduled');
+                assert.equal(BuildRequest.findById(700).statusUrl(), 'http://build.webkit.org/builders/some-builder-1/');
                 assert.equal(BuildRequest.findById(701).status(), 'pending');
+                assert.equal(BuildRequest.findById(701).statusUrl(), null);
                 assert.equal(BuildRequest.findById(702).status(), 'pending');
+                assert.equal(BuildRequest.findById(702).statusUrl(), null);
                 assert.equal(BuildRequest.findById(703).status(), 'pending');
+                assert.equal(BuildRequest.findById(703).statusUrl(), null);
                 done();
             }).catch(done);
         });
@@ -301,7 +317,7 @@ describe('BuildbotTriggerable', function () {
                 assert.equal(MockRemoteAPI.requests[0].url, '/json/builders/some-builder-1/pendingBuilds');
                 MockRemoteAPI.requests[0].resolve([]);
                 assert.equal(MockRemoteAPI.requests[1].method, 'GET');
-                assert.equal(MockRemoteAPI.requests[1].url, '/json/builders/some-builder-2/pendingBuilds');
+                assert.equal(MockRemoteAPI.requests[1].url, '/json/builders/some%20builder%202/pendingBuilds');
                 MockRemoteAPI.requests[1].resolve([]);
                 return MockRemoteAPI.waitForRequest();
             }).then(function () {
@@ -310,7 +326,7 @@ describe('BuildbotTriggerable', function () {
                 assert.equal(MockRemoteAPI.requests[2].url, '/json/builders/some-builder-1/builds/?select=-1&select=-2');
                 MockRemoteAPI.requests[2].resolve({[-1]: MockData.runningBuild({buildRequestId: 701}), [-2]: MockData.finishedBuild({buildRequestId: 700})});
                 assert.equal(MockRemoteAPI.requests[3].method, 'GET');
-                assert.equal(MockRemoteAPI.requests[3].url, '/json/builders/some-builder-2/builds/?select=-1&select=-2');
+                assert.equal(MockRemoteAPI.requests[3].url, '/json/builders/some%20builder%202/builds/?select=-1&select=-2');
                 MockRemoteAPI.requests[3].resolve({});
                 return MockRemoteAPI.waitForRequest();
             }).then(function () {
@@ -326,7 +342,7 @@ describe('BuildbotTriggerable', function () {
                 assert.equal(MockRemoteAPI.requests[5].url, '/json/builders/some-builder-1/pendingBuilds');
                 MockRemoteAPI.requests[5].resolve([MockData.pendingBuild({buildRequestId: 702})]);
                 assert.equal(MockRemoteAPI.requests[6].method, 'GET');
-                assert.equal(MockRemoteAPI.requests[6].url, '/json/builders/some-builder-2/pendingBuilds');
+                assert.equal(MockRemoteAPI.requests[6].url, '/json/builders/some%20builder%202/pendingBuilds');
                 MockRemoteAPI.requests[6].resolve([]);
                 return MockRemoteAPI.waitForRequest();
             }).then(function () {
@@ -335,22 +351,94 @@ describe('BuildbotTriggerable', function () {
                 assert.equal(MockRemoteAPI.requests[7].url, '/json/builders/some-builder-1/builds/?select=-1&select=-2');
                 MockRemoteAPI.requests[7].resolve({[-1]: MockData.runningBuild({buildRequestId: 701}), [-2]: MockData.finishedBuild({buildRequestId: 700})});
                 assert.equal(MockRemoteAPI.requests[8].method, 'GET');
-                assert.equal(MockRemoteAPI.requests[8].url, '/json/builders/some-builder-2/builds/?select=-1&select=-2');
+                assert.equal(MockRemoteAPI.requests[8].url, '/json/builders/some%20builder%202/builds/?select=-1&select=-2');
                 MockRemoteAPI.requests[8].resolve({});
                 return syncPromise;
             }).then(function () {
                 assert.equal(BuildRequest.all().length, 4);
                 assert.equal(BuildRequest.findById(700).status(), 'pending');
+                assert.equal(BuildRequest.findById(700).statusUrl(), null);
                 assert.equal(BuildRequest.findById(701).status(), 'pending');
+                assert.equal(BuildRequest.findById(701).statusUrl(), null);
                 assert.equal(BuildRequest.findById(702).status(), 'pending');
+                assert.equal(BuildRequest.findById(702).statusUrl(), null);
                 assert.equal(BuildRequest.findById(703).status(), 'pending');
+                assert.equal(BuildRequest.findById(703).statusUrl(), null);
                 return BuildRequest.fetchForTriggerable(MockData.mockTestSyncConfigWithTwoBuilders().triggerableName);
             }).then(function () {
                 assert.equal(BuildRequest.all().length, 4);
                 assert.equal(BuildRequest.findById(700).status(), 'failed');
+                assert.equal(BuildRequest.findById(700).statusUrl(), 'http://build.webkit.org/builders/some-builder-1/builds/123');
                 assert.equal(BuildRequest.findById(701).status(), 'running');
+                assert.equal(BuildRequest.findById(701).statusUrl(), 'http://build.webkit.org/builders/some-builder-1/builds/124');
                 assert.equal(BuildRequest.findById(702).status(), 'scheduled');
+                assert.equal(BuildRequest.findById(702).statusUrl(), 'http://build.webkit.org/builders/some-builder-1/');
                 assert.equal(BuildRequest.findById(703).status(), 'pending');
+                assert.equal(BuildRequest.findById(703).statusUrl(), null);
+                done();
+            }).catch(done);
+        });
+
+        it('should update the status of a scheduled build if the pending build had started running', function (done) {
+            let db = TestServer.database();
+            let syncPromise;
+            db.connect().then(function () {
+                return MockData.addMockData(db, ['scheduled', 'pending', 'pending', 'pending']);
+            }).then(function () {
+                return Manifest.fetch();
+            }).then(function () {
+                let config = MockData.mockTestSyncConfigWithSingleBuilder();
+                let logger = new MockLogger;
+                let slaveInfo = {name: 'sync-slave', password: 'password'};
+                let triggerable = new BuildbotTriggerable(config, TestServer.remoteAPI(), MockRemoteAPI, slaveInfo, logger);
+                syncPromise = triggerable.syncOnce();
+                syncPromise.catch(done);
+                return MockRemoteAPI.waitForRequest();
+            }).then(function () {
+                assert.equal(MockRemoteAPI.requests.length, 1);
+                assert.equal(MockRemoteAPI.requests[0].method, 'GET');
+                assert.equal(MockRemoteAPI.requests[0].url, '/json/builders/some-builder-1/pendingBuilds');
+                MockRemoteAPI.requests[0].resolve([MockData.pendingBuild({buildRequestId: 700})]);
+                return MockRemoteAPI.waitForRequest();
+            }).then(function () {
+                assert.equal(MockRemoteAPI.requests.length, 2);
+                assert.equal(MockRemoteAPI.requests[1].method, 'GET');
+                assert.equal(MockRemoteAPI.requests[1].url, '/json/builders/some-builder-1/builds/?select=-1&select=-2');
+                MockRemoteAPI.requests[1].resolve({});
+                return MockRemoteAPI.waitForRequest();
+            }).then(function () {
+                assert.equal(MockRemoteAPI.requests.length, 3);
+                assert.equal(MockRemoteAPI.requests[2].method, 'GET');
+                assert.equal(MockRemoteAPI.requests[2].url, '/json/builders/some-builder-1/pendingBuilds');
+                MockRemoteAPI.requests[2].resolve([]);
+                return MockRemoteAPI.waitForRequest();
+            }).then(function () {
+                assert.equal(MockRemoteAPI.requests.length, 4);
+                assert.equal(MockRemoteAPI.requests[3].method, 'GET');
+                assert.equal(MockRemoteAPI.requests[3].url, '/json/builders/some-builder-1/builds/?select=-1&select=-2');
+                MockRemoteAPI.requests[3].resolve({[-1]: MockData.runningBuild({buildRequestId: 700})});
+                return syncPromise;
+            }).then(function () {
+                assert.equal(BuildRequest.all().length, 4);
+                assert.equal(BuildRequest.findById(700).status(), 'scheduled');
+                assert.equal(BuildRequest.findById(700).statusUrl(), null);
+                assert.equal(BuildRequest.findById(701).status(), 'pending');
+                assert.equal(BuildRequest.findById(701).statusUrl(), null);
+                assert.equal(BuildRequest.findById(702).status(), 'pending');
+                assert.equal(BuildRequest.findById(702).statusUrl(), null);
+                assert.equal(BuildRequest.findById(703).status(), 'pending');
+                assert.equal(BuildRequest.findById(703).statusUrl(), null);
+                return BuildRequest.fetchForTriggerable(MockData.mockTestSyncConfigWithTwoBuilders().triggerableName);
+            }).then(function () {
+                assert.equal(BuildRequest.all().length, 4);
+                assert.equal(BuildRequest.findById(700).status(), 'running');
+                assert.equal(BuildRequest.findById(700).statusUrl(), 'http://build.webkit.org/builders/some-builder-1/builds/124');
+                assert.equal(BuildRequest.findById(701).status(), 'pending');
+                assert.equal(BuildRequest.findById(701).statusUrl(), null);
+                assert.equal(BuildRequest.findById(702).status(), 'pending');
+                assert.equal(BuildRequest.findById(702).statusUrl(), null);
+                assert.equal(BuildRequest.findById(703).status(), 'pending');
+                assert.equal(BuildRequest.findById(703).statusUrl(), null);
                 done();
             }).catch(done);
         });
@@ -379,7 +467,7 @@ describe('BuildbotTriggerable', function () {
                 assert.equal(MockRemoteAPI.requests[0].url, '/json/builders/some-builder-1/pendingBuilds');
                 MockRemoteAPI.requests[0].resolve([MockData.pendingBuild({buildRequestId: 702})]);
                 assert.equal(MockRemoteAPI.requests[1].method, 'GET');
-                assert.equal(MockRemoteAPI.requests[1].url, '/json/builders/some-builder-2/pendingBuilds');
+                assert.equal(MockRemoteAPI.requests[1].url, '/json/builders/some%20builder%202/pendingBuilds');
                 MockRemoteAPI.requests[1].resolve([]);
                 return MockRemoteAPI.waitForRequest();
             }).then(function () {
@@ -388,13 +476,13 @@ describe('BuildbotTriggerable', function () {
                 assert.equal(MockRemoteAPI.requests[2].url, '/json/builders/some-builder-1/builds/?select=-1&select=-2');
                 MockRemoteAPI.requests[2].resolve({[-1]: MockData.runningBuild({buildRequestId: 701}), [-2]: MockData.finishedBuild({buildRequestId: 700})});
                 assert.equal(MockRemoteAPI.requests[3].method, 'GET');
-                assert.equal(MockRemoteAPI.requests[3].url, '/json/builders/some-builder-2/builds/?select=-1&select=-2');
+                assert.equal(MockRemoteAPI.requests[3].url, '/json/builders/some%20builder%202/builds/?select=-1&select=-2');
                 MockRemoteAPI.requests[3].resolve({});
                 return MockRemoteAPI.waitForRequest();
             }).then(function () {
                 assert.equal(MockRemoteAPI.requests.length, 5);
                 assert.equal(MockRemoteAPI.requests[4].method, 'POST');
-                assert.equal(MockRemoteAPI.requests[4].url, '/builders/some-builder-2/force');
+                assert.equal(MockRemoteAPI.requests[4].url, '/builders/some%20builder%202/force');
                 assert.deepEqual(MockRemoteAPI.requests[4].data, {'wk': '191622', 'os': '10.11 15A284', 'build-request-id': '710'});
                 MockRemoteAPI.requests[4].resolve('OK');
                 return MockRemoteAPI.waitForRequest();
@@ -404,8 +492,8 @@ describe('BuildbotTriggerable', function () {
                 assert.equal(MockRemoteAPI.requests[5].url, '/json/builders/some-builder-1/pendingBuilds');
                 MockRemoteAPI.requests[5].resolve([MockData.pendingBuild({buildRequestId: 702})]);
                 assert.equal(MockRemoteAPI.requests[6].method, 'GET');
-                assert.equal(MockRemoteAPI.requests[6].url, '/json/builders/some-builder-2/pendingBuilds');
-                MockRemoteAPI.requests[6].resolve([MockData.pendingBuild({builder: 'some-builder-2', buildRequestId: 710})]);
+                assert.equal(MockRemoteAPI.requests[6].url, '/json/builders/some%20builder%202/pendingBuilds');
+                MockRemoteAPI.requests[6].resolve([MockData.pendingBuild({builder: 'some builder 2', buildRequestId: 710})]);
                 return MockRemoteAPI.waitForRequest();
             }).then(function () {
                 assert.equal(MockRemoteAPI.requests.length, 9);
@@ -413,30 +501,46 @@ describe('BuildbotTriggerable', function () {
                 assert.equal(MockRemoteAPI.requests[7].url, '/json/builders/some-builder-1/builds/?select=-1&select=-2');
                 MockRemoteAPI.requests[7].resolve({[-1]: MockData.runningBuild({buildRequestId: 701}), [-2]: MockData.finishedBuild({buildRequestId: 700})});
                 assert.equal(MockRemoteAPI.requests[8].method, 'GET');
-                assert.equal(MockRemoteAPI.requests[8].url, '/json/builders/some-builder-2/builds/?select=-1&select=-2');
+                assert.equal(MockRemoteAPI.requests[8].url, '/json/builders/some%20builder%202/builds/?select=-1&select=-2');
                 MockRemoteAPI.requests[8].resolve({});
                 return syncPromise;
             }).then(function () {
                 assert.equal(BuildRequest.all().length, 8);
                 assert.equal(BuildRequest.findById(700).status(), 'completed');
+                assert.equal(BuildRequest.findById(700).statusUrl(), null);
                 assert.equal(BuildRequest.findById(701).status(), 'pending');
+                assert.equal(BuildRequest.findById(701).statusUrl(), null);
                 assert.equal(BuildRequest.findById(702).status(), 'pending');
+                assert.equal(BuildRequest.findById(702).statusUrl(), null);
                 assert.equal(BuildRequest.findById(703).status(), 'pending');
+                assert.equal(BuildRequest.findById(703).statusUrl(), null);
                 assert.equal(BuildRequest.findById(710).status(), 'pending');
+                assert.equal(BuildRequest.findById(710).statusUrl(), null);
                 assert.equal(BuildRequest.findById(711).status(), 'pending');
+                assert.equal(BuildRequest.findById(711).statusUrl(), null);
                 assert.equal(BuildRequest.findById(712).status(), 'pending');
+                assert.equal(BuildRequest.findById(712).statusUrl(), null);
                 assert.equal(BuildRequest.findById(713).status(), 'pending');
+                assert.equal(BuildRequest.findById(713).statusUrl(), null);
                 return BuildRequest.fetchForTriggerable(MockData.mockTestSyncConfigWithTwoBuilders().triggerableName);
             }).then(function () {
                 assert.equal(BuildRequest.all().length, 8);
                 assert.equal(BuildRequest.findById(700).status(), 'completed');
+                assert.equal(BuildRequest.findById(700).statusUrl(), 'http://build.webkit.org/builders/some-builder-1/builds/123');
                 assert.equal(BuildRequest.findById(701).status(), 'running');
+                assert.equal(BuildRequest.findById(701).statusUrl(), 'http://build.webkit.org/builders/some-builder-1/builds/124');
                 assert.equal(BuildRequest.findById(702).status(), 'scheduled');
+                assert.equal(BuildRequest.findById(702).statusUrl(), 'http://build.webkit.org/builders/some-builder-1/');
                 assert.equal(BuildRequest.findById(703).status(), 'pending');
+                assert.equal(BuildRequest.findById(703).statusUrl(), null);
                 assert.equal(BuildRequest.findById(710).status(), 'scheduled');
+                assert.equal(BuildRequest.findById(710).statusUrl(), 'http://build.webkit.org/builders/some%20builder%202/');
                 assert.equal(BuildRequest.findById(711).status(), 'pending');
+                assert.equal(BuildRequest.findById(711).statusUrl(), null);
                 assert.equal(BuildRequest.findById(712).status(), 'pending');
+                assert.equal(BuildRequest.findById(712).statusUrl(), null);
                 assert.equal(BuildRequest.findById(713).status(), 'pending');
+                assert.equal(BuildRequest.findById(713).statusUrl(), null);
                 done();
             }).catch(done);
         });
index 5d94ebd..465ffba 100644 (file)
@@ -44,7 +44,7 @@ class BuildbotBuildEntry {
             if (request.isPending())
                 return 'scheduled';
         } else if (this.isInProgress()) {
-            if (!request.hasStarted())
+            if (!request.hasStarted() || request.isScheduled())
                 return 'running';
         } else if (this.hasFinished()) {
             if (!request.hasFinished())
@@ -179,16 +179,16 @@ class BuildbotSyncer {
         });
     }
 
-    pathForPendingBuildsJSON() { return `/json/builders/${this._builderName}/pendingBuilds`; }
+    pathForPendingBuildsJSON() { return `/json/builders/${escape(this._builderName)}/pendingBuilds`; }
     pathForBuildJSON(selectedBuilds)
     {
-        return `/json/builders/${this._builderName}/builds/?`
+        return `/json/builders/${escape(this._builderName)}/builds/?`
             + selectedBuilds.map(function (number) { return 'select=' + number; }).join('&');
     }
-    pathForForceBuild() { return `/builders/${this._builderName}/force`; }
+    pathForForceBuild() { return `/builders/${escape(this._builderName)}/force`; }
 
-    url() { return this._remote.url(`/builders/${this._builderName}/`); }
-    urlForBuildNumber(number) { return this._remote.url(`/builders/${this._builderName}/builds/${number}`); }
+    url() { return this._remote.url(`/builders/${escape(this._builderName)}/`); }
+    urlForBuildNumber(number) { return this._remote.url(`/builders/${escape(this._builderName)}/builds/${number}`); }
 
     _propertiesForBuildRequest(buildRequest)
     {
index e649511..95039dd 100644 (file)
@@ -102,12 +102,16 @@ class BuildbotTriggerable {
                     if (newStatus) {
                         self._logger.log(`Updating the status of build request ${request.id()} from ${request.status()} to ${newStatus}`);
                         updates[entry.buildRequestId()] = {status: newStatus, url: entry.url()};
+                    } else if (!request.statusUrl()) {
+                        self._logger.log(`Setting the status URL of build request ${request.id()} to ${entry.url()}`);
+                        updates[entry.buildRequestId()] = {status: request.status(), url: entry.url()};
                     }
                 }
             });
         })).then(function () {
             for (let request of BuildRequest.all()) {
                 if (request.hasStarted() && !request.hasFinished() && !associatedRequests.has(request)) {
+                    self._logger.log(`Updating the status of build request ${request.id()} from ${request.status()} to failedIfNotCompleted`);
                     assert(!(request.id() in updates));
                     updates[request.id()] = {status: 'failedIfNotCompleted'};
                 }
@@ -132,7 +136,7 @@ class BuildbotTriggerable {
 
         let firstRequest = !nextRequest.order();
         if (firstRequest) {
-            this._logger.log(`Syncing build request ${nextRequest.id()} on ${groupInfo.slaveName} in ${groupInfo.syncer.builderName()}`);
+            this._logger.log(`Scheduling build request ${nextRequest.id()} on ${groupInfo.slaveName} in ${groupInfo.syncer.builderName()}`);
             return groupInfo.syncer.scheduleRequest(request, groupInfo.slaveName);
         }
 
@@ -140,7 +144,7 @@ class BuildbotTriggerable {
             let promise = syncer.scheduleFirstRequestInGroupIfAvailable(nextRequest);
             if (promise) {
                 let slaveName = groupInfo.slaveName ? ` on ${groupInfo.slaveName}` : '';
-                this._logger.log(`Syncing build request ${nextRequest.id()}${slaveName} in ${syncer.builderName()}`);
+                this._logger.log(`Scheduling build request ${nextRequest.id()}${slaveName} in ${syncer.builderName()}`);
                 return promise;
             }
         }
index d251019..e6de4f1 100644 (file)
@@ -93,7 +93,7 @@ class RemoteAPI {
                 port: server.port,
                 auth: server.auth ? server.auth.username + ':' + server.auth.password : null,
                 method: method,
-                path: escape(path),
+                path: path,
             };
 
             let request = (server.scheme == 'http' ? http : https).request(options, function (response) {
index 5bb8ccf..9bd4dbf 100644 (file)
@@ -850,7 +850,7 @@ describe('BuildbotSyncer', function () {
                 syncer.scheduleFirstRequestInGroupIfAvailable(createSampleBuildRequest(MockModels.somePlatform, MockModels.someTest));
             }).then(function () {
                 assert.equal(requests.length, 1);
-                assert.equal(requests[0].url, '/builders/some builder/force');
+                assert.equal(requests[0].url, '/builders/some%20builder/force');
                 assert.equal(requests[0].method, 'POST');
                 assert.deepEqual(requests[0].data, {id: '16733-' + MockModels.somePlatform.id()});
                 done();
@@ -864,7 +864,7 @@ describe('BuildbotSyncer', function () {
                 syncer.scheduleFirstRequestInGroupIfAvailable(createSampleBuildRequest(MockModels.somePlatform, MockModels.someTest));
             }).then(function () {
                 assert.equal(requests.length, 1);
-                assert.equal(requests[0].url, '/builders/some builder/force');
+                assert.equal(requests[0].url, '/builders/some%20builder/force');
                 assert.equal(requests[0].method, 'POST');
                 assert.deepEqual(requests[0].data, {id: '16733-' + MockModels.somePlatform.id()});
                 done();