Skip to content

Commit 0d62b38

Browse files
patrickhulcepatrickhulce
committed
core(fr): extract warnings from gather-runner
1 parent 40b8df8 commit 0d62b38

File tree

3 files changed

+114
-76
lines changed

3 files changed

+114
-76
lines changed

‎lighthouse-core/gather/driver/environment.js

+63
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,28 @@
66
'use strict';
77

88
const log = require('lighthouse-logger');
9+
const constants = require('../../config/constants.js');
910
const pageFunctions = require('../../lib/page-functions.js');
11+
const i18n = require('../../lib/i18n/i18n.js');
12+
13+
const UIStrings = {
14+
/**
15+
* @description Warning that the host device where Lighthouse is running appears to have a slower
16+
* CPU than the expected Lighthouse baseline.
17+
*/
18+
warningSlowHostCpu: 'The tested device appears to have a slower CPU than ' +
19+
'Lighthouse expects. This can negatively affect your performance score. Learn more about ' +
20+
'[calibrating an appropriate CPU slowdown multiplier](https://github.com/GoogleChrome/lighthouse/blob/master/docs/throttling.md#cpu-throttling).',
21+
};
22+
23+
/**
24+
* We want to warn when the CPU seemed to be at least ~2x weaker than our regular target device.
25+
* We're starting with a more conservative value that will increase over time to our true target threshold.
26+
* @see https://github.com/GoogleChrome/lighthouse/blob/ccbc8002fd058770d14e372a8301cc4f7d256414/docs/throttling.md#calibrating-multipliers
27+
*/
28+
const SLOW_CPU_BENCHMARK_INDEX_THRESHOLD = 1000;
29+
30+
const str_ = i18n.createMessageInstanceIdFn(__filename, UIStrings);
1031

1132
/**
1233
* @param {LH.Gatherer.FRProtocolSession} session
@@ -37,7 +58,49 @@ async function getBenchmarkIndex(executionContext) {
3758
return indexVal;
3859
}
3960

61+
/**
62+
* Returns a warning if the host device appeared to be underpowered according to BenchmarkIndex.
63+
*
64+
* @param {{settings: LH.Config.Settings; BenchmarkIndex: number}} context
65+
* @return {LH.IcuMessage | undefined}
66+
*/
67+
function getSlowHostCpuWarning(context) {
68+
const {settings, BenchmarkIndex} = context;
69+
const {throttling, throttlingMethod} = settings;
70+
const defaultThrottling = constants.defaultSettings.throttling;
71+
72+
// We only want to warn when the user can take an action to fix it.
73+
// Eventually, this should expand to cover DevTools.
74+
if (settings.channel !== 'cli') return;
75+
76+
// Only warn if they are using the default throttling settings.
77+
const isThrottledMethod = throttlingMethod === 'simulate' || throttlingMethod === 'devtools';
78+
const isDefaultMultiplier =
79+
throttling.cpuSlowdownMultiplier === defaultThrottling.cpuSlowdownMultiplier;
80+
if (!isThrottledMethod || !isDefaultMultiplier) return;
81+
82+
// Only warn if the device didn't meet the threshold.
83+
// See https://github.com/GoogleChrome/lighthouse/blob/master/docs/throttling.md#cpu-throttling
84+
if (BenchmarkIndex > SLOW_CPU_BENCHMARK_INDEX_THRESHOLD) return;
85+
86+
return str_(UIStrings.warningSlowHostCpu);
87+
}
88+
89+
/**
90+
* @param {{settings: LH.Config.Settings, baseArtifacts: Pick<LH.Artifacts, 'BenchmarkIndex'>}} context
91+
* @return {Array<LH.IcuMessage>}
92+
*/
93+
function getEnvironmentWarnings(context) {
94+
const {settings, baseArtifacts} = context;
95+
96+
return [
97+
getSlowHostCpuWarning({settings, BenchmarkIndex: baseArtifacts.BenchmarkIndex}),
98+
].filter(/** @return {s is LH.IcuMessage} */ s => !!s);
99+
}
100+
40101
module.exports = {
102+
UIStrings,
41103
getBrowserVersion,
42104
getBenchmarkIndex,
105+
getEnvironmentWarnings,
43106
};

‎lighthouse-core/gather/driver/navigation.js

+45-2
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,27 @@
88
const NetworkMonitor = require('./network-monitor.js');
99
const {waitForFullyLoaded, waitForFrameNavigated} = require('./wait-for-condition.js');
1010
const constants = require('../../config/constants.js');
11+
const i18n = require('../../lib/i18n/i18n.js');
12+
const URL = require('../../lib/url-shim.js');
13+
14+
const UIStrings = {
15+
/**
16+
* @description Warning that the web page redirected during testing and that may have affected the load.
17+
* @example {https://example.com/requested/page} requested
18+
* @example {https://example.com/final/resolved/page} final
19+
*/
20+
warningRedirected: 'The page may not be loading as expected because your test URL ' +
21+
`({requested}) was redirected to {final}. ` +
22+
'Try testing the second URL directly.',
23+
/**
24+
* @description Warning that Lighthouse timed out while waiting for the page to load.
25+
*/
26+
warningTimeout: 'The page loaded too slowly to finish within the time limit. ' +
27+
'Results may be incomplete.',
28+
};
29+
30+
const str_ = i18n.createMessageInstanceIdFn(__filename, UIStrings);
31+
1132

1233
// Controls how long to wait after FCP before continuing
1334
const DEFAULT_PAUSE_AFTER_FCP = 0;
@@ -58,7 +79,7 @@ function resolveWaitForFullyLoadedOptions(options) {
5879
* @param {LH.Gatherer.FRTransitionalDriver} driver
5980
* @param {string} url
6081
* @param {NavigationOptions} options
61-
* @return {Promise<{finalUrl: string, timedOut: boolean}>}
82+
* @return {Promise<{finalUrl: string, timedOut: boolean, warnings: Array<LH.IcuMessage>}>}
6283
*/
6384
async function gotoURL(driver, url, options) {
6485
const session = driver.defaultSession;
@@ -103,7 +124,29 @@ async function gotoURL(driver, url, options) {
103124
return {
104125
finalUrl,
105126
timedOut,
127+
warnings: getNavigationWarnings({timedOut, finalUrl, requestedUrl: url}),
106128
};
107129
}
108130

109-
module.exports = {gotoURL};
131+
/**
132+
* @param {{timedOut: boolean, requestedUrl: string, finalUrl: string; }} navigation
133+
* @return {Array<LH.IcuMessage>}
134+
*/
135+
function getNavigationWarnings(navigation) {
136+
const {requestedUrl, finalUrl} = navigation;
137+
/** @type {Array<LH.IcuMessage>} */
138+
const warnings = [];
139+
140+
if (navigation.timedOut) warnings.push(str_(UIStrings.warningTimeout));
141+
142+
if (!URL.equalWithExcludedFragments(requestedUrl, finalUrl)) {
143+
warnings.push(str_(UIStrings.warningRedirected, {
144+
requested: requestedUrl,
145+
final: finalUrl,
146+
}));
147+
}
148+
149+
return warnings;
150+
}
151+
152+
module.exports = {gotoURL, UIStrings};

‎lighthouse-core/gather/gather-runner.js

+6-74
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,7 @@ const NetworkRecorder = require('../lib/network-recorder.js');
1111
const emulation = require('../lib/emulation.js');
1212
const constants = require('../config/constants.js');
1313
const i18n = require('../lib/i18n/i18n.js');
14-
const URL = require('../lib/url-shim.js');
15-
const {getBenchmarkIndex} = require('./driver/environment.js');
14+
const {getBenchmarkIndex, getEnvironmentWarnings} = require('./driver/environment.js');
1615
const prepare = require('./driver/prepare.js');
1716
const storage = require('./driver/storage.js');
1817
const navigation = require('./driver/navigation.js');
@@ -22,38 +21,6 @@ const InstallabilityErrors = require('./gatherers/installability-errors.js');
2221
const NetworkUserAgent = require('./gatherers/network-user-agent.js');
2322
const Stacks = require('./gatherers/stacks.js');
2423

25-
const UIStrings = {
26-
/**
27-
* @description Warning that the web page redirected during testing and that may have affected the load.
28-
* @example {https://example.com/requested/page} requested
29-
* @example {https://example.com/final/resolved/page} final
30-
*/
31-
warningRedirected: 'The page may not be loading as expected because your test URL ' +
32-
`({requested}) was redirected to {final}. ` +
33-
'Try testing the second URL directly.',
34-
/**
35-
* @description Warning that Lighthouse timed out while waiting for the page to load.
36-
*/
37-
warningTimeout: 'The page loaded too slowly to finish within the time limit. ' +
38-
'Results may be incomplete.',
39-
/**
40-
* @description Warning that the host device where Lighthouse is running appears to have a slower
41-
* CPU than the expected Lighthouse baseline.
42-
*/
43-
warningSlowHostCpu: 'The tested device appears to have a slower CPU than ' +
44-
'Lighthouse expects. This can negatively affect your performance score. Learn more about ' +
45-
'[calibrating an appropriate CPU slowdown multiplier](https://github.com/GoogleChrome/lighthouse/blob/master/docs/throttling.md#cpu-throttling).',
46-
};
47-
48-
/**
49-
* We want to warn when the CPU seemed to be at least ~2x weaker than our regular target device.
50-
* We're starting with a more conservative value that will increase over time to our true target threshold.
51-
* @see https://github.com/GoogleChrome/lighthouse/blob/ccbc8002fd058770d14e372a8301cc4f7d256414/docs/throttling.md#calibrating-multipliers
52-
*/
53-
const SLOW_CPU_BENCHMARK_INDEX_THRESHOLD = 1000;
54-
55-
const str_ = i18n.createMessageInstanceIdFn(__filename, UIStrings);
56-
5724
/** @typedef {import('../gather/driver.js')} Driver */
5825

5926
/**
@@ -101,15 +68,16 @@ class GatherRunner {
10168
};
10269
log.time(status);
10370
try {
104-
const {finalUrl, timedOut} = await navigation.gotoURL(driver, passContext.url, {
71+
const requestedUrl = passContext.url;
72+
const {finalUrl, warnings} = await navigation.gotoURL(driver, requestedUrl, {
10573
waitUntil: passContext.passConfig.recordTrace ?
10674
['load', 'fcp'] : ['load'],
10775
maxWaitForFcp: passContext.settings.maxWaitForFcp,
10876
maxWaitForLoad: passContext.settings.maxWaitForLoad,
10977
...passContext.passConfig,
11078
});
11179
passContext.url = finalUrl;
112-
if (timedOut) passContext.LighthouseRunWarnings.push(str_(UIStrings.warningTimeout));
80+
passContext.LighthouseRunWarnings.push(...warnings);
11381
} catch (err) {
11482
// If it's one of our loading-based LHErrors, we'll treat it as a page load error.
11583
if (err.code === 'NO_FCP' || err.code === 'PAGE_HUNG') {
@@ -222,34 +190,6 @@ class GatherRunner {
222190
log.timeEnd(status);
223191
}
224192

225-
/**
226-
* Returns a warning if the host device appeared to be underpowered according to BenchmarkIndex.
227-
*
228-
* @param {Pick<LH.Gatherer.PassContext, 'settings'|'baseArtifacts'>} passContext
229-
* @return {LH.IcuMessage | undefined}
230-
*/
231-
static getSlowHostCpuWarning(passContext) {
232-
const {settings, baseArtifacts} = passContext;
233-
const {throttling, throttlingMethod} = settings;
234-
const defaultThrottling = constants.defaultSettings.throttling;
235-
236-
// We only want to warn when the user can take an action to fix it.
237-
// Eventually, this should expand to cover DevTools.
238-
if (settings.channel !== 'cli') return;
239-
240-
// Only warn if they are using the default throttling settings.
241-
const isThrottledMethod = throttlingMethod === 'simulate' || throttlingMethod === 'devtools';
242-
const isDefaultMultiplier =
243-
throttling.cpuSlowdownMultiplier === defaultThrottling.cpuSlowdownMultiplier;
244-
if (!isThrottledMethod || !isDefaultMultiplier) return;
245-
246-
// Only warn if the device didn't meet the threshold.
247-
// See https://github.com/GoogleChrome/lighthouse/blob/master/docs/throttling.md#cpu-throttling
248-
if (baseArtifacts.BenchmarkIndex > SLOW_CPU_BENCHMARK_INDEX_THRESHOLD) return;
249-
250-
return str_(UIStrings.warningSlowHostCpu);
251-
}
252-
253193
/**
254194
* Beging recording devtoolsLog and trace (if requested).
255195
* @param {LH.Gatherer.PassContext} passContext
@@ -481,13 +421,6 @@ class GatherRunner {
481421

482422
// Copy redirected URL to artifact.
483423
baseArtifacts.URL.finalUrl = passContext.url;
484-
/* eslint-disable max-len */
485-
if (!URL.equalWithExcludedFragments(baseArtifacts.URL.requestedUrl, baseArtifacts.URL.finalUrl)) {
486-
baseArtifacts.LighthouseRunWarnings.push(str_(UIStrings.warningRedirected, {
487-
requested: baseArtifacts.URL.requestedUrl,
488-
final: baseArtifacts.URL.finalUrl,
489-
}));
490-
}
491424

492425
// Fetch the manifest, if it exists.
493426
baseArtifacts.WebAppManifest = await WebAppManifest.getWebAppManifest(
@@ -504,8 +437,8 @@ class GatherRunner {
504437
const devtoolsLog = baseArtifacts.devtoolsLogs[passContext.passConfig.passName];
505438
baseArtifacts.NetworkUserAgent = NetworkUserAgent.getNetworkUserAgent(devtoolsLog);
506439

507-
const slowCpuWarning = GatherRunner.getSlowHostCpuWarning(passContext);
508-
if (slowCpuWarning) baseArtifacts.LighthouseRunWarnings.push(slowCpuWarning);
440+
const environmentWarnings = getEnvironmentWarnings(passContext);
441+
baseArtifacts.LighthouseRunWarnings.push(...environmentWarnings);
509442

510443
log.timeEnd(status);
511444
}
@@ -673,4 +606,3 @@ class GatherRunner {
673606
}
674607

675608
module.exports = GatherRunner;
676-
module.exports.UIStrings = UIStrings;

0 commit comments

Comments
 (0)