[webkit-changes] [269892] trunk/Websites/perf.webkit.org

[dewei_zhu]

Title: [269892] trunk/Websites/perf.webkit.org

Revision

269892

Author

dewei_...@apple.com

Date

2020-11-16 20:33:06 -0800 (Mon, 16 Nov 2020)

Log Message

BuildbotTriggerable should not process a completed build request from a completed test group.
https://bugs.webkit.org/show_bug.cgi?id=219016

Reviewed by Ryosuke Niwa.

BuildBotTriggerable._pullBuildbotOnAllSyncers relies on buildReqeustsByGroup to contain all build requests found by
BuildRequest.findById. However, since re-use root change is landed, above assumption is no longer valid. BuildRequest.all()
may includes build requests under same analysis task. Thus, `_pullBuildbotOnAllSyncers` will fail due to 'info' is undefined.

* server-tests/tools-buildbot-triggerable-tests.js: Added a unit test.
* tools/js/buildbot-triggerable.js:
(BuildbotTriggerable.prototype._pullBuildbotOnAllSyncers): Added logic to stop processing the build request if it's not
from incomplete build requests under a triggerable.

Modified Paths

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

Diff

Modified: trunk/Websites/perf.webkit.org/ChangeLog (269891 => 269892)

--- trunk/Websites/perf.webkit.org/ChangeLog	2020-11-17 03:15:47 UTC (rev 269891)
+++ trunk/Websites/perf.webkit.org/ChangeLog	2020-11-17 04:33:06 UTC (rev 269892)
@@ -1,3 +1,19 @@
+2020-11-16  Dewei Zhu  <dewei_...@apple.com>
+
+        BuildbotTriggerable should not process a completed build request from a completed test group.
+        https://bugs.webkit.org/show_bug.cgi?id=219016
+
+        Reviewed by Ryosuke Niwa.
+
+        BuildBotTriggerable._pullBuildbotOnAllSyncers relies on buildReqeustsByGroup to contain all build requests found by
+        BuildRequest.findById. However, since re-use root change is landed, above assumption is no longer valid. BuildRequest.all()
+        may includes build requests under same analysis task. Thus, `_pullBuildbotOnAllSyncers` will fail due to 'info' is undefined.
+
+        * server-tests/tools-buildbot-triggerable-tests.js: Added a unit test.
+        * tools/js/buildbot-triggerable.js:
+        (BuildbotTriggerable.prototype._pullBuildbotOnAllSyncers): Added logic to stop processing the build request if it's not
+        from incomplete build requests under a triggerable.
+
 2020-10-30  Dewei Zhu  <dewei_...@apple.com>
 
         Performance dashboard should avoid building same configuration under one analysis task.

Modified: trunk/Websites/perf.webkit.org/server-tests/tools-buildbot-triggerable-tests.js (269891 => 269892)

--- trunk/Websites/perf.webkit.org/server-tests/tools-buildbot-triggerable-tests.js	2020-11-17 03:15:47 UTC (rev 269891)
+++ trunk/Websites/perf.webkit.org/server-tests/tools-buildbot-triggerable-tests.js	2020-11-17 04:33:06 UTC (rev 269892)
@@ -947,6 +947,85 @@
             });
         });
 
+        it('should skip updating a completed build request whose test group has completed and not listed in a triggerable', async () => {
+            await MockData.addMockBuildRequestsWithRoots(TestServer.database(), ['completed', 'completed', 'completed', 'completed', 'pending', 'pending', 'pending', 'pending']);
+            await Manifest.fetch();
+            const config = MockData.mockTestSyncConfigWithPatchAcceptingBuilder();
+            const logger = new MockLogger;
+            const slaveInfo = {name: 'sync-slave', password: 'password'};
+            const triggerable = new BuildbotTriggerable(config, TestServer.remoteAPI(), MockRemoteAPI, slaveInfo, logger);
+            const syncPromise = triggerable.initSyncers().then(() => triggerable.syncOnce());
+            assertRequestAndResolve(MockRemoteAPI.requests[0], 'GET', MockData.buildbotBuildersURL(), MockData.mockBuildbotBuilders());
+            MockRemoteAPI.reset();
+            await MockRemoteAPI.waitForRequest();
+
+            assert.equal(BuildRequest.all().length, 4);
+            let buildRequest = BuildRequest.findById(900);
+            assert.equal(buildRequest.status(), 'pending');
+            assert.equal(buildRequest.statusUrl(), null);
+
+            assert.equal(MockRemoteAPI.requests.length, 3);
+            assert.equal(MockRemoteAPI.requests[0].method, 'GET');
+            assert.equal(MockRemoteAPI.requests[0].url, MockData.pendingBuildsUrl('some tester'));
+            MockRemoteAPI.requests[0].resolve({});
+            assert.equal(MockRemoteAPI.requests[1].method, 'GET');
+            assert.equal(MockRemoteAPI.requests[1].url, MockData.pendingBuildsUrl('some-builder-1'));
+            MockRemoteAPI.requests[1].resolve({});
+            assert.equal(MockRemoteAPI.requests[2].method, 'GET');
+            assert.equal(MockRemoteAPI.requests[2].url, MockData.pendingBuildsUrl('some builder 2'));
+            MockRemoteAPI.requests[2].resolve({});
+            MockRemoteAPI.reset();
+            await MockRemoteAPI.waitForRequest();
+
+            assert.equal(MockRemoteAPI.requests.length, 3);
+            assert.equal(MockRemoteAPI.requests[0].method, 'GET');
+            assert.equal(MockRemoteAPI.requests[0].url, MockData.recentBuildsUrl('some tester', 2));
+            MockRemoteAPI.requests[0].resolve({});
+            assert.equal(MockRemoteAPI.requests[1].method, 'GET');
+            assert.equal(MockRemoteAPI.requests[1].url, MockData.recentBuildsUrl('some-builder-1', 2));
+            MockRemoteAPI.requests[1].resolve({'builds': [MockData.finishedBuildData({buildRequestId: 801}), MockData.finishedBuildData({buildRequestId: 800})]});
+            assert.equal(MockRemoteAPI.requests[2].method, 'GET');
+            assert.equal(MockRemoteAPI.requests[2].url, MockData.recentBuildsUrl('some builder 2', 2));
+            MockRemoteAPI.requests[2].resolve({});
+            MockRemoteAPI.reset();
+            await MockRemoteAPI.waitForRequest();
+
+            assert.equal(MockRemoteAPI.requests.length, 3);
+            assert.equal(MockRemoteAPI.requests[0].method, 'GET');
+            assert.equal(MockRemoteAPI.requests[0].url, MockData.pendingBuildsUrl('some tester'));
+            MockRemoteAPI.requests[0].resolve({});
+            assert.equal(MockRemoteAPI.requests[1].method, 'GET');
+            assert.equal(MockRemoteAPI.requests[1].url, MockData.pendingBuildsUrl('some-builder-1'));
+            MockRemoteAPI.requests[1].resolve({});
+            assert.equal(MockRemoteAPI.requests[2].method, 'GET');
+            assert.equal(MockRemoteAPI.requests[2].url, MockData.pendingBuildsUrl('some builder 2'));
+            MockRemoteAPI.requests[2].resolve({});
+            MockRemoteAPI.reset();
+            await MockRemoteAPI.waitForRequest();
+
+            assert.equal(MockRemoteAPI.requests.length, 3);
+            assert.equal(MockRemoteAPI.requests[0].method, 'GET');
+            assert.equal(MockRemoteAPI.requests[0].url, MockData.recentBuildsUrl('some tester', 2));
+            MockRemoteAPI.requests[0].resolve({});
+            assert.equal(MockRemoteAPI.requests[1].method, 'GET');
+            assert.equal(MockRemoteAPI.requests[1].url, MockData.recentBuildsUrl('some-builder-1', 2));
+            MockRemoteAPI.requests[1].resolve({'builds': [MockData.finishedBuildData({buildRequestId: 801}), MockData.finishedBuildData({buildRequestId: 800})]});
+            assert.equal(MockRemoteAPI.requests[2].method, 'GET');
+            assert.equal(MockRemoteAPI.requests[2].url, MockData.recentBuildsUrl('some builder 2', 2));
+            MockRemoteAPI.requests[2].resolve({});
+            MockRemoteAPI.reset();
+
+            await syncPromise;
+            await BuildRequest.fetchForTriggerable(MockData.mockTestSyncConfigWithPatchAcceptingBuilder().triggerableName);
+            assert.equal(BuildRequest.all().length, 8);
+            buildRequest = BuildRequest.findById(800);
+            let anotherBuildRequest = BuildRequest.findById(900);
+            assert.equal(buildRequest.status(), 'completed');
+            assert.equal(anotherBuildRequest.status(), 'completed');
+            assert.equal(buildRequest.statusUrl(), 'http://build.webkit.org/buids/1');
+            assert.equal(anotherBuildRequest.statusUrl(), 'http://build.webkit.org/buids/1');
+        });
+
         it('should reuse the roots from a completed build request with the same commit set', async () => {
             await MockData.addMockBuildRequestsWithRoots(TestServer.database());
             await Manifest.fetch();

Modified: trunk/Websites/perf.webkit.org/tools/js/buildbot-triggerable.js (269891 => 269892)

--- trunk/Websites/perf.webkit.org/tools/js/buildbot-triggerable.js	2020-11-17 03:15:47 UTC (rev 269891)
+++ trunk/Websites/perf.webkit.org/tools/js/buildbot-triggerable.js	2020-11-17 04:33:06 UTC (rev 269892)
@@ -176,9 +176,15 @@
                     const request = BuildRequest.findById(entry.buildRequestId());
                     if (!request)
                         continue;
+
+                    const info = buildReqeustsByGroup.get(request.testGroupId());
+                    if (!info) {
+                        assert(request.hasFinished());
+                        continue;
+                    }
+
                     associatedRequests.add(request);
 
-                    const info = buildReqeustsByGroup.get(request.testGroupId());
                     if (request.isBuild()) {
                         assert(!info.buildSyncer || info.buildSyncer == syncer);
                         if (entry.slaveName()) {

_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes