From df56548dbb7d9e826f7630227864d0de2f3a57a7 Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Fri, 24 May 2024 16:30:55 -0700 Subject: [PATCH 1/3] tests: fix devtools e2e test runner (#16018) --- core/test/devtools-tests/run-e2e-tests.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/test/devtools-tests/run-e2e-tests.sh b/core/test/devtools-tests/run-e2e-tests.sh index 181482609216..72104ee0ca6b 100644 --- a/core/test/devtools-tests/run-e2e-tests.sh +++ b/core/test/devtools-tests/run-e2e-tests.sh @@ -15,4 +15,4 @@ export LH_ROOT="$SCRIPT_DIR/../../.." cd "$DEVTOOLS_PATH" TEST_PATTERN="${1:-lighthouse/*}" -npm run e2etest -- "$TEST_PATTERN" --target=$BUILD_FOLDER +vpython3 third_party/node/node.py --output scripts/test/run_test_suite.js --config=test/e2e/test-runner-config.json "$TEST_PATTERN" --target=$BUILD_FOLDER From c73113df0cf1547db99b0990c22483de7873c845 Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Fri, 24 May 2024 16:31:18 -0700 Subject: [PATCH 2/3] tests: add timeout to runSmokeTest (#16017) --- cli/test/smokehouse/smokehouse.js | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/cli/test/smokehouse/smokehouse.js b/cli/test/smokehouse/smokehouse.js index 4e29e378c212..0a329abdaac8 100644 --- a/cli/test/smokehouse/smokehouse.js +++ b/cli/test/smokehouse/smokehouse.js @@ -163,8 +163,18 @@ async function runSmokeTest(smokeTestDefn, testOptions) { // Run Lighthouse. try { + // Each individual runner has internal timeouts, but we've had bugs where + // that didn't cover some edge case. So to be safe give a (long) timeout here. + const timeoutPromise = new Promise((_, reject) => { + setTimeout(() => + reject(new Error('Timed out waiting for provided lighthouseRunner')), 1000 * 120); + }); + const timedResult = await Promise.race([ + lighthouseRunner(requestedUrl, config, testRunnerOptions), + timeoutPromise, + ]); result = { - ...await lighthouseRunner(requestedUrl, config, testRunnerOptions), + ...timedResult, networkRequests: takeNetworkRequestUrls ? takeNetworkRequestUrls() : undefined, }; From 8d1d78b06818ae8e7aa808353a3d9321924c74b3 Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Fri, 24 May 2024 16:51:29 -0700 Subject: [PATCH 3/3] core(network): fix time units in network quiet calc (#16013) Co-authored-by: Paul Irish --- core/computed/metrics/interactive.js | 4 +- core/gather/driver/network-monitor.js | 8 +-- .../test/computed/metrics/interactive-test.js | 4 +- .../gather/driver/network-monitor-test.js | 55 +++++++++++-------- 4 files changed, 39 insertions(+), 32 deletions(-) diff --git a/core/computed/metrics/interactive.js b/core/computed/metrics/interactive.js index 90f67633d927..2526490d025b 100644 --- a/core/computed/metrics/interactive.js +++ b/core/computed/metrics/interactive.js @@ -187,6 +187,6 @@ export {InteractiveComputed as Interactive}; /** * @typedef TimePeriod - * @property {number} start - * @property {number} end + * @property {number} start In ms. + * @property {number} end In ms. */ diff --git a/core/gather/driver/network-monitor.js b/core/gather/driver/network-monitor.js index 8c6dc73f22f8..814c357a3703 100644 --- a/core/gather/driver/network-monitor.js +++ b/core/gather/driver/network-monitor.js @@ -202,9 +202,10 @@ class NetworkMonitor extends NetworkMonitorEventEmitter { /** * Finds all time periods where the number of inflight requests is less than or equal to the * number of allowed concurrent requests. + * The time periods returned are in ms. * @param {Array} requests * @param {number} allowedConcurrentRequests - * @param {number=} endTime + * @param {number=} endTime In ms * @return {Array<{start: number, end: number}>} */ static findNetworkQuietPeriods(requests, allowedConcurrentRequests, endTime = Infinity) { @@ -215,10 +216,9 @@ class NetworkMonitor extends NetworkMonitorEventEmitter { if (UrlUtils.isNonNetworkProtocol(request.protocol)) return; if (request.protocol === 'ws' || request.protocol === 'wss') return; - // convert the network timestamp to ms - timeBoundaries.push({time: request.networkRequestTime * 1000, isStart: true}); + timeBoundaries.push({time: request.networkRequestTime, isStart: true}); if (request.finished) { - timeBoundaries.push({time: request.networkEndTime * 1000, isStart: false}); + timeBoundaries.push({time: request.networkEndTime, isStart: false}); } }); diff --git a/core/test/computed/metrics/interactive-test.js b/core/test/computed/metrics/interactive-test.js index 216ef384286d..569c1621e55f 100644 --- a/core/test/computed/metrics/interactive-test.js +++ b/core/test/computed/metrics/interactive-test.js @@ -37,8 +37,8 @@ function generateNetworkRecords(partialRecords, timeOrigin) { statusCode: item.statusCode || 200, requestMethod: item.requestMethod || 'GET', finished: typeof item.finished === 'undefined' ? true : item.finished, - networkRequestTime: (item.start + timeOriginInMs) / 1000, - networkEndTime: item.end === -1 ? -1 : (item.end + timeOriginInMs) / 1000, + networkRequestTime: item.start + timeOriginInMs, + networkEndTime: item.end === -1 ? -1 : item.end + timeOriginInMs, }; return /** @type {LH.Artifacts.NetworkRequest} */ (record); }); diff --git a/core/test/gather/driver/network-monitor-test.js b/core/test/gather/driver/network-monitor-test.js index 80337d65cb94..faa66ca424ff 100644 --- a/core/test/gather/driver/network-monitor-test.js +++ b/core/test/gather/driver/network-monitor-test.js @@ -381,9 +381,9 @@ describe('NetworkMonitor', () => { it('should find the 0-quiet periods', () => { const records = [ - record({networkRequestTime: 0, networkEndTime: 1}), - record({networkRequestTime: 2, networkEndTime: 3}), - record({networkRequestTime: 4, networkEndTime: 5}), + record({networkRequestTime: 0, networkEndTime: 1000}), + record({networkRequestTime: 2000, networkEndTime: 3000}), + record({networkRequestTime: 4000, networkEndTime: 5000}), ]; const periods = NetworkMonitor.findNetworkQuietPeriods(records, 0); @@ -392,15 +392,22 @@ describe('NetworkMonitor', () => { {start: 3000, end: 4000}, {start: 5000, end: Infinity}, ]); + // Same thing but verifying these numbers round trip without a problem. + expect(periods).toEqual([ + // The time between the first two, and so on… + {start: records[0].networkEndTime, end: records[1].networkRequestTime}, + {start: records[1].networkEndTime, end: records[2].networkRequestTime}, + {start: records[2].networkEndTime, end: Infinity}, + ]); }); it('should find the 2-quiet periods', () => { const records = [ - record({networkRequestTime: 0, networkEndTime: 1.5}), - record({networkRequestTime: 0, networkEndTime: 2}), - record({networkRequestTime: 0, networkEndTime: 2.5}), - record({networkRequestTime: 2, networkEndTime: 3}), - record({networkRequestTime: 4, networkEndTime: 5}), + record({networkRequestTime: 0, networkEndTime: 1500}), + record({networkRequestTime: 0, networkEndTime: 2000}), + record({networkRequestTime: 0, networkEndTime: 2500}), + record({networkRequestTime: 2000, networkEndTime: 3000}), + record({networkRequestTime: 4000, networkEndTime: 5000}), ]; const periods = NetworkMonitor.findNetworkQuietPeriods(records, 2); @@ -409,14 +416,14 @@ describe('NetworkMonitor', () => { it('should handle unfinished requests', () => { const records = [ - record({networkRequestTime: 0, networkEndTime: 1.5}), - record({networkRequestTime: 0, networkEndTime: 2}), - record({networkRequestTime: 0, networkEndTime: 2.5}), - record({networkRequestTime: 2, networkEndTime: 3}), - record({networkRequestTime: 2}), - record({networkRequestTime: 2}), - record({networkRequestTime: 4, networkEndTime: 5}), - record({networkRequestTime: 5.5}), + record({networkRequestTime: 0, networkEndTime: 1500}), + record({networkRequestTime: 0, networkEndTime: 2000}), + record({networkRequestTime: 0, networkEndTime: 2500}), + record({networkRequestTime: 2000, networkEndTime: 3000}), + record({networkRequestTime: 2000}), + record({networkRequestTime: 2000}), + record({networkRequestTime: 4000, networkEndTime: 5000}), + record({networkRequestTime: 5500}), ]; const periods = NetworkMonitor.findNetworkQuietPeriods(records, 2); @@ -429,8 +436,8 @@ describe('NetworkMonitor', () => { it('should ignore data URIs', () => { const records = [ - record({networkRequestTime: 0, networkEndTime: 1}), - record({networkRequestTime: 0, networkEndTime: 2, url: 'data:image/png;base64,', + record({networkRequestTime: 0, networkEndTime: 1000}), + record({networkRequestTime: 0, networkEndTime: 2000, url: 'data:image/png;base64,', protocol: 'data'}), ]; @@ -443,12 +450,12 @@ describe('NetworkMonitor', () => { finished: false, url: 'https://iframe.com', documentURL: 'https://iframe.com', - responseHeadersEndTime: 1.2, + responseHeadersEndTime: 1200, }; const records = [ - record({networkRequestTime: 0, networkEndTime: 1}), - record({networkRequestTime: 0, networkEndTime: 1.2, ...iframeRequest}), + record({networkRequestTime: 0, networkEndTime: 1000}), + record({networkRequestTime: 0, networkEndTime: 1200, ...iframeRequest}), ]; const periods = NetworkMonitor.findNetworkQuietPeriods(records, 0); @@ -460,12 +467,12 @@ describe('NetworkMonitor', () => { const quicRequest = { finished: false, responseHeaders: [{name: 'ALT-SVC', value: 'hq=":49288";quic="1,1abadaba,51303334,0"'}], - timing: /** @type {*} */ ({receiveHeadersEnd: 1.28}), + timing: /** @type {*} */ ({receiveHeadersEnd: 1280}), }; const records = [ - record({networkRequestTime: 0, networkEndTime: 1}), - record({networkRequestTime: 0, networkEndTime: 2, ...quicRequest}), + record({networkRequestTime: 0, networkEndTime: 1000}), + record({networkRequestTime: 0, networkEndTime: 2000, ...quicRequest}), ]; const periods = NetworkMonitor.findNetworkQuietPeriods(records, 0);