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

Tue, 27 Oct 2020 01:23:03 -0700

Title: [269026] trunk/Websites/perf.webkit.org
Revision
269026
Author
[email protected]
Date
2020-10-27 01:21:48 -0700 (Tue, 27 Oct 2020)

Log Message

Fix and update performance dashboard tests
https://bugs.webkit.org/show_bug.cgi?id=218222

Reviewed by Ryosuke Niwa.

* public/api/upload-root.php: Add a null check against empty array
when accessing invalid key which will show warning since php 7.4.
Per https://wiki.php.net/rfc/notice-for-non-valid-array-container.
* server-tests/tools-sync-buildbot-integration-tests.js:
Fixed a unit test that assumes build request IDs under one test group is
one after another. This assumption is wrong when 'StartServers' under
'mpm_prefork_module' is set more than one in apache config.
Fixed antoher incorrect unit test.
* unit-tests/analysis-results-notifier-tests.js:
Fixed unit tests which incorrectly used 'assert.throws' per
https://nodejs.org/docs/latest-v7.x/api/assert.html#assert_assert_throws_block_error_message.

Modified Paths

Diff

Modified: trunk/Websites/perf.webkit.org/ChangeLog (269025 => 269026)


--- trunk/Websites/perf.webkit.org/ChangeLog	2020-10-27 08:01:43 UTC (rev 269025)
+++ trunk/Websites/perf.webkit.org/ChangeLog	2020-10-27 08:21:48 UTC (rev 269026)
@@ -1,3 +1,22 @@
+2020-10-26  Dewei Zhu  <[email protected]>
+
+        Fix and update performance dashboard tests
+        https://bugs.webkit.org/show_bug.cgi?id=218222
+
+        Reviewed by Ryosuke Niwa.
+
+        * public/api/upload-root.php: Add a null check against empty array
+        when accessing invalid key which will show warning since php 7.4.
+        Per https://wiki.php.net/rfc/notice-for-non-valid-array-container.
+        * server-tests/tools-sync-buildbot-integration-tests.js:
+        Fixed a unit test that assumes build request IDs under one test group is
+        one after another. This assumption is wrong when 'StartServers' under
+        'mpm_prefork_module' is set more than one in apache config.
+        Fixed antoher incorrect unit test.
+        * unit-tests/analysis-results-notifier-tests.js:
+        Fixed unit tests which incorrectly used 'assert.throws' per
+        https://nodejs.org/docs/latest-v7.x/api/assert.html#assert_assert_throws_block_error_message.
+
 2020-03-04  Zhifei Fang  <[email protected]>
 
         Test-freshness page table heaer misplaced

Modified: trunk/Websites/perf.webkit.org/public/api/upload-root.php (269025 => 269026)


--- trunk/Websites/perf.webkit.org/public/api/upload-root.php	2020-10-27 08:01:43 UTC (rev 269025)
+++ trunk/Websites/perf.webkit.org/public/api/upload-root.php	2020-10-27 08:21:48 UTC (rev 269026)
@@ -27,7 +27,7 @@
     $build_request_id = $arguments['buildRequest'];
 
     $request_row = $db->select_first_row('build_requests', 'request', array('id' => $build_request_id));
-    if ($request_row['request_test'] || $request_row['request_order'] >= 0)
+    if (!$request_row || $request_row['request_test'] || $request_row['request_order'] >= 0)
         exit_with_error('InvalidBuildRequestType', array('buildRequest' => $build_request_id));
 
     $test_group = $db->select_first_row('analysis_test_groups', 'testgroup', array('id' => $request_row['request_group']));

Modified: trunk/Websites/perf.webkit.org/server-tests/tools-sync-buildbot-integration-tests.js (269025 => 269026)


--- trunk/Websites/perf.webkit.org/server-tests/tools-sync-buildbot-integration-tests.js	2020-10-27 08:01:43 UTC (rev 269025)
+++ trunk/Websites/perf.webkit.org/server-tests/tools-sync-buildbot-integration-tests.js	2020-10-27 08:21:48 UTC (rev 269026)
@@ -243,7 +243,9 @@
     it('should not schedule on another builder if the build was scheduled on one builder before', () => {
         const requests = MockRemoteAPI.requests;
         let firstTestGroup;
+        let firstTestGroupFirstBuildRequest;
         let secondTestGroup;
+        let secondTestGroupFirstBuildRequest;
         let syncPromise;
         let triggerable;
         let taskId = null;
@@ -254,6 +256,8 @@
         }).then((testGroups) => {
             firstTestGroup = testGroups[0];
             secondTestGroup = testGroups[1];
+            firstTestGroupFirstBuildRequest = firstTestGroup.buildRequests()[0].id();
+            secondTestGroupFirstBuildRequest = secondTestGroup.buildRequests()[0].id();
             taskId = firstTestGroup.task().id();
             anotherTaskId = secondTestGroup.task().id();
             syncPromise = triggerable.initSyncers().then(() => triggerable.syncOnce());
@@ -262,13 +266,13 @@
             return MockRemoteAPI.waitForRequest();
         }).then(() => {
             assert.equal(requests.length, 2);
-            assertAndResolveRequest(requests[0], 'GET', MockData.pendingBuildsUrl('some tester'), MockData.pendingBuild({builderId: MockData.builderIDForName('some tester'), buildRequestId: 5}));
+            assertAndResolveRequest(requests[0], 'GET', MockData.pendingBuildsUrl('some tester'), MockData.pendingBuild({builderId: MockData.builderIDForName('some tester'), buildRequestId: secondTestGroupFirstBuildRequest}));
             assertAndResolveRequest(requests[1], 'GET', MockData.pendingBuildsUrl('another tester'), {});
             return MockRemoteAPI.waitForRequest();
         }).then(() => {
             assert.equal(requests.length, 4);
             assertAndResolveRequest(requests[2], 'GET', MockData.recentBuildsUrl('some tester', 2),
-                MockData.runningBuild({builderId: MockData.builderIDForName('some tester'), buildRequestId: 1}));
+                MockData.runningBuild({builderId: MockData.builderIDForName('some tester'), buildRequestId: firstTestGroupFirstBuildRequest}));
             assertAndResolveRequest(requests[3], 'GET', MockData.recentBuildsUrl('another tester', 2), {});
             return MockRemoteAPI.waitForRequest();
         }).then(() => {
@@ -280,8 +284,8 @@
             assert.equal(requests.length, 8);
             assertAndResolveRequest(requests[6], 'GET', MockData.recentBuildsUrl('some tester', 2), {
                 'builds': [
-                    MockData.runningBuildData({builderId: MockData.builderIDForName('some tester'), buildRequestId: 5}),
-                    MockData.finishedBuildData({builderId: MockData.builderIDForName('some tester'), buildRequestId: 1})]
+                    MockData.runningBuildData({builderId: MockData.builderIDForName('some tester'), buildRequestId: secondTestGroupFirstBuildRequest}),
+                    MockData.finishedBuildData({builderId: MockData.builderIDForName('some tester'), buildRequestId: firstTestGroupFirstBuildRequest})]
             });
             assertAndResolveRequest(requests[7], 'GET', MockData.recentBuildsUrl('another tester', 2), {});
             return syncPromise;
@@ -334,6 +338,8 @@
         const requests = MockRemoteAPI.requests;
         let syncPromise;
         const triggerable = await createTriggerable();
+        const firstBuildNumber = 123;
+        const secondBuildNumber = 124;
         let testGroup = await createTestGroupWithPatch();
 
         const taskId = testGroup.task().id();
@@ -403,7 +409,7 @@
         assert.equal(requests.length, 13);
         assertAndResolveRequest(requests[10], 'GET', MockData.recentBuildsUrl('some tester', 2), {});
         assertAndResolveRequest(requests[11], 'GET', MockData.recentBuildsUrl('some builder', 2),
-            MockData.runningBuild({builderId: MockData.builderIDForName('some builder'), buildRequestId: 1, statusDescription: 'Building WebKit'}));
+            MockData.runningBuild({builderId: MockData.builderIDForName('some builder'), buildRequestId: 1, buildNumber: firstBuildNumber, statusDescription: 'Building WebKit'}));
         assertAndResolveRequest(requests[12], 'GET', MockData.recentBuildsUrl('other builder', 2), {});
         await syncPromise;
 
@@ -459,7 +465,7 @@
         assert.equal(requests.length, 6);
         assertAndResolveRequest(requests[3], 'GET', MockData.recentBuildsUrl('some tester', 2), {});
         assertAndResolveRequest(requests[4], 'GET', MockData.recentBuildsUrl('some builder', 2),
-            MockData.runningBuild({builderId: MockData.builderIDForName('some builder'), buildRequestId: 1, buildNumber: 124, state_string: 'Compiling WTF'}));
+            MockData.runningBuild({builderId: MockData.builderIDForName('some builder'), buildRequestId: 1, buildTag: firstBuildNumber, statusDescription: 'Compiling WTF'}));
         assertAndResolveRequest(requests[5], 'GET', MockData.recentBuildsUrl('other builder', 2), {});
         await MockRemoteAPI.waitForRequest();
 
@@ -471,10 +477,8 @@
 
         assert.equal(requests.length, 12);
         assertAndResolveRequest(requests[9], 'GET', MockData.recentBuildsUrl('some tester', 2), {});
-        assertAndResolveRequest(requests[10], 'GET', MockData.recentBuildsUrl('some builder', 2), {
-            'builds': [
-                MockData.runningBuildData({builderId: MockData.builderIDForName('some builder'), buildRequestId: 1, state_string: 'Compiling WTF'})]
-        });
+        assertAndResolveRequest(requests[10], 'GET', MockData.recentBuildsUrl('some builder', 2),
+            MockData.runningBuild({builderId: MockData.builderIDForName('some builder'), buildRequestId: 1, buildTag: firstBuildNumber, statusDescription: 'Compiling WTF'}));
         assertAndResolveRequest(requests[11], 'GET', MockData.recentBuildsUrl('other builder', 2), {});
         await syncPromise;
 
@@ -489,7 +493,7 @@
         assert(buildRequest.isBuild());
         assert(!buildRequest.isTest());
         assert.equal(buildRequest.statusLabel(), 'Running');
-        assert.equal(buildRequest.statusUrl(), MockData.statusUrl('some builder', 124));
+        assert.equal(buildRequest.statusUrl(), MockData.statusUrl('some builder', firstBuildNumber));
         assert.equal(buildRequest.statusDescription(), 'Compiling WTF');
         assert.equal(buildRequest.buildId(), null);
 
@@ -515,7 +519,7 @@
         assert.equal(otherCommitSet.rootForRepository(webkit), null);
         assert.deepEqual(otherCommitSet.allRootFiles(), []);
 
-        await uploadRoot(buildRequest.id(), 123);
+        await uploadRoot(buildRequest.id(), firstBuildNumber);
 
         testGroups = await TestGroup.fetchForTask(taskId, true);
 
@@ -528,7 +532,7 @@
         assert(buildRequest.isBuild());
         assert(!buildRequest.isTest());
         assert.equal(buildRequest.statusLabel(), 'Completed');
-        assert.equal(buildRequest.statusUrl(), MockData.statusUrl('some builder', 124));
+        assert.equal(buildRequest.statusUrl(), MockData.statusUrl('some builder', firstBuildNumber));
         assert.equal(buildRequest.statusDescription(), 'Compiling WTF');
         assert.notEqual(buildRequest.buildId(), null);
 
@@ -571,7 +575,7 @@
         assert.equal(requests.length, 6);
         assertAndResolveRequest(requests[3], 'GET', MockData.recentBuildsUrl('some tester', 2), {});
         assertAndResolveRequest(requests[4], 'GET', MockData.recentBuildsUrl('some builder', 2),
-            MockData.finishedBuild({builderId: MockData.builderIDForName('some builder'), buildRequestId: 1, buildNumber: 124}));
+            MockData.finishedBuild({builderId: MockData.builderIDForName('some builder'), buildRequestId: 1, buildTag: firstBuildNumber}));
         assertAndResolveRequest(requests[5], 'GET', MockData.recentBuildsUrl('other builder', 2), {});
         await MockRemoteAPI.waitForRequest();
 
@@ -591,8 +595,8 @@
         assertAndResolveRequest(requests[10], 'GET', MockData.recentBuildsUrl('some tester', 2), {});
         assertAndResolveRequest(requests[11], 'GET', MockData.recentBuildsUrl('some builder', 2), {
             'builds': [
-                MockData.runningBuildData({builderId: MockData.builderIDForName('some builder'), buildRequestId: 2}),
-                MockData.finishedBuildData({builderId: MockData.builderIDForName('some builder'), buildRequestId: 1, buildNumber: 124})]
+                MockData.runningBuildData({builderId: MockData.builderIDForName('some builder'), buildRequestId: 2, buildTag: secondBuildNumber}),
+                MockData.finishedBuildData({builderId: MockData.builderIDForName('some builder'), buildRequestId: 1, buildTag: firstBuildNumber})]
         });
         assertAndResolveRequest(requests[12], 'GET', MockData.recentBuildsUrl('other builder', 2), {});
         await syncPromise;
@@ -608,7 +612,7 @@
         assert(buildRequest.isBuild());
         assert(!buildRequest.isTest());
         assert.equal(buildRequest.statusLabel(), 'Completed');
-        assert.equal(buildRequest.statusUrl(), MockData.statusUrl('some builder', 124));
+        assert.equal(buildRequest.statusUrl(), MockData.statusUrl('some builder', firstBuildNumber));
         assert.equal(buildRequest.statusDescription(), null);
         assert.notEqual(buildRequest.buildId(), null);
 
@@ -626,7 +630,7 @@
         assert(otherBuildRequest.isBuild());
         assert(!otherBuildRequest.isTest());
         assert.equal(otherBuildRequest.statusLabel(), 'Running');
-        assert.equal(otherBuildRequest.statusUrl(), MockData.statusUrl('some builder', 124));
+        assert.equal(otherBuildRequest.statusUrl(), MockData.statusUrl('some builder', secondBuildNumber));
         assert.equal(otherBuildRequest.statusDescription(), null);
         assert.equal(otherBuildRequest.buildId(), null);
 
@@ -648,7 +652,7 @@
         assert(buildRequest.isBuild());
         assert(!buildRequest.isTest());
         assert.equal(buildRequest.statusLabel(), 'Completed');
-        assert.equal(buildRequest.statusUrl(), MockData.statusUrl('some builder', 124));
+        assert.equal(buildRequest.statusUrl(), MockData.statusUrl('some builder', firstBuildNumber));
         assert.equal(buildRequest.statusDescription(), null);
         assert.notEqual(buildRequest.buildId(), null);
 
@@ -666,7 +670,7 @@
         assert(otherBuildRequest.isBuild());
         assert(!otherBuildRequest.isTest());
         assert.equal(otherBuildRequest.statusLabel(), 'Completed');
-        assert.equal(otherBuildRequest.statusUrl(), MockData.statusUrl('some builder', 124));
+        assert.equal(otherBuildRequest.statusUrl(), MockData.statusUrl('some builder', secondBuildNumber));
         assert.equal(otherBuildRequest.statusDescription(), null);
         assert.notEqual(otherBuildRequest.buildId(), null);

Modified: trunk/Websites/perf.webkit.org/unit-tests/analysis-results-notifier-tests.js (269025 => 269026)


--- trunk/Websites/perf.webkit.org/unit-tests/analysis-results-notifier-tests.js	2020-10-27 08:01:43 UTC (rev 269025)
+++ trunk/Websites/perf.webkit.org/unit-tests/analysis-results-notifier-tests.js	2020-10-27 08:21:48 UTC (rev 269026)
@@ -86,11 +86,11 @@
 
     describe('_validateRules', () => {
         it('should fail the validation for empty rule', () => {
-            assert.throws(() => AnalysisResultsNotifier._validateRules([{}]), 'AssertionError [ERR_ASSERTION]', 'Either tests or platforms should be an array of strings');
+            assert.throws(() => AnalysisResultsNotifier._validateRules([{}]), /AssertionError \[ERR_ASSERTION\]/, 'Either tests or platforms should be an array of strings');
         });
 
         it('should fail the validation for platforms of a rule is not an array of string', () => {
-            assert.throws(() => AnalysisResultsNotifier._validateRules([{tests: [{}]}]), 'AssertionError [ERR_ASSERTION]', 'Either tests or platforms should be an array of strings');
+            assert.throws(() => AnalysisResultsNotifier._validateRules([{tests: [{}]}]), /AssertionError \[ERR_ASSERTION\]/, 'Either tests or platforms should be an array of strings');
         });
 
         it('should pass the validation for if a rule only has tests specified', () => {

_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to