diff --git a/lighthouse-cli/run.js b/lighthouse-cli/run.js index 4c4d32c0bd36..98546dc2141c 100644 --- a/lighthouse-cli/run.js +++ b/lighthouse-cli/run.js @@ -232,6 +232,10 @@ async function runLighthouse(url, flags, config) { let launchedChrome; try { + if (url && flags.auditMode && !flags.gatherMode) { + log.warn('CLI', 'URL parameter is ignored if -A flag is used without -G flag'); + } + const shouldGather = flags.gatherMode || flags.gatherMode === flags.auditMode; const shouldUseLocalChrome = URL.isLikeLocalhost(flags.hostname); if (shouldGather && shouldUseLocalChrome) { diff --git a/lighthouse-core/fraggle-rock/gather/navigation-runner.js b/lighthouse-core/fraggle-rock/gather/navigation-runner.js index 29b1a1abebd0..0f37c81d66ea 100644 --- a/lighthouse-core/fraggle-rock/gather/navigation-runner.js +++ b/lighthouse-core/fraggle-rock/gather/navigation-runner.js @@ -22,6 +22,7 @@ const {initializeConfig} = require('../config/config.js'); const {getBaseArtifacts, finalizeArtifacts} = require('./base-artifacts.js'); const format = require('../../../shared/localization/format.js'); const LighthouseError = require('../../lib/lh-error.js'); +const URL = require('../../lib/url-shim.js'); const {getPageLoadError} = require('../../lib/navigation-error.js'); const Trace = require('../../gather/gatherers/trace.js'); const DevtoolsLog = require('../../gather/gatherers/devtools-log.js'); @@ -279,7 +280,7 @@ async function _cleanup({requestedUrl, driver, config}) { * @return {Promise} */ async function navigation(options) { - const {url: requestedUrl, page, configContext = {}} = options; + const {url, page, configContext = {}} = options; const {config} = initializeConfig(options.config, {...configContext, gatherMode: 'navigation'}); const computedCache = new Map(); const internalOptions = { @@ -289,6 +290,7 @@ async function navigation(options) { return Runner.run( async () => { const driver = new Driver(page); + const requestedUrl = URL.normalizeUrl(url); const context = {driver, config, requestedUrl, options: internalOptions}; const {baseArtifacts} = await _setup(context); const {artifacts} = await _navigations({...context, baseArtifacts, computedCache}); @@ -297,7 +299,6 @@ async function navigation(options) { return finalizeArtifacts(baseArtifacts, artifacts); }, { - url: requestedUrl, config, computedCache: new Map(), } diff --git a/lighthouse-core/fraggle-rock/gather/snapshot-runner.js b/lighthouse-core/fraggle-rock/gather/snapshot-runner.js index da756af669b1..ce3235dbc16f 100644 --- a/lighthouse-core/fraggle-rock/gather/snapshot-runner.js +++ b/lighthouse-core/fraggle-rock/gather/snapshot-runner.js @@ -51,7 +51,6 @@ async function snapshot(options) { return finalizeArtifacts(baseArtifacts, artifacts); }, { - url, config, computedCache, } diff --git a/lighthouse-core/fraggle-rock/gather/timespan-runner.js b/lighthouse-core/fraggle-rock/gather/timespan-runner.js index e4eb3d2a8bb2..4598eb808ffe 100644 --- a/lighthouse-core/fraggle-rock/gather/timespan-runner.js +++ b/lighthouse-core/fraggle-rock/gather/timespan-runner.js @@ -64,7 +64,6 @@ async function startTimespan(options) { return finalizeArtifacts(baseArtifacts, artifacts); }, { - url: finalUrl, config, computedCache, } diff --git a/lighthouse-core/index.js b/lighthouse-core/index.js index 373a8cc1eee9..65001c173482 100644 --- a/lighthouse-core/index.js +++ b/lighthouse-core/index.js @@ -9,6 +9,7 @@ const Runner = require('./runner.js'); const log = require('lighthouse-logger'); const ChromeProtocol = require('./gather/connections/cri.js'); const Config = require('./config/config.js'); +const URL = require('./lib/url-shim.js'); /** @typedef {import('./gather/connections/connection.js')} Connection */ @@ -42,12 +43,12 @@ async function lighthouse(url, flags = {}, configJSON, userConnection) { const config = generateConfig(configJSON, flags); const computedCache = new Map(); - const options = {url, config, computedCache}; + const options = {config, computedCache}; const connection = userConnection || new ChromeProtocol(flags.port, flags.hostname); // kick off a lighthouse run - /** @param {{requestedUrl: string}} runnerData */ - const gatherFn = ({requestedUrl}) => { + const gatherFn = () => { + const requestedUrl = URL.normalizeUrl(url); return Runner._gatherArtifactsFromBrowser(requestedUrl, options, connection); }; return Runner.run(gatherFn, options); diff --git a/lighthouse-core/lib/url-shim.js b/lighthouse-core/lib/url-shim.js index bcaded9a7a81..2ce5cc944e62 100644 --- a/lighthouse-core/lib/url-shim.js +++ b/lighthouse-core/lib/url-shim.js @@ -10,6 +10,7 @@ */ const {Util} = require('../util-commonjs.js'); +const LHError = require('../lib/lh-error.js'); /** @typedef {import('./network-request.js')} NetworkRequest */ @@ -248,6 +249,20 @@ class URLShim extends URL { if (ext === 'jpg') return 'image/jpeg'; return `image/${ext}`; } + + /** + * @param {string|undefined} url + * @return {string} + */ + static normalizeUrl(url) { + // Verify the url is valid and that protocol is allowed + if (url && this.isValid(url) && this.isProtocolAllowed(url)) { + // Use canonicalized URL (with trailing slashes and such) + return new URL(url).href; + } else { + throw new LHError(LHError.errors.INVALID_URL); + } + } } URLShim.URL = URL; diff --git a/lighthouse-core/runner.js b/lighthouse-core/runner.js index fdc552dc8424..a7c1ab5b7455 100644 --- a/lighthouse-core/runner.js +++ b/lighthouse-core/runner.js @@ -16,7 +16,6 @@ const stackPacks = require('./lib/stack-packs.js'); const assetSaver = require('./lib/asset-saver.js'); const fs = require('fs'); const path = require('path'); -const URL = require('./lib/url-shim.js'); const Sentry = require('./lib/sentry.js'); const generateReport = require('../report/generator/report-generator.js').generateReport; const LHError = require('./lib/lh-error.js'); @@ -29,8 +28,8 @@ const {version: lighthouseVersion} = require('../package.json'); class Runner { /** * @template {LH.Config.Config | LH.Config.FRConfig} TConfig - * @param {(runnerData: {requestedUrl: string, config: TConfig, driverMock?: Driver}) => Promise} gatherFn - * @param {{config: TConfig, computedCache: Map, url?: string, driverMock?: Driver}} runOpts + * @param {(runnerData: {config: TConfig, driverMock?: Driver}) => Promise} gatherFn + * @param {{config: TConfig, computedCache: Map, driverMock?: Driver}} runOpts * @return {Promise} */ static async run(gatherFn, runOpts) { @@ -52,47 +51,7 @@ class Runner { data: sentryContext?.extra, }); - // User can run -G solo, -A solo, or -GA together - // -G and -A will run partial lighthouse pipelines, - // and -GA will run everything plus save artifacts and lhr to disk. - - // Gather phase - // Either load saved artifacts off disk or from the browser - let artifacts; - let requestedUrl; - if (settings.auditMode && !settings.gatherMode) { - // No browser required, just load the artifacts from disk. - const path = Runner._getDataSavePath(settings); - artifacts = assetSaver.loadArtifacts(path); - requestedUrl = artifacts.URL.requestedUrl; - - if (!requestedUrl) { - throw new Error('Cannot run audit mode on empty URL'); - } - if (runOpts.url && !URL.equalWithExcludedFragments(runOpts.url, requestedUrl)) { - throw new Error('Cannot run audit mode on different URL'); - } - } else { - // verify the url is valid and that protocol is allowed - if (runOpts.url && URL.isValid(runOpts.url) && URL.isProtocolAllowed(runOpts.url)) { - // Use canonicalized URL (with trailing slashes and such) - requestedUrl = new URL(runOpts.url).href; - } else { - throw new LHError(LHError.errors.INVALID_URL); - } - - artifacts = await gatherFn({ - requestedUrl, - config: runOpts.config, - driverMock: runOpts.driverMock, - }); - - // -G means save these to ./latest-run, etc. - if (settings.gatherMode) { - const path = Runner._getDataSavePath(settings); - await assetSaver.saveArtifacts(artifacts, path); - } - } + const artifacts = await this.gatherAndManageArtifacts(gatherFn, runOpts); // Potentially quit early if (settings.gatherMode && !settings.auditMode) return; @@ -133,7 +92,7 @@ class Runner { /** @type {LH.RawIcu} */ const i18nLhr = { lighthouseVersion, - requestedUrl, + requestedUrl: artifacts.URL.requestedUrl, finalUrl: artifacts.URL.finalUrl, fetchTime: artifacts.fetchTime, gatherMode: artifacts.GatherContext.gatherMode, @@ -184,6 +143,47 @@ class Runner { } } + /** + * User can run -G solo, -A solo, or -GA together + * -G and -A will run partial lighthouse pipelines, + * and -GA will run everything plus save artifacts and lhr to disk. + * + * @template {LH.Config.Config | LH.Config.FRConfig} TConfig + * @param {(runnerData: {config: TConfig, driverMock?: Driver}) => Promise} gatherFn + * @param {{config: TConfig, driverMock?: Driver}} options + * @return {Promise} + */ + static async gatherAndManageArtifacts(gatherFn, options) { + const settings = options.config.settings; + + // Gather phase + // Either load saved artifacts from disk or from the browser. + let artifacts; + if (settings.auditMode && !settings.gatherMode) { + // No browser required, just load the artifacts from disk. + const path = this._getDataSavePath(settings); + artifacts = assetSaver.loadArtifacts(path); + const requestedUrl = artifacts.URL.requestedUrl; + + if (!requestedUrl) { + throw new Error('Cannot run audit mode on empty URL'); + } + } else { + artifacts = await gatherFn({ + config: options.config, + driverMock: options.driverMock, + }); + + // -G means save these to disk (e.g. ./latest-run). + if (settings.gatherMode) { + const path = this._getDataSavePath(settings); + await assetSaver.saveArtifacts(artifacts, path); + } + } + + return artifacts; + } + /** * This handles both the auditMode case where gatherer entries need to be merged in and * the gather/audit case where timingEntriesFromRunner contains all entries from this run, diff --git a/lighthouse-core/test/fraggle-rock/gather/mock-driver.js b/lighthouse-core/test/fraggle-rock/gather/mock-driver.js index 14ccd2118b7a..36c3f91ec32b 100644 --- a/lighthouse-core/test/fraggle-rock/gather/mock-driver.js +++ b/lighthouse-core/test/fraggle-rock/gather/mock-driver.js @@ -143,12 +143,22 @@ function createMockDriver() { }; } -/** @param {() => jest.Mock} runProvider */ -function mockRunnerModule(runProvider) { - const runnerModule = {getGathererList: () => []}; - Object.defineProperty(runnerModule, 'run', { - get: runProvider, - }); +function mockRunnerModule() { + const runnerModule = { + getGathererList: jest.fn().mockReturnValue([]), + gatherAndManageArtifacts: jest.fn(), + run: jest.fn(), + reset, + }; + + jest.mock('../../../runner.js', () => runnerModule); + + function reset() { + runnerModule.getGathererList.mockReturnValue([]); + runnerModule.gatherAndManageArtifacts.mockReset(); + runnerModule.run.mockReset(); + } + return runnerModule; } diff --git a/lighthouse-core/test/fraggle-rock/gather/navigation-runner-test.js b/lighthouse-core/test/fraggle-rock/gather/navigation-runner-test.js index 09ac9a7d8838..c3475d7d7344 100644 --- a/lighthouse-core/test/fraggle-rock/gather/navigation-runner-test.js +++ b/lighthouse-core/test/fraggle-rock/gather/navigation-runner-test.js @@ -22,9 +22,7 @@ const TraceGatherer = require('../../../gather/gatherers/trace.js'); const toDevtoolsLog = require('../../network-records-to-devtools-log.js'); // Establish the mocks before we require our file under test. -let mockRunnerRun = jest.fn(); - -jest.mock('../../../runner.js', () => mockRunnerModule(() => mockRunnerRun)); +const mockRunner = mockRunnerModule(); const runner = require('../../../fraggle-rock/gather/navigation-runner.js'); @@ -93,7 +91,7 @@ describe('NavigationRunner', () => { beforeEach(() => { requestedUrl = 'http://example.com'; - mockRunnerRun = jest.fn(); + mockRunner.reset(); config = initializeConfig(undefined, {gatherMode: 'navigation'}).config; navigation = createNavigation().navigation; computedCache = new Map(); @@ -483,6 +481,19 @@ describe('NavigationRunner', () => { }); describe('navigation', () => { + it('should throw on invalid URL', async () => { + const runnerActual = jest.requireActual('../../../runner.js'); + mockRunner.run.mockImplementation(runnerActual.run); + mockRunner.gatherAndManageArtifacts.mockImplementation(runnerActual.gatherAndManageArtifacts); + + const navigatePromise = runner.navigation({ + url: '', + page: mockDriver._page.asPage(), + }); + + await expect(navigatePromise).rejects.toThrow('INVALID_URL'); + }); + it('should initialize config', async () => { const settingsOverrides = { formFactor: /** @type {const} */ ('desktop'), @@ -497,7 +508,7 @@ describe('NavigationRunner', () => { configContext, }); - expect(mockRunnerRun.mock.calls[0][1]).toMatchObject({ + expect(mockRunner.run.mock.calls[0][1]).toMatchObject({ config: { settings: settingsOverrides, }, diff --git a/lighthouse-core/test/fraggle-rock/gather/snapshot-runner-test.js b/lighthouse-core/test/fraggle-rock/gather/snapshot-runner-test.js index 215c4e90cde6..8f4664ebe3e8 100644 --- a/lighthouse-core/test/fraggle-rock/gather/snapshot-runner-test.js +++ b/lighthouse-core/test/fraggle-rock/gather/snapshot-runner-test.js @@ -16,11 +16,10 @@ const { } = require('./mock-driver.js'); // Establish the mocks before we require our file under test. -let mockRunnerRun = jest.fn(); /** @type {ReturnType} */ let mockDriver; +const mockRunner = mockRunnerModule(); -jest.mock('../../../runner.js', () => mockRunnerModule(() => mockRunnerRun)); jest.mock('../../../fraggle-rock/gather/driver.js', () => mockDriverModule(() => mockDriver.asDriver()) ); @@ -42,7 +41,7 @@ describe('Snapshot Runner', () => { beforeEach(() => { mockPage = createMockPage(); mockDriver = createMockDriver(); - mockRunnerRun = jest.fn(); + mockRunner.reset(); page = mockPage.asPage(); mockDriver._session.sendCommand.mockResponse('Browser.getVersion', { @@ -67,14 +66,14 @@ describe('Snapshot Runner', () => { it('should connect to the page and run', async () => { await snapshot({page, config}); expect(mockDriver.connect).toHaveBeenCalled(); - expect(mockRunnerRun).toHaveBeenCalled(); + expect(mockRunner.run).toHaveBeenCalled(); }); it('should collect base artifacts', async () => { mockPage.url.mockResolvedValue('https://lighthouse.example.com/'); await snapshot({page, config}); - const artifacts = await mockRunnerRun.mock.calls[0][0](); + const artifacts = await mockRunner.run.mock.calls[0][0](); expect(artifacts).toMatchObject({ fetchTime: expect.any(String), URL: {finalUrl: 'https://lighthouse.example.com/'}, @@ -83,7 +82,7 @@ describe('Snapshot Runner', () => { it('should collect snapshot artifacts', async () => { await snapshot({page, config}); - const artifacts = await mockRunnerRun.mock.calls[0][0](); + const artifacts = await mockRunner.run.mock.calls[0][0](); expect(artifacts).toMatchObject({A: 'Artifact A', B: 'Artifact B'}); expect(gathererA.getArtifact).toHaveBeenCalled(); expect(gathererB.getArtifact).toHaveBeenCalled(); @@ -100,7 +99,7 @@ describe('Snapshot Runner', () => { const configContext = {settingsOverrides}; await snapshot({page, config, configContext}); - expect(mockRunnerRun.mock.calls[0][1]).toMatchObject({ + expect(mockRunner.run.mock.calls[0][1]).toMatchObject({ config: { settings: settingsOverrides, }, @@ -109,7 +108,7 @@ describe('Snapshot Runner', () => { it('should not invoke instrumentation methods', async () => { await snapshot({page, config}); - await mockRunnerRun.mock.calls[0][0](); + await mockRunner.run.mock.calls[0][0](); expect(gathererA.startInstrumentation).not.toHaveBeenCalled(); expect(gathererA.startSensitiveInstrumentation).not.toHaveBeenCalled(); expect(gathererA.stopSensitiveInstrumentation).not.toHaveBeenCalled(); @@ -120,7 +119,7 @@ describe('Snapshot Runner', () => { gathererB.meta.supportedModes = ['timespan']; await snapshot({page, config}); - const artifacts = await mockRunnerRun.mock.calls[0][0](); + const artifacts = await mockRunner.run.mock.calls[0][0](); expect(artifacts).toMatchObject({A: 'Artifact A'}); expect(artifacts).not.toHaveProperty('B'); expect(gathererB.getArtifact).not.toHaveBeenCalled(); @@ -133,7 +132,7 @@ describe('Snapshot Runner', () => { gathererB.meta.dependencies = {ImageElements: dependencySymbol}; await snapshot({page, config}); - const artifacts = await mockRunnerRun.mock.calls[0][0](); + const artifacts = await mockRunner.run.mock.calls[0][0](); expect(artifacts).toMatchObject({A: 'Artifact A', B: 'Artifact B'}); expect(gathererB.getArtifact.mock.calls[0][0]).toMatchObject({ dependencies: { diff --git a/lighthouse-core/test/fraggle-rock/gather/timespan-runner-test.js b/lighthouse-core/test/fraggle-rock/gather/timespan-runner-test.js index 76bbaaef1f2f..d1f1eaba8803 100644 --- a/lighthouse-core/test/fraggle-rock/gather/timespan-runner-test.js +++ b/lighthouse-core/test/fraggle-rock/gather/timespan-runner-test.js @@ -17,12 +17,11 @@ const { } = require('./mock-driver.js'); // Establish the mocks before we require our file under test. -let mockRunnerRun = jest.fn(); /** @type {ReturnType} */ let mockDriver; const mockSubmodules = mockDriverSubmodules(); +const mockRunner = mockRunnerModule(); -jest.mock('../../../runner.js', () => mockRunnerModule(() => mockRunnerRun)); jest.mock('../../../fraggle-rock/gather/driver.js', () => mockDriverModule(() => mockDriver.asDriver()) ); @@ -45,7 +44,7 @@ describe('Timespan Runner', () => { mockSubmodules.reset(); mockPage = createMockPage(); mockDriver = createMockDriver(); - mockRunnerRun = jest.fn(); + mockRunner.reset(); page = mockPage.asPage(); mockDriver._session.sendCommand.mockResponse('Browser.getVersion', { @@ -71,7 +70,7 @@ describe('Timespan Runner', () => { const timespan = await startTimespan({page, config}); await timespan.endTimespan(); expect(mockDriver.connect).toHaveBeenCalled(); - expect(mockRunnerRun).toHaveBeenCalled(); + expect(mockRunner.run).toHaveBeenCalled(); }); it('should prepare the target', async () => { @@ -97,7 +96,7 @@ describe('Timespan Runner', () => { mockPage.url.mockResolvedValue('https://end.example.com/'); await timespan.endTimespan(); - const artifacts = await mockRunnerRun.mock.calls[0][0](); + const artifacts = await mockRunner.run.mock.calls[0][0](); expect(artifacts).toMatchObject({ fetchTime: expect.any(String), URL: { @@ -118,7 +117,7 @@ describe('Timespan Runner', () => { const timespan = await startTimespan({page, config, configContext}); await timespan.endTimespan(); - expect(mockRunnerRun.mock.calls[0][1]).toMatchObject({ + expect(mockRunner.run.mock.calls[0][1]).toMatchObject({ config: { settings: settingsOverrides, }, @@ -128,7 +127,7 @@ describe('Timespan Runner', () => { it('should invoke stop instrumentation', async () => { const timespan = await startTimespan({page, config}); await timespan.endTimespan(); - await mockRunnerRun.mock.calls[0][0](); + await mockRunner.run.mock.calls[0][0](); expect(gathererA.stopSensitiveInstrumentation).toHaveBeenCalled(); expect(gathererB.stopSensitiveInstrumentation).toHaveBeenCalled(); expect(gathererA.stopInstrumentation).toHaveBeenCalled(); @@ -138,7 +137,7 @@ describe('Timespan Runner', () => { it('should collect timespan artifacts', async () => { const timespan = await startTimespan({page, config}); await timespan.endTimespan(); - const artifacts = await mockRunnerRun.mock.calls[0][0](); + const artifacts = await mockRunner.run.mock.calls[0][0](); expect(artifacts).toMatchObject({A: 'Artifact A', B: 'Artifact B'}); }); @@ -148,7 +147,7 @@ describe('Timespan Runner', () => { const timespan = await startTimespan({page, config}); await timespan.endTimespan(); - const artifacts = await mockRunnerRun.mock.calls[0][0](); + const artifacts = await mockRunner.run.mock.calls[0][0](); expect(artifacts).toMatchObject({A: artifactError, B: 'Artifact B'}); expect(gathererA.stopInstrumentation).not.toHaveBeenCalled(); expect(gathererB.stopInstrumentation).toHaveBeenCalled(); @@ -159,7 +158,7 @@ describe('Timespan Runner', () => { const timespan = await startTimespan({page, config}); await timespan.endTimespan(); - const artifacts = await mockRunnerRun.mock.calls[0][0](); + const artifacts = await mockRunner.run.mock.calls[0][0](); expect(artifacts).toMatchObject({A: 'Artifact A'}); expect(artifacts).not.toHaveProperty('B'); expect(gathererB.startInstrumentation).not.toHaveBeenCalled(); @@ -174,7 +173,7 @@ describe('Timespan Runner', () => { const timespan = await startTimespan({page, config}); await timespan.endTimespan(); - const artifacts = await mockRunnerRun.mock.calls[0][0](); + const artifacts = await mockRunner.run.mock.calls[0][0](); expect(artifacts).toMatchObject({A: 'Artifact A', B: 'Artifact B'}); expect(gathererB.getArtifact.mock.calls[0][0]).toMatchObject({ dependencies: { diff --git a/lighthouse-core/test/lib/url-shim-test.js b/lighthouse-core/test/lib/url-shim-test.js index a5c1ad9fceb2..d281cd2a96aa 100644 --- a/lighthouse-core/test/lib/url-shim-test.js +++ b/lighthouse-core/test/lib/url-shim-test.js @@ -364,4 +364,34 @@ describe('URL Shim', () => { expect(URL.guessMimeType('https://example.com/')).toEqual(undefined); }); }); + + describe('normalizeUrl', () => { + it('returns normalized URL', () => { + expect(URL.normalizeUrl('https://example.com')).toEqual('https://example.com/'); + }); + + it('rejects when not given a URL', () => { + expect(() => { + URL.normalizeUrl(undefined); + }).toThrow('INVALID_URL'); + }); + + it('rejects when given a URL of zero length', () => { + expect(() => { + URL.normalizeUrl(''); + }).toThrow('INVALID_URL'); + }); + + it('rejects when given a URL without protocol', () => { + expect(() => { + URL.normalizeUrl('localhost'); + }).toThrow('INVALID_URL'); + }); + + it('rejects when given a URL without hostname', () => { + expect(() => { + URL.normalizeUrl('https://'); + }).toThrow('INVALID_URL'); + }); + }); }); diff --git a/lighthouse-core/test/runner-test.js b/lighthouse-core/test/runner-test.js index 5d52d407144a..f45f4568acbd 100644 --- a/lighthouse-core/test/runner-test.js +++ b/lighthouse-core/test/runner-test.js @@ -28,11 +28,15 @@ const i18n = require('../lib/i18n/i18n.js'); /* eslint-env jest */ describe('Runner', () => { - const defaultGatherFn = opts => Runner._gatherArtifactsFromBrowser( - opts.requestedUrl, - {...opts, computedCache: new Map()}, - null - ); + const createGatherFn = url => { + return opts => { + return Runner._gatherArtifactsFromBrowser( + url, + {...opts, computedCache: new Map()}, + null + ); + }; + }; /** @type {jest.Mock} */ let saveArtifactsSpy; @@ -86,8 +90,8 @@ describe('Runner', () => { }); it('-G gathers, quits, and doesn\'t run audits', () => { - const opts = {url, config: generateConfig({gatherMode: artifactsPath}), driverMock}; - return Runner.run(defaultGatherFn, opts).then(_ => { + const opts = {config: generateConfig({gatherMode: artifactsPath}), driverMock}; + return Runner.run(createGatherFn(url), opts).then(_ => { expect(loadArtifactsSpy).not.toHaveBeenCalled(); expect(saveArtifactsSpy).toHaveBeenCalled(); @@ -107,7 +111,7 @@ describe('Runner', () => { // uses the files on disk from the -G test. ;) it('-A audits from saved artifacts and doesn\'t gather', () => { const opts = {config: generateConfig({auditMode: artifactsPath}), driverMock}; - return Runner.run(defaultGatherFn, opts).then(_ => { + return Runner.run(createGatherFn(), opts).then(_ => { expect(loadArtifactsSpy).toHaveBeenCalled(); expect(gatherRunnerRunSpy).not.toHaveBeenCalled(); expect(saveArtifactsSpy).not.toHaveBeenCalled(); @@ -121,24 +125,13 @@ describe('Runner', () => { const settings = {auditMode: artifactsPath, throttlingMethod: 'provided'}; const opts = {config: generateConfig(settings), driverMock}; try { - await Runner.run(defaultGatherFn, opts); + await Runner.run(createGatherFn(), opts); assert.fail('should have thrown'); } catch (err) { assert.ok(/Cannot change settings/.test(err.message), 'should have prevented run'); } }); - it('-A throws if the URL changes', async () => { - const settings = {auditMode: artifactsPath}; - const opts = {url: 'https://differenturl.com', config: generateConfig(settings), driverMock}; - try { - await Runner.run(defaultGatherFn, opts); - assert.fail('should have thrown'); - } catch (err) { - assert.ok(/different URL/.test(err.message), 'should have prevented run'); - } - }); - it('does not include a top-level runtimeError when gatherers were successful', async () => { const config = new Config({ settings: { @@ -156,8 +149,8 @@ describe('Runner', () => { it('-GA is a normal run but it saves artifacts and LHR to disk', () => { const settings = {auditMode: artifactsPath, gatherMode: artifactsPath}; - const opts = {url, config: generateConfig(settings), driverMock}; - return Runner.run(defaultGatherFn, opts).then(_ => { + const opts = {config: generateConfig(settings), driverMock}; + return Runner.run(createGatherFn(url), opts).then(_ => { expect(loadArtifactsSpy).not.toHaveBeenCalled(); expect(gatherRunnerRunSpy).toHaveBeenCalled(); expect(saveArtifactsSpy).toHaveBeenCalled(); @@ -167,8 +160,8 @@ describe('Runner', () => { }); it('non -G/-A run doesn\'t save artifacts to disk', () => { - const opts = {url, config: generateConfig(), driverMock}; - return Runner.run(defaultGatherFn, opts).then(_ => { + const opts = {config: generateConfig(), driverMock}; + return Runner.run(createGatherFn(url), opts).then(_ => { expect(loadArtifactsSpy).not.toHaveBeenCalled(); expect(gatherRunnerRunSpy).toHaveBeenCalled(); expect(saveArtifactsSpy).not.toHaveBeenCalled(); @@ -194,7 +187,7 @@ describe('Runner', () => { settings: {gatherMode: artifactsPath}, passes: [{gatherers: [WarningAndErrorGatherer]}], }); - await Runner.run(defaultGatherFn, {url, config: gatherConfig, driverMock}); + await Runner.run(createGatherFn(url), {config: gatherConfig, driverMock}); // Artifacts are still localizable. const artifacts = assetSaver.loadArtifacts(resolvedPath); @@ -224,7 +217,7 @@ describe('Runner', () => { settings: {auditMode: artifactsPath}, audits: [{implementation: DummyAudit}], }); - const {lhr} = await Runner.run(defaultGatherFn, {url, config: auditConfig}); + const {lhr} = await Runner.run(createGatherFn(url), {config: auditConfig}); // Messages are now localized and formatted. expect(lhr.runWarnings[0]).toBe('Potential savings of 2 KiB'); @@ -247,7 +240,7 @@ describe('Runner', () => { ], }); - return Runner.run(defaultGatherFn, {url, config, driverMock}).then(_ => { + return Runner.run(createGatherFn(url), {config, driverMock}).then(_ => { expect(gatherRunnerRunSpy).toHaveBeenCalled(); assert.ok(typeof config.passes[0].gatherers[0] === 'object'); }); @@ -262,7 +255,7 @@ describe('Runner', () => { ], }); - return Runner.run(defaultGatherFn, {url, config, driverMock}) + return Runner.run(createGatherFn(url), {config, driverMock}) .then(_ => { assert.ok(false); }, err => { @@ -572,7 +565,7 @@ describe('Runner', () => { }], }); - return Runner.run(defaultGatherFn, {url, config, driverMock}) + return Runner.run(createGatherFn(url), {config, driverMock}) .then(_ => { assert.ok(false); }, err => { @@ -591,7 +584,7 @@ describe('Runner', () => { ], }); - return Runner.run(defaultGatherFn, {url, config, driverMock}).then(results => { + return Runner.run(createGatherFn(url), {config, driverMock}).then(results => { assert.ok(results.lhr.lighthouseVersion); assert.ok(results.lhr.fetchTime); assert.equal(results.lhr.requestedUrl, url); @@ -621,7 +614,7 @@ describe('Runner', () => { }, }); - return Runner.run(defaultGatherFn, {url, config, driverMock}).then(results => { + return Runner.run(createGatherFn(url), {config, driverMock}).then(results => { expect(gatherRunnerRunSpy).toHaveBeenCalled(); assert.ok(results.lhr.lighthouseVersion); assert.ok(results.lhr.fetchTime); @@ -633,27 +626,6 @@ describe('Runner', () => { }); }); - - it('rejects when not given a URL', async () => { - const config = new Config({}); - await expect(Runner.run({}, {config})).rejects.toThrow('INVALID_URL'); - }); - - it('rejects when given a URL of zero length', async () => { - const config = new Config({}); - await expect(Runner.run({}, {url: '', config})).rejects.toThrow('INVALID_URL'); - }); - - it('rejects when given a URL without protocol', async () => { - const config = new Config({}); - await expect(Runner.run({}, {url: 'localhost', config})).rejects.toThrow('INVALID_URL'); - }); - - it('rejects when given a URL without hostname', async () => { - const config = new Config({}); - await expect(Runner.run({}, {url: 'https://', config})).rejects.toThrow('INVALID_URL'); - }); - it('only supports core audits with names matching their filename', () => { const coreAudits = Runner.getAuditList(); coreAudits.forEach(auditFilename => { @@ -693,7 +665,7 @@ describe('Runner', () => { ], }); - return Runner.run(defaultGatherFn, {url, config, driverMock}).then(results => { + return Runner.run(createGatherFn(url), {config, driverMock}).then(results => { // User-specified artifact. assert.ok(results.artifacts.ViewportDimensions); @@ -712,7 +684,7 @@ describe('Runner', () => { audits: [], }); - return Runner.run(defaultGatherFn, {config, driverMock}).then(results => { + return Runner.run(createGatherFn(), {config, driverMock}).then(results => { assert.deepStrictEqual(results.lhr.runWarnings, [ 'I\'m a warning!', 'Also a warning', @@ -743,7 +715,7 @@ describe('Runner', () => { ], }); - return Runner.run(defaultGatherFn, {config, driverMock}).then(results => { + return Runner.run(createGatherFn(), {config, driverMock}).then(results => { assert.deepStrictEqual(results.lhr.runWarnings, [warningString]); }); }); @@ -785,7 +757,7 @@ describe('Runner', () => { it('includes a top-level runtimeError when a gatherer throws one', async () => { const config = new Config(configJson); - const {lhr} = await Runner.run(defaultGatherFn, {url: 'https://example.com/', config, driverMock}); + const {lhr} = await Runner.run(createGatherFn('https://example.com/'), {config, driverMock}); // Audit error included the runtimeError expect(lhr.audits['test-audit'].scoreDisplayMode).toEqual('error'); @@ -816,7 +788,7 @@ describe('Runner', () => { }); const config = new Config(configJson); - const {lhr} = await Runner.run(defaultGatherFn, {url, config, driverMock: errorDriverMock}); + const {lhr} = await Runner.run(createGatherFn(url), {config, driverMock: errorDriverMock}); // Audit error still includes the gatherer runtimeError. expect(lhr.audits['test-audit'].scoreDisplayMode).toEqual('error'); @@ -840,7 +812,7 @@ describe('Runner', () => { }; try { - await Runner.run(defaultGatherFn, {url: 'https://example.com/', driverMock: erroringDriver, config: new Config()}); + await Runner.run(createGatherFn('https://example.com/'), {driverMock: erroringDriver, config: new Config()}); assert.fail('should have thrown'); } catch (err) { assert.equal(err.code, LHError.errors.PROTOCOL_TIMEOUT.code); @@ -859,7 +831,7 @@ describe('Runner', () => { }, }); - const results = await Runner.run(defaultGatherFn, {url, config, driverMock}); + const results = await Runner.run(createGatherFn(url), {config, driverMock}); assert.ok(Array.isArray(results.report) && results.report.length === 2, 'did not return multiple reports'); assert.ok(JSON.parse(results.report[0]), 'did not return json output');