diff --git a/core/lib/asset-saver.js b/core/lib/asset-saver.js index a6a5317bb521..e9a7836d7a57 100644 --- a/core/lib/asset-saver.js +++ b/core/lib/asset-saver.js @@ -23,11 +23,12 @@ const optionsFilename = 'options.json'; const artifactsFilename = 'artifacts.json'; const traceSuffix = '.trace.json'; const devtoolsLogSuffix = '.devtoolslog.json'; +const defaultPrefix = 'defaultPass'; +const errorPrefix = 'pageLoadError-defaultPass'; const stepDirectoryRegex = /^step(\d+)$/; /** * @typedef {object} PreparedAssets - * @property {string} passName * @property {LH.Trace} traceData * @property {LH.DevtoolsLog} devtoolsLog */ @@ -53,32 +54,30 @@ function loadArtifacts(basePath) { const filenames = fs.readdirSync(basePath); - // load devtoolsLogs - artifacts.devtoolsLogs = {}; filenames.filter(f => f.endsWith(devtoolsLogSuffix)).forEach(filename => { - const passName = filename.replace(devtoolsLogSuffix, ''); + if (!artifacts.devtoolsLogs) artifacts.devtoolsLogs = {}; + const prefix = filename.replace(devtoolsLogSuffix, ''); const devtoolsLog = JSON.parse(fs.readFileSync(path.join(basePath, filename), 'utf8')); - artifacts.devtoolsLogs[passName] = devtoolsLog; - if (passName === 'defaultPass') { + artifacts.devtoolsLogs[prefix] = devtoolsLog; + if (prefix === defaultPrefix) { artifacts.DevtoolsLog = devtoolsLog; } - if (passName === 'pageLoadError-defaultPass') { + if (prefix === errorPrefix) { artifacts.DevtoolsLogError = devtoolsLog; } }); - // load traces - artifacts.traces = {}; filenames.filter(f => f.endsWith(traceSuffix)).forEach(filename => { + if (!artifacts.traces) artifacts.traces = {}; const file = fs.readFileSync(path.join(basePath, filename), {encoding: 'utf-8'}); const trace = JSON.parse(file); - const passName = filename.replace(traceSuffix, ''); - artifacts.traces[passName] = Array.isArray(trace) ? {traceEvents: trace} : trace; - if (passName === 'defaultPass') { - artifacts.Trace = artifacts.traces[passName]; + const prefix = filename.replace(traceSuffix, ''); + artifacts.traces[prefix] = Array.isArray(trace) ? {traceEvents: trace} : trace; + if (prefix === defaultPrefix) { + artifacts.Trace = artifacts.traces[prefix]; } - if (passName === 'pageLoadError-defaultPass') { - artifacts.TraceError = artifacts.traces[passName]; + if (prefix === errorPrefix) { + artifacts.TraceError = artifacts.traces[prefix]; } }); @@ -220,23 +219,34 @@ async function saveArtifacts(artifacts, basePath) { } } - // `DevtoolsLog` and `Trace` will always be the 'defaultPass' version. + // `devtoolsLogs` and `traces` are duplicate compat artifacts. // We don't need to save them twice, so extract them here. - // eslint-disable-next-line no-unused-vars - const {traces, devtoolsLogs, DevtoolsLog, Trace, ...restArtifacts} = artifacts; + const { + // eslint-disable-next-line no-unused-vars + traces, + // eslint-disable-next-line no-unused-vars + devtoolsLogs, + DevtoolsLog, + Trace, + DevtoolsLogError, + TraceError, + ...restArtifacts + } = artifacts; + + if (Trace) { + await saveTrace(Trace, `${basePath}/${defaultPrefix}${traceSuffix}`); + } - // save traces - if (traces) { - for (const [passName, trace] of Object.entries(traces)) { - await saveTrace(trace, `${basePath}/${passName}${traceSuffix}`); - } + if (TraceError) { + await saveTrace(TraceError, `${basePath}/${errorPrefix}${traceSuffix}`); } - // save devtools log - if (devtoolsLogs) { - for (const [passName, devtoolsLog] of Object.entries(devtoolsLogs)) { - await saveDevtoolsLog(devtoolsLog, `${basePath}/${passName}${devtoolsLogSuffix}`); - } + if (DevtoolsLog) { + await saveDevtoolsLog(DevtoolsLog, `${basePath}/${defaultPrefix}${devtoolsLogSuffix}`); + } + + if (DevtoolsLogError) { + await saveDevtoolsLog(DevtoolsLogError, `${basePath}/${errorPrefix}${devtoolsLogSuffix}`); } // save everything else, using a replacer to serialize LighthouseErrors in the artifacts. @@ -256,33 +266,49 @@ function saveLhr(lhr, basePath) { } /** - * Filter traces and extract screenshots to prepare for saving. + * Filter trace and extract screenshots to prepare for saving. + * @param {LH.Trace} trace + * @param {LH.Result['audits']} [audits] + * @return {LH.Trace} + */ +function prepareTraceAsset(trace, audits) { + if (!trace) return trace; + + const traceData = Object.assign({}, trace); + if (audits) { + const evts = new MetricTraceEvents(traceData.traceEvents, audits).generateFakeEvents(); + traceData.traceEvents = traceData.traceEvents.concat(evts); + } + return traceData; +} + +/** * @param {LH.Artifacts} artifacts * @param {LH.Result['audits']} [audits] * @return {Promise>} */ async function prepareAssets(artifacts, audits) { - const passNames = Object.keys(artifacts.traces); /** @type {Array} */ const assets = []; - for (const passName of passNames) { - const trace = artifacts.traces[passName]; - const devtoolsLog = artifacts.devtoolsLogs[passName]; - - const traceData = Object.assign({}, trace); - if (audits) { - const evts = new MetricTraceEvents(traceData.traceEvents, audits).generateFakeEvents(); - traceData.traceEvents = traceData.traceEvents.concat(evts); - } - + const devtoolsLog = artifacts.DevtoolsLog; + const traceData = prepareTraceAsset(artifacts.Trace, audits); + if (traceData || devtoolsLog) { assets.push({ - passName, traceData, devtoolsLog, }); } + const devtoolsLogError = artifacts.DevtoolsLogError; + const traceErrorData = prepareTraceAsset(artifacts.TraceError, audits); + if (devtoolsLogError || traceErrorData) { + assets.push({ + traceData: traceErrorData, + devtoolsLog: devtoolsLogError, + }); + } + return assets; } diff --git a/core/runner.js b/core/runner.js index fe6796b1abbe..0cbc88652dee 100644 --- a/core/runner.js +++ b/core/runner.js @@ -375,9 +375,10 @@ class Runner { // If trace/devtoolsLog required, check that DEFAULT_PASS trace/devtoolsLog exists. // NOTE: for now, not a pass-specific check of traces or devtoolsLogs. - const noRequiredTrace = artifactName === 'traces' && !artifacts.traces[Audit.DEFAULT_PASS]; + const noRequiredTrace = artifactName === 'traces' && + !artifacts.traces?.[Audit.DEFAULT_PASS]; const noRequiredDevtoolsLog = artifactName === 'devtoolsLogs' && - !artifacts.devtoolsLogs[Audit.DEFAULT_PASS]; + !artifacts.devtoolsLogs?.[Audit.DEFAULT_PASS]; if (noArtifact || noRequiredTrace || noRequiredDevtoolsLog) { log.warn('Runner', diff --git a/core/test/lib/asset-saver-test.js b/core/test/lib/asset-saver-test.js index 2a2321a88aa9..5c9b0477bd41 100644 --- a/core/test/lib/asset-saver-test.js +++ b/core/test/lib/asset-saver-test.js @@ -10,7 +10,6 @@ import fs from 'fs'; import * as assetSaver from '../../lib/asset-saver.js'; import {MetricTraceEvents} from '../../lib/traces/metric-trace-events.js'; import {LighthouseError} from '../../lib/lh-error.js'; -import {Audit} from '../../audits/audit.js'; import {LH_ROOT} from '../../../root.js'; import {getModuleDirectory} from '../../../esm-utils.js'; import {readJson} from '../test-utils.js'; @@ -37,14 +36,8 @@ describe('asset-saver helper', () => { before(() => { fs.mkdirSync(tmpDir, {recursive: true}); const artifacts = { - devtoolsLogs: { - [Audit.DEFAULT_PASS]: [{message: 'first'}, {message: 'second'}], - }, - traces: { - [Audit.DEFAULT_PASS]: { - traceEvents, - }, - }, + DevtoolsLog: [{message: 'first'}, {message: 'second'}], + Trace: {traceEvents}, }; return assetSaver.saveAssets(artifacts, dbwResults.audits, `${tmpDir}/the_file`); @@ -73,10 +66,20 @@ describe('asset-saver helper', () => { it('adds fake events to trace', () => { const countEvents = trace => trace.traceEvents.length; const mockArtifacts = { - devtoolsLogs: {}, - traces: { - defaultPass: dbwTrace, - }, + Trace: dbwTrace, + }; + const beforeCount = countEvents(dbwTrace); + return assetSaver.prepareAssets(mockArtifacts, dbwResults.audits).then(preparedAssets => { + const afterCount = countEvents(preparedAssets[0].traceData); + const metricsMinusTimeOrigin = MetricTraceEvents.metricsDefinitions.length - 1; + assert.equal(afterCount, beforeCount + (2 * metricsMinusTimeOrigin)); + }); + }); + + it('adds fake events to error trace', () => { + const countEvents = trace => trace.traceEvents.length; + const mockArtifacts = { + TraceError: dbwTrace, }; const beforeCount = countEvents(dbwTrace); return assetSaver.prepareAssets(mockArtifacts, dbwResults.audits).then(preparedAssets => { @@ -351,8 +354,6 @@ describe('asset-saver helper', () => { error.code = 'ECONNREFUSED'; const artifacts = { - traces: {}, - devtoolsLogs: {}, ViewportDimensions: error, }; @@ -375,8 +376,6 @@ describe('asset-saver helper', () => { {cause: new Error('the cause')}); const artifacts = { - traces: {}, - devtoolsLogs: {}, ScriptElements: lhError, }; @@ -397,12 +396,8 @@ describe('asset-saver helper', () => { it('saves artifacts in files concluding with a newline', async () => { const artifacts = { - devtoolsLogs: { - [Audit.DEFAULT_PASS]: [{method: 'first'}, {method: 'second'}], - }, - traces: { - [Audit.DEFAULT_PASS]: {traceEvents: traceEvents.slice(0, 100)}, - }, + DevtoolsLog: [{method: 'first'}, {method: 'second'}], + Trace: {traceEvents: traceEvents.slice(0, 100)}, RobotsTxt: {status: 404, content: null}, }; await assetSaver.saveArtifacts(artifacts, outputPath);