Skip to content

Commit

Permalink
core(fr): replace configContext with flags (GoogleChrome#14050)
Browse files Browse the repository at this point in the history
  • Loading branch information
adamraine authored and alexnj committed Aug 24, 2022
1 parent e33d237 commit 9583242
Show file tree
Hide file tree
Showing 26 changed files with 220 additions and 232 deletions.
2 changes: 2 additions & 0 deletions cli/test/cli/__snapshots__/index-test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,7 @@ Object {
"mobile": true,
"width": 360,
},
"skipAboutBlank": false,
"skipAudits": null,
"throttling": Object {
"cpuSlowdownMultiplier": 4,
Expand Down Expand Up @@ -359,6 +360,7 @@ Object {
"mobile": true,
"width": 360,
},
"skipAboutBlank": false,
"skipAudits": null,
"throttling": Object {
"cpuSlowdownMultiplier": 4,
Expand Down
15 changes: 0 additions & 15 deletions core/config/config-helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -606,20 +606,6 @@ function deepCloneConfigJson(json) {
return cloned;
}

/**
* @param {LH.Flags} flags
* @return {LH.Config.FRContext}
*/
function flagsToFRContext(flags) {
return {
configPath: flags?.configPath,
settingsOverrides: flags,
logLevel: flags?.logLevel,
hostname: flags?.hostname,
port: flags?.port,
};
}

export {
deepClone,
deepCloneConfigJson,
Expand All @@ -631,5 +617,4 @@ export {
resolveGathererToDefn,
resolveModulePath,
resolveSettings,
flagsToFRContext,
};
1 change: 1 addition & 0 deletions core/config/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ const defaultSettings = {
onlyAudits: null,
onlyCategories: null,
skipAudits: null,
skipAboutBlank: false,
};

/** @type {LH.Config.Pass} */
Expand Down
25 changes: 12 additions & 13 deletions core/fraggle-rock/config/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,6 @@ const defaultConfigPath = path.join(
'../../config/default-config.js'
);

/** @typedef {LH.Config.FRContext & {gatherMode: LH.Gatherer.GatherMode}} ConfigContext */

/**
* @param {LH.Config.Json|undefined} configJSON
* @param {{configPath?: string}} context
Expand Down Expand Up @@ -176,10 +174,10 @@ async function resolveArtifactsToDefns(artifacts, configDir) {
* Overrides the settings that may not apply to the chosen gather mode.
*
* @param {LH.Config.Settings} settings
* @param {ConfigContext} context
* @param {LH.Gatherer.GatherMode} gatherMode
*/
function overrideSettingsForGatherMode(settings, context) {
if (context.gatherMode === 'timespan') {
function overrideSettingsForGatherMode(settings, gatherMode) {
if (gatherMode === 'timespan') {
if (settings.throttlingMethod === 'simulate') {
settings.throttlingMethod = 'devtools';
}
Expand Down Expand Up @@ -251,21 +249,22 @@ function resolveNavigationsToDefns(navigations, artifactDefns, settings) {
}

/**
* @param {LH.Config.Json|undefined} configJSON
* @param {ConfigContext} context
* @param {LH.Gatherer.GatherMode} gatherMode
* @param {LH.Config.Json=} configJSON
* @param {LH.Flags=} flags
* @return {Promise<{config: LH.Config.FRConfig, warnings: string[]}>}
*/
async function initializeConfig(configJSON, context) {
async function initializeConfig(gatherMode, configJSON, flags = {}) {
const status = {msg: 'Initialize config', id: 'lh:config'};
log.time(status, 'verbose');

let {configWorkingCopy, configDir} = resolveWorkingCopy(configJSON, context); // eslint-disable-line prefer-const
let {configWorkingCopy, configDir} = resolveWorkingCopy(configJSON, flags);

configWorkingCopy = resolveExtensions(configWorkingCopy);
configWorkingCopy = await mergePlugins(configWorkingCopy, configDir, context.settingsOverrides);
configWorkingCopy = await mergePlugins(configWorkingCopy, configDir, flags);

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

const artifacts = await resolveArtifactsToDefns(configWorkingCopy.artifacts, configDir);
const navigations = resolveNavigationsToDefns(configWorkingCopy.navigations, artifacts, settings);
Expand All @@ -282,7 +281,7 @@ async function initializeConfig(configJSON, context) {

const {warnings} = assertValidConfig(config);

config = filterConfigByGatherMode(config, context.gatherMode);
config = filterConfigByGatherMode(config, gatherMode);
config = filterConfigByExplicitFilters(config, settings);

log.timeEnd(status);
Expand Down
41 changes: 16 additions & 25 deletions core/fraggle-rock/gather/navigation-runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,6 @@ import Trace from '../../gather/gatherers/trace.js';
import DevtoolsLog from '../../gather/gatherers/devtools-log.js';
import NetworkRecords from '../../computed/network-records.js';

/** @typedef {{skipAboutBlank?: boolean}} InternalOptions */

/**
* @typedef NavigationContext
* @property {Driver} driver
Expand All @@ -35,7 +33,6 @@ import NetworkRecords from '../../computed/network-records.js';
* @property {LH.NavigationRequestor} requestor
* @property {LH.FRBaseArtifacts} baseArtifacts
* @property {Map<string, LH.ArbitraryEqualityMap>} computedCache
* @property {InternalOptions} [options]
*/

/** @typedef {Omit<Parameters<typeof collectPhaseArtifacts>[0], 'phase'>} PhaseState */
Expand All @@ -44,12 +41,14 @@ const DEFAULT_HOSTNAME = '127.0.0.1';
const DEFAULT_PORT = 9222;

/**
* @param {{driver: Driver, config: LH.Config.FRConfig, options?: InternalOptions}} args
* @param {{driver: Driver, config: LH.Config.FRConfig, requestor: LH.NavigationRequestor}} args
* @return {Promise<{baseArtifacts: LH.FRBaseArtifacts}>}
*/
async function _setup({driver, config, options}) {
async function _setup({driver, config, requestor}) {
await driver.connect();
if (!options?.skipAboutBlank) {

// We can't trigger the navigation through user interaction if we reset the page before starting.
if (typeof requestor === 'string' && !config.settings.skipAboutBlank) {
await gotoURL(driver, defaultNavigationConfig.blankPage, {waitUntil: ['navigated']});
}

Expand All @@ -64,10 +63,12 @@ async function _setup({driver, config, options}) {
* @param {NavigationContext} navigationContext
* @return {Promise<{warnings: Array<LH.IcuMessage>}>}
*/
async function _setupNavigation({requestor, driver, navigation, config, options}) {
if (!options?.skipAboutBlank) {
async function _setupNavigation({requestor, driver, navigation, config}) {
// We can't trigger the navigation through user interaction if we reset the page before starting.
if (typeof requestor === 'string' && !config.settings.skipAboutBlank) {
await gotoURL(driver, navigation.blankPage, {...navigation, waitUntil: ['navigated']});
}

const {warnings} = await prepare.prepareTargetForIndividualNavigation(
driver.defaultSession,
config.settings,
Expand Down Expand Up @@ -249,10 +250,10 @@ async function _navigation(navigationContext) {
}

/**
* @param {{driver: Driver, config: LH.Config.FRConfig, requestor: LH.NavigationRequestor; baseArtifacts: LH.FRBaseArtifacts, computedCache: NavigationContext['computedCache'], options?: InternalOptions}} args
* @param {{driver: Driver, config: LH.Config.FRConfig, requestor: LH.NavigationRequestor; baseArtifacts: LH.FRBaseArtifacts, computedCache: NavigationContext['computedCache']}} args
* @return {Promise<{artifacts: Partial<LH.FRArtifacts & LH.FRBaseArtifacts>}>}
*/
async function _navigations({driver, config, requestor, baseArtifacts, computedCache, options}) {
async function _navigations({driver, config, requestor, baseArtifacts, computedCache}) {
if (!config.navigations) throw new Error('No navigations configured');

/** @type {Partial<LH.FRArtifacts & LH.FRBaseArtifacts>} */
Expand All @@ -268,7 +269,6 @@ async function _navigations({driver, config, requestor, baseArtifacts, computedC
config,
baseArtifacts,
computedCache,
options,
};

let shouldHaltNavigations = false;
Expand Down Expand Up @@ -300,25 +300,17 @@ async function _cleanup({requestedUrl, driver, config}) {

/**
* @param {LH.NavigationRequestor|undefined} requestor
* @param {{page?: LH.Puppeteer.Page, config?: LH.Config.Json, configContext?: LH.Config.FRContext}} options
* @param {{page?: LH.Puppeteer.Page, config?: LH.Config.Json, flags?: LH.Flags}} options
* @return {Promise<LH.Gatherer.FRGatherResult>}
*/
async function navigationGather(requestor, options) {
const {configContext = {}} = options;
log.setLevel(configContext.logLevel || 'error');
const {flags = {}} = options;
log.setLevel(flags.logLevel || 'error');

const {config} =
await initializeConfig(options.config, {...configContext, gatherMode: 'navigation'});
const {config} = await initializeConfig('navigation', options.config, flags);
const computedCache = new Map();
const internalOptions = {
skipAboutBlank: configContext.skipAboutBlank,
};

// We can't trigger the navigation through user interaction if we reset the page before starting.
const isCallback = typeof requestor === 'function';
if (isCallback) {
internalOptions.skipAboutBlank = true;
}

const runnerOptions = {config, computedCache};
const artifacts = await Runner.gather(
Expand All @@ -329,7 +321,7 @@ async function navigationGather(requestor, options) {
// For navigation mode, we shouldn't connect to a browser in audit mode,
// therefore we connect to the browser in the gatherFn callback.
if (!page) {
const {hostname = DEFAULT_HOSTNAME, port = DEFAULT_PORT} = configContext;
const {hostname = DEFAULT_HOSTNAME, port = DEFAULT_PORT} = flags;
const browser = await puppeteer.connect({browserURL: `http://${hostname}:${port}`});
page = await browser.newPage();
}
Expand All @@ -339,7 +331,6 @@ async function navigationGather(requestor, options) {
driver,
config,
requestor: normalizedRequestor,
options: internalOptions,
};
const {baseArtifacts} = await _setup(context);
const {artifacts} = await _navigations({...context, baseArtifacts, computedCache});
Expand Down
9 changes: 4 additions & 5 deletions core/fraggle-rock/gather/snapshot-runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,14 @@ import {initializeConfig} from '../config/config.js';
import {getBaseArtifacts, finalizeArtifacts} from './base-artifacts.js';

/**
* @param {{page: LH.Puppeteer.Page, config?: LH.Config.Json, configContext?: LH.Config.FRContext}} options
* @param {{page: LH.Puppeteer.Page, config?: LH.Config.Json, flags?: LH.Flags}} options
* @return {Promise<LH.Gatherer.FRGatherResult>}
*/
async function snapshotGather(options) {
const {configContext = {}} = options;
log.setLevel(configContext.logLevel || 'error');
const {flags = {}} = options;
log.setLevel(flags.logLevel || 'error');

const {config} =
await initializeConfig(options.config, {...configContext, gatherMode: 'snapshot'});
const {config} = await initializeConfig('snapshot', options.config, flags);
const driver = new Driver(options.page);
await driver.connect();

Expand Down
9 changes: 4 additions & 5 deletions core/fraggle-rock/gather/timespan-runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,14 @@ import {initializeConfig} from '../config/config.js';
import {getBaseArtifacts, finalizeArtifacts} from './base-artifacts.js';

/**
* @param {{page: LH.Puppeteer.Page, config?: LH.Config.Json, configContext?: LH.Config.FRContext}} options
* @param {{page: LH.Puppeteer.Page, config?: LH.Config.Json, flags?: LH.Flags}} options
* @return {Promise<{endTimespanGather(): Promise<LH.Gatherer.FRGatherResult>}>}
*/
async function startTimespanGather(options) {
const {configContext = {}} = options;
log.setLevel(configContext.logLevel || 'error');
const {flags = {}} = options;
log.setLevel(flags.logLevel || 'error');

const {config} =
await initializeConfig(options.config, {...configContext, gatherMode: 'timespan'});
const {config} = await initializeConfig('timespan', options.config, flags);
const driver = new Driver(options.page);
await driver.connect();

Expand Down
20 changes: 9 additions & 11 deletions core/fraggle-rock/user-flow.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,24 +63,22 @@ class UserFlow {
*/
_getNextNavigationOptions(stepOptions) {
const options = {...this.options, ...stepOptions};
const configContext = {...options.configContext};
const settingsOverrides = {...configContext.settingsOverrides};
const flags = {...options.flags};

if (configContext.skipAboutBlank === undefined) {
configContext.skipAboutBlank = true;
if (flags.skipAboutBlank === undefined) {
flags.skipAboutBlank = true;
}

// On repeat navigations, we want to disable storage reset by default (i.e. it's not a cold load).
const isSubsequentNavigation = this._gatherSteps
.some(step => step.artifacts.GatherContext.gatherMode === 'navigation');
if (isSubsequentNavigation) {
if (settingsOverrides.disableStorageReset === undefined) {
settingsOverrides.disableStorageReset = true;
if (flags.disableStorageReset === undefined) {
flags.disableStorageReset = true;
}
}

configContext.settingsOverrides = settingsOverrides;
options.configContext = configContext;
options.flags = flags;

return options;
}
Expand All @@ -96,7 +94,7 @@ class UserFlow {
artifacts: gatherResult.artifacts,
name: providedName || this._getDefaultStepName(gatherResult.artifacts),
config: options.config,
configContext: options.configContext,
flags: options.flags,
};
this._gatherSteps.push(gatherStep);
this._gatherStepRunnerOptions.set(gatherStep, gatherResult.runnerOptions);
Expand Down Expand Up @@ -246,7 +244,7 @@ async function auditGatherSteps(gatherSteps, options) {
/** @type {LH.FlowResult['steps']} */
const steps = [];
for (const gatherStep of gatherSteps) {
const {artifacts, name, configContext} = gatherStep;
const {artifacts, name, flags} = gatherStep;

let runnerOptions = options.gatherStepRunnerOptions?.get(gatherStep);

Expand All @@ -255,7 +253,7 @@ async function auditGatherSteps(gatherSteps, options) {
// Step specific configs take precedence over a config for the entire flow.
const configJson = gatherStep.config || options.config;
const {gatherMode} = artifacts.GatherContext;
const {config} = await initializeConfig(configJson, {...configContext, gatherMode});
const {config} = await initializeConfig(gatherMode, configJson, flags);
runnerOptions = {
config,
computedCache: new Map(),
Expand Down
7 changes: 2 additions & 5 deletions core/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import {Config} from './config/config.js';
import URL from './lib/url-shim.js';
import * as fraggleRock from './fraggle-rock/api.js';
import {Driver} from './gather/driver.js';
import {flagsToFRContext} from './config/config-helpers.js';
import {initializeConfig} from './fraggle-rock/config/config.js';

/** @typedef {import('./gather/connections/connection.js').Connection} Connection */
Expand Down Expand Up @@ -41,8 +40,7 @@ import {initializeConfig} from './fraggle-rock/config/config.js';
* @return {Promise<LH.RunnerResult|undefined>}
*/
async function lighthouse(url, flags = {}, configJSON, page) {
const configContext = flagsToFRContext(flags);
return fraggleRock.navigation(url, {page, config: configJSON, configContext});
return fraggleRock.navigation(url, {page, config: configJSON, flags});
}

/**
Expand Down Expand Up @@ -86,8 +84,7 @@ async function legacyNavigation(url, flags = {}, configJSON, userConnection) {
* @return {Promise<LH.Config.FRConfig>}
*/
async function generateConfig(configJson, flags = {}, gatherMode = 'navigation') {
const configContext = flagsToFRContext(flags);
const {config} = await initializeConfig(configJson, {...configContext, gatherMode});
const {config} = await initializeConfig(gatherMode, configJson, flags);
return config;
}

Expand Down
2 changes: 1 addition & 1 deletion core/scripts/print-a11y-scoring.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

import {initializeConfig} from '../fraggle-rock/config/config.js';

const {config} = await initializeConfig(undefined, {gatherMode: 'navigation'});
const {config} = await initializeConfig('navigation');
if (!config.categories || !config.audits) throw new Error('wut');

const auditRefs = config.categories.accessibility.auditRefs;
Expand Down
Loading

0 comments on commit 9583242

Please sign in to comment.