sync-buildbot.js doesn't mark disappeared builds as failed
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 8 Apr 2016 04:21:32 +0000 (04:21 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 8 Apr 2016 04:21:32 +0000 (04:21 +0000)
https://bugs.webkit.org/show_bug.cgi?id=156386

Reviewed by Chris Dumez.

Fix a bug that new syncing script doesn't mark builds that it scheduled but doesn't appear when queried
by buildbot's JSON API. These are builds that got canceled by humans (e.g. buildbot was restarted, data
loss, pending build was canceled, etc...)

* server-tests/tools-buildbot-triggerable-tests.js: Added a test case.
* tools/js/buildbot-triggerable.js:
(BuildbotTriggerable.prototype._pullBuildbotOnAllSyncers): Added a set of build requests we've matched
against BuildbotBuildEntry's. Mark build requests that didn't have any entry but supposed to be in either
'scheduled' or 'running' status as failed.

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

Websites/perf.webkit.org/ChangeLog
Websites/perf.webkit.org/server-tests/tools-buildbot-triggerable-tests.js
Websites/perf.webkit.org/tools/js/buildbot-triggerable.js

index 57915b6..12953f6 100644 (file)
@@ -1,5 +1,22 @@
 2016-04-07  Ryosuke Niwa  <rniwa@webkit.org>
 
+        sync-buildbot.js doesn't mark disappeared builds as failed
+        https://bugs.webkit.org/show_bug.cgi?id=156386
+
+        Reviewed by Chris Dumez.
+
+        Fix a bug that new syncing script doesn't mark builds that it scheduled but doesn't appear when queried
+        by buildbot's JSON API. These are builds that got canceled by humans (e.g. buildbot was restarted, data
+        loss, pending build was canceled, etc...)
+
+        * server-tests/tools-buildbot-triggerable-tests.js: Added a test case.
+        * tools/js/buildbot-triggerable.js:
+        (BuildbotTriggerable.prototype._pullBuildbotOnAllSyncers): Added a set of build requests we've matched
+        against BuildbotBuildEntry's. Mark build requests that didn't have any entry but supposed to be in either
+        'scheduled' or 'running' status as failed.
+
+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
 
index 95d2f7c..42c5ae3 100644 (file)
@@ -441,6 +441,62 @@ describe('BuildbotTriggerable', function () {
             }).catch(done);
         });
 
+        it('should update the status of a supposedly scheduled build that went missing', 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([]);
+                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({});
+                return syncPromise;
+            }).then(function () {
+                assert.equal(BuildRequest.all().length, 4);
+                assert.equal(BuildRequest.findById(700).status(), 'scheduled');
+                assert.equal(BuildRequest.findById(701).status(), 'pending');
+                assert.equal(BuildRequest.findById(702).status(), 'pending');
+                assert.equal(BuildRequest.findById(703).status(), 'pending');
+                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(701).status(), 'pending');
+                assert.equal(BuildRequest.findById(702).status(), 'pending');
+                assert.equal(BuildRequest.findById(703).status(), 'pending');
+                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;
index 6287ce6..62602c9 100644 (file)
@@ -80,6 +80,7 @@ class BuildbotTriggerable {
     _pullBuildbotOnAllSyncers(buildReqeustsByGroup)
     {
         let updates = {};
+        let associatedRequests = new Set;
         let self = this;
         return Promise.all(this._syncers.map(function (syncer) {
             self._logger.log(`Fetching builds on ${syncer.builderName()}`);
@@ -88,6 +89,7 @@ class BuildbotTriggerable {
                     let request = BuildRequest.findById(entry.buildRequestId());
                     if (!request)
                         continue;
+                    associatedRequests.add(request);
 
                     let info = buildReqeustsByGroup.get(request.testGroupId());
                     assert(!info.syncer || info.syncer == syncer);
@@ -104,7 +106,14 @@ class BuildbotTriggerable {
                     }
                 }
             });
-        })).then(function () { return updates; });
+        })).then(function () {
+            for (let request of BuildRequest.all()) {
+                if (request.hasStarted() && !request.hasFinished() && !associatedRequests.has(request)) {
+                    assert(!(request.id() in updates));
+                    updates[request.id()] = {status: 'failedIfNotCompleted'};
+                }
+            }
+        }).then(function () { return updates; });
     }
 
     _scheduleNextRequestInGroupIfSlaveIsAvailable(groupInfo, pendingUpdates)