From f536a65d83849d47a016a5f516e9158adea5987a Mon Sep 17 00:00:00 2001 From: Patrick Hulce Date: Tue, 19 Jan 2021 08:43:57 -0600 Subject: [PATCH 1/5] core(fr): add navigation runner --- lighthouse-core/fraggle-rock/api.js | 2 + .../fraggle-rock/gather/navigation-runner.js | 210 +++++++++++++++ .../test/fraggle-rock/api-test-pptr.js | 15 ++ .../test/fraggle-rock/config/config-test.js | 1 + .../test/fraggle-rock/gather/mock-driver.js | 75 ++++++ .../gather/navigation-runner-test.js | 242 ++++++++++++++++++ .../gather/driver/execution-context-test.js | 6 +- types/config.d.ts | 3 +- 8 files changed, 551 insertions(+), 3 deletions(-) create mode 100644 lighthouse-core/fraggle-rock/gather/navigation-runner.js create mode 100644 lighthouse-core/test/fraggle-rock/gather/mock-driver.js create mode 100644 lighthouse-core/test/fraggle-rock/gather/navigation-runner-test.js diff --git a/lighthouse-core/fraggle-rock/api.js b/lighthouse-core/fraggle-rock/api.js index a20720ce4f88..cd8e519016f4 100644 --- a/lighthouse-core/fraggle-rock/api.js +++ b/lighthouse-core/fraggle-rock/api.js @@ -7,8 +7,10 @@ const {snapshot} = require('./gather/snapshot-runner.js'); const {startTimespan} = require('./gather/timespan-runner.js'); +const {navigation} = require('./gather/navigation-runner.js'); module.exports = { snapshot, startTimespan, + navigation, }; diff --git a/lighthouse-core/fraggle-rock/gather/navigation-runner.js b/lighthouse-core/fraggle-rock/gather/navigation-runner.js new file mode 100644 index 000000000000..ae1084e81fa4 --- /dev/null +++ b/lighthouse-core/fraggle-rock/gather/navigation-runner.js @@ -0,0 +1,210 @@ +/** + * @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'; + +const Driver = require('./driver.js'); +const Runner = require('../../runner.js'); +const {defaultNavigationConfig} = require('../../config/constants.js'); +const {initializeConfig} = require('../config/config.js'); +const {getBaseArtifacts} = require('./base-artifacts.js'); + +/** + * @typedef NavigationContext + * @property {Driver} driver + * @property {LH.Config.NavigationDefn} navigation + * @property {string} requestedUrl + */ + +/** @typedef {Record>} IntermediateArtifacts */ + +/** + * @param {{driver: Driver, config: LH.Config.FRConfig, requestedUrl: string}} args + */ +async function _setup({driver, config, requestedUrl}) { + await driver.connect(); + // TODO(FR-COMPAT): use frameNavigated-based navigation + await driver._page.goto(defaultNavigationConfig.blankPage); + + // TODO(FR-COMPAT): setupDriver + + const baseArtifacts = getBaseArtifacts(config); + baseArtifacts.URL.requestedUrl = requestedUrl; + + return {baseArtifacts}; +} + +/** + * @param {NavigationContext} navigationContext + */ +async function _setupNavigation({driver, navigation}) { + // TODO(FR-COMPAT): use frameNavigated-based navigation + await driver._page.goto(navigation.blankPage); + + // TODO(FR-COMPAT): setup network conditions (throttling & cache state) +} + +/** + * @param {NavigationContext} navigationContext + * @param {IntermediateArtifacts} artifacts + */ +async function _beforeTimespanPhase(navigationContext, artifacts) { + for (const artifactDefn of navigationContext.navigation.artifacts) { + const gatherer = artifactDefn.gatherer.instance; + if (!gatherer.meta.supportedModes.includes('timespan')) continue; + + const artifactPromise = Promise.resolve().then(() => + gatherer.beforeTimespan({driver: navigationContext.driver, gatherMode: 'navigation'}) + ); + artifacts[artifactDefn.id] = artifactPromise; + await artifactPromise.catch(() => {}); + } +} + +/** + * @param {NavigationContext} navigationContext + */ +async function _navigate(navigationContext) { + const {driver, requestedUrl} = navigationContext; + // TODO(FR-COMPAT): use waitForCondition-based navigation + await driver._page.goto(requestedUrl, {waitUntil: ['load', 'networkidle2']}); + + // TODO(FR-COMPAT): disable all throttling settings + // TODO(FR-COMPAT): capture page load errors +} + +/** + * @param {NavigationContext} navigationContext + * @param {IntermediateArtifacts} artifacts + */ +async function _afterTimespanPhase(navigationContext, artifacts) { + for (const artifactDefn of navigationContext.navigation.artifacts) { + const gatherer = artifactDefn.gatherer.instance; + if (!gatherer.meta.supportedModes.includes('timespan')) continue; + + const artifactPromise = (artifacts[artifactDefn.id] || Promise.resolve()).then(() => + gatherer.afterTimespan({driver: navigationContext.driver, gatherMode: 'navigation'}) + ); + artifacts[artifactDefn.id] = artifactPromise; + await artifactPromise.catch(() => {}); + } +} + +/** + * @param {NavigationContext} navigationContext + * @param {IntermediateArtifacts} artifacts + */ +async function _snapshotPhase(navigationContext, artifacts) { + for (const artifactDefn of navigationContext.navigation.artifacts) { + const gatherer = artifactDefn.gatherer.instance; + if (!gatherer.meta.supportedModes.includes('snapshot')) continue; + + const artifactPromise = Promise.resolve().then(() => + gatherer.snapshot({driver: navigationContext.driver, gatherMode: 'navigation'}) + ); + artifacts[artifactDefn.id] = artifactPromise; + await artifactPromise.catch(() => {}); + } +} + +/** + * @param {IntermediateArtifacts} timespanArtifacts + * @param {IntermediateArtifacts} snapshotArtifacts + * @return {Promise>} + */ +async function _mergeArtifacts(timespanArtifacts, snapshotArtifacts) { + /** @type {IntermediateArtifacts} */ + const artifacts = {}; + for (const [id, promise] of Object.entries({...timespanArtifacts, ...snapshotArtifacts})) { + artifacts[id] = await promise.catch(err => err); + } + + return artifacts; +} + +/** + * @param {NavigationContext} navigationContext + */ +async function _navigation(navigationContext) { + /** @type {IntermediateArtifacts} */ + const timespanArtifacts = {}; + /** @type {IntermediateArtifacts} */ + const snapshotArtifacts = {}; + + await _setupNavigation(navigationContext); + await _beforeTimespanPhase(navigationContext, timespanArtifacts); + await _navigate(navigationContext); + await _afterTimespanPhase(navigationContext, timespanArtifacts); + await _snapshotPhase(navigationContext, snapshotArtifacts); + + const artifacts = await _mergeArtifacts(timespanArtifacts, snapshotArtifacts); + return {artifacts}; +} + +/** + * @param {{driver: Driver, config: LH.Config.FRConfig, requestedUrl: string}} args + */ +async function _navigations({driver, config, requestedUrl}) { + if (!config.navigations) throw new Error('No navigations configured'); + + /** @type {Partial} */ + const artifacts = {}; + + for (const navigation of config.navigations) { + const navigationContext = { + driver, + navigation, + requestedUrl, + }; + + const navigationResult = await _navigation(navigationContext); + Object.assign(artifacts, navigationResult.artifacts); + } + + return {artifacts}; +} + +/** + * @param {{driver: Driver}} args + */ +async function _cleanup({driver}) { // eslint-disable-line no-unused-vars + // TODO(FR-COMPAT): clear storage if necessary +} + +/** + * @param {{url: string, page: import('puppeteer').Page, config?: LH.Config.Json}} options + * @return {Promise} + */ +async function navigation(options) { + const {url: requestedUrl, page} = options; + const {config} = initializeConfig(options.config, {gatherMode: 'timespan'}); + + return Runner.run( + async () => { + const driver = new Driver(page); + const {baseArtifacts} = await _setup({driver, config, requestedUrl}); + const {artifacts} = await _navigations({driver, config, requestedUrl}); + + return /** @type {LH.Artifacts} */ ({...baseArtifacts, ...artifacts}); // Cast to drop Partial<> + }, + { + url: requestedUrl, + config, + } + ); +} + +module.exports = { + navigation, + _setup, + _setupNavigation, + _beforeTimespanPhase, + _afterTimespanPhase, + _snapshotPhase, + _navigate, + _navigation, + _navigations, + _cleanup, +}; diff --git a/lighthouse-core/test/fraggle-rock/api-test-pptr.js b/lighthouse-core/test/fraggle-rock/api-test-pptr.js index 175b2582d8df..7969cd92ae81 100644 --- a/lighthouse-core/test/fraggle-rock/api-test-pptr.js +++ b/lighthouse-core/test/fraggle-rock/api-test-pptr.js @@ -132,4 +132,19 @@ describe('Fraggle Rock API', () => { if (!matchingLog) expect(errorLogs).toContain({description: /violations added/}); }); }); + + describe('navigation', () => { + beforeEach(() => { + server.baseDir = path.join(__dirname, '../fixtures/fraggle-rock/snapshot-basic'); + }); + + it('should compute both Accessibility & ConsoleMessage results', async () => { + const result = await lighthouse.navigation({page, url: `${serverBaseUrl}/onclick.html`}); + if (!result) throw new Error('Lighthouse failed to produce a result'); + + const {lhr} = result; + const {erroredAudits} = getAuditsBreakdown(lhr); + expect(erroredAudits).toHaveLength(0); + }); + }); }); diff --git a/lighthouse-core/test/fraggle-rock/config/config-test.js b/lighthouse-core/test/fraggle-rock/config/config-test.js index bda5bd47526b..ea2a45489063 100644 --- a/lighthouse-core/test/fraggle-rock/config/config-test.js +++ b/lighthouse-core/test/fraggle-rock/config/config-test.js @@ -147,4 +147,5 @@ describe('Fraggle Rock Config', () => { it.todo('should filter configuration by inclusive settings'); it.todo('should filter configuration by exclusive settings'); it.todo('should validate audit/gatherer interdependencies'); + it.todo('should validate gatherers do not support all 3 modes'); }); diff --git a/lighthouse-core/test/fraggle-rock/gather/mock-driver.js b/lighthouse-core/test/fraggle-rock/gather/mock-driver.js new file mode 100644 index 000000000000..078c7ec6c156 --- /dev/null +++ b/lighthouse-core/test/fraggle-rock/gather/mock-driver.js @@ -0,0 +1,75 @@ +/** + * @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 { + createMockOnFn, + createMockOnceFn, + createMockSendCommandFn, +} = require('../../gather/mock-commands.js'); + +/** + * @fileoverview Mock fraggle rock driver for testing. + */ + +/** @typedef {import('../../../fraggle-rock/gather/driver.js')} Driver */ +/** @typedef {import('../../../gather/driver/execution-context.js')} ExecutionContext */ + +function createMockSession() { + return { + sendCommand: createMockSendCommandFn({useSessionId: false}), + once: createMockOnceFn(), + on: createMockOnFn(), + off: jest.fn(), + }; +} + +function createMockPage() { + return { + goto: jest.fn(), + target: () => ({createCDPSession: () => createMockSession()}), + + /** @return {import('puppeteer').Page} */ + asPage() { + // @ts-expect-error - We'll rely on the tests passing to know this matches. + return this; + }, + }; +} + +function createMockExecutionContext() { + const context = /** @type {ExecutionContext} */ ({}); + return {...context, evaluate: jest.fn(), evaluateAsync: jest.fn()}; +} + +function createMockDriver() { + const session = createMockSession(); + const context = createMockExecutionContext(); + + return { + _page: createMockPage(), + _executionContext: context, + _session: session, + defaultSession: session, + connect: jest.fn(), + evaluate: context.evaluate, + evaluateAsync: context.evaluateAsync, + + /** @return {Driver} */ + asDriver() { + // @ts-expect-error - We'll rely on the tests passing to know this matches. + return this; + }, + }; +} + +module.exports = { + createMockDriver, + createMockPage, + createMockSession, +}; diff --git a/lighthouse-core/test/fraggle-rock/gather/navigation-runner-test.js b/lighthouse-core/test/fraggle-rock/gather/navigation-runner-test.js new file mode 100644 index 000000000000..1c6d1ac819da --- /dev/null +++ b/lighthouse-core/test/fraggle-rock/gather/navigation-runner-test.js @@ -0,0 +1,242 @@ +/** + * @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'; + +const {createMockDriver} = require('./mock-driver.js'); +const runner = require('../../../fraggle-rock/gather/navigation-runner.js'); +const {initializeConfig} = require('../../../fraggle-rock/config/config.js'); +const {defaultNavigationConfig} = require('../../../config/constants.js'); + +/* eslint-env jest */ + +/** @typedef {{snapshot: jest.Mock, beforeTimespan:jest.Mock, afterTimespan: jest.Mock}} MockGatherer */ + +describe('NavigationRunner', () => { + let requestedUrl = ''; + /** @type {ReturnType} */ + let mockDriver; + /** @type {import('../../../fraggle-rock/gather/driver.js')} */ + let driver; + /** @type {LH.Config.FRConfig} */ + let config; + /** @type {LH.Config.NavigationDefn} */ + let navigation; + + /** @return {LH.Config.FRGathererDefn} */ + function createGathererDefn() { + return { + instance: { + name: 'Accessibility', + meta: {supportedModes: []}, + beforeTimespan: jest.fn(), + afterTimespan: jest.fn(), + snapshot: jest.fn(), + }, + }; + } + + /** @return {{navigation: LH.Config.NavigationDefn, gatherers: {timespan: MockGatherer, snapshot: MockGatherer}}} */ + function createNavigation() { + const timespanGatherer = createGathererDefn(); + timespanGatherer.instance.meta.supportedModes = ['timespan', 'navigation']; + timespanGatherer.instance.afterTimespan = jest.fn().mockResolvedValue({type: 'timespan'}); + const snapshotGatherer = createGathererDefn(); + snapshotGatherer.instance.meta.supportedModes = ['snapshot', 'navigation']; + snapshotGatherer.instance.snapshot = jest.fn().mockResolvedValue({type: 'snapshot'}); + + const navigation = { + ...defaultNavigationConfig, + artifacts: [ + {id: 'Timespan', gatherer: timespanGatherer}, + {id: 'Snapshot', gatherer: snapshotGatherer}, + ], + }; + + return { + navigation, + gatherers: { + timespan: /** @type {any} */ (timespanGatherer.instance), + snapshot: /** @type {any} */ (snapshotGatherer.instance), + }, + }; + } + + beforeEach(() => { + requestedUrl = 'http://example.com'; + config = initializeConfig(undefined, {gatherMode: 'navigation'}).config; + navigation = createNavigation().navigation; + + mockDriver = createMockDriver(); + driver = mockDriver.asDriver(); + }); + + describe('_setup', () => { + it('should connect the driver', async () => { + await runner._setup({driver, config, requestedUrl}); + expect(mockDriver.connect).toHaveBeenCalled(); + }); + + it('should navigate to the blank page', async () => { + await runner._setup({driver, config, requestedUrl}); + expect(mockDriver._page.goto).toHaveBeenCalledWith('about:blank'); + }); + + it.todo('should throw if service worker is currently controlling the page'); + it.todo('should enable emulation'); + it.todo('should enable important CDP domains'); + it.todo('should register the performance observer for navigation conditions'); + it.todo('should shim requestIdleCallback'); + it.todo('should reset storage'); + it.todo('should not reset storage when skipped'); + }); + + describe('_navigations', () => { + it('should throw if no navigations available', async () => { + config = {...config, navigations: null}; + await expect(runner._navigations({driver, requestedUrl, config})).rejects.toBeTruthy(); + }); + + it('should navigate as many times as there are navigations', async () => { + config = initializeConfig( + { + ...config, + navigations: [{id: 'default'}, {id: 'second'}, {id: 'third'}, {id: 'fourth'}], + }, + {gatherMode: 'navigation'} + ).config; + + await runner._navigations({driver, config, requestedUrl}); + const navigations = mockDriver._page.goto.mock.calls; + const pageNavigations = navigations.filter(call => call[0] === requestedUrl); + expect(pageNavigations).toHaveLength(4); + }); + + it('should merge artifacts between navigations', async () => { + config = initializeConfig( + { + ...config, + navigations: [ + {id: 'default', artifacts: ['Accessibility']}, + {id: 'second', artifacts: ['ConsoleMessages']}, + ], + }, + {gatherMode: 'navigation'} + ).config; + + const {artifacts} = await runner._navigations({driver, config, requestedUrl}); + const artifactIds = Object.keys(artifacts); + expect(artifactIds).toContain('Accessibility'); + expect(artifactIds).toContain('ConsoleMessages'); + }); + }); + + describe('_navigation', () => { + it('completes an end-to-end navigation', async () => { + const {artifacts} = await runner._navigation({driver, navigation, requestedUrl}); + const artifactIds = Object.keys(artifacts); + expect(artifactIds).toContain('Timespan'); + expect(artifactIds).toContain('Snapshot'); + + expect(mockDriver._page.goto).toHaveBeenCalled(); + }); + + it('collects both timespan and snapshot artifacts', async () => { + const {artifacts} = await runner._navigation({driver, navigation, requestedUrl}); + expect(artifacts).toEqual({ + Timespan: {type: 'timespan'}, + Snapshot: {type: 'snapshot'}, + }); + }); + }); + + describe('_setupNavigation', () => { + it('should setup the page on the blankPage', async () => { + navigation.blankPage = 'data:text/html;...'; + await runner._setupNavigation({driver, navigation, requestedUrl}); + expect(mockDriver._page.goto).toHaveBeenCalledWith('data:text/html;...'); + }); + + it.todo('should setup throttling'); + it.todo('should clear cache'); + it.todo('should skip clear cache when requested'); + }); + + describe('_beforeTimespanPhase', () => { + /** @type {Record>} */ + let artifacts; + + beforeEach(() => { + artifacts = {}; + }); + + it('should run the beforeTimespan phase of timespan gatherers', async () => { + const {navigation, gatherers} = createNavigation(); + await runner._beforeTimespanPhase({driver, navigation, requestedUrl}, artifacts); + expect(artifacts).toEqual({Timespan: expect.any(Promise)}); + expect(gatherers.timespan.beforeTimespan).toHaveBeenCalled(); + expect(gatherers.snapshot.beforeTimespan).not.toHaveBeenCalled(); + }); + }); + + describe('_navigate', () => { + it('should navigate the page', async () => { + await runner._navigate({driver, navigation, requestedUrl}); + expect(mockDriver._page.goto).toHaveBeenCalledWith(requestedUrl, expect.anything()); + }); + + it.todo('should wait for page conditions'); + it.todo('should disable throttling when finished'); + it.todo('should capture page load errors'); + }); + + describe('_afterTimespanPhase', () => { + /** @type {Record>} */ + let artifacts; + + beforeEach(() => { + artifacts = {}; + }); + + it('should run the afterTimespan phase of timespan gatherers', async () => { + const {navigation, gatherers} = createNavigation(); + await runner._afterTimespanPhase({driver, navigation, requestedUrl}, artifacts); + expect(artifacts).toEqual({Timespan: expect.any(Promise)}); + expect(await artifacts.Timespan).toEqual({type: 'timespan'}); + expect(gatherers.timespan.afterTimespan).toHaveBeenCalled(); + expect(gatherers.snapshot.afterTimespan).not.toHaveBeenCalled(); + }); + + it('should combine the previous promises', async () => { + artifacts = {Timespan: Promise.reject(new Error('beforeTimespan rejection'))}; + + const {navigation, gatherers} = createNavigation(); + await runner._afterTimespanPhase({driver, navigation, requestedUrl}, artifacts); + expect(artifacts).toEqual({Timespan: expect.any(Promise)}); + await expect(artifacts.Timespan).rejects.toMatchObject({message: 'beforeTimespan rejection'}); + expect(gatherers.timespan.afterTimespan).not.toHaveBeenCalled(); + expect(gatherers.snapshot.afterTimespan).not.toHaveBeenCalled(); + }); + }); + + describe('_snapshotPhase', () => { + /** @type {Record>} */ + let artifacts; + + beforeEach(() => { + artifacts = {}; + }); + + it('should run the snapshot phase of timespan gatherers', async () => { + const {navigation, gatherers} = createNavigation(); + await runner._snapshotPhase({driver, navigation, requestedUrl}, artifacts); + expect(artifacts).toEqual({Snapshot: expect.any(Promise)}); + expect(await artifacts.Snapshot).toEqual({type: 'snapshot'}); + expect(gatherers.timespan.snapshot).not.toHaveBeenCalled(); + expect(gatherers.snapshot.snapshot).toHaveBeenCalled(); + }); + }); +}); + diff --git a/lighthouse-core/test/gather/driver/execution-context-test.js b/lighthouse-core/test/gather/driver/execution-context-test.js index f06a3db56340..d656d858dc97 100644 --- a/lighthouse-core/test/gather/driver/execution-context-test.js +++ b/lighthouse-core/test/gather/driver/execution-context-test.js @@ -77,6 +77,8 @@ describe('ExecutionContext', () => { executionDestroyed[1]({executionContextId: 42}); expect(executionContext.getContextId()).toEqual(undefined); }); + + it.todo('should cache native objects in page'); }); describe('.evaluateAsync', () => { @@ -205,7 +207,7 @@ describe('.evaluate', () => { return new __nativePromise(function (resolve) { return __nativePromise.resolve() .then(_ => (() => { - + function main(value) { return value; } @@ -245,7 +247,7 @@ describe('.evaluate', () => { const code = mockFn.mock.calls[0][0]; expect(code).toBe(`(() => { - + function mainFn(value) { return value; } diff --git a/types/config.d.ts b/types/config.d.ts index d2f002d3a0ab..a62dd4d88174 100644 --- a/types/config.d.ts +++ b/types/config.d.ts @@ -102,7 +102,7 @@ declare global { export interface ArtifactJson { id: string; - gatherer: GathererJson; + gatherer: FRGathererJson; } export type GathererJson = { @@ -116,6 +116,7 @@ declare global { options?: {}; } | Gatherer.GathererInstance | ClassOf | string; + export type FRGathererJson = string | {instance: Gatherer.FRGathererInstance} export interface CategoryJson { title: string | IcuMessage; From 63120985601ca13a57d77f5a12c324bb010560ad Mon Sep 17 00:00:00 2001 From: Patrick Hulce Date: Wed, 20 Jan 2021 17:01:53 -0600 Subject: [PATCH 2/5] address feedback --- .../fraggle-rock/gather/navigation-runner.js | 1 + .../fraggle-rock/navigation-basic/index.html | 24 +++++++++++++++++++ .../test/fraggle-rock/api-test-pptr.js | 8 +++++-- .../gather/navigation-runner-test.js | 2 +- 4 files changed, 32 insertions(+), 3 deletions(-) create mode 100644 lighthouse-core/test/fixtures/fraggle-rock/navigation-basic/index.html diff --git a/lighthouse-core/fraggle-rock/gather/navigation-runner.js b/lighthouse-core/fraggle-rock/gather/navigation-runner.js index ae1084e81fa4..8a639bd89d2a 100644 --- a/lighthouse-core/fraggle-rock/gather/navigation-runner.js +++ b/lighthouse-core/fraggle-rock/gather/navigation-runner.js @@ -186,6 +186,7 @@ async function navigation(options) { const driver = new Driver(page); const {baseArtifacts} = await _setup({driver, config, requestedUrl}); const {artifacts} = await _navigations({driver, config, requestedUrl}); + await _cleanup({driver}); return /** @type {LH.Artifacts} */ ({...baseArtifacts, ...artifacts}); // Cast to drop Partial<> }, diff --git a/lighthouse-core/test/fixtures/fraggle-rock/navigation-basic/index.html b/lighthouse-core/test/fixtures/fraggle-rock/navigation-basic/index.html new file mode 100644 index 000000000000..501dac968e78 --- /dev/null +++ b/lighthouse-core/test/fixtures/fraggle-rock/navigation-basic/index.html @@ -0,0 +1,24 @@ + + + + + + + + + + Hello, Fraggle Rock! + + This page has some basic violations when the page has loaded. + + + + + + diff --git a/lighthouse-core/test/fraggle-rock/api-test-pptr.js b/lighthouse-core/test/fraggle-rock/api-test-pptr.js index 7969cd92ae81..aa091b8d3301 100644 --- a/lighthouse-core/test/fraggle-rock/api-test-pptr.js +++ b/lighthouse-core/test/fraggle-rock/api-test-pptr.js @@ -135,7 +135,7 @@ describe('Fraggle Rock API', () => { describe('navigation', () => { beforeEach(() => { - server.baseDir = path.join(__dirname, '../fixtures/fraggle-rock/snapshot-basic'); + server.baseDir = path.join(__dirname, '../fixtures/fraggle-rock/navigation-basic'); }); it('should compute both Accessibility & ConsoleMessage results', async () => { @@ -143,8 +143,12 @@ describe('Fraggle Rock API', () => { if (!result) throw new Error('Lighthouse failed to produce a result'); const {lhr} = result; - const {erroredAudits} = getAuditsBreakdown(lhr); + const {failedAudits, erroredAudits} = getAuditsBreakdown(lhr); expect(erroredAudits).toHaveLength(0); + + const failedAuditIds = failedAudits.map(audit => audit.id); + expect(failedAuditIds).toContain('label'); + expect(failedAuditIds).toContain('errors-in-console'); }); }); }); diff --git a/lighthouse-core/test/fraggle-rock/gather/navigation-runner-test.js b/lighthouse-core/test/fraggle-rock/gather/navigation-runner-test.js index 1c6d1ac819da..40d2f248afcb 100644 --- a/lighthouse-core/test/fraggle-rock/gather/navigation-runner-test.js +++ b/lighthouse-core/test/fraggle-rock/gather/navigation-runner-test.js @@ -229,7 +229,7 @@ describe('NavigationRunner', () => { artifacts = {}; }); - it('should run the snapshot phase of timespan gatherers', async () => { + it('should run the snapshot phase of snapshot gatherers', async () => { const {navigation, gatherers} = createNavigation(); await runner._snapshotPhase({driver, navigation, requestedUrl}, artifacts); expect(artifacts).toEqual({Snapshot: expect.any(Promise)}); From 38c93aa23bea7e1db01e250aba3a641ed73dfb41 Mon Sep 17 00:00:00 2001 From: Patrick Hulce Date: Wed, 20 Jan 2021 17:04:33 -0600 Subject: [PATCH 3/5] handle type error --- lighthouse-core/fraggle-rock/config/config.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lighthouse-core/fraggle-rock/config/config.js b/lighthouse-core/fraggle-rock/config/config.js index 0bbe801f46b1..223489484fd0 100644 --- a/lighthouse-core/fraggle-rock/config/config.js +++ b/lighthouse-core/fraggle-rock/config/config.js @@ -61,7 +61,11 @@ function resolveArtifactsToDefns(artifacts, configDir) { const coreGathererList = Runner.getGathererList(); const artifactDefns = artifacts.map(artifactJson => { - const gatherer = resolveGathererToDefn(artifactJson.gatherer, coreGathererList, configDir); + /** @type {LH.Config.GathererJson} */ + // @ts-expect-error FR-COMPAT - eventually move the config-helpers to support new types + const gathererJson = artifactJson.gatherer; + + const gatherer = resolveGathererToDefn(gathererJson, coreGathererList, configDir); if (!isFRGathererDefn(gatherer)) { throw new Error(`${gatherer.instance.name} gatherer does not support Fraggle Rock`); } From fce808ae54c924e7e74de6ed0c5c208ea692a834 Mon Sep 17 00:00:00 2001 From: Patrick Hulce Date: Wed, 20 Jan 2021 19:53:38 -0600 Subject: [PATCH 4/5] try in ci --- .../fraggle-rock/gather/navigation-runner.js | 2 +- .../fixtures/fraggle-rock/navigation-basic/index.html | 10 +++++++++- .../test/gather/driver/execution-context-test.js | 11 ++++++++--- 3 files changed, 18 insertions(+), 5 deletions(-) diff --git a/lighthouse-core/fraggle-rock/gather/navigation-runner.js b/lighthouse-core/fraggle-rock/gather/navigation-runner.js index 8a639bd89d2a..78ec7cc7fb3a 100644 --- a/lighthouse-core/fraggle-rock/gather/navigation-runner.js +++ b/lighthouse-core/fraggle-rock/gather/navigation-runner.js @@ -179,7 +179,7 @@ async function _cleanup({driver}) { // eslint-disable-line no-unused-vars */ async function navigation(options) { const {url: requestedUrl, page} = options; - const {config} = initializeConfig(options.config, {gatherMode: 'timespan'}); + const {config} = initializeConfig(options.config, {gatherMode: 'navigation'}); return Runner.run( async () => { diff --git a/lighthouse-core/test/fixtures/fraggle-rock/navigation-basic/index.html b/lighthouse-core/test/fixtures/fraggle-rock/navigation-basic/index.html index 501dac968e78..76569632af42 100644 --- a/lighthouse-core/test/fixtures/fraggle-rock/navigation-basic/index.html +++ b/lighthouse-core/test/fixtures/fraggle-rock/navigation-basic/index.html @@ -15,9 +15,17 @@ This page has some basic violations when the page has loaded. - + +
+
+ +
+
diff --git a/lighthouse-core/test/gather/driver/execution-context-test.js b/lighthouse-core/test/gather/driver/execution-context-test.js index d656d858dc97..d7d91e21a801 100644 --- a/lighthouse-core/test/gather/driver/execution-context-test.js +++ b/lighthouse-core/test/gather/driver/execution-context-test.js @@ -28,6 +28,11 @@ function createMockSession() { return session; } +/** @param {string} s */ +function trimTrailingWhitespace(s) { + return s.split('\n').map(line => line.trimEnd()).join('\n'); +} + describe('ExecutionContext', () => { /** @type {LH.Gatherer.FRProtocolSession} */ let sessionMock; @@ -228,7 +233,7 @@ describe('.evaluate', () => { .then(resolve); }); }())`.trim(); - expect(expression).toBe(expected); + expect(trimTrailingWhitespace(expression)).toBe(expected); expect(await eval(expression)).toBe(1); }); @@ -246,7 +251,7 @@ describe('.evaluate', () => { const value = await executionContext.evaluate(mainFn, {args: [1]}); // eslint-disable-line no-unused-vars const code = mockFn.mock.calls[0][0]; - expect(code).toBe(`(() => { + expect(trimTrailingWhitespace(code)).toBe(`(() => { function mainFn(value) { return value; @@ -289,7 +294,7 @@ describe('.evaluate', () => { }); const code = mockFn.mock.calls[0][0]; - expect(code).toEqual(`(() => { + expect(trimTrailingWhitespace(code)).toEqual(`(() => { function abs(val) { return Math.abs(val); } From 426a07c5d44e04d5f6f375c9542b8d8f40fc133e Mon Sep 17 00:00:00 2001 From: Patrick Hulce Date: Thu, 21 Jan 2021 10:14:31 -0600 Subject: [PATCH 5/5] use correct navigation page --- lighthouse-core/test/fraggle-rock/api-test-pptr.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lighthouse-core/test/fraggle-rock/api-test-pptr.js b/lighthouse-core/test/fraggle-rock/api-test-pptr.js index aa091b8d3301..82df6221bb46 100644 --- a/lighthouse-core/test/fraggle-rock/api-test-pptr.js +++ b/lighthouse-core/test/fraggle-rock/api-test-pptr.js @@ -139,7 +139,7 @@ describe('Fraggle Rock API', () => { }); it('should compute both Accessibility & ConsoleMessage results', async () => { - const result = await lighthouse.navigation({page, url: `${serverBaseUrl}/onclick.html`}); + const result = await lighthouse.navigation({page, url: `${serverBaseUrl}/index.html`}); if (!result) throw new Error('Lighthouse failed to produce a result'); const {lhr} = result;