Skip to content

Commit

Permalink
core(fr): add UserFlow usability improvements (#13139)
Browse files Browse the repository at this point in the history
  • Loading branch information
patrickhulce authored Sep 29, 2021
1 parent 214d8bc commit 70a32c2
Show file tree
Hide file tree
Showing 3 changed files with 184 additions and 3 deletions.
10 changes: 10 additions & 0 deletions lighthouse-core/fraggle-rock/api.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,19 @@
const {snapshot} = require('./gather/snapshot-runner.js');
const {startTimespan} = require('./gather/timespan-runner.js');
const {navigation} = require('./gather/navigation-runner.js');
const UserFlow = require('./user-flow.js');

/**
* @param {import('puppeteer').Page} page
* @param {UserFlow.UserFlowOptions} [options]
*/
async function startFlow(page, options) {
return new UserFlow(page, options);
}

module.exports = {
snapshot,
startTimespan,
navigation,
startFlow,
};
30 changes: 27 additions & 3 deletions lighthouse-core/fraggle-rock/user-flow.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@
'use strict';

const {generateFlowReportHtml} = require('../../report/generator/report-generator.js');
const {navigation, startTimespan, snapshot} = require('./api.js');
const {snapshot} = require('./gather/snapshot-runner.js');
const {startTimespan} = require('./gather/timespan-runner.js');
const {navigation} = require('./gather/navigation-runner.js');

/** @typedef {Parameters<snapshot>[0]} FrOptions */
/** @typedef {Omit<FrOptions, 'page'>} UserFlowOptions */
Expand Down Expand Up @@ -49,15 +51,37 @@ class UserFlow {
}
}

/**
* @param {string} url
* @param {StepOptions=} stepOptions
*/
_getNextNavigationOptions(url, stepOptions) {
const options = {url, ...this.options, ...stepOptions};

// 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');
if (isSubsequentNavigation) {
const configContext = {...options.configContext};
const settingsOverrides = {...configContext.settingsOverrides};
if (settingsOverrides.disableStorageReset === undefined) {
settingsOverrides.disableStorageReset = true;
}

configContext.settingsOverrides = settingsOverrides;
options.configContext = configContext;
}

return options;
}

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

const options = {url, ...this.options, ...stepOptions};
const result = await navigation(options);
const result = await navigation(this._getNextNavigationOptions(url, stepOptions));
if (!result) throw Error('Navigation returned undefined');

const providedName = stepOptions && stepOptions.stepName;
Expand Down
147 changes: 147 additions & 0 deletions lighthouse-core/test/fraggle-rock/user-flow-test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,147 @@
/**
* @license Copyright 2021 The Lighthouse Authors. All Rights Reserved.
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0
* Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License.
*/
'use strict';

/* eslint-env jest */

const snapshotModule = {snapshot: jest.fn()};
jest.mock('../../fraggle-rock/gather/snapshot-runner.js', () => snapshotModule);
const navigationModule = {navigation: jest.fn()};
jest.mock('../../fraggle-rock/gather/navigation-runner.js', () => navigationModule);
const timespanModule = {startTimespan: jest.fn()};
jest.mock('../../fraggle-rock/gather/timespan-runner.js', () => timespanModule);

const {createMockPage} = require('./gather/mock-driver.js');
const UserFlow = require('../../fraggle-rock/user-flow.js');

describe('UserFlow', () => {
let mockPage = createMockPage();

beforeEach(() => {
mockPage = createMockPage();
const lhr = {finalUrl: 'https://www.example.com'};

snapshotModule.snapshot.mockReset();
snapshotModule.snapshot.mockResolvedValue({lhr: {...lhr, gatherMode: 'snapshot'}});

navigationModule.navigation.mockReset();
navigationModule.navigation.mockResolvedValue({lhr: {...lhr, gatherMode: 'navigation'}});

const timespanLhr = {...lhr, gatherMode: 'timespan'};
const timespan = {endTimespan: jest.fn().mockResolvedValue({lhr: timespanLhr})};
timespanModule.startTimespan.mockReset();
timespanModule.startTimespan.mockResolvedValue(timespan);
});

describe('.navigate()', () => {
it('should throw if a timespan is active', async () => {
const flow = new UserFlow(mockPage.asPage());
await flow.startTimespan();
await expect(flow.navigate('https://example.com')).rejects.toBeTruthy();
});

it('should invoke navigation runner', async () => {
const flow = new UserFlow(mockPage.asPage());

await flow.navigate('https://example.com/1', {stepName: 'My Step'});

const configContext = {settingsOverrides: {maxWaitForLoad: 1000}};
await flow.navigate('https://example.com/2', {configContext});

await flow.navigate('https://example.com/3');

expect(navigationModule.navigation).toHaveBeenCalledTimes(3);
expect(flow.steps).toMatchObject([
{name: 'My Step', lhr: {finalUrl: 'https://www.example.com'}},
{name: 'Navigation report (www.example.com/)', lhr: {finalUrl: 'https://www.example.com'}},
{name: 'Navigation report (www.example.com/)', lhr: {finalUrl: 'https://www.example.com'}},
]);
});

it('should disable storage reset on subsequent navigations', async () => {
const flow = new UserFlow(mockPage.asPage());
await flow.navigate('https://example.com/1');

// Try once when we have some other settings.
const configContext = {settingsOverrides: {maxWaitForLoad: 1000}};
await flow.navigate('https://example.com/2', {configContext});

// Try once when we don't have any other settings.
await flow.navigate('https://example.com/3');

// Try once when we explicitly set it.
const configContextExplicit = {settingsOverrides: {disableStorageReset: false}};
await flow.navigate('https://example.com/4', {configContext: configContextExplicit});

// Check that we have the property set.
expect(navigationModule.navigation).toHaveBeenCalledTimes(4);
const [[call1], [call2], [call3], [call4]] = navigationModule.navigation.mock.calls;
expect(call1).not.toHaveProperty('configContext.settingsOverrides.disableStorageReset');
expect(call2).toHaveProperty('configContext.settingsOverrides.disableStorageReset');
expect(call3).toHaveProperty('configContext.settingsOverrides.disableStorageReset');
expect(call4).toHaveProperty('configContext.settingsOverrides.disableStorageReset');
expect(call2.configContext.settingsOverrides.disableStorageReset).toBe(true);
expect(call3.configContext.settingsOverrides.disableStorageReset).toBe(true);
expect(call4.configContext.settingsOverrides.disableStorageReset).toBe(false);

// Check that we didn't mutate the original objects.
expect(configContext).toEqual({settingsOverrides: {maxWaitForLoad: 1000}});
expect(configContextExplicit).toEqual({settingsOverrides: {disableStorageReset: false}});
});
});

describe('.startTimespan()', () => {
it('should throw if a timespan is active', async () => {
const flow = new UserFlow(mockPage.asPage());
await flow.startTimespan();
await expect(flow.startTimespan()).rejects.toBeTruthy();
});

it('should invoke timespan runner', async () => {
const flow = new UserFlow(mockPage.asPage());

await flow.startTimespan({stepName: 'My Timespan'});
await flow.endTimespan();

await flow.startTimespan();
await flow.endTimespan();

expect(timespanModule.startTimespan).toHaveBeenCalledTimes(2);
expect(flow.steps).toMatchObject([
{name: 'My Timespan', lhr: {finalUrl: 'https://www.example.com'}},
{name: 'Timespan report (www.example.com/)', lhr: {finalUrl: 'https://www.example.com'}},
]);
});
});

describe('.endTimespan()', () => {
it('should throw if a timespan has not started', async () => {
const flow = new UserFlow(mockPage.asPage());
await expect(flow.endTimespan()).rejects.toBeTruthy();
});
});

describe('.snapshot()', () => {
it('should throw if a timespan is active', async () => {
const flow = new UserFlow(mockPage.asPage());
await flow.startTimespan();
await expect(flow.snapshot()).rejects.toBeTruthy();
});

it('should invoke snapshot runner', async () => {
const flow = new UserFlow(mockPage.asPage());

await flow.snapshot({stepName: 'My Snapshot'});
await flow.snapshot();

expect(snapshotModule.snapshot).toHaveBeenCalledTimes(2);
expect(flow.steps).toMatchObject([
{name: 'My Snapshot', lhr: {finalUrl: 'https://www.example.com'}},
{name: 'Snapshot report (www.example.com/)', lhr: {finalUrl: 'https://www.example.com'}},
]);
});
});
});

0 comments on commit 70a32c2

Please sign in to comment.