Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

core: remove config navigations #15397

Merged
merged 13 commits into from
Oct 18, 2023
80 changes: 18 additions & 62 deletions core/config/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,11 @@ import log from 'lighthouse-logger';

import {Runner} from '../runner.js';
import defaultConfig from './default-config.js';
import {defaultNavigationConfig, nonSimulatedPassConfigOverrides} from './constants.js'; // eslint-disable-line max-len
import {nonSimulatedSettingsOverrides} from './constants.js'; // eslint-disable-line max-len
import {
throwInvalidDependencyOrder,
isValidArtifactDependency,
throwInvalidArtifactDependency,
assertArtifactTopologicalOrder,
assertValidConfig,
} from './validation.js';
import {filterConfigByGatherMode, filterConfigByExplicitFilters} from './filters.js';
Expand Down Expand Up @@ -192,66 +191,34 @@ function overrideSettingsForGatherMode(settings, gatherMode) {
/**
* Overrides the quiet windows when throttlingMethod requires observation.
*
* @param {LH.Config.NavigationDefn} navigation
* @param {LH.Config.Settings} settings
*/
function overrideNavigationThrottlingWindows(navigation, settings) {
if (navigation.disableThrottling) return;
function overrideThrottlingWindows(settings) {
if (settings.throttlingMethod === 'simulate') return;

navigation.cpuQuietThresholdMs = Math.max(
navigation.cpuQuietThresholdMs || 0,
nonSimulatedPassConfigOverrides.cpuQuietThresholdMs
settings.cpuQuietThresholdMs = Math.max(
settings.cpuQuietThresholdMs || 0,
nonSimulatedSettingsOverrides.cpuQuietThresholdMs
);
navigation.networkQuietThresholdMs = Math.max(
navigation.networkQuietThresholdMs || 0,
nonSimulatedPassConfigOverrides.networkQuietThresholdMs
settings.networkQuietThresholdMs = Math.max(
settings.networkQuietThresholdMs || 0,
nonSimulatedSettingsOverrides.networkQuietThresholdMs
);
navigation.pauseAfterFcpMs = Math.max(
navigation.pauseAfterFcpMs || 0,
nonSimulatedPassConfigOverrides.pauseAfterFcpMs
settings.pauseAfterFcpMs = Math.max(
settings.pauseAfterFcpMs || 0,
nonSimulatedSettingsOverrides.pauseAfterFcpMs
);
navigation.pauseAfterLoadMs = Math.max(
navigation.pauseAfterLoadMs || 0,
nonSimulatedPassConfigOverrides.pauseAfterLoadMs
settings.pauseAfterLoadMs = Math.max(
settings.pauseAfterLoadMs || 0,
nonSimulatedSettingsOverrides.pauseAfterLoadMs
);
}

/**
* @param {LH.Config.AnyArtifactDefn[]|null|undefined} artifactDefns
* @param {LH.Config.Settings} settings
* @return {LH.Config.NavigationDefn[] | null}
*/
function resolveFakeNavigations(artifactDefns, settings) {
if (!artifactDefns) return null;

const status = {msg: 'Resolve navigation definitions', id: 'lh:config:resolveNavigationsToDefns'};
log.time(status, 'verbose');

const resolvedNavigation = {
...defaultNavigationConfig,
artifacts: artifactDefns,
pauseAfterFcpMs: settings.pauseAfterFcpMs,
pauseAfterLoadMs: settings.pauseAfterLoadMs,
networkQuietThresholdMs: settings.networkQuietThresholdMs,
cpuQuietThresholdMs: settings.cpuQuietThresholdMs,
blankPage: settings.blankPage,
};

overrideNavigationThrottlingWindows(resolvedNavigation, settings);

const navigations = [resolvedNavigation];
assertArtifactTopologicalOrder(navigations);

log.timeEnd(status);
return navigations;
}

/**
* @param {LH.Gatherer.GatherMode} gatherMode
* @param {LH.Config=} config
* @param {LH.Flags=} flags
* @return {Promise<{resolvedConfig: LH.Config.ResolvedConfig, warnings: string[]}>}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are no config warnings anymore but there could be in the future I guess. Shouldn't be too hard to add this if we ever want them again.

* @return {Promise<{resolvedConfig: LH.Config.ResolvedConfig}>}
*/
async function initializeConfig(gatherMode, config, flags = {}) {
const status = {msg: 'Initialize config', id: 'lh:config'};
Expand All @@ -264,28 +231,26 @@ async function initializeConfig(gatherMode, config, flags = {}) {

const settings = resolveSettings(configWorkingCopy.settings || {}, flags);
overrideSettingsForGatherMode(settings, gatherMode);
overrideThrottlingWindows(settings);

const artifacts = await resolveArtifactsToDefns(configWorkingCopy.artifacts, configDir);

const navigations = resolveFakeNavigations(artifacts, settings);

/** @type {LH.Config.ResolvedConfig} */
let resolvedConfig = {
artifacts,
navigations,
audits: await resolveAuditsToDefns(configWorkingCopy.audits, configDir),
categories: configWorkingCopy.categories || null,
groups: configWorkingCopy.groups || null,
settings,
};

const {warnings} = assertValidConfig(resolvedConfig);
assertValidConfig(resolvedConfig);

resolvedConfig = filterConfigByGatherMode(resolvedConfig, gatherMode);
resolvedConfig = filterConfigByExplicitFilters(resolvedConfig, settings);

log.timeEnd(status);
return {resolvedConfig, warnings};
return {resolvedConfig};
}

/**
Expand All @@ -296,15 +261,6 @@ function getConfigDisplayString(resolvedConfig) {
/** @type {LH.Config.ResolvedConfig} */
const resolvedConfigCopy = JSON.parse(JSON.stringify(resolvedConfig));

if (resolvedConfigCopy.navigations) {
for (const navigation of resolvedConfigCopy.navigations) {
for (let i = 0; i < navigation.artifacts.length; ++i) {
// @ts-expect-error Breaking the Config.AnyArtifactDefn type.
navigation.artifacts[i] = navigation.artifacts[i].id;
}
}
}

if (resolvedConfigCopy.artifacts) {
for (const artifactDefn of resolvedConfigCopy.artifacts) {
// @ts-expect-error Breaking the Config.AnyArtifactDefn type.
Expand Down
20 changes: 2 additions & 18 deletions core/config/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -127,22 +127,7 @@ const defaultSettings = {
skipAudits: null,
};

/** @type {Required<LH.Config.NavigationJson>} */
const defaultNavigationConfig = {
id: 'defaultPass',
loadFailureMode: 'fatal',
disableThrottling: false,
disableStorageReset: false,
pauseAfterFcpMs: 0,
pauseAfterLoadMs: 0,
networkQuietThresholdMs: 0,
cpuQuietThresholdMs: 0,
blockedUrlPatterns: [],
blankPage: 'about:blank',
artifacts: [],
};

const nonSimulatedPassConfigOverrides = {
const nonSimulatedSettingsOverrides = {
pauseAfterFcpMs: 5250,
pauseAfterLoadMs: 5250,
networkQuietThresholdMs: 5250,
Expand All @@ -154,6 +139,5 @@ export {
screenEmulationMetrics,
userAgents,
defaultSettings,
defaultNavigationConfig,
nonSimulatedPassConfigOverrides,
nonSimulatedSettingsOverrides,
};
28 changes: 0 additions & 28 deletions core/config/filters.js
Original file line number Diff line number Diff line change
Expand Up @@ -107,30 +107,6 @@ function filterArtifactsByGatherMode(artifacts, mode) {
});
}

/**
* Filters an array of navigations down to the set supported by the available artifacts.
*
* @param {LH.Config.ResolvedConfig['navigations']} navigations
* @param {Array<LH.Config.AnyArtifactDefn>} availableArtifacts
* @return {LH.Config.ResolvedConfig['navigations']}
*/
function filterNavigationsByAvailableArtifacts(navigations, availableArtifacts) {
if (!navigations) return navigations;

const availableArtifactIds = new Set(
availableArtifacts.map(artifact => artifact.id).concat(baseArtifactKeys)
);

return navigations
.map(navigation => {
return {
...navigation,
artifacts: navigation.artifacts.filter((artifact) => availableArtifactIds.has(artifact.id)),
};
})
.filter(navigation => navigation.artifacts.length);
}

/**
* Filters an array of audits down to the set that can be computed using only the specified artifacts.
*
Expand Down Expand Up @@ -318,13 +294,10 @@ function filterConfigByExplicitFilters(resolvedConfig, filters) {
if (artifacts && resolvedConfig.settings.disableFullPageScreenshot) {
artifacts = artifacts.filter(({id}) => id !== 'FullPageScreenshot');
}
const navigations =
filterNavigationsByAvailableArtifacts(resolvedConfig.navigations, artifacts || []);

return {
...resolvedConfig,
artifacts,
navigations,
audits,
categories,
};
Expand All @@ -335,7 +308,6 @@ export {
filterConfigByExplicitFilters,
filterArtifactsByGatherMode,
filterArtifactsByAvailableAudits,
filterNavigationsByAvailableArtifacts,
filterAuditsByAvailableArtifacts,
filterAuditsByGatherMode,
filterCategoriesByAvailableAudits,
Expand Down
81 changes: 18 additions & 63 deletions core/config/validation.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,43 +68,6 @@
}
}

/**
* Throws an error if the provided object does not implement the required navigations interface.
* @param {LH.Config.ResolvedConfig['navigations']} navigationsDefn
* @return {{warnings: string[]}}
*/
function assertValidNavigations(navigationsDefn) {
if (!navigationsDefn || !navigationsDefn.length) return {warnings: []};

/** @type {string[]} */
const warnings = [];

// Assert that the first navigation has loadFailureMode fatal.
const firstNavigation = navigationsDefn[0];
if (firstNavigation.loadFailureMode !== 'fatal') {
const currentMode = firstNavigation.loadFailureMode;
const warning = [
`"${firstNavigation.id}" is the first navigation but had a failure mode of ${currentMode}.`,
`The first navigation will always be treated as loadFailureMode=fatal.`,
].join(' ');

warnings.push(warning);
firstNavigation.loadFailureMode = 'fatal';
}

// Assert that navigations have unique IDs.
const navigationIds = navigationsDefn.map(navigation => navigation.id);
const duplicateId = navigationIds.find(
(id, i) => navigationIds.slice(i + 1).some(other => id === other)
);

if (duplicateId) {
throw new Error(`Navigation must have unique identifiers, but "${duplicateId}" was repeated.`);
}

return {warnings};
}

/**
* Throws an error if the provided object does not implement the required properties of an audit
* definition.
Expand Down Expand Up @@ -223,50 +186,43 @@
}

/**
* Asserts that artifacts are in a valid dependency order that can be computed.
* Asserts that artifacts are unique, valid and are in a dependency order that can be computed.
*
* @param {Array<LH.Config.NavigationDefn>} navigations
* @param {Array<LH.Config.AnyArtifactDefn>} artifactDefns
*/
function assertArtifactTopologicalOrder(navigations) {
function assertValidArtifacts(artifactDefns) {
/** @type {Set<string>} */
const availableArtifacts = new Set();

for (const navigation of navigations) {
for (const artifact of navigation.artifacts) {
availableArtifacts.add(artifact.id);
if (!artifact.dependencies) continue;
for (const artifact of artifactDefns) {
assertValidArtifact(artifact);

for (const [dependencyKey, {id: dependencyId}] of Object.entries(artifact.dependencies)) {
if (availableArtifacts.has(dependencyId)) continue;
throwInvalidDependencyOrder(artifact.id, dependencyKey);
}
if (availableArtifacts.has(artifact.id)) {
throw new Error(`Config defined multiple artifacts with id '${artifact.id}'`);
}

Check warning on line 202 in core/config/validation.js

View check run for this annotation

Codecov / codecov/patch

core/config/validation.js#L201-L202

Added lines #L201 - L202 were not covered by tests

availableArtifacts.add(artifact.id);
if (!artifact.dependencies) continue;

for (const [dependencyKey, {id: dependencyId}] of Object.entries(artifact.dependencies)) {
if (availableArtifacts.has(dependencyId)) continue;
throwInvalidDependencyOrder(artifact.id, dependencyKey);

Check warning on line 209 in core/config/validation.js

View check run for this annotation

Codecov / codecov/patch

core/config/validation.js#L209

Added line #L209 was not covered by tests
}
}
}

/**
* @param {LH.Config.ResolvedConfig} resolvedConfig
* @return {{warnings: string[]}}
*/
function assertValidConfig(resolvedConfig) {
const {warnings} = assertValidNavigations(resolvedConfig.navigations);

/** @type {Set<string>} */
const artifactIds = new Set();
for (const artifactDefn of resolvedConfig.artifacts || []) {
if (artifactIds.has(artifactDefn.id)) {
throw new Error(`Config defined multiple artifacts with id '${artifactDefn.id}'`);
}
artifactIds.add(artifactDefn.id);
assertValidArtifact(artifactDefn);
}
assertValidArtifacts(resolvedConfig.artifacts || []);

for (const auditDefn of resolvedConfig.audits || []) {
assertValidAudit(auditDefn);
}

assertValidCategories(resolvedConfig.categories, resolvedConfig.audits, resolvedConfig.groups);
assertValidSettings(resolvedConfig.settings);
return {warnings};
}

/**
Expand Down Expand Up @@ -303,11 +259,10 @@
isValidArtifactDependency,
assertValidPluginName,
assertValidArtifact,
assertValidNavigations,
assertValidAudit,
assertValidCategories,
assertValidSettings,
assertArtifactTopologicalOrder,
assertValidArtifacts,
assertValidConfig,
throwInvalidDependencyOrder,
throwInvalidArtifactDependency,
Expand Down
10 changes: 5 additions & 5 deletions core/gather/base-artifacts.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,20 +60,20 @@ function deduplicateWarnings(warnings) {

/**
* @param {LH.BaseArtifacts} baseArtifacts
* @param {Partial<LH.Artifacts>} gathererArtifacts
* @param {Partial<LH.GathererArtifacts>} gathererArtifacts
* @return {LH.Artifacts}
*/
function finalizeArtifacts(baseArtifacts, gathererArtifacts) {
const warnings = baseArtifacts.LighthouseRunWarnings
.concat(gathererArtifacts.LighthouseRunWarnings || [])
.concat(getEnvironmentWarnings({settings: baseArtifacts.settings, baseArtifacts}));
baseArtifacts.LighthouseRunWarnings.push(
...getEnvironmentWarnings({settings: baseArtifacts.settings, baseArtifacts})
);

// Cast to remove the partial from gathererArtifacts.
const artifacts = /** @type {LH.Artifacts} */ ({...baseArtifacts, ...gathererArtifacts});

// Set the post-run meta artifacts.
artifacts.Timing = log.getTimeEntries();
artifacts.LighthouseRunWarnings = deduplicateWarnings(warnings);
artifacts.LighthouseRunWarnings = deduplicateWarnings(baseArtifacts.LighthouseRunWarnings);

if (artifacts.PageLoadError && !artifacts.URL.finalDisplayedUrl) {
artifacts.URL.finalDisplayedUrl = artifacts.URL.requestedUrl || '';
Expand Down
2 changes: 1 addition & 1 deletion core/gather/driver/navigation.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ const DEFAULT_NETWORK_QUIET_THRESHOLD = 5000;
// Controls how long to wait between longtasks before determining the CPU is idle, off by default
const DEFAULT_CPU_QUIET_THRESHOLD = 0;

/** @typedef {{waitUntil: Array<'fcp'|'load'|'navigated'>} & LH.Config.SharedPassNavigationJson & Partial<Pick<LH.Config.Settings, 'maxWaitForFcp'|'maxWaitForLoad'|'debugNavigation'>>} NavigationOptions */
/** @typedef {{waitUntil: Array<'fcp'|'load'|'navigated'>} & Partial<LH.Config.Settings>} NavigationOptions */

/** @param {NavigationOptions} options */
function resolveWaitForFullyLoadedOptions(options) {
Expand Down
Loading
Loading