Skip to content

Commit

Permalink
Merge branch 'main' into hide-layout-shift-elements
Browse files Browse the repository at this point in the history
  • Loading branch information
connorjclark committed Jan 8, 2024
2 parents ef484dd + d1cf7e1 commit ffbc26b
Show file tree
Hide file tree
Showing 12 changed files with 92 additions and 46 deletions.
2 changes: 1 addition & 1 deletion build/build-bundle.js
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ async function buildBundle(entryPath, distPath, opts = {minify: true}) {
treeShaking: true,
sourcemap: 'linked',
banner: {js: banner},
lineLimit: 120,
lineLimit: 1000,
// Because of page-functions!
keepNames: true,
inject: ['./build/process-global.js'],
Expand Down
2 changes: 1 addition & 1 deletion core/audits/byte-efficiency/unused-css-rules.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ class UnusedCSSRules extends ByteEfficiencyAudit {
title: str_(UIStrings.title),
description: str_(UIStrings.description),
scoreDisplayMode: ByteEfficiencyAudit.SCORING_MODES.METRIC_SAVINGS,
guidanceLevel: 2,
guidanceLevel: 1,
requiredArtifacts: ['CSSUsage', 'URL', 'devtoolsLogs', 'traces', 'GatherContext'],
};
}
Expand Down
2 changes: 1 addition & 1 deletion core/audits/byte-efficiency/unused-javascript.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ class UnusedJavaScript extends ByteEfficiencyAudit {
title: str_(UIStrings.title),
description: str_(UIStrings.description),
scoreDisplayMode: ByteEfficiencyAudit.SCORING_MODES.METRIC_SAVINGS,
guidanceLevel: 2,
guidanceLevel: 1,
requiredArtifacts: ['JsUsage', 'Scripts', 'SourceMaps', 'GatherContext',
'devtoolsLogs', 'traces', 'URL'],
};
Expand Down
2 changes: 1 addition & 1 deletion core/gather/driver/wait-for-condition.js
Original file line number Diff line number Diff line change
Expand Up @@ -540,7 +540,7 @@ function waitForUserToContinue(driver) {
}
/* c8 ignore stop */

driver.defaultSession.setNextProtocolTimeout(2 ** 31 - 1);
driver.defaultSession.setNextProtocolTimeout(Infinity);
return driver.executionContext.evaluate(createInPagePromise, {args: []});
}

Expand Down
19 changes: 16 additions & 3 deletions core/gather/session.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,17 @@ import {LighthouseError} from '../lib/lh-error.js';

// Controls how long to wait for a response after sending a DevTools protocol command.
const DEFAULT_PROTOCOL_TIMEOUT = 30000;
const PPTR_BUFFER = 50;

/**
* Puppeteer timeouts must fit into an int32 and the maximum timeout for `setTimeout` is a *signed*
* int32. However, this also needs to account for the puppeteer buffer we add to the timeout later.
*
* So this is defined as the max *signed* int32 minus PPTR_BUFFER.
*
* In human terms, this timeout is ~25 days which is as good as infinity for all practical purposes.
*/
const MAX_TIMEOUT = 2147483647 - PPTR_BUFFER;

/** @typedef {LH.Protocol.StrictEventEmitterClass<LH.CrdpEvents>} CrdpEventMessageEmitter */
const CrdpEventEmitter = /** @type {CrdpEventMessageEmitter} */ (EventEmitter);
Expand Down Expand Up @@ -70,6 +81,7 @@ class ProtocolSession extends CrdpEventEmitter {
* @param {number} ms
*/
setNextProtocolTimeout(ms) {
if (ms > MAX_TIMEOUT) ms = MAX_TIMEOUT;
this._nextProtocolTimeout = ms;
}

Expand All @@ -86,15 +98,16 @@ class ProtocolSession extends CrdpEventEmitter {
/** @type {NodeJS.Timer|undefined} */
let timeout;
const timeoutPromise = new Promise((resolve, reject) => {
if (timeoutMs === Infinity) return;

// eslint-disable-next-line max-len
timeout = setTimeout(reject, timeoutMs, new LighthouseError(LighthouseError.errors.PROTOCOL_TIMEOUT, {
protocolMethod: method,
}));
});

const resultPromise = this._cdpSession.send(method, ...params);
const resultPromise = this._cdpSession.send(method, ...params, {
// Add 50ms to the Puppeteer timeout to ensure the Lighthouse timeout finishes first.
timeout: timeoutMs + PPTR_BUFFER,
});
const resultWithTimeoutPromise = Promise.race([resultPromise, timeoutPromise]);

return resultWithTimeoutPromise.finally(() => {
Expand Down
12 changes: 6 additions & 6 deletions core/test/fixtures/user-flows/reports/sample-flow-result.json
Original file line number Diff line number Diff line change
Expand Up @@ -3084,7 +3084,7 @@
}
}
},
"guidanceLevel": 2
"guidanceLevel": 1
},
"unused-javascript": {
"id": "unused-javascript",
Expand Down Expand Up @@ -3116,7 +3116,7 @@
}
}
},
"guidanceLevel": 2
"guidanceLevel": 1
},
"modern-image-formats": {
"id": "modern-image-formats",
Expand Down Expand Up @@ -10460,7 +10460,7 @@
}
}
},
"guidanceLevel": 2
"guidanceLevel": 1
},
"unused-javascript": {
"id": "unused-javascript",
Expand Down Expand Up @@ -10492,7 +10492,7 @@
}
}
},
"guidanceLevel": 2
"guidanceLevel": 1
},
"modern-image-formats": {
"id": "modern-image-formats",
Expand Down Expand Up @@ -21056,7 +21056,7 @@
}
}
},
"guidanceLevel": 2
"guidanceLevel": 1
},
"unused-javascript": {
"id": "unused-javascript",
Expand Down Expand Up @@ -21088,7 +21088,7 @@
}
}
},
"guidanceLevel": 2
"guidanceLevel": 1
},
"modern-image-formats": {
"id": "modern-image-formats",
Expand Down
25 changes: 23 additions & 2 deletions core/test/gather/session-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,13 @@ describe('ProtocolSession', () => {
let puppeteerSession;
/** @type {ProtocolSession} */
let session;
let rawSend = fnAny();

beforeEach(() => {
rawSend = fnAny().mockResolvedValue(Promise.resolve());

// @ts-expect-error - Individual mock functions are applied as necessary.
puppeteerSession = new CdpCDPSession({_rawSend: fnAny(), send: fnAny()}, '', 'root');
puppeteerSession = new CdpCDPSession({_rawSend: rawSend}, '', 'root');
session = new ProtocolSession(puppeteerSession);
});

Expand Down Expand Up @@ -127,7 +130,7 @@ describe('ProtocolSession', () => {

const result = await session.sendCommand('Page.navigate', {url: 'foo'});
expect(result).toEqual(123);
expect(send).toHaveBeenCalledWith('Page.navigate', {url: 'foo'});
expect(send).toHaveBeenCalledWith('Page.navigate', {url: 'foo'}, {timeout: 30050});
});

it('times out a request by default', async () => {
Expand Down Expand Up @@ -186,6 +189,12 @@ describe('ProtocolSession', () => {
expect(resultPromise).toBeDone();
expect(await resultPromise).toBe('result');
});

it('rejects on error from protocol', async () => {
rawSend.mockRejectedValue(new Error('Url is not valid'));
const resultPromise = session.sendCommand('Page.navigate', {url: ''});
await expect(resultPromise).rejects.toThrow('Url is not valid');
});
});

describe('.has/get/setNextProtocolTimeout', () => {
Expand Down Expand Up @@ -216,5 +225,17 @@ describe('ProtocolSession', () => {
expect(session.hasNextProtocolTimeout()).toBe(false);
expect(session.getNextProtocolTimeout()).toBe(DEFAULT_TIMEOUT);
});

it('should handle infinite timeout', () => {
session.setNextProtocolTimeout(Infinity);
expect(session.hasNextProtocolTimeout()).toBe(true);
expect(session.getNextProtocolTimeout()).toBe(2147483597);
});

it('should handle extremely large (but not infinite) timeout', () => {
session.setNextProtocolTimeout(2 ** 40);
expect(session.hasNextProtocolTimeout()).toBe(true);
expect(session.getNextProtocolTimeout()).toBe(2147483597);
});
});
});
4 changes: 2 additions & 2 deletions core/test/results/sample_v2.json
Original file line number Diff line number Diff line change
Expand Up @@ -4674,7 +4674,7 @@
}
}
},
"guidanceLevel": 2
"guidanceLevel": 1
},
"unused-javascript": {
"id": "unused-javascript",
Expand Down Expand Up @@ -4745,7 +4745,7 @@
}
}
},
"guidanceLevel": 2
"guidanceLevel": 1
},
"modern-image-formats": {
"id": "modern-image-formats",
Expand Down
14 changes: 13 additions & 1 deletion core/test/test-env/mocha-setup.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ function getSnapshotState(testFile) {
const snapshotDir = path.join(path.dirname(testFile), '__snapshots__');
const snapshotFile = path.join(snapshotDir, path.basename(testFile) + '.snap');
snapshotState = new SnapshotState(snapshotFile, {
updateSnapshot: process.env.SNAPSHOT_UPDATE ? 'all' : 'new',
updateSnapshot: process.env.SNAPSHOT_UPDATE ? 'all' : 'none',
prettierPath: '',
snapshotFormat: {},
});
Expand Down Expand Up @@ -98,6 +98,7 @@ expect.extend({

const title = makeTestTitle(test);
const snapshotState = getSnapshotState(test.file);

const context = {snapshotState, currentTestName: title};
// @ts-expect-error - this is enough for snapshots to work.
const matcher = toMatchSnapshot.bind(context);
Expand Down Expand Up @@ -146,6 +147,17 @@ const rootHooks = {

// Needed so `expect` extension method can access information about the current test.
mochaCurrentTest = this.currentTest;

// If a test is retried the snapshot indices will start where the previous attempt left off.
// This can lead to several problems including the test passing where it should be failing.
//
// Jest itself clears the snapshot state on retries although they seem to execute retries after
// all tests finish and not immediately after the initial test failure.
// https://github.com/jestjs/jest/pull/8629
if (this.currentTest.retries() && this.currentTest.file) {
const snapshotState = getSnapshotState(this.currentTest.file);
snapshotState.clear();
}
},
/** @this {Mocha.Context} */
afterEach() {
Expand Down
2 changes: 1 addition & 1 deletion docs/readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ You can also craft your own config (e.g. [experimental-config.js](https://github

### Differences from CLI flags

Note that some flag functionality is only available to the CLI. The set of shared flags that work in both node and CLI can be found [in our typedefs](https://github.com/GoogleChrome/lighthouse/blob/888bd6dc9d927a734a8e20ea8a0248baa5b425ed/typings/externs.d.ts#L82-L119). In most cases, the functionality is not offered in the node module simply because it is easier and more flexible to do it yourself.
Note that some flag functionality is only available to the CLI. The set of shared flags that work in both node and CLI can be found [in our typedefs](https://github.com/GoogleChrome/lighthouse/blob/main/types/lhr/settings.d.ts#:~:text=interface%20SharedFlagsSettings). In most cases, the functionality is not offered in the node module simply because it is easier and more flexible to do it yourself.

| CLI Flag | Differences in Node |
| - | - |
Expand Down
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@
"pako": "^2.0.3",
"preact": "^10.7.2",
"pretty-json-stringify": "^0.0.2",
"puppeteer": "^21.5.2",
"puppeteer": "^21.7.0",
"resolve": "^1.22.1",
"rollup": "^2.52.7",
"rollup-plugin-polyfill-node": "^0.12.0",
Expand Down Expand Up @@ -200,7 +200,7 @@
"open": "^8.4.0",
"parse-cache-control": "1.0.1",
"ps-list": "^8.0.0",
"puppeteer-core": "^21.5.2",
"puppeteer-core": "^21.7.0",
"robots-parser": "^3.0.1",
"semver": "^5.3.0",
"speedline-core": "^1.4.3",
Expand Down
50 changes: 25 additions & 25 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -1138,10 +1138,10 @@
resolved "https://registry.yarnpkg.com/@protobufjs/utf8/-/utf8-1.1.0.tgz#a777360b5b39a1a2e5106f8e858f2fd2d060c570"
integrity sha1-p3c2C1s5oaLlEG+OhY8v0tBgxXA=

"@puppeteer/browsers@1.8.0":
version "1.8.0"
resolved "https://registry.yarnpkg.com/@puppeteer/browsers/-/browsers-1.8.0.tgz#fb6ee61de15e7f0e67737aea9f9bab1512dbd7d8"
integrity sha512-TkRHIV6k2D8OlUe8RtG+5jgOF/H98Myx0M6AOafC8DdNVOFiBSFa5cpRDtpm8LXOa9sVwe0+e6Q3FC56X/DZfg==
"@puppeteer/browsers@1.9.1":
version "1.9.1"
resolved "https://registry.yarnpkg.com/@puppeteer/browsers/-/browsers-1.9.1.tgz#384ee8b09786f0e8f62b1925e4c492424cb549ee"
integrity sha512-PuvK6xZzGhKPvlx3fpfdM2kYY3P/hB1URtK8wA7XUJ6prn6pp22zvJHu48th0SGcHL9SutbPHrFuQgfXTFobWA==
dependencies:
debug "4.3.4"
extract-zip "2.0.1"
Expand Down Expand Up @@ -2333,10 +2333,10 @@ chrome-launcher@^1.1.0:
is-wsl "^2.2.0"
lighthouse-logger "^2.0.1"

chromium-bidi@0.4.33:
version "0.4.33"
resolved "https://registry.yarnpkg.com/chromium-bidi/-/chromium-bidi-0.4.33.tgz#9a9aba5a5b07118c8e7d6405f8ee79f47418dd1d"
integrity sha512-IxoFM5WGQOIAd95qrSXzJUv4eXIrh+RvU3rwwqIiwYuvfE7U/Llj4fejbsJnjJMUYCuGtVQsY2gv7oGl4aTNSQ==
chromium-bidi@0.5.2:
version "0.5.2"
resolved "https://registry.yarnpkg.com/chromium-bidi/-/chromium-bidi-0.5.2.tgz#358b03bb7c53e0f8d0fd77d596ea67ee30f7ff06"
integrity sha512-PbVOSddxgKyj+JByqavWMNqWPCoCaT6XK5Z1EFe168sxnB/BM51LnZEPXSbFcFAJv/+u2B4XNTs9uXxy4GW3cQ==
dependencies:
mitt "3.0.1"
urlpattern-polyfill "9.0.0"
Expand Down Expand Up @@ -6029,26 +6029,26 @@ punycode@^2.1.0, punycode@^2.1.1:
resolved "https://registry.yarnpkg.com/punycode/-/punycode-2.1.1.tgz#b58b010ac40c22c5657616c8d2c2c02c7bf479ec"
integrity sha512-XRsRjdf+j5ml+y/6GKHPZbrF/8p2Yga0JPtdqTIY2Xe5ohJPD9saDJJLPvp9+NSBprVvevdXZybnj2cv8OEd0A==

puppeteer-core@21.5.2, puppeteer-core@^21.5.2:
version "21.5.2"
resolved "https://registry.yarnpkg.com/puppeteer-core/-/puppeteer-core-21.5.2.tgz#6d3de4efb2ae65f1ee072043787b75594e88035f"
integrity sha512-v4T0cWnujSKs+iEfmb8ccd7u4/x8oblEyKqplqKnJ582Kw8PewYAWvkH4qUWhitN3O2q9RF7dzkvjyK5HbzjLA==
puppeteer-core@21.7.0, puppeteer-core@^21.7.0:
version "21.7.0"
resolved "https://registry.yarnpkg.com/puppeteer-core/-/puppeteer-core-21.7.0.tgz#c0abb98cbd17dbd7ee317b4257958337fa25d2c7"
integrity sha512-elPYPozrgiM3phSy7VDUJCVWQ07SPnOm78fpSaaSNFoQx5sur/MqhTSro9Wz8lOEjqCykGC6WRkwxDgmqcy1dQ==
dependencies:
"@puppeteer/browsers" "1.8.0"
chromium-bidi "0.4.33"
"@puppeteer/browsers" "1.9.1"
chromium-bidi "0.5.2"
cross-fetch "4.0.0"
debug "4.3.4"
devtools-protocol "0.0.1203626"
ws "8.14.2"
ws "8.16.0"

puppeteer@^21.5.2:
version "21.5.2"
resolved "https://registry.yarnpkg.com/puppeteer/-/puppeteer-21.5.2.tgz#0a4a72175c0fd0944d6486f4734807e1671d527b"
integrity sha512-BaAGJOq8Fl6/cck6obmwaNLksuY0Bg/lIahCLhJPGXBFUD2mCffypa4A592MaWnDcye7eaHmSK9yot0pxctY8A==
puppeteer@^21.7.0:
version "21.7.0"
resolved "https://registry.yarnpkg.com/puppeteer/-/puppeteer-21.7.0.tgz#c4b46ef28a2986f9c536eb086ab47c8dea80e4f9"
integrity sha512-Yy+UUy0b9siJezbhHO/heYUoZQUwyqDK1yOQgblTt0l97tspvDVFkcW9toBlnSvSfkDmMI3Dx9cZL6R8bDArHA==
dependencies:
"@puppeteer/browsers" "1.8.0"
"@puppeteer/browsers" "1.9.1"
cosmiconfig "8.3.6"
puppeteer-core "21.5.2"
puppeteer-core "21.7.0"

q@^1.5.1:
version "1.5.1"
Expand Down Expand Up @@ -7433,10 +7433,10 @@ write-file-atomic@^4.0.1:
imurmurhash "^0.1.4"
signal-exit "^3.0.7"

ws@8.14.2:
version "8.14.2"
resolved "https://registry.yarnpkg.com/ws/-/ws-8.14.2.tgz#6c249a806eb2db7a20d26d51e7709eab7b2e6c7f"
integrity sha512-wEBG1ftX4jcglPxgFCMJmZ2PLtSbJ2Peg6TmpJFTbe9GZYOQCDPdMYu/Tm0/bGZkw8paZnJY45J4K2PZrLYq8g==
ws@8.16.0:
version "8.16.0"
resolved "https://registry.yarnpkg.com/ws/-/ws-8.16.0.tgz#d1cd774f36fbc07165066a60e40323eab6446fd4"
integrity sha512-HS0c//TP7Ina87TfiPUz1rQzMhHrl/SG2guqRcTOIUYD2q8uhUdNHZYJUaQ8aTGPzCh+c6oawMKW35nFl1dxyQ==

ws@>=7.4.6:
version "8.13.0"
Expand Down

0 comments on commit ffbc26b

Please sign in to comment.