Diff
Modified: trunk/Websites/perf.webkit.org/ChangeLog (199387 => 199388)
--- trunk/Websites/perf.webkit.org/ChangeLog 2016-04-12 21:03:52 UTC (rev 199387)
+++ trunk/Websites/perf.webkit.org/ChangeLog 2016-04-12 21:04:09 UTC (rev 199388)
@@ -1,3 +1,29 @@
+2016-04-12 Ryosuke Niwa <[email protected]>
+
+ Make sync-buildbot.js fault safe
+ https://bugs.webkit.org/show_bug.cgi?id=156498
+
+ Reviewed by Chris Dumez.
+
+ Fixed a bug that sync-buildbot.js will continue to schedule build requests from multiple test groups
+ if multiple test groups are simultaneously in-progress on the same builder. Also fixed a bug that if
+ a build request had failed without leaving a trace (i.e. no entry on any of the builders we know of),
+ sync-buildbot.js throws an exception.
+
+ * server-tests/tools-buildbot-triggerable-tests.js: Added test cases.
+ * tools/js/buildbot-syncer.js:
+ (BuildbotSyncer.prototype.scheduleRequestInGroupIfAvailable): Renamed. Optionally takes the slave name.
+ When this parameter is specified, schedule the request only if the specified slave is available.
+ * tools/js/buildbot-triggerable.js:
+ (BuildbotTriggerable.prototype._scheduleNextRequestInGroupIfSlaveIsAvailable): Always use
+ scheduleRequestInGroupIfAvailable to schedule a new build request. Using scheduleRequest for non-first
+ build requests was problematic when there were multiple test groups with pending requests because then
+ we would schedule those pending requests without checking whether there is already a pending job or if
+ we have previously scheduled a job. Also fallback to use any syncer / builder when groupInfo.syncer is
+ not set even if the next request was not the first one in the test group since we can't determine on
+ which builder preceding requests are processed in such cases.
+ * unit-tests/buildbot-syncer-tests.js:
+
2016-04-11 Ryosuke Niwa <[email protected]>
Replace script runner to use mocha.js tests
Modified: trunk/Websites/perf.webkit.org/server-tests/tools-buildbot-triggerable-tests.js (199387 => 199388)
--- trunk/Websites/perf.webkit.org/server-tests/tools-buildbot-triggerable-tests.js 2016-04-12 21:03:52 UTC (rev 199387)
+++ trunk/Websites/perf.webkit.org/server-tests/tools-buildbot-triggerable-tests.js 2016-04-12 21:04:09 UTC (rev 199388)
@@ -651,6 +651,161 @@
}).catch(done);
});
+ it('should recover from multiple test groups running simultenously', function (done) {
+ let db = TestServer.database();
+ let syncPromise;
+ db.connect().then(function () {
+ return Promise.all([
+ MockData.addMockData(db, ['completed', 'pending', 'pending', 'pending']),
+ MockData.addAnotherMockTestGroup(db, ['completed', '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: 711})]);
+ 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({[-1]: MockData.runningBuild({buildRequestId: 700}), [-2]: MockData.finishedBuild({buildRequestId: 710})});
+ 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([MockData.pendingBuild({buildRequestId: 701})]);
+ 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}), [-2]: MockData.finishedBuild({buildRequestId: 710})});
+ 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(), 'completed');
+ 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/124');
+ assert.equal(BuildRequest.findById(701).status(), 'scheduled');
+ assert.equal(BuildRequest.findById(701).statusUrl(), 'http://build.webkit.org/builders/some-builder-1/');
+ 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(), 'completed');
+ assert.equal(BuildRequest.findById(710).statusUrl(), 'http://build.webkit.org/builders/some-builder-1/builds/123');
+ 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);
+ });
+
+ it('should recover from missing failed build request', function (done) {
+ let db = TestServer.database();
+ let syncPromise;
+ db.connect().then(function () {
+ return Promise.all([
+ MockData.addMockData(db, ['failed', '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([]);
+ 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, 'POST');
+ assert.equal(MockRemoteAPI.requests[2].url, '/builders/some-builder-1/force');
+ assert.deepEqual(MockRemoteAPI.requests[2].data, {'wk': '192736', 'os': '10.11 15A284', 'build-request-id': '701'});
+ MockRemoteAPI.requests[2].resolve('OK');
+ }).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/pendingBuilds');
+ MockRemoteAPI.requests[3].resolve([MockData.pendingBuild({buildRequestId: 701})]);
+ return MockRemoteAPI.waitForRequest();
+ }).then(function () {
+ assert.equal(MockRemoteAPI.requests.length, 5);
+ assert.equal(MockRemoteAPI.requests[4].method, 'GET');
+ assert.equal(MockRemoteAPI.requests[4].url, '/json/builders/some-builder-1/builds/?select=-1&select=-2');
+ MockRemoteAPI.requests[4].resolve({});
+ return syncPromise;
+ }).then(function () {
+ assert.equal(BuildRequest.all().length, 4);
+ assert.equal(BuildRequest.findById(700).status(), 'failed');
+ 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(), null);
+ assert.equal(BuildRequest.findById(701).status(), 'scheduled');
+ assert.equal(BuildRequest.findById(701).statusUrl(), 'http://build.webkit.org/builders/some-builder-1/');
+ 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);
+ });
+
it('should update the status of a supposedly scheduled build that went missing', function (done) {
let db = TestServer.database();
let syncPromise;
Modified: trunk/Websites/perf.webkit.org/tools/js/buildbot-syncer.js (199387 => 199388)
--- trunk/Websites/perf.webkit.org/tools/js/buildbot-syncer.js 2016-04-12 21:03:52 UTC (rev 199387)
+++ trunk/Websites/perf.webkit.org/tools/js/buildbot-syncer.js 2016-04-12 21:04:09 UTC (rev 199388)
@@ -101,7 +101,7 @@
return this._remote.postFormUrlencodedData(this.pathForForceBuild(), properties);
}
- scheduleFirstRequestInGroupIfAvailable(newRequest)
+ scheduleRequestInGroupIfAvailable(newRequest, slaveName)
{
assert(newRequest instanceof BuildRequest);
@@ -124,6 +124,12 @@
return this.scheduleRequest(newRequest, null);
}
+ if (slaveName) {
+ if (!usedSlaves.has(slaveName) && !this._slavesWithNewRequests.has(slaveName))
+ return this.scheduleRequest(newRequest, slaveName);
+ return null;
+ }
+
for (let slaveName of this._slaveList) {
if (!usedSlaves.has(slaveName) && !this._slavesWithNewRequests.has(slaveName))
return this.scheduleRequest(newRequest, slaveName);
Modified: trunk/Websites/perf.webkit.org/tools/js/buildbot-triggerable.js (199387 => 199388)
--- trunk/Websites/perf.webkit.org/tools/js/buildbot-triggerable.js 2016-04-12 21:03:52 UTC (rev 199387)
+++ trunk/Websites/perf.webkit.org/tools/js/buildbot-triggerable.js 2016-04-12 21:04:09 UTC (rev 199388)
@@ -132,19 +132,30 @@
if (!nextRequest)
return null;
+ let promise;
+ let syncer;
if (!!nextRequest.order()) {
- this._logger.log(`Scheduling build request ${nextRequest.id()} on ${groupInfo.slaveName} in ${groupInfo.syncer.builderName()}`);
- return groupInfo.syncer.scheduleRequest(nextRequest, groupInfo.slaveName);
+ syncer = groupInfo.syncer;
+ if (!syncer)
+ this._logger.error(`Could not identify the syncer for ${nextRequest.id()}.`);
+ else
+ promise = syncer.scheduleRequestInGroupIfAvailable(nextRequest, groupInfo.slaveName);
}
- for (let syncer of this._syncers) {
- let promise = syncer.scheduleFirstRequestInGroupIfAvailable(nextRequest);
- if (promise) {
- let slaveName = groupInfo.slaveName ? ` on ${groupInfo.slaveName}` : '';
- this._logger.log(`Scheduling build request ${nextRequest.id()}${slaveName} in ${syncer.builderName()}`);
- return promise;
+ if (!syncer) {
+ for (syncer of this._syncers) {
+ let promise = syncer.scheduleRequestInGroupIfAvailable(nextRequest);
+ if (promise)
+ break;
}
}
+
+ if (promise) {
+ let slaveName = groupInfo.slaveName ? ` on ${groupInfo.slaveName}` : '';
+ this._logger.log(`Scheduling build request ${nextRequest.id()}${slaveName} in ${syncer.builderName()}`);
+ return promise;
+ }
+
return null;
}
Modified: trunk/Websites/perf.webkit.org/unit-tests/buildbot-syncer-tests.js (199387 => 199388)
--- trunk/Websites/perf.webkit.org/unit-tests/buildbot-syncer-tests.js 2016-04-12 21:03:52 UTC (rev 199387)
+++ trunk/Websites/perf.webkit.org/unit-tests/buildbot-syncer-tests.js 2016-04-12 21:04:09 UTC (rev 199388)
@@ -827,7 +827,7 @@
});
});
- describe('scheduleFirstRequestInGroupIfAvailable', function () {
+ describe('scheduleRequestInGroupIfAvailable', function () {
function pullBuildbotWithAssertion(syncer, pendingBuilds, inProgressAndFinishedBuilds)
{
@@ -847,7 +847,7 @@
let syncer = BuildbotSyncer._loadConfig(MockRemoteAPI, {'configurations': [smallConfiguration()]})[0];
pullBuildbotWithAssertion(syncer, [], {}).then(function () {
- syncer.scheduleFirstRequestInGroupIfAvailable(createSampleBuildRequest(MockModels.somePlatform, MockModels.someTest));
+ syncer.scheduleRequestInGroupIfAvailable(createSampleBuildRequest(MockModels.somePlatform, MockModels.someTest));
}).then(function () {
assert.equal(requests.length, 1);
assert.equal(requests[0].url, '/builders/some%20builder/force');
@@ -861,7 +861,7 @@
let syncer = BuildbotSyncer._loadConfig(MockRemoteAPI, {'configurations': [smallConfiguration()]})[0];
pullBuildbotWithAssertion(syncer, [], {[-1]: smallFinishedBuild()}).then(function () {
- syncer.scheduleFirstRequestInGroupIfAvailable(createSampleBuildRequest(MockModels.somePlatform, MockModels.someTest));
+ syncer.scheduleRequestInGroupIfAvailable(createSampleBuildRequest(MockModels.somePlatform, MockModels.someTest));
}).then(function () {
assert.equal(requests.length, 1);
assert.equal(requests[0].url, '/builders/some%20builder/force');
@@ -875,7 +875,7 @@
let syncer = BuildbotSyncer._loadConfig(MockRemoteAPI, {'configurations': [smallConfiguration()]})[0];
pullBuildbotWithAssertion(syncer, [smallPendingBuild()], {}).then(function () {
- syncer.scheduleFirstRequestInGroupIfAvailable(createSampleBuildRequest(MockModels.somePlatform, MockModels.someTest));
+ syncer.scheduleRequestInGroupIfAvailable(createSampleBuildRequest(MockModels.somePlatform, MockModels.someTest));
}).then(function () {
assert.equal(requests.length, 0);
done();
@@ -886,7 +886,7 @@
let syncer = BuildbotSyncer._loadConfig(MockRemoteAPI, sampleiOSConfig())[0];
pullBuildbotWithAssertion(syncer, [], {}).then(function () {
- syncer.scheduleFirstRequestInGroupIfAvailable(createSampleBuildRequest(MockModels.iphone, MockModels.speedometer));
+ syncer.scheduleRequestInGroupIfAvailable(createSampleBuildRequest(MockModels.iphone, MockModels.speedometer));
}).then(function () {
assert.equal(requests.length, 1);
assert.equal(requests[0].url, '/builders/ABTest-iPhone-RunBenchmark-Tests/force');
@@ -899,7 +899,7 @@
let syncer = BuildbotSyncer._loadConfig(MockRemoteAPI, sampleiOSConfig())[1];
pullBuildbotWithAssertion(syncer, [], {[-1]: sampleFinishedBuild()}).then(function () {
- syncer.scheduleFirstRequestInGroupIfAvailable(createSampleBuildRequest(MockModels.ipad, MockModels.speedometer));
+ syncer.scheduleRequestInGroupIfAvailable(createSampleBuildRequest(MockModels.ipad, MockModels.speedometer));
}).then(function () {
assert.equal(requests.length, 1);
assert.equal(requests[0].url, '/builders/ABTest-iPad-RunBenchmark-Tests/force');
@@ -912,7 +912,7 @@
let syncer = BuildbotSyncer._loadConfig(MockRemoteAPI, sampleiOSConfig())[1];
pullBuildbotWithAssertion(syncer, [samplePendingBuild()], {}).then(function () {
- syncer.scheduleFirstRequestInGroupIfAvailable(createSampleBuildRequest(MockModels.ipad, MockModels.speedometer));
+ syncer.scheduleRequestInGroupIfAvailable(createSampleBuildRequest(MockModels.ipad, MockModels.speedometer));
}).then(function () {
assert.equal(requests.length, 0);
done();
@@ -923,7 +923,7 @@
let syncer = BuildbotSyncer._loadConfig(MockRemoteAPI, sampleiOSConfig())[1];
pullBuildbotWithAssertion(syncer, [samplePendingBuild(1, 1, 'another-slave')], {}).then(function () {
- syncer.scheduleFirstRequestInGroupIfAvailable(createSampleBuildRequest(MockModels.ipad, MockModels.speedometer));
+ syncer.scheduleRequestInGroupIfAvailable(createSampleBuildRequest(MockModels.ipad, MockModels.speedometer));
}).then(function () {
assert.equal(requests.length, 1);
done();
@@ -934,7 +934,7 @@
let syncer = BuildbotSyncer._loadConfig(MockRemoteAPI, sampleiOSConfig())[1];
pullBuildbotWithAssertion(syncer, [], {[-1]: sampleInProgressBuild()}).then(function () {
- syncer.scheduleFirstRequestInGroupIfAvailable(createSampleBuildRequest(MockModels.ipad, MockModels.speedometer));
+ syncer.scheduleRequestInGroupIfAvailable(createSampleBuildRequest(MockModels.ipad, MockModels.speedometer));
}).then(function () {
assert.equal(requests.length, 1);
done();
@@ -945,7 +945,7 @@
let syncer = BuildbotSyncer._loadConfig(MockRemoteAPI, sampleiOSConfig())[1];
pullBuildbotWithAssertion(syncer, [], {[-1]: sampleInProgressBuild('other-slave')}).then(function () {
- syncer.scheduleFirstRequestInGroupIfAvailable(createSampleBuildRequest(MockModels.ipad, MockModels.speedometer));
+ syncer.scheduleRequestInGroupIfAvailable(createSampleBuildRequest(MockModels.ipad, MockModels.speedometer));
}).then(function () {
assert.equal(requests.length, 1);
done();
@@ -956,7 +956,7 @@
let syncer = BuildbotSyncer._loadConfig(MockRemoteAPI, sampleiOSConfig())[0];
pullBuildbotWithAssertion(syncer, [], {}).then(function () {
- syncer.scheduleFirstRequestInGroupIfAvailable(createSampleBuildRequest(MockModels.ipad, MockModels.speedometer));
+ syncer.scheduleRequestInGroupIfAvailable(createSampleBuildRequest(MockModels.ipad, MockModels.speedometer));
}).then(function () {
assert.equal(requests.length, 0);
done();
@@ -971,7 +971,7 @@
syncer.scheduleRequest(createSampleBuildRequest(MockModels.ipad, MockModels.speedometer), 'ABTest-iPad-1');
}).then(function () {
assert.equal(requests.length, 2);
- syncer.scheduleFirstRequestInGroupIfAvailable(createSampleBuildRequest(MockModels.ipad, MockModels.speedometer));
+ syncer.scheduleRequestInGroupIfAvailable(createSampleBuildRequest(MockModels.ipad, MockModels.speedometer));
}).then(function () {
assert.equal(requests.length, 2);
done();
@@ -985,7 +985,7 @@
syncer.scheduleRequest(createSampleBuildRequest(MockModels.ipad, MockModels.speedometer), 'ABTest-iPad-0');
}).then(function () {
assert.equal(requests.length, 1);
- syncer.scheduleFirstRequestInGroupIfAvailable(createSampleBuildRequest(MockModels.ipad, MockModels.speedometer), 'ABTest-iPad-1');
+ syncer.scheduleRequestInGroupIfAvailable(createSampleBuildRequest(MockModels.ipad, MockModels.speedometer), 'ABTest-iPad-1');
}).then(function () {
assert.equal(requests.length, 2);
done();
@@ -999,7 +999,7 @@
syncer.scheduleRequest(createSampleBuildRequest(MockModels.somePlatform, MockModels.someTest), null);
}).then(function () {
assert.equal(requests.length, 1);
- syncer.scheduleFirstRequestInGroupIfAvailable(createSampleBuildRequest(MockModels.somePlatform, MockModels.someTest));
+ syncer.scheduleRequestInGroupIfAvailable(createSampleBuildRequest(MockModels.somePlatform, MockModels.someTest));
}).then(function () {
assert.equal(requests.length, 1);
done();
