core(runner): asset manager helper (#13519)

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) {

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<LH.RunnerResult|undefined>}
  */
 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(),
     }

lighthouse-core/fraggle-rock/gather/snapshot-runner.js

@@ -51,7 +51,6 @@ async function snapshot(options) {
       return finalizeArtifacts(baseArtifacts, artifacts);
     },
     {
-      url,
       config,
       computedCache,
     }

lighthouse-core/fraggle-rock/gather/timespan-runner.js

@@ -64,7 +64,6 @@ async function startTimespan(options) {
           return finalizeArtifacts(baseArtifacts, artifacts);
         },
         {
-          url: finalUrl,
           config,
           computedCache,
         }

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);

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;

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<LH.Artifacts>} gatherFn
-   * @param {{config: TConfig, computedCache: Map<string, ArbitraryEqualityMap>, url?: string, driverMock?: Driver}} runOpts
+   * @param {(runnerData: {config: TConfig, driverMock?: Driver}) => Promise<LH.Artifacts>} gatherFn
+   * @param {{config: TConfig, computedCache: Map<string, ArbitraryEqualityMap>, driverMock?: Driver}} runOpts
    * @return {Promise<LH.RunnerResult|undefined>}
    */
   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<LH.Result>} */
       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<LH.Artifacts>} gatherFn
+   * @param {{config: TConfig, driverMock?: Driver}} options
+   * @return {Promise<LH.Artifacts>}
+   */
+  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,

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;
 }
 

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,
         },

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<typeof createMockDriver>} */
 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: {