From 53c45f7990ae00ec9800ec50017272b66a7bfe0d Mon Sep 17 00:00:00 2001 From: "rniwa@webkit.org" Date: Wed, 30 Mar 2016 19:46:23 +0000 Subject: [PATCH] BuildRequest should have a method to fetch all in-progress and pending requests for a triggerable https://bugs.webkit.org/show_bug.cgi?id=156008 Reviewed by Darin Adler. Add a method to BuildRequest that fetches all pending and in-progress requests for a triggerable. Now, new syncing scripts must be able to figure out the build slave the first build requests in a given test group had used in order to schedule subsequent build requests in the test group. For this purpose, /api/build-requests has been modified to return all build requests whose test group had not finished yet. A test group is finished if all build requests in the test group had finished (completed, failed, or canceled). * public/include/build-requests-fetcher.php: (BuildRequestFetcher::fetch_incomplete_requests_for_triggerable): Return all build requests in test groups that have not been finished. * public/v3/models/build-request.js: (BuildRequest): (BuildRequest.prototype.testGroupId): Added. (BuildRequest.prototype.isPending): Renamed from hasPending to fix a bad grammar. (BuildRequest.fetchForTriggerable): Added. (BuildRequest.constructBuildRequestsFromData): Extracted from _createModelsFromFetchedTestGroups in TestGroup. * public/v3/models/manifest.js: (Manifest.fetch): Use the full path from root so that it works in server tests. * public/v3/models/test-group.js: (TestGroup.hasPending): (TestGroup._createModelsFromFetchedTestGroups): * server-tests/api-build-requests-tests.js: Added tests to ensure all build requests for a test group is present in the response returned by /api/build-requests iff any build request in the group had not finished yet. (.addMockData): (.addAnotherMockTestGroup): Added. * unit-tests/test-groups-tests.js: git-svn-id: http://svn.webkit.org/repository/webkit/trunk@198853 268f45cc-cd09-0410-ab3c-d52691b4dbfc --- Websites/perf.webkit.org/ChangeLog | 38 +++ .../public/include/build-requests-fetcher.php | 7 +- .../public/v3/models/build-request.js | 34 ++- .../public/v3/models/manifest.js | 4 +- .../public/v3/models/test-group.js | 19 +- .../server-tests/api-build-requests-tests.js | 227 +++++++++++++++++- .../unit-tests/test-groups-tests.js | 4 +- 7 files changed, 301 insertions(+), 32 deletions(-) diff --git a/Websites/perf.webkit.org/ChangeLog b/Websites/perf.webkit.org/ChangeLog index 1d54f57c0421..e6257fc53c7c 100644 --- a/Websites/perf.webkit.org/ChangeLog +++ b/Websites/perf.webkit.org/ChangeLog @@ -1,3 +1,41 @@ +2016-03-30 Ryosuke Niwa + + BuildRequest should have a method to fetch all in-progress and pending requests for a triggerable + https://bugs.webkit.org/show_bug.cgi?id=156008 + + Reviewed by Darin Adler. + + Add a method to BuildRequest that fetches all pending and in-progress requests for a triggerable. + + Now, new syncing scripts must be able to figure out the build slave the first build requests in + a given test group had used in order to schedule subsequent build requests in the test group. + + For this purpose, /api/build-requests has been modified to return all build requests whose test + group had not finished yet. A test group is finished if all build requests in the test group had + finished (completed, failed, or canceled). + + * public/include/build-requests-fetcher.php: + (BuildRequestFetcher::fetch_incomplete_requests_for_triggerable): Return all build requests in test + groups that have not been finished. + * public/v3/models/build-request.js: + (BuildRequest): + (BuildRequest.prototype.testGroupId): Added. + (BuildRequest.prototype.isPending): Renamed from hasPending to fix a bad grammar. + (BuildRequest.fetchForTriggerable): Added. + (BuildRequest.constructBuildRequestsFromData): Extracted from _createModelsFromFetchedTestGroups in + TestGroup. + * public/v3/models/manifest.js: + (Manifest.fetch): Use the full path from root so that it works in server tests. + * public/v3/models/test-group.js: + (TestGroup.hasPending): + (TestGroup._createModelsFromFetchedTestGroups): + * server-tests/api-build-requests-tests.js: Added tests to ensure all build requests for a test group + is present in the response returned by /api/build-requests iff any build request in the group had not + finished yet. + (.addMockData): + (.addAnotherMockTestGroup): Added. + * unit-tests/test-groups-tests.js: + 2016-03-29 Ryosuke Niwa Make dependency injection in unit tests more explicit diff --git a/Websites/perf.webkit.org/public/include/build-requests-fetcher.php b/Websites/perf.webkit.org/public/include/build-requests-fetcher.php index 918bd8f25579..71d9d52917cc 100644 --- a/Websites/perf.webkit.org/public/include/build-requests-fetcher.php +++ b/Websites/perf.webkit.org/public/include/build-requests-fetcher.php @@ -27,8 +27,11 @@ class BuildRequestsFetcher { 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_status != \'completed\' - ORDER BY request_created_at, request_group, request_order', array($triggerable_id)); + WHERE request_triggerable = $1 AND request_group + IN (SELECT testgroup_id 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)); } function fetch_request($request_id) { diff --git a/Websites/perf.webkit.org/public/v3/models/build-request.js b/Websites/perf.webkit.org/public/v3/models/build-request.js index 7077e52789a2..ed714514648d 100644 --- a/Websites/perf.webkit.org/public/v3/models/build-request.js +++ b/Websites/perf.webkit.org/public/v3/models/build-request.js @@ -5,6 +5,7 @@ class BuildRequest extends DataModelObject { constructor(id, object) { super(id, object); + this._testGroupId = object.testGroupId; console.assert(!object.testGroup || object.testGroup instanceof TestGroup); this._testGroup = object.testGroup; if (this._testGroup) @@ -28,13 +29,14 @@ class BuildRequest extends DataModelObject { this._buildId = object.build; } + testGroupId() { return this._testGroupId; } testGroup() { return this._testGroup; } order() { return this._order; } rootSet() { return this._rootSet; } hasFinished() { return this._status == 'failed' || this._status == 'completed' || this._status == 'canceled'; } hasStarted() { return this._status != 'pending'; } - hasPending() { return this._status == 'pending'; } + isPending() { return this._status == 'pending'; } statusLabel() { switch (this._status) { @@ -62,6 +64,36 @@ class BuildRequest extends DataModelObject { this._result = result; this._testGroup.didSetResult(this); } + + static fetchForTriggerable(triggerable) + { + return RemoteAPI.getJSONWithStatus('/api/build-requests/' + triggerable).then(function (data) { + return BuildRequest.constructBuildRequestsFromData(data); + }); + } + + static constructBuildRequestsFromData(data) + { + var rootIdMap = {}; + for (var root of data['roots']) { + rootIdMap[root.id] = root; + root.repository = Repository.findById(root.repository); + } + + var rootSets = data['rootSets'].map(function (row) { + row.roots = row.roots.map(function (rootId) { return rootIdMap[rootId]; }); + return RootSet.ensureSingleton(row.id, row); + }); + + return data['buildRequests'].map(function (rawData) { + rawData.platform = Platform.findById(rawData.platform); + rawData.test = Test.findById(rawData.test); + rawData.testGroupId = rawData.testGroup; + rawData.testGroup = TestGroup.findById(rawData.testGroup); + rawData.rootSet = RootSet.findById(rawData.rootSet); + return BuildRequest.ensureSingleton(rawData.id, rawData); + }); + } } if (typeof module != 'undefined') diff --git a/Websites/perf.webkit.org/public/v3/models/manifest.js b/Websites/perf.webkit.org/public/v3/models/manifest.js index f429793a5fcf..045796916753 100644 --- a/Websites/perf.webkit.org/public/v3/models/manifest.js +++ b/Websites/perf.webkit.org/public/v3/models/manifest.js @@ -4,8 +4,8 @@ class Manifest { static fetch() { - return RemoteAPI.getJSON('../data/manifest.json').catch(function () { - return RemoteAPI.getJSON('../api/manifest/'); + return RemoteAPI.getJSON('/data/manifest.json').catch(function () { + return RemoteAPI.getJSON('/api/manifest/'); }).then(this._didFetchManifest.bind(this)); } diff --git a/Websites/perf.webkit.org/public/v3/models/test-group.js b/Websites/perf.webkit.org/public/v3/models/test-group.js index 9a131b402400..e863da0c775a 100644 --- a/Websites/perf.webkit.org/public/v3/models/test-group.js +++ b/Websites/perf.webkit.org/public/v3/models/test-group.js @@ -112,7 +112,7 @@ class TestGroup extends LabeledObject { hasPending() { - return this._buildRequests.some(function (request) { return request.hasPending(); }); + return this._buildRequests.some(function (request) { return request.isPending(); }); } compareTestResults(rootSetA, rootSetB) @@ -222,22 +222,7 @@ class TestGroup extends LabeledObject { return TestGroup.ensureSingleton(row.id, row); }); - var rootIdMap = {}; - for (var root of data['roots']) { - rootIdMap[root.id] = root; - root.repository = Repository.findById(root.repository); - } - - var rootSets = data['rootSets'].map(function (row) { - row.roots = row.roots.map(function (rootId) { return rootIdMap[rootId]; }); - return RootSet.ensureSingleton(row.id, row); - }); - - var buildRequests = data['buildRequests'].map(function (rawData) { - rawData.testGroup = TestGroup.findById(rawData.testGroup); - rawData.rootSet = RootSet.findById(rawData.rootSet); - return BuildRequest.ensureSingleton(rawData.id, rawData); - }); + BuildRequest.constructBuildRequestsFromData(data); return testGroups; } diff --git a/Websites/perf.webkit.org/server-tests/api-build-requests-tests.js b/Websites/perf.webkit.org/server-tests/api-build-requests-tests.js index 0576628d6722..9a0ee2aa8480 100644 --- a/Websites/perf.webkit.org/server-tests/api-build-requests-tests.js +++ b/Websites/perf.webkit.org/server-tests/api-build-requests-tests.js @@ -1,12 +1,28 @@ 'use strict'; let assert = require('assert'); + +require('../tools/js/v3-models.js'); + let TestServer = require('./resources/test-server.js'); describe('/api/build-requests', function () { this.timeout(10000); TestServer.inject(); + beforeEach(function () { + AnalysisTask._fetchAllPromise = null; + AnalysisTask.clearStaticMap(); + BuildRequest.clearStaticMap(); + CommitLog.clearStaticMap(); + Metric.clearStaticMap(); + Platform.clearStaticMap(); + Repository.clearStaticMap(); + RootSet.clearStaticMap(); + Test.clearStaticMap(); + TestGroup.clearStaticMap(); + }) + it('should return "TriggerableNotFound" when the database is empty', function (done) { TestServer.remoteAPI().getJSON('/api/build-requests/build-webkit').then(function (content) { assert.equal(content['status'], 'TriggerableNotFound'); @@ -29,18 +45,21 @@ describe('/api/build-requests', function () { }).catch(done); }); - function addMockData(db) + function addMockData(db, statusList) { + if (!statusList) + statusList = ['pending', 'pending', 'pending', 'pending']; return Promise.all([ db.insert('build_triggerables', {id: 1, name: 'build-webkit'}), db.insert('repositories', {id: 9, name: 'OS X'}), db.insert('repositories', {id: 11, name: 'WebKit'}), db.insert('commits', {id: 87832, repository: 9, revision: '10.11 15A284'}), - db.insert('commits', {id: 93116, repository: 11, revision: '191622', time: new Date(1445945816878)}), - db.insert('commits', {id: 96336, repository: 11, revision: '192736', time: new Date(1448225325650)}), + db.insert('commits', {id: 93116, repository: 11, revision: '191622', time: (new Date(1445945816878)).toISOString()}), + db.insert('commits', {id: 96336, repository: 11, revision: '192736', time: (new Date(1448225325650)).toISOString()}), db.insert('platforms', {id: 65, name: 'some platform'}), db.insert('tests', {id: 200, name: 'some test'}), db.insert('test_metrics', {id: 300, test: 200, name: 'some metric'}), + db.insert('test_configurations', {id: 301, metric: 300, platform: 65, type: 'current'}), db.insert('root_sets', {id: 401}), db.insert('roots', {set: 401, commit: 87832}), db.insert('roots', {set: 401, commit: 93116}), @@ -49,10 +68,23 @@ describe('/api/build-requests', function () { db.insert('roots', {set: 402, commit: 96336}), db.insert('analysis_tasks', {id: 500, platform: 65, metric: 300, name: 'some task'}), db.insert('analysis_test_groups', {id: 600, task: 500, name: 'some test group'}), - db.insert('build_requests', {id: 700, triggerable: 1, platform: 65, test: 200, group: 600, order: 0, root_set: 401}), - db.insert('build_requests', {id: 701, triggerable: 1, platform: 65, test: 200, group: 600, order: 1, root_set: 402}), - db.insert('build_requests', {id: 702, triggerable: 1, platform: 65, test: 200, group: 600, order: 2, root_set: 401}), - db.insert('build_requests', {id: 703, triggerable: 1, platform: 65, test: 200, group: 600, order: 3, root_set: 402}), + db.insert('build_requests', {id: 700, status: statusList[0], triggerable: 1, platform: 65, test: 200, group: 600, order: 0, root_set: 401}), + db.insert('build_requests', {id: 701, status: statusList[1], triggerable: 1, platform: 65, test: 200, group: 600, order: 1, root_set: 402}), + db.insert('build_requests', {id: 702, status: statusList[2], triggerable: 1, platform: 65, test: 200, group: 600, order: 2, root_set: 401}), + db.insert('build_requests', {id: 703, status: statusList[3], triggerable: 1, platform: 65, test: 200, group: 600, order: 3, root_set: 402}), + ]); + } + + function addAnotherMockTestGroup(db, statusList) + { + 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('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}), + db.insert('build_requests', {id: 711, status: statusList[1], triggerable: 1, platform: 65, test: 200, group: 599, order: 1, root_set: 402}), ]); } @@ -114,7 +146,7 @@ describe('/api/build-requests', function () { }).catch(done); }); - it('should return support useLegacyIdResolution option', function (done) { + it('should support useLegacyIdResolution option', function (done) { let db = TestServer.database(); db.connect().then(function () { return addMockData(db); @@ -172,4 +204,183 @@ describe('/api/build-requests', function () { }).catch(done); }); + it('should be fetchable by BuildRequest.fetchForTriggerable', function (done) { + let db = TestServer.database(); + db.connect().then(function () { + return addMockData(db); + }).then(function () { + return Manifest.fetch(); + }).then(function () { + return BuildRequest.fetchForTriggerable('build-webkit'); + }).then(function (buildRequests) { + assert.equal(buildRequests.length, 4); + + assert.equal(buildRequests[0].id(), 700); + assert.equal(buildRequests[0].order(), 0); + assert.ok(buildRequests[0].rootSet() instanceof RootSet); + assert.ok(!buildRequests[0].hasFinished()); + assert.ok(!buildRequests[0].hasStarted()); + assert.ok(buildRequests[0].isPending()); + assert.equal(buildRequests[0].statusLabel(), 'Waiting to be scheduled'); + + assert.equal(buildRequests[1].id(), 701); + assert.equal(buildRequests[1].order(), 1); + assert.ok(buildRequests[1].rootSet() instanceof RootSet); + assert.ok(!buildRequests[1].hasFinished()); + assert.ok(!buildRequests[1].hasStarted()); + assert.ok(buildRequests[1].isPending()); + assert.equal(buildRequests[1].statusLabel(), 'Waiting to be scheduled'); + + assert.equal(buildRequests[2].id(), 702); + assert.equal(buildRequests[2].order(), 2); + assert.ok(buildRequests[2].rootSet() instanceof RootSet); + assert.ok(!buildRequests[2].hasFinished()); + assert.ok(!buildRequests[2].hasStarted()); + assert.ok(buildRequests[2].isPending()); + assert.equal(buildRequests[2].statusLabel(), 'Waiting to be scheduled'); + + assert.equal(buildRequests[3].id(), 703); + assert.equal(buildRequests[3].order(), 3); + assert.ok(buildRequests[3].rootSet() instanceof RootSet); + assert.ok(!buildRequests[3].hasFinished()); + assert.ok(!buildRequests[3].hasStarted()); + assert.ok(buildRequests[3].isPending()); + assert.equal(buildRequests[3].statusLabel(), 'Waiting to be scheduled'); + + let osx = Repository.findById(9); + assert.equal(osx.name(), 'OS X'); + + let webkit = Repository.findById(11); + assert.equal(webkit.name(), 'WebKit'); + + let firstRootSet = buildRequests[0].rootSet(); + assert.equal(buildRequests[2].rootSet(), firstRootSet); + + let secondRootSet = buildRequests[1].rootSet(); + assert.equal(buildRequests[3].rootSet(), secondRootSet); + + assert.equal(firstRootSet.revisionForRepository(osx), '10.11 15A284'); + assert.equal(firstRootSet.revisionForRepository(webkit), '191622'); + + assert.equal(secondRootSet.revisionForRepository(osx), '10.11 15A284'); + assert.equal(secondRootSet.revisionForRepository(webkit), '192736'); + + let osxCommit = firstRootSet.commitForRepository(osx); + assert.equal(osxCommit.revision(), '10.11 15A284'); + assert.equal(osxCommit, secondRootSet.commitForRepository(osx)); + + let firstWebKitCommit = firstRootSet.commitForRepository(webkit); + assert.equal(firstWebKitCommit.revision(), '191622'); + assert.equal(+firstWebKitCommit.time(), 1445945816878); + + let secondWebKitCommit = secondRootSet.commitForRepository(webkit); + assert.equal(secondWebKitCommit.revision(), '192736'); + assert.equal(+secondWebKitCommit.time(), 1448225325650); + + done(); + }).catch(done); + }); + + it('should not include a build request if all requests in the same group had been completed', function (done) { + let db = TestServer.database(); + db.connect().then(function () { + return addMockData(db, ['completed', 'completed', 'completed', 'completed']); + }).then(function () { + return Manifest.fetch(); + }).then(function () { + return BuildRequest.fetchForTriggerable('build-webkit'); + }).then(function (buildRequests) { + assert.equal(buildRequests.length, 0); + done(); + }).catch(done); + }); + + it('should not include a build request if all requests in the same group had been failed or cancled', function (done) { + let db = TestServer.database(); + db.connect().then(function () { + return addMockData(db, ['failed', 'failed', 'canceled', 'canceled']); + }).then(function () { + return Manifest.fetch(); + }).then(function () { + return BuildRequest.fetchForTriggerable('build-webkit'); + }).then(function (buildRequests) { + assert.equal(buildRequests.length, 0); + done(); + }).catch(done); + }); + + it('should include all build requests of a test group if one of the reqeusts in the group had not been finished', function (done) { + let db = TestServer.database(); + db.connect().then(function () { + return addMockData(db, ['completed', 'completed', 'scheduled', 'pending']); + }).then(function () { + return Manifest.fetch(); + }).then(function () { + return BuildRequest.fetchForTriggerable('build-webkit'); + }).then(function (buildRequests) { + assert.equal(buildRequests.length, 4); + assert.ok(buildRequests[0].hasFinished()); + assert.ok(buildRequests[0].hasStarted()); + assert.ok(!buildRequests[0].isPending()); + assert.ok(buildRequests[1].hasFinished()); + assert.ok(buildRequests[1].hasStarted()); + assert.ok(!buildRequests[1].isPending()); + assert.ok(!buildRequests[2].hasFinished()); + assert.ok(buildRequests[2].hasStarted()); + assert.ok(!buildRequests[2].isPending()); + assert.ok(!buildRequests[3].hasFinished()); + assert.ok(!buildRequests[3].hasStarted()); + assert.ok(buildRequests[3].isPending()); + done(); + }).catch(done); + }); + + it('should include all build requests of a test group if one of the reqeusts in the group is still running', function (done) { + let db = TestServer.database(); + db.connect().then(function () { + return addMockData(db, ['completed', 'completed', 'completed', 'running']); + }).then(function () { + return Manifest.fetch(); + }).then(function () { + return BuildRequest.fetchForTriggerable('build-webkit'); + }).then(function (buildRequests) { + assert.equal(buildRequests.length, 4); + assert.ok(buildRequests[0].hasFinished()); + assert.ok(buildRequests[0].hasStarted()); + assert.ok(!buildRequests[0].isPending()); + assert.ok(buildRequests[1].hasFinished()); + assert.ok(buildRequests[1].hasStarted()); + assert.ok(!buildRequests[1].isPending()); + assert.ok(buildRequests[2].hasFinished()); + assert.ok(buildRequests[2].hasStarted()); + assert.ok(!buildRequests[2].isPending()); + assert.ok(!buildRequests[3].hasFinished()); + assert.ok(buildRequests[3].hasStarted()); + assert.ok(!buildRequests[3].isPending()); + done(); + }).catch(done); + }); + + it('should order build requests based on test group and order', function (done) { + let db = TestServer.database(); + db.connect().then(function () { + return Promise.all([addMockData(db), 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(), 710); + assert.equal(buildRequests[0].testGroupId(), 599); + assert.equal(buildRequests[1].id(), 711); + assert.equal(buildRequests[1].testGroupId(), 599); + assert.equal(buildRequests[2].id(), 712); + assert.equal(buildRequests[2].testGroupId(), 599); + assert.equal(buildRequests[3].id(), 713); + assert.equal(buildRequests[3].testGroupId(), 599); + done(); + }).catch(done); + }); + }); diff --git a/Websites/perf.webkit.org/unit-tests/test-groups-tests.js b/Websites/perf.webkit.org/unit-tests/test-groups-tests.js index 083e8d3da5b3..fbb6a8557493 100644 --- a/Websites/perf.webkit.org/unit-tests/test-groups-tests.js +++ b/Websites/perf.webkit.org/unit-tests/test-groups-tests.js @@ -139,7 +139,7 @@ describe('TestGroup', function () { assert.equal(buildRequests[0].order(), 0); assert.ok(!buildRequests[0].hasFinished()); assert.ok(!buildRequests[0].hasStarted()); - assert.ok(buildRequests[0].hasPending()); + assert.ok(buildRequests[0].isPending()); assert.equal(buildRequests[0].statusLabel(), 'Waiting to be scheduled'); assert.equal(buildRequests[0].buildId(), null); assert.equal(buildRequests[0].result(), null); @@ -148,7 +148,7 @@ describe('TestGroup', function () { assert.equal(buildRequests[1].order(), 1); assert.ok(!buildRequests[1].hasFinished()); assert.ok(!buildRequests[1].hasStarted()); - assert.ok(buildRequests[1].hasPending()); + assert.ok(buildRequests[1].isPending()); assert.equal(buildRequests[1].statusLabel(), 'Waiting to be scheduled'); assert.equal(buildRequests[1].buildId(), null); assert.equal(buildRequests[1].result(), null); -- 2.36.0