+2017-10-30 Dewei Zhu <dewei_zhu@apple.com>
+
+ Limit the number of results to be submitted in one submission.
+ https://bugs.webkit.org/show_bug.cgi?id=179045
+
+ Reviewed by Ryosuke Niwa.
+
+ Submitting results for a large number of builds with owned commit information may exceed the size limit of php.
+ Added a way to split the results into groups of certain sizes, and submit them one by one.
+
+ * server-tests/tools-os-build-fetcher-tests.js: Updated the unit tests.
+ * tools/js/os-build-fetcher.js: Added '_maxNumberOfResultsPerSubmit' which can be specified by a configuration but also use 20 as default value.
+ (prototype.fetchAndReportNewBuilds): Instead of submitting all results once, split them into groups and submit them one by one.
+ (prototype._fetchAvailableBuilds): 'label' is already quoted, should remove unnecessary quotes.
+ (prototype._addOwnedCommitsForBuild): Added logging to log the size of owned commits.
+
2017-10-31 Dewei Zhu <dewei_zhu@apple.com>
OwnedCommitViewer should include the preceding commit.
describe('OSBuilderFetcher._computeOrder', () => {
it('should calculate the right order for a given valid revision', () => {
- const fetcher = new OSBuildFetcher();
+ const fetcher = new OSBuildFetcher({});
assert.equal(fetcher._computeOrder('Sierra16D32'), 1603003200);
assert.equal(fetcher._computeOrder('16D321'), 1603032100);
assert.equal(fetcher._computeOrder('16d321'), 1603032100);
});
it('should throw assertion error when given a invalid revision', () => {
- const fetcher = new OSBuildFetcher();
+ const fetcher = new OSBuildFetcher({});
assert.throws(() => fetcher._computeOrder('invalid'), (error) => error.name == 'AssertionError');
assert.throws(() => fetcher._computeOrder(''), (error) => error.name == 'AssertionError');
assert.throws(() => fetcher._computeOrder('16'), (error) => error.name == 'AssertionError');
describe('OSBuilderFetcher._commitsForAvailableBuilds', () => {
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 fetchter = new OSBuildFetcher({}, null, null, MockSubprocess, logger);
const waitForInvocationPromise = MockSubprocess.waitForInvocation();
const fetchCommitsPromise = fetchter._commitsForAvailableBuilds('OSX', ['list', 'build1'], '^\\.*$', 1604000000);
describe('OSBuildFetcher._addOwnedCommitsForBuild', () => {
it('should add owned-commit info for commits', () => {
const logger = new MockLogger;
- const fetchter = new OSBuildFetcher(null, null, null, MockSubprocess, logger);
+ const fetchter = new OSBuildFetcher({}, null, null, MockSubprocess, logger);
const waitForInvocationPromise = MockSubprocess.waitForInvocation();
const addownedCommitPromise = fetchter._addOwnedCommitsForBuild([osxCommit, anotherOSXCommit], ['ownedCommit', 'for', 'revision']);
it('should fail if the command to get owned-commit info fails', () => {
const logger = new MockLogger;
- const fetchter = new OSBuildFetcher(null, null, null, MockSubprocess, logger);
+ const fetchter = new OSBuildFetcher({}, null, null, MockSubprocess, logger);
const waitForInvocationPromise = MockSubprocess.waitForInvocation();
const addownedCommitPromise = fetchter._addOwnedCommitsForBuild([osxCommit], ['ownedCommit', 'for', 'revision'])
it('should fail if entries in owned-commits does not contain revision', () => {
const logger = new MockLogger;
- const fetchter = new OSBuildFetcher(null, null, null, MockSubprocess, logger);
+ const fetchter = new OSBuildFetcher({}, null, null, MockSubprocess, logger);
const waitForInvocationPromise = MockSubprocess.waitForInvocation();
const addownedCommitPromise = fetchter._addOwnedCommitsForBuild([osxCommit], ['ownedCommit', 'for', 'revision'])
assert.equal(webkitRepository.length, 1);
assert.equal(webkitRepository[0]['owner'], 10);
- assert.equal(jscRepository.length, 1)
+ assert.equal(jscRepository.length, 1);
assert.equal(jscRepository[0]['owner'], 10);
assert.equal(osxCommit16D69.length, 1);
const osxCommit16E34 = results[4];
assert.equal(webkitRepository.length, 0);
- assert.equal(jscRepository.length, 0)
+ assert.equal(jscRepository.length, 0);
assert.equal(osxCommit16D69.length, 1);
assert.equal(osxCommit16D69[0]['repository'], 10);
constructor(osConfig, remoteAPI, slaveAuth, subprocess, logger)
{
this._osConfig = osConfig;
- this._reportedRevisions = new Set();
this._logger = logger;
this._slaveAuth = slaveAuth;
this._remoteAPI = remoteAPI;
this._subprocess = subprocess;
+ this._maxSubmitCount = osConfig['maxSubmitCount'] || 20;
}
static fetchAndReportAllInOrder(fetcherList)
{
return this._fetchAvailableBuilds().then((results) => {
this._logger.log(`Submitting ${results.length} builds for ${this._osConfig['name']}`);
- return this._submitCommits(results);
+ if (results.length == 0)
+ return this._submitCommits(results);
+
+ const splittedResults = [];
+ for (let startIndex = 0; startIndex < results.length; startIndex += this._maxSubmitCount)
+ splittedResults.push(results.slice(startIndex, startIndex + this._maxSubmitCount));
+
+ return mapInSerialPromiseChain(splittedResults, this._submitCommits.bind(this)).then((responses) => {
+ assert(responses.every((response) => response['status'] == 'OK'));
+ assert(responses.length > 0);
+ return responses[0];
+ });
});
}
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']"`;
+ const label = 'name' in command ? `"${command['name']}"` : `"${command['minRevision']}" to "${command['maxRevision']}"`;
this._logger.log(`Found ${commits.length} builds for ${label}`);
if ('ownedCommitCommand' in command) {
- this._logger.log(`Resolving ownedCommits for "${label}"`);
+ this._logger.log(`Resolving ownedCommits for ${label}`);
return this._addOwnedCommitsForBuild(commits, command['ownedCommitCommand']);
}
return mapInSerialPromiseChain(commits, (commit) => {
return this._subprocess.execute(command.concat(commit['revision'])).then((ownedCommitOutput) => {
const ownedCommits = JSON.parse(ownedCommitOutput);
+ this._logger.log(`Got ${Object.keys((ownedCommits)).length} owned commits for "${commit['revision']}"`);
for (let repositoryName in ownedCommits) {
const ownedCommit = ownedCommits[repositoryName];
assert.deepEqual(Object.keys(ownedCommit), ['revision']);