From ecb3a8c78c87afda21aba4ac72363f389ae8d8a3 Mon Sep 17 00:00:00 2001 From: Patrick Hulce Date: Tue, 27 Mar 2018 15:15:52 -0700 Subject: [PATCH] core: unify config and CLI settings (#4849) --- lighthouse-cli/cli-flags.js | 15 ++-- lighthouse-cli/test/cli/run-test.js | 6 ++ lighthouse-core/config/config.js | 33 +++++--- lighthouse-core/config/default.js | 6 +- lighthouse-core/config/full-config.js | 1 + lighthouse-core/config/mixed-content.js | 1 + lighthouse-core/gather/driver.js | 44 +++++----- lighthouse-core/gather/gather-runner.js | 59 +++++++------- lighthouse-core/index.js | 4 +- lighthouse-core/runner.js | 36 ++++----- lighthouse-core/test/config/config-test.js | 26 +++++- lighthouse-core/test/gather/driver-test.js | 6 +- .../test/gather/gather-runner-test.js | 80 +++++++++---------- lighthouse-core/test/runner-test.js | 11 +-- typings/externs.d.ts | 36 ++++++--- typings/gatherer.d.ts | 5 +- 16 files changed, 216 insertions(+), 153 deletions(-) diff --git a/lighthouse-cli/cli-flags.js b/lighthouse-cli/cli-flags.js index 23b7e917320e..c0f87fc9ac2e 100644 --- a/lighthouse-cli/cli-flags.js +++ b/lighthouse-cli/cli-flags.js @@ -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'); /** @@ -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({ @@ -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'}) @@ -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 diff --git a/lighthouse-cli/test/cli/run-test.js b/lighthouse-cli/test/cli/run-test.js index 52e068ceee74..f1cbd956bf22 100644 --- a/lighthouse-cli/test/cli/run-test.js +++ b/lighthouse-cli/test/cli/run-test.js @@ -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']); diff --git a/lighthouse-core/config/config.js b/lighthouse-core/config/config.js index 755c45d65af9..3faca313beb8 100644 --- a/lighthouse-core/config/config.js +++ b/lighthouse-core/config/config.js @@ -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 @@ -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); @@ -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; @@ -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); @@ -563,8 +566,13 @@ class Config { * @return {!Set} */ static getAuditIdsInCategories(categories) { - const audits = _flatten(Object.keys(categories).map(id => categories[id].audits)); - return new Set(audits.map(audit => audit.id)); + /** @type {Array} */ + let audits = []; + for (const category of Object.values(categories)) { + audits = audits.concat(category.audits.map(audit => audit.id)); + } + + return new Set(audits); } /** @@ -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} */ static getMapOfAuditPathToName(config) { const auditObjectsAll = Config.requireAudits(config.audits); @@ -748,6 +756,11 @@ class Config { get groups() { return this._groups; } + + /** @type {LH.ConfigSettings} */ + get settings() { + return this._settings; + } } /** diff --git a/lighthouse-core/config/default.js b/lighthouse-core/config/default.js index e1df90b0069f..99ccdf7dd935 100644 --- a/lighthouse-core/config/default.js +++ b/lighthouse-core/config/default.js @@ -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, diff --git a/lighthouse-core/config/full-config.js b/lighthouse-core/config/full-config.js index bafd4976dc48..851927cb888f 100644 --- a/lighthouse-core/config/full-config.js +++ b/lighthouse-core/config/full-config.js @@ -7,6 +7,7 @@ module.exports = { extends: 'lighthouse:default', + settings: {}, passes: [ { passName: 'extraPass', diff --git a/lighthouse-core/config/mixed-content.js b/lighthouse-core/config/mixed-content.js index ac3ad41fe6c1..ccdce63c7d62 100644 --- a/lighthouse-core/config/mixed-content.js +++ b/lighthouse-core/config/mixed-content.js @@ -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. diff --git a/lighthouse-core/gather/driver.js b/lighthouse-core/gather/driver.js index 6af017cee75f..8cff234a4673 100644 --- a/lighthouse-core/gather/driver.js +++ b/lighthouse-core/gather/driver.js @@ -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'); } @@ -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} */ 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(); @@ -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; @@ -893,12 +893,12 @@ class Driver { } /** - * @param {{additionalTraceCategories: string=}=} flags + * @param {{additionalTraceCategories: string=}=} settings * @return {Promise} */ - 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 = { @@ -1016,25 +1016,25 @@ class Driver { } /** - * @param {LH.Flags} flags + * @param {LH.ConfigSettings} settings * @return {Promise} */ - 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} */ - 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); @@ -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} */ async goOnline(options) { - await this.setThrottling(options.flags, options.config); + await this.setThrottling(options.settings, options.passConfig); this.online = true; } diff --git a/lighthouse-core/gather/gather-runner.js b/lighthouse-core/gather/gather-runner.js index ae7409905c2d..7f72f442970c 100644 --- a/lighthouse-core/gather/gather-runner.js +++ b/lighthouse-core/gather/gather-runner.js @@ -75,17 +75,15 @@ class GatherRunner { * will always represent the post-redirected URL. options.initialUrl is the * pre-redirect starting URL. * @param {!Driver} driver - * @param {!Object} options + * @param {!Object} passContext * @return {!Promise} */ - static loadPage(driver, options) { - return driver.gotoURL(options.url, { + static loadPage(driver, passContext) { + return driver.gotoURL(passContext.url, { waitForLoad: true, - disableJavaScript: !!options.disableJavaScript, - flags: options.flags, - config: options.config, + passContext, }).then(finalUrl => { - options.url = finalUrl; + passContext.url = finalUrl; }); } @@ -97,8 +95,8 @@ class GatherRunner { */ static setupDriver(driver, gathererResults, options) { log.log('status', 'Initializing…'); - const resetStorage = !options.flags.disableStorageReset; - // Enable emulation based on flags + const resetStorage = !options.settings.disableStorageReset; + // Enable emulation based on settings return driver.assertNoSameOriginServiceWorkerClients(options.url) .then(_ => driver.getUserAgent()) .then(userAgent => { @@ -106,7 +104,7 @@ class GatherRunner { GatherRunner.warnOnHeadless(userAgent, gathererResults); gathererResults.fetchedAt = [(new Date()).toJSON()]; }) - .then(_ => driver.beginEmulation(options.flags)) + .then(_ => driver.beginEmulation(options.settings)) .then(_ => driver.enableRuntimeEvents()) .then(_ => driver.cacheNatives()) .then(_ => driver.registerPerformanceObserver()) @@ -193,18 +191,18 @@ class GatherRunner { * @return {!Promise} */ static beforePass(passContext, gathererResults) { - const blockedUrls = (passContext.config.blockedUrlPatterns || []) - .concat(passContext.flags.blockedUrlPatterns || []); - const blankPage = passContext.config.blankPage; - const blankDuration = passContext.config.blankDuration; + const blockedUrls = (passContext.passConfig.blockedUrlPatterns || []) + .concat(passContext.settings.blockedUrlPatterns || []); + const blankPage = passContext.passConfig.blankPage; + const blankDuration = passContext.passConfig.blankDuration; const pass = GatherRunner.loadBlank(passContext.driver, blankPage, blankDuration) // Set request blocking before any network activity // No "clearing" is done at the end of the pass since blockUrlPatterns([]) will unset all if // neccessary at the beginning of the next pass. .then(() => passContext.driver.blockUrlPatterns(blockedUrls)) - .then(() => passContext.driver.setExtraHTTPHeaders(passContext.flags.extraHeaders)); + .then(() => passContext.driver.setExtraHTTPHeaders(passContext.settings.extraHeaders)); - return passContext.config.gatherers.reduce((chain, gathererDefn) => { + return passContext.passConfig.gatherers.reduce((chain, gathererDefn) => { return chain.then(_ => { const gatherer = gathererDefn.instance; // Abuse the passContext to pass through gatherer options @@ -225,11 +223,12 @@ class GatherRunner { */ static pass(passContext, gathererResults) { const driver = passContext.driver; - const config = passContext.config; + const config = passContext.passConfig; + const settings = passContext.settings; const gatherers = config.gatherers; const recordTrace = config.recordTrace; - const isPerfRun = !passContext.flags.disableStorageReset && recordTrace && config.useThrottling; + const isPerfRun = !settings.disableStorageReset && recordTrace && config.useThrottling; const gatherernames = gatherers.map(g => g.instance.name).join(', '); const status = 'Loading page & waiting for onload'; @@ -241,7 +240,7 @@ class GatherRunner { // Always record devtoolsLog .then(_ => driver.beginDevtoolsLog()) // Begin tracing if requested by config. - .then(_ => recordTrace && driver.beginTrace(passContext.flags)) + .then(_ => recordTrace && driver.beginTrace(settings)) // Navigate. .then(_ => GatherRunner.loadPage(driver, passContext)) .then(_ => log.log('statusEnd', status)); @@ -268,7 +267,7 @@ class GatherRunner { */ static afterPass(passContext, gathererResults) { const driver = passContext.driver; - const config = passContext.config; + const config = passContext.passConfig; const gatherers = config.gatherers; const passData = {}; @@ -312,7 +311,7 @@ class GatherRunner { }); // Disable throttling so the afterPass analysis isn't throttled - pass = pass.then(_ => driver.setThrottling(passContext.flags, {useThrottling: false})); + pass = pass.then(_ => driver.setThrottling(passContext.settings, {useThrottling: false})); pass = gatherers.reduce((chain, gathererDefn) => { const gatherer = gathererDefn.instance; @@ -391,16 +390,12 @@ class GatherRunner { return Promise.reject(new Error('You must provide a url to the gather-runner')); } - if (typeof options.flags === 'undefined') { - options.flags = {}; - } - if (typeof options.config === 'undefined') { return Promise.reject(new Error('You must provide a config')); } - if (typeof options.flags.disableCpuThrottling === 'undefined') { - options.flags.disableCpuThrottling = false; + if (typeof options.settings === 'undefined') { + options.settings = {}; } const gathererResults = { @@ -415,21 +410,21 @@ class GatherRunner { .then(_ => { // If the main document redirects, we'll update this to keep track let urlAfterRedirects; - return passes.reduce((chain, config, passIndex) => { - const passContext = Object.assign({}, options, {config}); + return passes.reduce((chain, passConfig, passIndex) => { + const passContext = Object.assign({}, options, {passConfig}); return chain - .then(_ => driver.setThrottling(options.flags, config)) + .then(_ => driver.setThrottling(options.settings, passConfig)) .then(_ => GatherRunner.beforePass(passContext, gathererResults)) .then(_ => GatherRunner.pass(passContext, gathererResults)) .then(_ => GatherRunner.afterPass(passContext, gathererResults)) .then(passData => { - const passName = config.passName || Audit.DEFAULT_PASS; + const passName = passConfig.passName || Audit.DEFAULT_PASS; // networkRecords are discarded and not added onto artifacts. tracingData.devtoolsLogs[passName] = passData.devtoolsLog; // If requested by config, add trace to pass's tracingData - if (config.recordTrace) { + if (passConfig.recordTrace) { tracingData.traces[passName] = passData.trace; } diff --git a/lighthouse-core/index.js b/lighthouse-core/index.js index b832fe7671cb..f2f81f2192dd 100644 --- a/lighthouse-core/index.js +++ b/lighthouse-core/index.js @@ -40,11 +40,11 @@ function lighthouse(url, flags = {}, configJSON) { log.setLevel(flags.logLevel); // Use ConfigParser to generate a valid config file - const config = new Config(configJSON, flags.configPath); + const config = new Config(configJSON, flags); const connection = new ChromeProtocol(flags.port, flags.hostname); // kick off a lighthouse run - return Runner.run(connection, {url, flags, config}) + return Runner.run(connection, {url, config}) .then((lighthouseResults = {}) => { // Annotate with time to run lighthouse. const endTime = Date.now(); diff --git a/lighthouse-core/runner.js b/lighthouse-core/runner.js index 74cc78d5f5b6..5a1692f805c2 100644 --- a/lighthouse-core/runner.js +++ b/lighthouse-core/runner.js @@ -21,7 +21,7 @@ const Sentry = require('./lib/sentry'); class Runner { static run(connection, opts) { // Clean opts input. - opts.flags = opts.flags || {}; + opts.settings = opts.config && opts.config.settings || {}; // List of top-level warnings for this Lighthouse run. const lighthouseRunWarnings = []; @@ -65,17 +65,17 @@ class Runner { // Gather phase // Either load saved artifacts off disk, from config, or get from the browser - if (opts.flags.auditMode && !opts.flags.gatherMode) { - const path = Runner._getArtifactsPath(opts.flags); + if (opts.settings.auditMode && !opts.settings.gatherMode) { + const path = Runner._getArtifactsPath(opts.settings); run = run.then(_ => Runner._loadArtifactsFromDisk(path)); } else if (opts.config.artifacts) { run = run.then(_ => opts.config.artifacts); } else { run = run.then(_ => Runner._gatherArtifactsFromBrowser(opts, connection)); // -G means save these to ./latest-run, etc. - if (opts.flags.gatherMode) { + if (opts.settings.gatherMode) { run = run.then(async artifacts => { - const path = Runner._getArtifactsPath(opts.flags); + const path = Runner._getArtifactsPath(opts.settings); await Runner._saveArtifacts(artifacts, path); return artifacts; }); @@ -83,7 +83,7 @@ class Runner { } // Potentially quit early - if (opts.flags.gatherMode && !opts.flags.auditMode) return run; + if (opts.settings.gatherMode && !opts.settings.auditMode) return run; // Audit phase run = run.then(artifacts => Runner._runAudits(opts, artifacts)); @@ -115,7 +115,7 @@ class Runner { runWarnings: lighthouseRunWarnings, audits: resultsById, artifacts: runResults.artifacts, - runtimeConfig: Runner.getRuntimeConfig(opts.flags), + runtimeConfig: Runner.getRuntimeConfig(opts.settings), reportCategories: categories, reportGroups: opts.config.groups, }; @@ -385,45 +385,45 @@ class Runner { /** * Get runtime configuration specified by the flags - * @param {!Object} flags + * @param {!LH.ConfigSettings} settings * @return {!Object} runtime config */ - static getRuntimeConfig(flags) { + static getRuntimeConfig(settings) { const emulationDesc = emulation.getEmulationDesc(); const environment = [ { name: 'Device Emulation', - enabled: !flags.disableDeviceEmulation, + enabled: !settings.disableDeviceEmulation, description: emulationDesc['deviceEmulation'], }, { name: 'Network Throttling', - enabled: !flags.disableNetworkThrottling, + enabled: !settings.disableNetworkThrottling, description: emulationDesc['networkThrottling'], }, { name: 'CPU Throttling', - enabled: !flags.disableCpuThrottling, + enabled: !settings.disableCpuThrottling, description: emulationDesc['cpuThrottling'], }, ]; return { environment, - blockedUrlPatterns: flags.blockedUrlPatterns || [], - extraHeaders: flags.extraHeaders || {}, + blockedUrlPatterns: settings.blockedUrlPatterns || [], + extraHeaders: settings.extraHeaders || {}, }; } /** * Get path to use for -G and -A modes. Defaults to $CWD/latest-run - * @param {Flags} flags + * @param {LH.ConfigSettings} settings * @return {string} */ - static _getArtifactsPath(flags) { + static _getArtifactsPath({auditMode, gatherMode}) { // This enables usage like: -GA=./custom-folder - if (typeof flags.auditMode === 'string') return path.resolve(process.cwd(), flags.auditMode); - if (typeof flags.gatherMode === 'string') return path.resolve(process.cwd(), flags.gatherMode); + if (typeof auditMode === 'string') return path.resolve(process.cwd(), auditMode); + if (typeof gatherMode === 'string') return path.resolve(process.cwd(), gatherMode); return path.join(process.cwd(), 'latest-run'); } } diff --git a/lighthouse-core/test/config/config-test.js b/lighthouse-core/test/config/config-test.js index 1fd0d9759d79..44c34c061ab4 100644 --- a/lighthouse-core/test/config/config-test.js +++ b/lighthouse-core/test/config/config-test.js @@ -142,7 +142,7 @@ describe('Config', () => { return assert.throws(_ => new Config({ audits: [], - }, configPath), /absolute path/); + }, {configPath}), /absolute path/); }); it('loads an audit relative to a config path', () => { @@ -150,7 +150,7 @@ describe('Config', () => { return assert.doesNotThrow(_ => new Config({ audits: ['../fixtures/valid-custom-audit'], - }, configPath)); + }, {configPath})); }); it('loads an audit from node_modules/', () => { @@ -440,10 +440,28 @@ describe('Config', () => { const auditNames = new Set(config.audits.map(audit => audit.implementation.meta.name)); assert.ok(config, 'failed to generate config'); assert.ok(auditNames.has('custom-audit'), 'did not include custom audit'); - assert.ok(auditNames.has('unused-css-rules'), 'did not include full audits'); + assert.ok(auditNames.has('unused-javascript'), 'did not include full audits'); assert.ok(auditNames.has('first-meaningful-paint'), 'did not include default audits'); }); + it('merges settings with correct priority', () => { + const config = new Config( + { + extends: 'lighthouse:full', + settings: { + disableStorageReset: true, + disableDeviceEmulation: false, + }, + }, + {disableDeviceEmulation: true} + ); + + assert.ok(config, 'failed to generate config'); + assert.ok(typeof config.settings.maxWaitForLoad === 'number', 'missing setting from default'); + assert.ok(config.settings.disableStorageReset, 'missing setting from extension config'); + assert.ok(config.settings.disableDeviceEmulation, 'missing setting from flags'); + }); + describe('artifact loading', () => { it('expands artifacts', () => { const config = new Config({ @@ -676,7 +694,7 @@ describe('Config', () => { it('loads a gatherer relative to a config path', () => { const config = new Config({ passes: [{gatherers: ['../fixtures/valid-custom-gatherer']}], - }, __filename); + }, {configPath: __filename}); const gatherer = config.passes[0].gatherers[0]; assert.equal(gatherer.instance.name, 'CustomGatherer'); diff --git a/lighthouse-core/test/gather/driver-test.js b/lighthouse-core/test/gather/driver-test.js index 10648d867f4e..63b3aa2afb2c 100644 --- a/lighthouse-core/test/gather/driver-test.js +++ b/lighthouse-core/test/gather/driver-test.js @@ -208,8 +208,10 @@ describe('Browser Driver', () => { const loadOptions = { waitForLoad: true, - config: { - networkQuietThresholdMs: 1, + passContext: { + passConfig: { + networkQuietThresholdMs: 1, + }, }, }; diff --git a/lighthouse-core/test/gather/gather-runner-test.js b/lighthouse-core/test/gather/gather-runner-test.js index 0989f6d44950..702ed1728d86 100644 --- a/lighthouse-core/test/gather/gather-runner-test.js +++ b/lighthouse-core/test/gather/gather-runner-test.js @@ -97,8 +97,8 @@ describe('GatherRunner', function() { const options = { url: url1, - flags: {}, - config: {}, + settings: {}, + passConfig: {}, }; return GatherRunner.loadPage(driver, options).then(_ => { @@ -106,14 +106,14 @@ describe('GatherRunner', function() { }); }); - it('creates flags if needed', () => { + it('creates settings if needed', () => { const url = 'https://example.com'; const driver = fakeDriver; const config = new Config({}); const options = {url, driver, config}; return GatherRunner.run([], options).then(_ => { - assert.equal(typeof options.flags, 'object'); + assert.equal(typeof options.settings, 'object'); }); }); @@ -146,7 +146,7 @@ describe('GatherRunner', function() { ); return GatherRunner.setupDriver(driver, {}, { - flags: {}, + settings: {}, }).then(_ => { assert.equal(tests.calledDeviceEmulation, true); assert.equal(tests.calledNetworkEmulation, true); @@ -171,7 +171,7 @@ describe('GatherRunner', function() { ); return GatherRunner.setupDriver(driver, {}, { - flags: { + settings: { disableDeviceEmulation: true, }, }).then(_ => { @@ -198,7 +198,7 @@ describe('GatherRunner', function() { ); return GatherRunner.setupDriver(driver, {}, { - flags: { + settings: { disableNetworkThrottling: true, }, }).then(_ => { @@ -227,7 +227,7 @@ describe('GatherRunner', function() { ); return GatherRunner.setupDriver(driver, {}, { - flags: { + settings: { disableCpuThrottling: true, }, }).then(_ => { @@ -262,7 +262,7 @@ describe('GatherRunner', function() { getUserAgent: () => Promise.resolve('Fake user agent'), }; - return GatherRunner.setupDriver(driver, {}, {flags: {}}).then(_ => { + return GatherRunner.setupDriver(driver, {}, {settings: {}}).then(_ => { assert.equal(tests.calledCleanBrowserCaches, false); assert.equal(tests.calledClearStorage, true); }); @@ -283,15 +283,15 @@ describe('GatherRunner', function() { gotoURL: asyncFunc, cleanBrowserCaches: createCheck('calledCleanBrowserCaches'), }; - const config = { + const passConfig = { recordTrace: true, useThrottling: true, gatherers: [], }; - const flags = { + const settings = { disableStorageReset: false, }; - return GatherRunner.pass({driver, config, flags}, {TestGatherer: []}).then(_ => { + return GatherRunner.pass({driver, passConfig, settings}, {TestGatherer: []}).then(_ => { assert.equal(tests.calledCleanBrowserCaches, true); }); }); @@ -322,7 +322,7 @@ describe('GatherRunner', function() { }; return GatherRunner.setupDriver(driver, {}, { - flags: {disableStorageReset: true}, + settings: {disableStorageReset: true}, }).then(_ => { assert.equal(tests.calledCleanBrowserCaches, false); assert.equal(tests.calledClearStorage, false); @@ -337,10 +337,10 @@ describe('GatherRunner', function() { return GatherRunner.beforePass({ driver, - flags: { + settings: { blockedUrlPatterns: ['http://*.evil.com', '.jpg', '.woff2'], }, - config: { + passConfig: { blockedUrlPatterns: ['*.jpeg'], gatherers: [], }, @@ -358,8 +358,8 @@ describe('GatherRunner', function() { return GatherRunner.beforePass({ driver, - flags: {}, - config: {gatherers: []}, + settings: {}, + passConfig: {gatherers: []}, }).then(() => assert.deepStrictEqual(receivedUrlPatterns, [])); }); @@ -376,10 +376,10 @@ describe('GatherRunner', function() { return GatherRunner.beforePass({ driver, - flags: { + settings: { extraHeaders: headers, }, - config: {gatherers: []}, + passConfig: {gatherers: []}, }).then(() => assert.deepStrictEqual( receivedHeaders, headers @@ -401,15 +401,15 @@ describe('GatherRunner', function() { }, }; - const config = { + const passConfig = { recordTrace: true, gatherers: [ {instance: new TestGatherer()}, ], }; - const flags = {}; + const settings = {}; - return GatherRunner.pass({driver, config, flags}, {TestGatherer: []}).then(_ => { + return GatherRunner.pass({driver, passConfig, settings}, {TestGatherer: []}).then(_ => { assert.equal(calledTrace, true); }); }); @@ -426,14 +426,14 @@ describe('GatherRunner', function() { }, }); - const config = { + const passConfig = { recordTrace: true, gatherers: [ {instance: new TestGatherer()}, ], }; - return GatherRunner.afterPass({url, driver, config}, {TestGatherer: []}).then(passData => { + return GatherRunner.afterPass({url, driver, passConfig}, {TestGatherer: []}).then(passData => { assert.equal(calledTrace, true); assert.equal(passData.trace, fakeTraceData); }); @@ -451,14 +451,14 @@ describe('GatherRunner', function() { }, }; - const config = { + const passConfig = { gatherers: [ {instance: new TestGatherer()}, ], }; - const flags = {}; + const settings = {}; - return GatherRunner.pass({driver, config, flags}, {TestGatherer: []}).then(_ => { + return GatherRunner.pass({driver, passConfig, settings}, {TestGatherer: []}).then(_ => { assert.equal(calledDevtoolsLogCollect, true); }); }); @@ -477,13 +477,13 @@ describe('GatherRunner', function() { }, }); - const config = { + const passConfig = { gatherers: [ {instance: new TestGatherer()}, ], }; - return GatherRunner.afterPass({url, driver, config}, {TestGatherer: []}).then(vals => { + return GatherRunner.afterPass({url, driver, passConfig}, {TestGatherer: []}).then(vals => { assert.equal(calledDevtoolsLogCollect, true); assert.strictEqual(vals.devtoolsLog[0], fakeDevtoolsMessage); }); @@ -508,7 +508,7 @@ describe('GatherRunner', function() { const t1 = new TestGatherer(); const t2 = new TestGatherer(); const config = new Config({}); - const flags = {}; + const settings = {}; const passes = [{ blankDuration: 0, @@ -528,7 +528,7 @@ describe('GatherRunner', function() { return GatherRunner.run(passes, { driver: fakeDriver, url: 'https://example.com', - flags, + settings, config, }).then(_ => { assert.ok(t1.called); @@ -548,7 +548,7 @@ describe('GatherRunner', function() { passName: 'secondPass', gatherers: [{instance: new TestGatherer()}], }]; - const options = {driver: fakeDriver, url: 'https://example.com', flags: {}, config: {}}; + const options = {driver: fakeDriver, url: 'https://example.com', settings: {}, config: {}}; return GatherRunner.run(passes, options) .then(artifacts => { @@ -571,7 +571,7 @@ describe('GatherRunner', function() { passName: 'secondPass', gatherers: [{instance: new TestGatherer()}], }]; - const options = {driver: fakeDriver, url: 'https://example.com', flags: {}, config: {}}; + const options = {driver: fakeDriver, url: 'https://example.com', settings: {}, config: {}}; return GatherRunner.run(passes, options) .then(artifacts => { @@ -655,7 +655,7 @@ describe('GatherRunner', function() { return GatherRunner.run(passes, { driver: fakeDriver, url: 'https://example.com', - flags: {}, + settings: {}, config: new Config({}), }).then(artifacts => { gathererNames.forEach(gathererName => { @@ -689,7 +689,7 @@ describe('GatherRunner', function() { return GatherRunner.run(passes, { driver: fakeDriver, url: 'https://example.com', - flags: {}, + settings: {}, config: new Config({}), }).then(artifacts => { assert.equal(artifacts.EavesdropGatherer1, 1); @@ -815,7 +815,7 @@ describe('GatherRunner', function() { return GatherRunner.run(passes, { driver: fakeDriver, url: 'https://example.com', - flags: {}, + settings: {}, config: new Config({}), }).then(artifacts => { gathererNames.forEach(gathererName => { @@ -851,7 +851,7 @@ describe('GatherRunner', function() { return GatherRunner.run(passes, { driver: fakeDriver, url: 'https://example.com', - flags: {}, + settings: {}, config: new Config({}), }).then( _ => assert.ok(false), @@ -871,7 +871,7 @@ describe('GatherRunner', function() { return GatherRunner.run(passes, { driver: fakeDriver, url: 'https://example.com', - flags: {}, + settings: {}, config: new Config({}), }).then(_ => assert.ok(false), _ => assert.ok(true)); }); @@ -899,7 +899,7 @@ describe('GatherRunner', function() { return GatherRunner.run(passes, { driver: unresolvedDriver, url, - flags: {}, + settings: {}, config: new Config({}), }).then(artifacts => { assert.equal(artifacts.LighthouseRunWarnings.length, 1); @@ -930,7 +930,7 @@ describe('GatherRunner', function() { return GatherRunner.run(passes, { driver: unresolvedDriver, url, - flags: {}, + settings: {}, config: new Config({}), }) .then(_ => { diff --git a/lighthouse-core/test/runner-test.js b/lighthouse-core/test/runner-test.js index c713c7b10679..0dfb553714f0 100644 --- a/lighthouse-core/test/runner-test.js +++ b/lighthouse-core/test/runner-test.js @@ -40,11 +40,12 @@ describe('Runner', () => { describe('Gather Mode & Audit Mode', () => { const url = 'https://example.com'; - const generateConfig = _ => new Config({ + const generateConfig = settings => new Config({ passes: [{ gatherers: ['viewport-dimensions'], }], audits: ['content-width'], + settings, }); const artifactsPath = '.tmp/test_artifacts'; const resolvedPath = path.resolve(process.cwd(), artifactsPath); @@ -54,7 +55,7 @@ describe('Runner', () => { }); it('-G gathers, quits, and doesn\'t run audits', () => { - const opts = {url, config: generateConfig(), driverMock, flags: {gatherMode: artifactsPath}}; + const opts = {url, config: generateConfig({gatherMode: artifactsPath}), driverMock}; return Runner.run(null, opts).then(_ => { assert.equal(loadArtifactsSpy.called, false, 'loadArtifacts was called'); @@ -73,7 +74,7 @@ describe('Runner', () => { // uses the files on disk from the -G test. ;) it('-A audits from saved artifacts and doesn\'t gather', () => { - const opts = {url, config: generateConfig(), driverMock, flags: {auditMode: artifactsPath}}; + const opts = {url, config: generateConfig({auditMode: artifactsPath}), driverMock}; return Runner.run(null, opts).then(_ => { assert.equal(loadArtifactsSpy.called, true, 'loadArtifacts was not called'); assert.equal(gatherRunnerRunSpy.called, false, 'GatherRunner.run was called'); @@ -83,8 +84,8 @@ describe('Runner', () => { }); it('-GA is a normal run but it saves artifacts to disk', () => { - const opts = {url, config: generateConfig(), driverMock, - flags: {auditMode: artifactsPath, gatherMode: artifactsPath}}; + const settings = {auditMode: artifactsPath, gatherMode: artifactsPath}; + const opts = {url, config: generateConfig(settings), driverMock}; return Runner.run(null, opts).then(_ => { assert.equal(loadArtifactsSpy.called, false, 'loadArtifacts was called'); assert.equal(gatherRunnerRunSpy.called, true, 'GatherRunner.run was not called'); diff --git a/typings/externs.d.ts b/typings/externs.d.ts index 90c11a264488..b861c5f4788c 100644 --- a/typings/externs.d.ts +++ b/typings/externs.d.ts @@ -4,13 +4,28 @@ * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. */ -import _Crdp from "../node_modules/vscode-chrome-debug-core/lib/crdp/crdp"; +import _Crdp from '../node_modules/vscode-chrome-debug-core/lib/crdp/crdp'; declare global { module LH { export import Crdp = _Crdp; - export interface Flags { + interface SharedFlagsSettings { + maxWaitForLoad?: number; + blockedUrlPatterns?: string[]; + additionalTraceCategories?: string[]; + auditMode?: boolean | string; + gatherMode?: boolean | string; + disableStorageReset?: boolean; + disableDeviceEmulation?: boolean; + disableCpuThrottling?: boolean; + disableNetworkThrottling?: boolean; + onlyAudits?: string[]; + onlyCategories?: string[]; + skipAudits?: string[]; + } + + export interface Flags extends SharedFlagsSettings { _: string[]; port: number; chromeFlags: string; @@ -18,29 +33,28 @@ declare global { outputPath: string; saveAssets: boolean; view: boolean; - maxWaitForLoad?: number; logLevel: string; hostname: string; - blockedUrlPatterns: string[]; - extraHeaders: string; enableErrorReporting: boolean; listAllAudits: boolean; listTraceCategories: boolean; - auditMode: boolean|string; - gatherMode: boolean|string; configPath?: string; perf: boolean; mixedContent: boolean; verbose: boolean; quiet: boolean; - disableDeviceEmulation?: boolean; - disableCpuThrottling?: boolean; - disableNetworkThrottling?: boolean; + + extraHeaders?: string; } // TODO: type checking for Config export interface Config { passes: ConfigPass[]; + settings: ConfigSettings; + } + + export interface ConfigSettings extends SharedFlagsSettings { + extraHeaders?: Crdp.Network.Headers; } export interface ConfigPass { @@ -105,7 +119,7 @@ declare global { export interface NetworkRequestTiming { connectStart: number; - connectEnd: number + connectEnd: number; sslStart: number; sslEnd: number; sendStart: number; diff --git a/typings/gatherer.d.ts b/typings/gatherer.d.ts index 1f5b7fa5340c..06e17c547909 100644 --- a/typings/gatherer.d.ts +++ b/typings/gatherer.d.ts @@ -7,7 +7,10 @@ declare global { module LH.Gatherer { export interface PassContext { - options: object; + disableJavaScript?: boolean; + passConfig?: ConfigPass; + settings?: ConfigSettings; + options?: object; } export interface LoadData {