https://bugs.webkit.org/show_bug.cgi?id=156375
Reviewed by Chris Dumez.
Order build requests preferring user created ones over ones automatically created by detect-changes.js.
Also fixed a bug in BuildbotSyncer.scheduleFirstRequestInGroupIfAvailable that it was scheduling a new
build request on a builder/slave even when we had previously scheduled another build request.
* public/include/build-requests-fetcher.php:
(BuildRequestsFetcher::fetch_incomplete_requests_for_triggerable): Order build requested based on
author_order which is 0 when it's created by an user and 1 when it's created by detect-changes.js.
Since we're using ascending order, this would put user created test groups first.
* server-tests/api-build-requests-tests.js: Updated an existing test case and added a new test case
for testing that build requests for an user created test group shows up first.
* server-tests/resources/mock-data.js:
(MockData.addAnotherMockTestGroup): Takes an extra argument to specify the author name.
* server-tests/tools-buildbot-triggerable-tests.js: Added a test case for testing that build requests
for an user created test group shows up first.
* tools/js/buildbot-syncer.js:
(BuildbotSyncer): Added _slavesWithNewRequests to keep track of build slaves on which we have already
scheduled new build requests. Don't schedule more requests on these slaves.
(BuildbotSyncer.prototype.scheduleRequest):
(BuildbotSyncer.prototype.scheduleFirstRequestInGroupIfAvailable): Add the specified slave name (or null
when slaveList is not specified) to _slavesWithNewRequests.
(BuildbotSyncer.prototype.pullBuildbot): Clear the set after pulling buildbot since any build request
we have previously scheduled should be included in one of the entires now.
* unit-tests/buildbot-syncer-tests.js: Added test cases for the aforementioned bug.
(sampleiOSConfig): Added a second slave for new test cases.
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@199210
268f45cc-cd09-0410-ab3c-
d52691b4dbfc
+2016-04-07 Ryosuke Niwa <rniwa@webkit.org>
+
+ A/B testing bots should prioritize user created test groups
+ https://bugs.webkit.org/show_bug.cgi?id=156375
+
+ Reviewed by Chris Dumez.
+
+ Order build requests preferring user created ones over ones automatically created by detect-changes.js.
+
+ Also fixed a bug in BuildbotSyncer.scheduleFirstRequestInGroupIfAvailable that it was scheduling a new
+ build request on a builder/slave even when we had previously scheduled another build request.
+
+ * public/include/build-requests-fetcher.php:
+ (BuildRequestsFetcher::fetch_incomplete_requests_for_triggerable): Order build requested based on
+ author_order which is 0 when it's created by an user and 1 when it's created by detect-changes.js.
+ Since we're using ascending order, this would put user created test groups first.
+ * server-tests/api-build-requests-tests.js: Updated an existing test case and added a new test case
+ for testing that build requests for an user created test group shows up first.
+ * server-tests/resources/mock-data.js:
+ (MockData.addAnotherMockTestGroup): Takes an extra argument to specify the author name.
+ * server-tests/tools-buildbot-triggerable-tests.js: Added a test case for testing that build requests
+ for an user created test group shows up first.
+ * tools/js/buildbot-syncer.js:
+ (BuildbotSyncer): Added _slavesWithNewRequests to keep track of build slaves on which we have already
+ scheduled new build requests. Don't schedule more requests on these slaves.
+ (BuildbotSyncer.prototype.scheduleRequest):
+ (BuildbotSyncer.prototype.scheduleFirstRequestInGroupIfAvailable): Add the specified slave name (or null
+ when slaveList is not specified) to _slavesWithNewRequests.
+ (BuildbotSyncer.prototype.pullBuildbot): Clear the set after pulling buildbot since any build request
+ we have previously scheduled should be included in one of the entires now.
+ * unit-tests/buildbot-syncer-tests.js: Added test cases for the aforementioned bug.
+ (sampleiOSConfig): Added a second slave for new test cases.
+
2016-04-07 Ryosuke Niwa <rniwa@webkit.org>
Migrate legacy perf dashboard tests to mocha.js based tests
}
function fetch_incomplete_requests_for_triggerable($triggerable_id) {
- $this->rows = $this->db->query_and_fetch_all('SELECT * FROM build_requests
- WHERE request_triggerable = $1 AND request_group
- IN (SELECT testgroup_id FROM analysis_test_groups WHERE EXISTS
+ $this->rows = $this->db->query_and_fetch_all('SELECT * FROM build_requests,
+ (SELECT testgroup_id, (case when testgroup_author is not null then 0 else 1 end) as author_order, testgroup_created_at
+ FROM analysis_test_groups WHERE EXISTS
(SELECT 1 FROM build_requests WHERE testgroup_id = request_group AND request_status
- IN (\'pending\', \'scheduled\', \'running\')))
- ORDER BY request_group, request_order', array($triggerable_id));
+ IN (\'pending\', \'scheduled\', \'running\'))) AS test_groups
+ WHERE request_triggerable = $1 AND request_group = test_groups.testgroup_id
+ ORDER BY author_order, testgroup_created_at, request_order', array($triggerable_id));
}
function fetch_request($request_id) {
}).catch(done);
});
- it('should order build requests based on test group and order', function (done) {
+ it('should order build requests based on test group creation time and order', function (done) {
let db = TestServer.database();
db.connect().then(function () {
- return Promise.all([MockData.addMockData(db), MockData.addAnotherMockTestGroup(db)])
+ return Promise.all([MockData.addMockData(db), MockData.addAnotherMockTestGroup(db)]);
+ }).then(function () {
+ return Manifest.fetch();
+ }).then(function () {
+ return BuildRequest.fetchForTriggerable('build-webkit');
+ }).then(function (buildRequests) {
+ assert.equal(buildRequests.length, 8);
+ assert.equal(buildRequests[0].id(), 700);
+ assert.equal(buildRequests[0].testGroupId(), 600);
+ assert.equal(buildRequests[1].id(), 701);
+ assert.equal(buildRequests[1].testGroupId(), 600);
+ assert.equal(buildRequests[2].id(), 702);
+ assert.equal(buildRequests[2].testGroupId(), 600);
+ assert.equal(buildRequests[3].id(), 703);
+ assert.equal(buildRequests[3].testGroupId(), 600);
+
+ assert.equal(buildRequests[4].id(), 710);
+ assert.equal(buildRequests[4].testGroupId(), 599);
+ assert.equal(buildRequests[5].id(), 711);
+ assert.equal(buildRequests[5].testGroupId(), 599);
+ assert.equal(buildRequests[6].id(), 712);
+ assert.equal(buildRequests[6].testGroupId(), 599);
+ assert.equal(buildRequests[7].id(), 713);
+ assert.equal(buildRequests[7].testGroupId(), 599);
+ done();
+ }).catch(done);
+ });
+
+ it('should place build requests created by user before automatically created ones', function (done) {
+ let db = TestServer.database();
+ db.connect().then(function () {
+ return Promise.all([MockData.addMockData(db), MockData.addAnotherMockTestGroup(db, null, 'rniwa')]);
}).then(function () {
return Manifest.fetch();
}).then(function () {
assert.equal(buildRequests[2].testGroupId(), 599);
assert.equal(buildRequests[3].id(), 713);
assert.equal(buildRequests[3].testGroupId(), 599);
+
+ assert.equal(buildRequests[4].id(), 700);
+ assert.equal(buildRequests[4].testGroupId(), 600);
+ assert.equal(buildRequests[5].id(), 701);
+ assert.equal(buildRequests[5].testGroupId(), 600);
+ assert.equal(buildRequests[6].id(), 702);
+ assert.equal(buildRequests[6].testGroupId(), 600);
+ assert.equal(buildRequests[7].id(), 703);
+ assert.equal(buildRequests[7].testGroupId(), 600);
done();
}).catch(done);
});
-
});
db.insert('build_requests', {id: 703, status: statusList[3], triggerable: 1, platform: 65, test: 200, group: 600, order: 3, root_set: 402}),
]);
},
- addAnotherMockTestGroup: function (db, statusList)
+ addAnotherMockTestGroup: function (db, statusList, author)
{
if (!statusList)
statusList = ['pending', 'pending', 'pending', 'pending'];
return Promise.all([
- db.insert('analysis_test_groups', {id: 599, task: 500, name: 'another test group'}),
+ db.insert('analysis_test_groups', {id: 599, task: 500, name: 'another test group', author: author}),
db.insert('build_requests', {id: 713, status: statusList[3], triggerable: 1, platform: 65, test: 200, group: 599, order: 3, root_set: 402}),
db.insert('build_requests', {id: 710, status: statusList[0], triggerable: 1, platform: 65, test: 200, group: 599, order: 0, root_set: 401}),
db.insert('build_requests', {id: 712, status: statusList[2], triggerable: 1, platform: 65, test: 200, group: 599, order: 2, root_set: 401}),
done();
}).catch(done);
});
+
+ it('should schedule a build request of an user created test group before ones created by automatic change detection', function (done) {
+ let db = TestServer.database();
+ let syncPromise;
+ db.connect().then(function () {
+ return Promise.all([
+ MockData.addMockData(db, ['pending', 'pending', 'pending', 'pending']),
+ MockData.addAnotherMockTestGroup(db, ['pending', 'pending', 'pending', 'pending'], 'rniwa'),
+ ]);
+ }).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': '191622', 'os': '10.11 15A284', 'build-request-id': '710'});
+ MockRemoteAPI.requests[2].resolve('OK');
+ done();
+ }).catch(done);
+ });
});
});
this._slaveList = object.slaveList;
this._buildRequestPropertyName = object.buildRequestArgument;
this._entryList = null;
+ this._slavesWithNewRequests = new Set;
}
builderName() { return this._builderName; }
scheduleRequest(newRequest, slaveName)
{
+ assert(!this._slavesWithNewRequests.has(slaveName));
let properties = this._propertiesForBuildRequest(newRequest);
assert.equal(!this._slavePropertyName, !slaveName);
if (this._slavePropertyName)
properties[this._slavePropertyName] = slaveName;
+ this._slavesWithNewRequests.add(slaveName);
return this._remote.postFormUrlencodedData(this.pathForForceBuild(), properties);
}
}
if (!this._slaveList || hasPendingBuildsWithoutSlaveNameSpecified) {
- if (usedSlaves.size)
+ if (usedSlaves.size || this._slavesWithNewRequests.size)
return null;
return this.scheduleRequest(newRequest, null);
}
for (let slaveName of this._slaveList) {
- if (!usedSlaves.has(slaveName))
+ if (!usedSlaves.has(slaveName) && !this._slavesWithNewRequests.has(slaveName))
return this.scheduleRequest(newRequest, slaveName);
}
entryList.push(entryByRequest[id]);
self._entryList = entryList;
+ self._slavesWithNewRequests.clear();
return entryList;
});
'iPad-bench': {
'builder': 'ABTest-iPad-RunBenchmark-Tests',
'arguments': { 'forcescheduler': 'ABTest-iPad-RunBenchmark-Tests-ForceScheduler' },
- 'slaveList': ['ABTest-iPad-0'],
+ 'slaveList': ['ABTest-iPad-0', 'ABTest-iPad-1'],
}
},
'configurations': [
}).catch(done);
});
+ it('should not schedule a build if a new request had been submitted to the same slave', function (done) {
+ let syncer = BuildbotSyncer._loadConfig(MockRemoteAPI, sampleiOSConfig())[1];
+
+ pullBuildbotWithAssertion(syncer, [], {}).then(function () {
+ syncer.scheduleRequest(createSampleBuildRequest(MockModels.ipad, MockModels.speedometer), 'ABTest-iPad-0');
+ syncer.scheduleRequest(createSampleBuildRequest(MockModels.ipad, MockModels.speedometer), 'ABTest-iPad-1');
+ }).then(function () {
+ assert.equal(requests.length, 2);
+ syncer.scheduleFirstRequestInGroupIfAvailable(createSampleBuildRequest(MockModels.ipad, MockModels.speedometer));
+ }).then(function () {
+ assert.equal(requests.length, 2);
+ done();
+ }).catch(done);
+ });
+
+ it('should schedule a build if a new request had been submitted to another slave', function (done) {
+ let syncer = BuildbotSyncer._loadConfig(MockRemoteAPI, sampleiOSConfig())[1];
+
+ pullBuildbotWithAssertion(syncer, [], {}).then(function () {
+ 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');
+ }).then(function () {
+ assert.equal(requests.length, 2);
+ done();
+ }).catch(done);
+ });
+
+ it('should not schedule a build if a new request had been submitted to the same builder without slaveList', function (done) {
+ let syncer = BuildbotSyncer._loadConfig(MockRemoteAPI, {'configurations': [smallConfiguration()]})[0];
+
+ pullBuildbotWithAssertion(syncer, [], {}).then(function () {
+ syncer.scheduleRequest(createSampleBuildRequest(MockModels.somePlatform, MockModels.someTest), null);
+ }).then(function () {
+ assert.equal(requests.length, 1);
+ syncer.scheduleFirstRequestInGroupIfAvailable(createSampleBuildRequest(MockModels.somePlatform, MockModels.someTest));
+ }).then(function () {
+ assert.equal(requests.length, 1);
+ done();
+ }).catch(done);
+ });
});
});
git://git.webkit.org / WebKit-https.git / commitdiff