Skip to content

Commit

Permalink
core: unify config and CLI settings (#4849)
Browse files Browse the repository at this point in the history
  • Loading branch information
patrickhulce authored Mar 27, 2018
1 parent 42d47ba commit ecb3a8c
Show file tree
Hide file tree
Showing 16 changed files with 216 additions and 153 deletions.
15 changes: 10 additions & 5 deletions lighthouse-cli/cli-flags.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
const yargs = require('yargs');
// @ts-ignore
const pkg = require('../package.json');
const Driver = require('../lighthouse-core/gather/driver.js');
const printer = require('./printer');

/**
Expand Down Expand Up @@ -61,6 +60,7 @@ function getFlags(manualArgv) {
'save-assets', 'list-all-audits', 'list-trace-categories', 'additional-trace-categories',
'config-path', 'chrome-flags', 'perf', 'mixed-content', 'port', 'hostname',
'max-wait-for-load', 'enable-error-reporting', 'gather-mode', 'audit-mode',
'only-audits', 'only-categories', 'skip-audits',
],
'Configuration:')
.describe({
Expand Down Expand Up @@ -91,6 +91,9 @@ function getFlags(manualArgv) {
'max-wait-for-load':
'The timeout (in milliseconds) to wait before the page is considered done loading and the run should continue. WARNING: Very high values can lead to large traces and instability',
'extra-headers': 'Set extra HTTP Headers to pass with request',
'only-audits': 'Only run the specified audits',
'only-categories': 'Only run the specified categories',
'skip-audits': 'Run everything except these audits',
})
// set aliases
.alias({'gather-mode': 'G', 'audit-mode': 'A'})
Expand All @@ -115,16 +118,18 @@ function getFlags(manualArgv) {
])
.choices('output', printer.getValidOutputOptions())
// force as an array
.array('blocked-url-patterns')
.string('extra-headers')
// note MUST use camelcase versions or only the kebab-case version will be forced
.array('blockedUrlPatterns')
.array('onlyAudits')
.array('onlyCategories')
.array('skipAudits')
.string('extraHeaders')

// default values
.default('chrome-flags', '')
.default('disable-cpu-throttling', false)
.default('output', 'html')
.default('port', 0)
.default('hostname', 'localhost')
.default('max-wait-for-load', Driver.MAX_WAIT_FOR_FULLY_LOADED)
.check(/** @param {!LH.Flags} argv */ (argv) => {
// Make sure lighthouse has been passed a url, or at least one of --list-all-audits
// or --list-trace-categories. If not, stop the program and ask for a url
Expand Down
6 changes: 6 additions & 0 deletions lighthouse-cli/test/cli/run-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,12 @@ describe('CLI run', function() {
}).timeout(20 * 1000);
});

describe('flag coercing', () => {
it('should force to array', () => {
assert.deepStrictEqual(getFlags(`--only-audits foo chrome://version`).onlyAudits, ['foo']);
});
});

describe('Parsing --chrome-flags', () => {
it('returns boolean flags that are true as a bare flag', () => {
assert.deepStrictEqual(parseChromeFlags('--debug'), ['--debug']);
Expand Down
33 changes: 23 additions & 10 deletions lighthouse-core/config/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@ const path = require('path');
const Audit = require('../audits/audit');
const Runner = require('../runner');

const _flatten = arr => [].concat(...arr);

// cleanTrace is run to remove duplicate TracingStartedInPage events,
// and to change TracingStartedInBrowser events into TracingStartedInPage.
// This is done by searching for most occuring threads and basing new events
Expand Down Expand Up @@ -281,9 +279,11 @@ class Config {
/**
* @constructor
* @param {!LighthouseConfig} configJSON
* @param {string=} configPath The absolute path to the config file, if there is one.
* @param {LH.Flags=} flags
*/
constructor(configJSON, configPath) {
constructor(configJSON, flags) {
let configPath = flags && flags.configPath;

if (!configJSON) {
configJSON = defaultConfig;
configPath = path.resolve(__dirname, defaultConfigPath);
Expand Down Expand Up @@ -321,11 +321,13 @@ class Config {
configJSON.audits = Config.expandAuditShorthandAndMergeOptions(configJSON.audits);
configJSON.passes = Config.expandGathererShorthandAndMergeOptions(configJSON.passes);

// Override any applicable settings with CLI flags
configJSON.settings = merge(configJSON.settings || {}, flags || {});

// Generate a limited config if specified
if (configJSON.settings &&
(Array.isArray(configJSON.settings.onlyCategories) ||
if (Array.isArray(configJSON.settings.onlyCategories) ||
Array.isArray(configJSON.settings.onlyAudits) ||
Array.isArray(configJSON.settings.skipAudits))) {
Array.isArray(configJSON.settings.skipAudits)) {
const categoryIds = configJSON.settings.onlyCategories;
const auditIds = configJSON.settings.onlyAudits;
const skipAuditIds = configJSON.settings.skipAudits;
Expand All @@ -341,6 +343,7 @@ class Config {
this._artifacts = expandArtifacts(configJSON.artifacts);
this._categories = configJSON.categories;
this._groups = configJSON.groups;
this._settings = configJSON.settings || {};

// validatePasses must follow after audits are required
validatePasses(configJSON.passes, this._audits);
Expand Down Expand Up @@ -563,8 +566,13 @@ class Config {
* @return {!Set<string>}
*/
static getAuditIdsInCategories(categories) {
const audits = _flatten(Object.keys(categories).map(id => categories[id].audits));
return new Set(audits.map(audit => audit.id));
/** @type {Array<string>} */
let audits = [];
for (const category of Object.values(categories)) {
audits = audits.concat(category.audits.map(audit => audit.id));
}

return new Set(audits);
}

/**
Expand All @@ -581,7 +589,7 @@ class Config {
/**
* Creates mapping from audit path (used in config.audits) to audit.name (used in categories)
* @param {!Object} config Lighthouse config object.
* @return {Map}
* @return {Map<string, string>}
*/
static getMapOfAuditPathToName(config) {
const auditObjectsAll = Config.requireAudits(config.audits);
Expand Down Expand Up @@ -748,6 +756,11 @@ class Config {
get groups() {
return this._groups;
}

/** @type {LH.ConfigSettings} */
get settings() {
return this._settings;
}
}

/**
Expand Down
6 changes: 5 additions & 1 deletion lighthouse-core/config/default.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,14 @@
*/
'use strict';

const Driver = require('../gather/driver');

/* eslint-disable max-len */

module.exports = {
settings: {},
settings: {
maxWaitForLoad: Driver.MAX_WAIT_FOR_FULLY_LOADED,
},
passes: [{
passName: 'defaultPass',
recordTrace: true,
Expand Down
1 change: 1 addition & 0 deletions lighthouse-core/config/full-config.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

module.exports = {
extends: 'lighthouse:default',
settings: {},
passes: [
{
passName: 'extraPass',
Expand Down
1 change: 1 addition & 0 deletions lighthouse-core/config/mixed-content.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
'use strict';

module.exports = {
settings: {},
// This performs two passes:
// (1) Gather the default resources requested by the page, and
// (2) Re-load page but attempt to upgrade each request to HTTPS.
Expand Down
44 changes: 22 additions & 22 deletions lighthouse-core/gather/driver.js
Original file line number Diff line number Diff line change
Expand Up @@ -431,7 +431,7 @@ class Driver {
};

// Check here for _networkStatusMonitor to satisfy type checker. Any further race condition
// will be caught at runtime on calls to it.
// will be caught at runtime on calls to it.
if (!this._networkStatusMonitor) {
throw new Error('Driver._waitForNetworkIdle called with no networkStatusMonitor');
}
Expand Down Expand Up @@ -717,12 +717,13 @@ class Driver {
* possible workaround.
* Resolves on the url of the loaded page, taking into account any redirects.
* @param {string} url
* @param {{waitForLoad?: boolean, disableJavaScript?: boolean, config?: LH.ConfigPass, flags?: LH.Flags}} options
* @param {{waitForLoad?: boolean, passContext?: LH.Gatherer.PassContext}} options
* @return {Promise<string>}
*/
async gotoURL(url, options = {}) {
const waitForLoad = options.waitForLoad || false;
const disableJS = options.disableJavaScript || false;
const passContext = options.passContext || {};
const disableJS = passContext.disableJavaScript || false;

await this._beginNetworkStatusMonitoring(url);
await this._clearIsolatedContextId();
Expand All @@ -735,10 +736,9 @@ class Driver {
this.sendCommand('Page.navigate', {url});

if (waitForLoad) {
let pauseAfterLoadMs = options.config && options.config.pauseAfterLoadMs;
let networkQuietThresholdMs = options.config && options.config.networkQuietThresholdMs;
let cpuQuietThresholdMs = options.config && options.config.cpuQuietThresholdMs;
let maxWaitMs = options.flags && options.flags.maxWaitForLoad;
const passConfig = passContext.passConfig || {};
let {pauseAfterLoadMs, networkQuietThresholdMs, cpuQuietThresholdMs} = passConfig;
let maxWaitMs = passContext.settings && passContext.settings.maxWaitForLoad;

/* eslint-disable max-len */
if (typeof pauseAfterLoadMs !== 'number') pauseAfterLoadMs = DEFAULT_PAUSE_AFTER_LOAD;
Expand Down Expand Up @@ -893,12 +893,12 @@ class Driver {
}

/**
* @param {{additionalTraceCategories: string=}=} flags
* @param {{additionalTraceCategories: string=}=} settings
* @return {Promise<void>}
*/
beginTrace(flags) {
const additionalCategories = (flags && flags.additionalTraceCategories &&
flags.additionalTraceCategories.split(',')) || [];
beginTrace(settings) {
const additionalCategories = (settings && settings.additionalTraceCategories &&
settings.additionalTraceCategories.split(',')) || [];
const traceCategories = this._traceCategories.concat(additionalCategories);
const uniqueCategories = Array.from(new Set(traceCategories));
const tracingOpts = {
Expand Down Expand Up @@ -1016,25 +1016,25 @@ class Driver {
}

/**
* @param {LH.Flags} flags
* @param {LH.ConfigSettings} settings
* @return {Promise<void>}
*/
async beginEmulation(flags) {
if (!flags.disableDeviceEmulation) {
async beginEmulation(settings) {
if (!settings.disableDeviceEmulation) {
await emulation.enableNexus5X(this);
}

await this.setThrottling(flags, {useThrottling: true});
await this.setThrottling(settings, {useThrottling: true});
}

/**
* @param {LH.Flags} flags
* @param {LH.ConfigSettings} settings
* @param {{useThrottling?: boolean}} passConfig
* @return {Promise<void>}
*/
async setThrottling(flags, passConfig) {
const throttleCpu = passConfig.useThrottling && !flags.disableCpuThrottling;
const throttleNetwork = passConfig.useThrottling && !flags.disableNetworkThrottling;
async setThrottling(settings, passConfig) {
const throttleCpu = passConfig.useThrottling && !settings.disableCpuThrottling;
const throttleNetwork = passConfig.useThrottling && !settings.disableNetworkThrottling;
const cpuPromise = throttleCpu ?
emulation.enableCPUThrottling(this) :
emulation.disableCPUThrottling(this);
Expand All @@ -1057,12 +1057,12 @@ class Driver {

/**
* Enable internet connection, using emulated mobile settings if
* `options.flags.disableNetworkThrottling` is false.
* @param {{flags: LH.Flags, config: LH.ConfigPass}} options
* `options.settings.disableNetworkThrottling` is false.
* @param {{settings: LH.ConfigSettings, passConfig: LH.ConfigPass}} options
* @return {Promise<void>}
*/
async goOnline(options) {
await this.setThrottling(options.flags, options.config);
await this.setThrottling(options.settings, options.passConfig);
this.online = true;
}

Expand Down
Loading

0 comments on commit ecb3a8c

Please sign in to comment.