Skip to content

Commit

Permalink
core(asset-saver): use new DevtoolsLog and Trace artifacts (#15345)
Browse files Browse the repository at this point in the history
  • Loading branch information
@adamraine
adamraine authored Aug 10, 2023
1 parent 2de79c0 commit 53c1b7b
Show file tree
Hide file tree
Showing 3 changed files with 88 additions and 66 deletions.
108 changes: 67 additions & 41 deletions core/lib/asset-saver.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand All @@ -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];
}
});

Expand Down Expand Up @@ -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.
Expand All @@ -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<Array<PreparedAssets>>}
*/
async function prepareAssets(artifacts, audits) {
const passNames = Object.keys(artifacts.traces);
/** @type {Array<PreparedAssets>} */
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;
}

Expand Down
5 changes: 3 additions & 2 deletions core/runner.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
41 changes: 18 additions & 23 deletions core/test/lib/asset-saver-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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`);
Expand Down Expand Up @@ -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 => {
Expand Down Expand Up @@ -351,8 +354,6 @@ describe('asset-saver helper', () => {
error.code = 'ECONNREFUSED';

const artifacts = {
traces: {},
devtoolsLogs: {},
ViewportDimensions: error,
};

Expand All @@ -375,8 +376,6 @@ describe('asset-saver helper', () => {
{cause: new Error('the cause')});

const artifacts = {
traces: {},
devtoolsLogs: {},
ScriptElements: lhError,
};

Expand All @@ -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);
Expand Down

0 comments on commit 53c1b7b

Please sign in to comment.