core(fr): align base artifacts with legacy gather-runner (#12510)

lighthouse-core/fraggle-rock/config/filters.js

@@ -7,6 +7,21 @@
 
 const Audit = require('../../audits/audit.js');
 
+/** @type {Record<keyof LH.FRBaseArtifacts, string>} */
+const baseArtifactKeySource = {
+  fetchTime: '',
+  LighthouseRunWarnings: '',
+  HostFormFactor: '',
+  HostUserAgent: '',
+  BenchmarkIndex: '',
+  settings: '',
+  Timing: '',
+  URL: '',
+  PageLoadError: '',
+};
+
+const baseArtifactKeys = Object.keys(baseArtifactKeySource);
+
 /**
  * Filters an array of artifacts down to the set that supports the specified gather mode.
  *
@@ -31,8 +46,9 @@ function filterArtifactsByGatherMode(artifacts, mode) {
 function filterAuditsByAvailableArtifacts(audits, availableArtifacts) {
   if (!audits) return null;
 
-  const availableAritfactIds = new Set(availableArtifacts.map(artifact => artifact.id));
-
+  const availableAritfactIds = new Set(
+    availableArtifacts.map(artifact => artifact.id).concat(baseArtifactKeys)
+  );
   return audits.filter(audit =>
     audit.implementation.meta.requiredArtifacts.every(id => availableAritfactIds.has(id))
   );

lighthouse-core/fraggle-rock/gather/base-artifacts.js

@@ -5,7 +5,13 @@
  */
 'use strict';
 
-const {getBrowserVersion, getBenchmarkIndex} = require('../../gather/driver/environment.js');
+const log = require('lighthouse-logger');
+const isEqual = require('lodash.isequal');
+const {
+  getBrowserVersion,
+  getBenchmarkIndex,
+  getEnvironmentWarnings,
+} = require('../../gather/driver/environment.js');
 
 /**
  * @param {LH.Config.FRConfig} config
@@ -21,35 +27,71 @@ async function getBaseArtifacts(config, driver) {
 
   const BenchmarkIndex = await getBenchmarkIndex(driver.executionContext);
 
-  /** @type {Array<string | LH.IcuMessage>} */
-  const LighthouseRunWarnings = [];
-
-  // TODO(FR-COMPAT): support slow host CPU warning
-  // TODO(FR-COMPAT): support redirected URL warning
-
   return {
     // Meta artifacts.
     fetchTime: new Date().toJSON(),
     Timing: [],
-    LighthouseRunWarnings,
+    LighthouseRunWarnings: [],
     settings: config.settings,
     // Environment artifacts that can always be computed.
     HostFormFactor,
     HostUserAgent,
     BenchmarkIndex,
     // Contextual artifacts whose collection changes based on gather mode.
     URL: {requestedUrl: '', finalUrl: ''},
-    PageLoadError: null, // TODO(FR-COMPAT): support PageLoadError
+    PageLoadError: null,
     // Artifacts that have been replaced by regular gatherers in Fraggle Rock.
     Stacks: [],
     NetworkUserAgent: '',
-    WebAppManifest: null, // replaced by standard gatherer
-    InstallabilityErrors: {errors: []}, // replaced by standard gatherer
+    WebAppManifest: null,
+    InstallabilityErrors: {errors: []},
     traces: {},
     devtoolsLogs: {},
   };
 }
 
+/**
+ * Deduplicates identical warnings.
+ * @param {Array<string | LH.IcuMessage>} warnings
+ * @return {Array<string | LH.IcuMessage>}
+ */
+function deduplicateWarnings(warnings) {
+  /** @type {Array<string | LH.IcuMessage>} */
+  const unique = [];
+
+  for (const warning of warnings) {
+    if (unique.some(existing => isEqual(warning, existing))) continue;
+    unique.push(warning);
+  }
+
+  return unique;
+}
+
+/**
+ * @param {LH.FRBaseArtifacts} baseArtifacts
+ * @param {Partial<LH.Artifacts>} gathererArtifacts
+ * @return {LH.Artifacts}
+ */
+function finalizeArtifacts(baseArtifacts, gathererArtifacts) {
+  const warnings = baseArtifacts.LighthouseRunWarnings
+    .concat(gathererArtifacts.LighthouseRunWarnings || [])
+    .concat(getEnvironmentWarnings({settings: baseArtifacts.settings, baseArtifacts}));
+
+  // Cast to remove the partial from gathererArtifacts.
+  const artifacts = /** @type {LH.Artifacts} */ ({...baseArtifacts, ...gathererArtifacts});
+
+  // Set the post-run meta artifacts.
+  artifacts.Timing = log.getTimeEntries();
+  artifacts.LighthouseRunWarnings = deduplicateWarnings(warnings);
+
+  // Check that the runner remembered to mutate the special-case URL artifact.
+  if (!artifacts.URL.requestedUrl) throw new Error('Runner did not set requestedUrl');
+  if (!artifacts.URL.finalUrl) throw new Error('Runner did not set finalUrl');
+
+  return artifacts;
+}
+
 module.exports = {
   getBaseArtifacts,
+  finalizeArtifacts,
 };

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

@@ -19,7 +19,7 @@ const storage = require('../../gather/driver/storage.js');
 const emulation = require('../../lib/emulation.js');
 const {defaultNavigationConfig} = require('../../config/constants.js');
 const {initializeConfig} = require('../config/config.js');
-const {getBaseArtifacts} = require('./base-artifacts.js');
+const {getBaseArtifacts, finalizeArtifacts} = require('./base-artifacts.js');
 const i18n = require('../../lib/i18n/i18n.js');
 const LighthouseError = require('../../lib/lh-error.js');
 const {getPageLoadError} = require('../../lib/navigation-error.js');
@@ -126,7 +126,7 @@ async function _collectNetworkRecords(navigationContext, phaseState) {
  * @param {PhaseState} phaseState
  * @param {UnPromise<ReturnType<typeof _setupNavigation>>} setupResult
  * @param {UnPromise<ReturnType<typeof _navigate>>} navigateResult
- * @return {Promise<{artifacts: Partial<LH.GathererArtifacts>, warnings: Array<LH.IcuMessage>, pageLoadError: LH.LighthouseError | undefined}>}
+ * @return {Promise<{finalUrl: string, artifacts: Partial<LH.GathererArtifacts>, warnings: Array<LH.IcuMessage>, pageLoadError: LH.LighthouseError | undefined}>}
  */
 async function _computeNavigationResult(
   navigationContext,
@@ -150,12 +150,17 @@ async function _computeNavigationResult(
     const localizedMessage = i18n.getFormatted(pageLoadError.friendlyMessage, locale);
     log.error('NavigationRunner', localizedMessage, navigationContext.requestedUrl);
 
-    return {artifacts: {}, warnings: [...warnings, pageLoadError.friendlyMessage], pageLoadError};
+    return {
+      finalUrl,
+      pageLoadError,
+      artifacts: {},
+      warnings: [...warnings, pageLoadError.friendlyMessage],
+    };
   } else {
     await collectPhaseArtifacts({phase: 'getArtifact', ...phaseState});
 
     const artifacts = await awaitArtifacts(phaseState.artifactState);
-    return {artifacts, warnings, pageLoadError: undefined};
+    return {finalUrl, artifacts, warnings, pageLoadError: undefined};
   }
 }
 
@@ -194,6 +199,8 @@ async function _navigations({driver, config, requestedUrl, computedCache}) {
 
   /** @type {Partial<LH.FRArtifacts & LH.FRBaseArtifacts>} */
   const artifacts = {};
+  /** @type {Array<LH.IcuMessage>} */
+  const LighthouseRunWarnings = [];
 
   for (const navigation of config.navigations) {
     const navigationContext = {
@@ -205,16 +212,20 @@ async function _navigations({driver, config, requestedUrl, computedCache}) {
     };
 
     const navigationResult = await _navigation(navigationContext);
-    if (navigationResult.pageLoadError && navigation.loadFailureMode === 'fatal') {
-      artifacts.PageLoadError = navigationResult.pageLoadError;
-      break;
+    if (navigation.loadFailureMode === 'fatal') {
+      if (navigationResult.pageLoadError) {
+        artifacts.PageLoadError = navigationResult.pageLoadError;
+        break;
+      }
+
+      artifacts.URL = {requestedUrl, finalUrl: navigationResult.finalUrl};
     }
 
-    // TODO(FR-COMPAT): merge RunWarnings between navigations
+    LighthouseRunWarnings.push(...navigationResult.warnings);
     Object.assign(artifacts, navigationResult.artifacts);
   }
 
-  return {artifacts};
+  return {artifacts: {...artifacts, LighthouseRunWarnings}};
 }
 
 /**
@@ -243,7 +254,7 @@ async function navigation(options) {
       const {artifacts} = await _navigations({driver, config, requestedUrl, computedCache});
       await _cleanup({driver, config, requestedUrl});
 
-      return /** @type {LH.Artifacts} */ ({...baseArtifacts, ...artifacts}); // Cast to drop Partial<>
+      return finalizeArtifacts(baseArtifacts, artifacts);
     },
     {
       url: requestedUrl,

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

@@ -13,7 +13,7 @@ const {
   awaitArtifacts,
 } = require('./runner-helpers.js');
 const {initializeConfig} = require('../config/config.js');
-const {getBaseArtifacts} = require('./base-artifacts.js');
+const {getBaseArtifacts, finalizeArtifacts} = require('./base-artifacts.js');
 
 /** @param {{page: import('puppeteer').Page, config?: LH.Config.Json}} options */
 async function snapshot(options) {
@@ -44,7 +44,7 @@ async function snapshot(options) {
       });
 
       const artifacts = await awaitArtifacts(artifactState);
-      return /** @type {LH.Artifacts} */ ({...baseArtifacts, ...artifacts}); // Cast to drop Partial<>
+      return finalizeArtifacts(baseArtifacts, artifacts);
     },
     {
       url,

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

@@ -13,7 +13,7 @@ const {
   awaitArtifacts,
 } = require('./runner-helpers.js');
 const {initializeConfig} = require('../config/config.js');
-const {getBaseArtifacts} = require('./base-artifacts.js');
+const {getBaseArtifacts, finalizeArtifacts} = require('./base-artifacts.js');
 
 /**
  * @param {{page: import('puppeteer').Page, config?: LH.Config.Json}} options
@@ -56,7 +56,7 @@ async function startTimespan(options) {
           await collectPhaseArtifacts({phase: 'getArtifact', ...phaseOptions});
 
           const artifacts = await awaitArtifacts(artifactState);
-          return /** @type {LH.Artifacts} */ ({...baseArtifacts, ...artifacts}); // Cast to drop Partial<>
+          return finalizeArtifacts(baseArtifacts, artifacts);
         },
         {
           url: finalUrl,

lighthouse-core/gather/gather-runner.js

@@ -20,6 +20,7 @@ const WebAppManifest = require('./gatherers/web-app-manifest.js');
 const InstallabilityErrors = require('./gatherers/installability-errors.js');
 const NetworkUserAgent = require('./gatherers/network-user-agent.js');
 const Stacks = require('./gatherers/stacks.js');
+const {finalizeArtifacts} = require('../fraggle-rock/gather/base-artifacts.js');
 
 /** @typedef {import('../gather/driver.js')} Driver */
 /** @typedef {import('../lib/arbitrary-equality-map.js')} ArbitraryEqualityMap */
@@ -446,18 +447,6 @@ class GatherRunner {
     log.timeEnd(status);
   }
 
-  /**
-   * Finalize baseArtifacts after gathering is fully complete.
-   * @param {LH.BaseArtifacts} baseArtifacts
-   */
-  static finalizeBaseArtifacts(baseArtifacts) {
-    // Take only unique LighthouseRunWarnings.
-    baseArtifacts.LighthouseRunWarnings = Array.from(new Set(baseArtifacts.LighthouseRunWarnings));
-
-    // Take the timing entries we've gathered so far.
-    baseArtifacts.Timing = log.getTimeEntries();
-  }
-
   /**
    * @param {Array<LH.Config.Pass>} passConfigs
    * @param {{driver: Driver, requestedUrl: string, settings: LH.Config.Settings, computedCache: Map<string, ArbitraryEqualityMap>}} options
@@ -515,8 +504,7 @@ class GatherRunner {
       }
 
       await GatherRunner.disposeDriver(driver, options);
-      GatherRunner.finalizeBaseArtifacts(baseArtifacts);
-      return /** @type {LH.Artifacts} */ ({...baseArtifacts, ...artifacts}); // Cast to drop Partial<>.
+      return finalizeArtifacts(baseArtifacts, artifacts);
     } catch (err) {
       // Clean up on error. Don't await so that the root error, not a disposal error, is shown.
       GatherRunner.disposeDriver(driver, options);

lighthouse-core/test/fraggle-rock/api-test-pptr.js

@@ -95,7 +95,7 @@ describe('Fraggle Rock API', () => {
 
       const {auditResults, erroredAudits, failedAudits} = getAuditsBreakdown(lhr);
       // TODO(FR-COMPAT): This assertion can be removed when full compatibility is reached.
-      expect(auditResults.length).toMatchInlineSnapshot(`74`);
+      expect(auditResults.length).toMatchInlineSnapshot(`78`);
 
       expect(erroredAudits).toHaveLength(0);
       expect(failedAudits.map(audit => audit.id)).toContain('label');
@@ -121,7 +121,7 @@ describe('Fraggle Rock API', () => {
 
       const {auditResults, erroredAudits, failedAudits} = getAuditsBreakdown(lhr);
       // TODO(FR-COMPAT): This assertion can be removed when full compatibility is reached.
-      expect(auditResults.length).toMatchInlineSnapshot(`28`);
+      expect(auditResults.length).toMatchInlineSnapshot(`35`);
 
       expect(erroredAudits).toHaveLength(0);
       expect(failedAudits.map(audit => audit.id)).toContain('errors-in-console');
@@ -159,8 +159,8 @@ describe('Fraggle Rock API', () => {
       const {lhr} = result;
       const {auditResults, failedAudits, erroredAudits} = getAuditsBreakdown(lhr);
       // TODO(FR-COMPAT): This assertion can be removed when full compatibility is reached.
-      expect(auditResults.length).toMatchInlineSnapshot(`114`);
-      expect(erroredAudits).toHaveLength(0);
+      expect(auditResults.length).toMatchInlineSnapshot(`145`);
+      expect(erroredAudits).toHaveLength(1); // FIXME: MainDocumentContent broken from merge
 
       const failedAuditIds = failedAudits.map(audit => audit.id);
       expect(failedAuditIds).toContain('label');

lighthouse-core/test/fraggle-rock/config/filters-test.js

@@ -88,6 +88,25 @@ describe('Fraggle Rock Config Filtering', () => {
       ]);
     });
 
+    it('should not filter audits with dependencies on base artifacts', () => {
+      class SnapshotWithBase extends BaseAudit {
+        static meta = {
+          id: 'snapshot',
+          requiredArtifacts: /** @type {any} */ (['Snapshot', 'URL', 'HostUserAgent']),
+          ...auditMeta,
+        };
+      }
+
+      const auditsWithBaseArtifacts = [SnapshotWithBase, TimespanAudit].map(audit => ({
+        implementation: audit,
+        options: {},
+      }));
+      const partialArtifacts = [{id: 'Snapshot', gatherer: {instance: snapshotGatherer}}];
+      expect(
+        filters.filterAuditsByAvailableArtifacts(auditsWithBaseArtifacts, partialArtifacts)
+      ).toEqual([{implementation: SnapshotWithBase, options: {}}]);
+    });
+
     it('should be noop when all artifacts available', () => {
       expect(filters.filterAuditsByAvailableArtifacts(audits, artifacts)).toEqual(audits);
     });
@@ -133,16 +152,16 @@ describe('Fraggle Rock Config Filtering', () => {
         {implementation: ManualAudit, options: {}},
       ];
 
-      const filteredCategories = filters.filterCategoriesByAvailableAudits({
-        snapshot: categories.snapshot,
-        timespanWithManual: {
-          title: 'Timespan + Manual',
-          auditRefs: [
-            {id: 'timespan', weight: 0},
-            {id: 'manual', weight: 0},
-          ],
+      const filteredCategories = filters.filterCategoriesByAvailableAudits(
+        {
+          snapshot: categories.snapshot,
+          timespanWithManual: {
+            title: 'Timespan + Manual',
+            auditRefs: [{id: 'timespan', weight: 0}, {id: 'manual', weight: 0}],
+          },
         },
-      }, partialAudits);
+        partialAudits
+      );
       expect(filteredCategories).not.toHaveProperty('timespanWithManual');
     });