Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

core(fr): separate audit phase for flows #13623

Merged
merged 14 commits into from
Mar 1, 2022
47 changes: 44 additions & 3 deletions lighthouse-core/fraggle-rock/api.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,11 @@
*/
'use strict';

const {snapshot} = require('./gather/snapshot-runner.js');
const {startTimespan} = require('./gather/timespan-runner.js');
const {navigation} = require('./gather/navigation-runner.js');
const {snapshotGather} = require('./gather/snapshot-runner.js');
const {startTimespanGather} = require('./gather/timespan-runner.js');
const {navigationGather} = require('./gather/navigation-runner.js');
const {generateFlowReportHtml} = require('../../report/generator/report-generator.js');
const Runner = require('../runner.js');
const UserFlow = require('./user-flow.js');

/**
Expand All @@ -18,9 +20,48 @@ async function startFlow(page, options) {
return new UserFlow(page, options);
}

/**
* @param {Parameters<navigationGather>} params
* @return {Promise<LH.RunnerResult|undefined>}
*/
async function navigation(...params) {
const gatherResult = await navigationGather(...params);
return Runner.audit(gatherResult.artifacts, gatherResult.runnerOptions);
}

/**
* @param {Parameters<snapshotGather>} params
* @return {Promise<LH.RunnerResult|undefined>}
*/
async function snapshot(...params) {
const gatherResult = await snapshotGather(...params);
return Runner.audit(gatherResult.artifacts, gatherResult.runnerOptions);
}

/**
* @param {Parameters<startTimespanGather>} params
* @return {Promise<{endTimespan: () => Promise<LH.RunnerResult|undefined>}>}
*/
async function startTimespan(...params) {
const {endTimespanGather} = await startTimespanGather(...params);
const endTimespan = async () => {
const gatherResult = await endTimespanGather();
return Runner.audit(gatherResult.artifacts, gatherResult.runnerOptions);
};
return {endTimespan};
}

/**
* @param {LH.FlowResult} flowResult
*/
async function generateFlowReport(flowResult) {
Copy link
Member Author

@adamraine adamraine Mar 1, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added this api function for 2 reasons:

  • Allows us to generate the flow result just once when we generate flow fixtures with --view flag
  • We will eventually need a way to generate a flow report for a result object generated from artifacts and not the UserFlow class

return generateFlowReportHtml(flowResult);
}

module.exports = {
snapshot,
startTimespan,
navigation,
startFlow,
generateFlowReport,
};
8 changes: 4 additions & 4 deletions lighthouse-core/fraggle-rock/gather/navigation-runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -290,9 +290,9 @@ async function _cleanup({requestedUrl, driver, config}) {
/**
* @param {LH.NavigationRequestor} requestor
* @param {{page: import('puppeteer').Page, config?: LH.Config.Json, configContext?: LH.Config.FRContext}} options
* @return {Promise<LH.RunnerResult|undefined>}
* @return {Promise<LH.Gatherer.FRGatherResult>}
*/
async function navigation(requestor, options) {
async function navigationGather(requestor, options) {
const {page, configContext = {}} = options;
log.setLevel(configContext.logLevel || 'error');

Expand Down Expand Up @@ -325,11 +325,11 @@ async function navigation(requestor, options) {
},
runnerOptions
);
return Runner.audit(artifacts, runnerOptions);
return {artifacts, runnerOptions};
}

module.exports = {
navigation,
navigationGather,
_setup,
_setupNavigation,
_navigate,
Expand Down
11 changes: 7 additions & 4 deletions lighthouse-core/fraggle-rock/gather/snapshot-runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,11 @@ const {
const {initializeConfig} = require('../config/config.js');
const {getBaseArtifacts, finalizeArtifacts} = require('./base-artifacts.js');

/** @param {{page: import('puppeteer').Page, config?: LH.Config.Json, configContext?: LH.Config.FRContext}} options */
async function snapshot(options) {
/**
* @param {{page: import('puppeteer').Page, config?: LH.Config.Json, configContext?: LH.Config.FRContext}} options
* @return {Promise<LH.Gatherer.FRGatherResult>}
*/
async function snapshotGather(options) {
const {configContext = {}} = options;
log.setLevel(configContext.logLevel || 'error');

Expand Down Expand Up @@ -57,9 +60,9 @@ async function snapshot(options) {
},
runnerOptions
);
return Runner.audit(artifacts, runnerOptions);
return {artifacts, runnerOptions};
}

module.exports = {
snapshot,
snapshotGather,
};
10 changes: 5 additions & 5 deletions lighthouse-core/fraggle-rock/gather/timespan-runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@ const {getBaseArtifacts, finalizeArtifacts} = require('./base-artifacts.js');

/**
* @param {{page: import('puppeteer').Page, config?: LH.Config.Json, configContext?: LH.Config.FRContext}} options
* @return {Promise<{endTimespan(): Promise<LH.RunnerResult|undefined>}>}
* @return {Promise<{endTimespanGather(): Promise<LH.Gatherer.FRGatherResult>}>}
*/
async function startTimespan(options) {
async function startTimespanGather(options) {
const {configContext = {}} = options;
log.setLevel(configContext.logLevel || 'error');

Expand Down Expand Up @@ -52,7 +52,7 @@ async function startTimespan(options) {
await collectPhaseArtifacts({phase: 'startSensitiveInstrumentation', ...phaseOptions});

return {
async endTimespan() {
async endTimespanGather() {
const finalUrl = await driver.url();
phaseOptions.url = finalUrl;

Expand All @@ -72,11 +72,11 @@ async function startTimespan(options) {
},
runnerOptions
);
return Runner.audit(artifacts, runnerOptions);
return {artifacts, runnerOptions};
},
};
}

module.exports = {
startTimespan,
startTimespanGather,
};
97 changes: 54 additions & 43 deletions lighthouse-core/fraggle-rock/user-flow.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,15 @@
'use strict';

const {generateFlowReportHtml} = require('../../report/generator/report-generator.js');
const {snapshot} = require('./gather/snapshot-runner.js');
const {startTimespan} = require('./gather/timespan-runner.js');
const {navigation} = require('./gather/navigation-runner.js');
const {snapshotGather} = require('./gather/snapshot-runner.js');
const {startTimespanGather} = require('./gather/timespan-runner.js');
const {navigationGather} = require('./gather/navigation-runner.js');
const Runner = require('../runner.js');

/** @typedef {Parameters<snapshot>[0]} FrOptions */
/** @typedef {Parameters<snapshotGather>[0]} FrOptions */
/** @typedef {Omit<FrOptions, 'page'> & {name?: string}} UserFlowOptions */
/** @typedef {Omit<FrOptions, 'page'> & {stepName?: string}} StepOptions */
/** @typedef {{gatherResult: LH.Gatherer.FRGatherResult, name: string}} StepArtifact */

class UserFlow {
/**
Expand All @@ -24,8 +26,8 @@ class UserFlow {
this.options = {page, ...options};
/** @type {string|undefined} */
this.name = options?.name;
/** @type {LH.FlowResult.Step[]} */
this.steps = [];
/** @type {StepArtifact[]} */
this.stepArtifacts = [];
}

/**
Expand All @@ -38,12 +40,12 @@ class UserFlow {
}

/**
* @param {LH.Result} lhr
* @param {LH.Artifacts} artifacts
* @return {string}
*/
_getDefaultStepName(lhr) {
const shortUrl = this._shortenUrl(lhr.finalUrl);
switch (lhr.gatherMode) {
_getDefaultStepName(artifacts) {
const shortUrl = this._shortenUrl(artifacts.URL.finalUrl);
switch (artifacts.GatherContext.gatherMode) {
case 'navigation':
return `Navigation report (${shortUrl})`;
case 'timespan':
Expand All @@ -66,7 +68,8 @@ class UserFlow {
}

// On repeat navigations, we want to disable storage reset by default (i.e. it's not a cold load).
const isSubsequentNavigation = this.steps.some(step => step.lhr.gatherMode === 'navigation');
const isSubsequentNavigation = this.stepArtifacts
.some(step => step.gatherResult.artifacts.GatherContext.gatherMode === 'navigation');
if (isSubsequentNavigation) {
if (settingsOverrides.disableStorageReset === undefined) {
settingsOverrides.disableStorageReset = true;
Expand All @@ -84,85 +87,93 @@ class UserFlow {
* @param {StepOptions=} stepOptions
*/
async navigate(requestor, stepOptions) {
if (this.currentTimespan) throw Error('Timespan already in progress');
if (this.currentTimespan) throw new Error('Timespan already in progress');

const result = await navigation(
const gatherResult = await navigationGather(
requestor,
this._getNextNavigationOptions(stepOptions)
);
if (!result) throw Error('Navigation returned undefined');

const providedName = stepOptions?.stepName;
this.steps.push({
lhr: result.lhr,
name: providedName || this._getDefaultStepName(result.lhr),
this.stepArtifacts.push({
gatherResult,
name: providedName || this._getDefaultStepName(gatherResult.artifacts),
});

return result;
return gatherResult;
}

/**
* @param {StepOptions=} stepOptions
*/
async startTimespan(stepOptions) {
if (this.currentTimespan) throw Error('Timespan already in progress');
if (this.currentTimespan) throw new Error('Timespan already in progress');

const options = {...this.options, ...stepOptions};
const timespan = await startTimespan(options);
const timespan = await startTimespanGather(options);
this.currentTimespan = {timespan, options};
}

async endTimespan() {
if (!this.currentTimespan) throw Error('No timespan in progress');
if (!this.currentTimespan) throw new Error('No timespan in progress');

const {timespan, options} = this.currentTimespan;
const result = await timespan.endTimespan();
const gatherResult = await timespan.endTimespanGather();
this.currentTimespan = undefined;
if (!result) throw Error('Timespan returned undefined');

const providedName = options?.stepName;
this.steps.push({
lhr: result.lhr,
name: providedName || this._getDefaultStepName(result.lhr),
this.stepArtifacts.push({
gatherResult,
name: providedName || this._getDefaultStepName(gatherResult.artifacts),
});

return result;
return gatherResult;
}

/**
* @param {StepOptions=} stepOptions
*/
async snapshot(stepOptions) {
if (this.currentTimespan) throw Error('Timespan already in progress');
if (this.currentTimespan) throw new Error('Timespan already in progress');

const options = {...this.options, ...stepOptions};
const result = await snapshot(options);
if (!result) throw Error('Snapshot returned undefined');
const gatherResult = await snapshotGather(options);

const providedName = stepOptions?.stepName;
this.steps.push({
lhr: result.lhr,
name: providedName || this._getDefaultStepName(result.lhr),
this.stepArtifacts.push({
gatherResult,
name: providedName || this._getDefaultStepName(gatherResult.artifacts),
});

return result;
return gatherResult;
}

/**
* @return {LH.FlowResult}
* @returns {Promise<LH.FlowResult>}
*/
getFlowResult() {
if (!this.steps.length) throw Error('Need at least one step before getting the flow result');
const url = new URL(this.steps[0].lhr.finalUrl);
const name = this.name || `User flow (${url.hostname})`;
return {steps: this.steps, name};
async createFlowResult() {
if (!this.stepArtifacts.length) {
throw new Error('Need at least one step before getting the result');
}
const url = new URL(this.stepArtifacts[0].gatherResult.artifacts.URL.finalUrl);
const flowName = this.name || `User flow (${url.hostname})`;

/** @type {LH.FlowResult['steps']} */
const steps = [];
for (const {gatherResult, name} of this.stepArtifacts) {
const result = await Runner.audit(gatherResult.artifacts, gatherResult.runnerOptions);
if (!result) throw new Error(`Step "${name}" did not return a result`);
steps.push({lhr: result.lhr, name});
}

return {steps, name: flowName};
}

/**
* @return {string}
* @return {Promise<string>}
*/
generateReport() {
const flowResult = this.getFlowResult();
async generateReport() {
const flowResult = await this.createFlowResult();
return generateFlowReportHtml(flowResult);
}
}
Expand Down
8 changes: 4 additions & 4 deletions lighthouse-core/scripts/update-flow-fixtures.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import waitForExpect from 'wait-for-expect';
import puppeteer from 'puppeteer';

import {LH_ROOT} from '../../root.js';
import UserFlow from '../fraggle-rock/user-flow.js';
import api from '../fraggle-rock/api.js';


/** @param {puppeteer.Page} page */
Expand Down Expand Up @@ -54,7 +54,7 @@ async function waitForImagesToLoad(page) {

try {
const page = await browser.newPage();
const flow = new UserFlow(page);
const flow = await api.startFlow(page);

await flow.navigate('https://www.mikescerealshack.co');

Expand All @@ -70,15 +70,15 @@ async function waitForImagesToLoad(page) {

await flow.navigate('https://www.mikescerealshack.co/corrections');

const flowResult = flow.getFlowResult();
const flowResult = await flow.createFlowResult();

fs.writeFileSync(
`${LH_ROOT}/lighthouse-core/test/fixtures/fraggle-rock/reports/sample-flow-result.json`,
JSON.stringify(flowResult, null, 2)
);

if (process.argv.includes('--view')) {
const htmlReport = flow.generateReport();
const htmlReport = await api.generateFlowReport(flowResult);
const filepath = `${LH_ROOT}/dist/sample-reports/flow-report/index.html`;
fs.writeFileSync(filepath, htmlReport);
open(filepath);
Expand Down
Loading