From 7ae8bd442d83efcca1cb48c5c48302d974b00756 Mon Sep 17 00:00:00 2001 From: Adam Raine <6752989+adamraine@users.noreply.github.com> Date: Wed, 26 Jul 2023 12:28:50 -0700 Subject: [PATCH] misc: remove residual references to legacy (#15292) --- cli/test/fixtures/dobetterweb/dbw_tester.js | 7 +--- .../test-definitions/dobetterweb.js | 30 ++++---------- core/config/validation.js | 8 ++-- core/gather/driver/network-monitor.js | 1 - core/test/config/config-test.js | 10 ++--- core/test/config/filters-test.js | 2 +- core/test/config/validation-test.js | 8 ++-- .../user-flows/navigation-basic/index.html | 2 +- .../navigation-basic/links-to-index.html | 2 +- .../user-flows/snapshot-basic/onclick.html | 2 +- .../test/gather/driver/target-manager-test.js | 6 +-- core/test/gather/gatherers/css-usage-test.js | 3 ++ core/test/gather/mock-commands.js | 40 +++++-------------- core/test/gather/mock-driver.js | 6 +-- .../__snapshots__/api-test-pptr.js.snap | 14 +++---- core/test/scenarios/api-test-pptr.js | 2 +- core/test/scripts/run-mocha-tests.js | 2 - tsconfig.json | 1 - types/gatherer.d.ts | 4 +- types/lhr/settings.d.ts | 2 +- 20 files changed, 56 insertions(+), 96 deletions(-) diff --git a/cli/test/fixtures/dobetterweb/dbw_tester.js b/cli/test/fixtures/dobetterweb/dbw_tester.js index 105ca159519d..fa1e4e57ff4d 100644 --- a/cli/test/fixtures/dobetterweb/dbw_tester.js +++ b/cli/test/fixtures/dobetterweb/dbw_tester.js @@ -8,10 +8,7 @@ (function() { -const params = new URLSearchParams(location.search); - -if (location.search === '' || params.has('deprecations')) { - window.webkitStorageInfo.PERSISTENT; // FAIL(deprecations) -} +// Just here to block rendering for now... +// Feel free to add code here if it's useful for the test. })(); diff --git a/cli/test/smokehouse/test-definitions/dobetterweb.js b/cli/test/smokehouse/test-definitions/dobetterweb.js index 1d10dc576ea2..e9bf8cec8515 100644 --- a/cli/test/smokehouse/test-definitions/dobetterweb.js +++ b/cli/test/smokehouse/test-definitions/dobetterweb.js @@ -269,37 +269,35 @@ const expectations = { 'errors-in-console': { score: 0, details: { - items: { - 0: { + items: [ + { source: 'exception', description: /^Error: A distinctive error\s+at http:\/\/localhost:10200\/dobetterweb\/dbw_tester.html:\d+:\d+$/, sourceLocation: {url: 'http://localhost:10200/dobetterweb/dbw_tester.html'}, }, - 1: { + { source: 'console.error', description: 'Error! Error!', sourceLocation: {url: 'http://localhost:10200/dobetterweb/dbw_tester.html'}, }, - 2: { + { source: 'network', description: 'Failed to load resource: the server responded with a status of 404 (Not Found)', sourceLocation: {url: 'http://localhost:10200/dobetterweb/unknown404.css?delay=200'}, }, - 3: { + { source: 'network', description: 'Failed to load resource: the server responded with a status of 404 (Not Found)', sourceLocation: {url: 'http://localhost:10200/dobetterweb/fcp-delayer.js?delay=5000'}, }, - 4: { + { // In the DT runner, the initial page load before staring Lighthouse will prevent this error. _excludeRunner: 'devtools', source: 'network', description: 'Failed to load resource: the server responded with a status of 404 (Not Found)', sourceLocation: {url: 'http://localhost:10200/favicon.ico'}, }, - // In legacy Lighthouse this audit will have additional duplicate failures which are a mistake. - // Fraggle Rock ordering of gatherer `stopInstrumentation` and `getArtifact` fixes the re-request issue. - }, + ], }, }, 'geolocation-on-start': { @@ -359,20 +357,6 @@ const expectations = { score: 0, details: { items: [ - { - // This feature was removed in M110. - // TODO: Remove this expectation once M110 reaches stable. - _maxChromiumVersion: '109', - value: /`window.webkitStorageInfo` is deprecated/, - source: { - type: 'source-location', - url: 'http://localhost:10200/dobetterweb/dbw_tester.js', - urlProvider: 'network', - line: '>0', - column: 9, - }, - subItems: undefined, - }, { value: /Synchronous `XMLHttpRequest` on the main thread is deprecated/, source: { diff --git a/core/config/validation.js b/core/config/validation.js index 04ebd1d5ece0..0128565cee1d 100644 --- a/core/config/validation.js +++ b/core/config/validation.js @@ -46,7 +46,7 @@ function assertValidPluginName(config, pluginName) { } /** - * Throws an error if the provided object does not implement the required Fraggle Rock gatherer interface. + * Throws an error if the provided object does not implement the required gatherer interface. * @param {LH.Config.AnyArtifactDefn} artifactDefn */ function assertValidArtifact(artifactDefn) { @@ -73,7 +73,7 @@ function assertValidArtifact(artifactDefn) { * @param {LH.Config.ResolvedConfig['navigations']} navigationsDefn * @return {{warnings: string[]}} */ -function assertValidFRNavigations(navigationsDefn) { +function assertValidNavigations(navigationsDefn) { if (!navigationsDefn || !navigationsDefn.length) return {warnings: []}; /** @type {string[]} */ @@ -248,7 +248,7 @@ function assertArtifactTopologicalOrder(navigations) { * @return {{warnings: string[]}} */ function assertValidConfig(resolvedConfig) { - const {warnings} = assertValidFRNavigations(resolvedConfig.navigations); + const {warnings} = assertValidNavigations(resolvedConfig.navigations); /** @type {Set} */ const artifactIds = new Set(); @@ -303,7 +303,7 @@ export { isValidArtifactDependency, assertValidPluginName, assertValidArtifact, - assertValidFRNavigations, + assertValidNavigations, assertValidAudit, assertValidCategories, assertValidSettings, diff --git a/core/gather/driver/network-monitor.js b/core/gather/driver/network-monitor.js index 9e9c2658b144..007373e49d77 100644 --- a/core/gather/driver/network-monitor.js +++ b/core/gather/driver/network-monitor.js @@ -31,7 +31,6 @@ class NetworkMonitor extends NetworkMonitorEventEmitter { /** @type {Array} */ _frameNavigations = []; - // TODO(FR-COMPAT): switch to real TargetManager when legacy removed. /** @param {LH.Gatherer.Driver['targetManager']} targetManager */ constructor(targetManager) { super(); diff --git a/core/test/config/config-test.js b/core/test/config/config-test.js index 2a3e2ab4fbd2..396e1eba08ac 100644 --- a/core/test/config/config-test.js +++ b/core/test/config/config-test.js @@ -16,7 +16,7 @@ import defaultConfig from '../../config/default-config.js'; const {nonSimulatedPassConfigOverrides} = constants; -describe('Fraggle Rock Config', () => { +describe('Config', () => { /** @type {LH.Gatherer.GatherMode} */ let gatherMode = 'snapshot'; @@ -86,10 +86,10 @@ describe('Fraggle Rock Config', () => { }); it('should throw on invalid artifact definitions', async () => { - const nonFRGatherer = new BaseGatherer(); - nonFRGatherer.getArtifact = jestMock.fn(); - const config = {artifacts: [{id: 'LegacyGather', gatherer: {instance: nonFRGatherer}}]}; - await expect(initializeConfig(gatherMode, config)).rejects.toThrow(/Gatherer for LegacyGather/); + const badGatherer = new BaseGatherer(); + badGatherer.getArtifact = jestMock.fn(); + const config = {artifacts: [{id: 'BadGatherer', gatherer: {instance: badGatherer}}]}; + await expect(initializeConfig(gatherMode, config)).rejects.toThrow(/Gatherer for BadGather/); }); it('should filter configuration by gatherMode', async () => { diff --git a/core/test/config/filters-test.js b/core/test/config/filters-test.js index 5fc72cd3414a..288cda2d9083 100644 --- a/core/test/config/filters-test.js +++ b/core/test/config/filters-test.js @@ -11,7 +11,7 @@ import BaseGatherer from '../../gather/base-gatherer.js'; import {defaultSettings, defaultNavigationConfig} from '../../config/constants.js'; import * as filters from '../../config/filters.js'; -describe('Fraggle Rock Config Filtering', () => { +describe('Config Filtering', () => { const snapshotGatherer = new BaseGatherer(); snapshotGatherer.meta = {supportedModes: ['snapshot']}; const timespanGatherer = new BaseGatherer(); diff --git a/core/test/config/validation-test.js b/core/test/config/validation-test.js index 835c4bff6557..1c454456c610 100644 --- a/core/test/config/validation-test.js +++ b/core/test/config/validation-test.js @@ -29,7 +29,7 @@ beforeEach(() => { ExampleAudit = ExampleAudit_; }); -describe('Fraggle Rock Config Validation', () => { +describe('Config Validation', () => { describe('assertValidConfig', () => { it('should throw if multiple artifacts have the same id', async () => { const {resolvedConfig} = await initializeConfig('navigation'); @@ -114,11 +114,11 @@ describe('Fraggle Rock Config Validation', () => { }); }); - describe('.assertValidFRNavigations', () => { + describe('.assertValidNavigations', () => { it('should add warning if navigations uses non-fatal loadFailureMode', () => { /** @type {Array} */ const navigations = [{...defaultNavigationConfig, loadFailureMode: 'warn', artifacts: []}]; - const {warnings} = validation.assertValidFRNavigations(navigations); + const {warnings} = validation.assertValidNavigations(navigations); expect(warnings).toHaveLength(1); expect(warnings[0]).toContain('but had a failure mode'); expect(navigations[0].loadFailureMode).toEqual('fatal'); @@ -132,7 +132,7 @@ describe('Fraggle Rock Config Validation', () => { {...defaultNavigationConfig, id: 'second', artifacts: []}, {...defaultNavigationConfig, id: 'first', artifacts: []}, ]; - const invocation = () => validation.assertValidFRNavigations(navigations); + const invocation = () => validation.assertValidNavigations(navigations); expect(invocation).toThrow(/must have unique.*but "first" was repeated/); }); }); diff --git a/core/test/fixtures/user-flows/navigation-basic/index.html b/core/test/fixtures/user-flows/navigation-basic/index.html index 76569632af42..6bee85e80b2c 100644 --- a/core/test/fixtures/user-flows/navigation-basic/index.html +++ b/core/test/fixtures/user-flows/navigation-basic/index.html @@ -11,7 +11,7 @@ - Hello, Fraggle Rock! + Hello, User Flows! This page has some basic violations when the page has loaded. diff --git a/core/test/fixtures/user-flows/navigation-basic/links-to-index.html b/core/test/fixtures/user-flows/navigation-basic/links-to-index.html index fa8868030097..af981e9a2173 100644 --- a/core/test/fixtures/user-flows/navigation-basic/links-to-index.html +++ b/core/test/fixtures/user-flows/navigation-basic/links-to-index.html @@ -11,7 +11,7 @@ - This page links to the normal fraggle rock index page. + This page links to the normal user flow index page. Link to index diff --git a/core/test/fixtures/user-flows/snapshot-basic/onclick.html b/core/test/fixtures/user-flows/snapshot-basic/onclick.html index 09c2a08e0648..ce444c39ac87 100644 --- a/core/test/fixtures/user-flows/snapshot-basic/onclick.html +++ b/core/test/fixtures/user-flows/snapshot-basic/onclick.html @@ -11,7 +11,7 @@ - Hello, Fraggle Rock! + Hello, User Flows! This page has no accessibility violations until the mouse is clicked. diff --git a/core/test/gather/driver/target-manager-test.js b/core/test/gather/driver/target-manager-test.js index c01df1515c2c..a208c42e716e 100644 --- a/core/test/gather/driver/target-manager-test.js +++ b/core/test/gather/driver/target-manager-test.js @@ -261,7 +261,7 @@ describe('TargetManager', () => { const rootTargetInfo = createTargetInfo(); // Still mock command responses at session level. - rootSession.send = createMockSendCommandFn({useSessionId: false}) + rootSession.send = createMockSendCommandFn() .mockResponse('Page.enable') .mockResponse('Page.getFrameTree', {frameTree: {frame: {id: ''}}}) .mockResponse('Runtime.enable') @@ -277,7 +277,7 @@ describe('TargetManager', () => { const iframeSession = createCdpSession('iframe'); const iframeTargetInfo = createTargetInfo({type: 'iframe', targetId: 'iframe'}); // Still mock command responses at session level. - iframeSession.send = createMockSendCommandFn({useSessionId: false}) + iframeSession.send = createMockSendCommandFn() .mockResponse('Target.getTargetInfo', {targetInfo: iframeTargetInfo}) .mockResponse('Network.enable') .mockResponse('Target.setAutoAttach') @@ -332,7 +332,7 @@ describe('TargetManager', () => { it('should stop listening for protocol events', async () => { const rootSession = createCdpSession('root'); // Still mock command responses at session level. - rootSession.send = createMockSendCommandFn({useSessionId: false}) + rootSession.send = createMockSendCommandFn() .mockResponse('Page.enable') .mockResponse('Page.getFrameTree', {frameTree: {frame: {id: ''}}}) .mockResponse('Runtime.enable') diff --git a/core/test/gather/gatherers/css-usage-test.js b/core/test/gather/gatherers/css-usage-test.js index 3addb103c5e5..3382f653a0fc 100644 --- a/core/test/gather/gatherers/css-usage-test.js +++ b/core/test/gather/gatherers/css-usage-test.js @@ -37,6 +37,7 @@ describe('.getArtifact', () => { /** @type {LH.Gatherer.Context} */ const context = { driver: driver.asDriver(), + page: driver._page.asPage(), gatherMode: 'snapshot', computedCache: new Map(), baseArtifacts: createMockBaseArtifacts(), @@ -94,6 +95,7 @@ describe('.getArtifact', () => { /** @type {LH.Gatherer.Context} */ const context = { driver: driver.asDriver(), + page: driver._page.asPage(), gatherMode: 'timespan', computedCache: new Map(), baseArtifacts: createMockBaseArtifacts(), @@ -149,6 +151,7 @@ describe('.getArtifact', () => { /** @type {LH.Gatherer.Context} */ const context = { driver: driver.asDriver(), + page: driver._page.asPage(), gatherMode: 'snapshot', computedCache: new Map(), baseArtifacts: createMockBaseArtifacts(), diff --git a/core/test/gather/mock-commands.js b/core/test/gather/mock-commands.js index a805df4d124f..f1a5d1d140ad 100644 --- a/core/test/gather/mock-commands.js +++ b/core/test/gather/mock-commands.js @@ -30,16 +30,8 @@ import {fnAny} from '../test-utils.js'; * returns the protocol message argument. * * To mock an error response, use `send.mockResponse('Command', () => Promise.reject(error))`. - * - * There are two variants of sendCommand, one that expects a sessionId as the second positional - * argument (legacy Lighthouse `Connection.sendCommand`) and one that does not (Fraggle Rock - * `ProtocolSession.sendCommand`). - * - * @param {{useSessionId: boolean}} [options] */ -function createMockSendCommandFn(options) { - const {useSessionId = true} = options || {}; - +function createMockSendCommandFn() { /** * Typescript fails to equate template type `C` here with `C` when pushing to this array. * Instead of sprinkling a couple ts-ignores, make `command` be any, but leave `C` for just @@ -52,18 +44,11 @@ function createMockSendCommandFn(options) { /** * @template {keyof LH.CrdpCommands} C * @param {C} command - * @param {string|undefined=} sessionId * @param {LH.CrdpCommands[C]['paramsType']} args */ - async (command, sessionId, ...args) => { - if (!useSessionId) { - // @ts-expect-error - If sessionId isn't used, it *is* args. - args = [sessionId, ...args]; - sessionId = undefined; - } - + async (command, ...args) => { const indexOfResponse = mockResponses - .findIndex(entry => entry.command === command && entry.sessionId === sessionId); + .findIndex(entry => entry.command === command && entry.sessionId === undefined); if (indexOfResponse === -1) throw new Error(`${command} unimplemented`); const {response, delay} = mockResponses[indexOfResponse]; mockResponses.splice(indexOfResponse, 1); @@ -96,25 +81,20 @@ function createMockSendCommandFn(options) { }, /** * @param {keyof LH.CrdpCommands} command - * @param {string=} sessionId */ - findInvocation(command, sessionId) { - const expectedArgs = useSessionId ? - [command, sessionId, expect.anything()] : - [command, expect.anything()]; - expect(mockFn).toHaveBeenCalledWith(...expectedArgs); + findInvocation(command) { + expect(mockFn).toHaveBeenCalledWith(command, expect.anything()); return mockFn.mock.calls.find( - call => call[0] === command && (!useSessionId || call[1] === sessionId) - )[useSessionId ? 2 : 1]; + call => call[0] === command + )[1]; }, /** * @param {keyof LH.CrdpCommands} command - * @param {string=} sessionId */ - findAllInvocations(command, sessionId) { + findAllInvocations(command) { return mockFn.mock.calls.filter( - call => call[0] === command && (!useSessionId || call[1] === sessionId) - ).map(invocation => useSessionId ? invocation[2] : invocation[1]); + call => call[0] === command + ).map(invocation => invocation[1]); }, }); diff --git a/core/test/gather/mock-driver.js b/core/test/gather/mock-driver.js index 378a06b79cf7..7d203fe58e6a 100644 --- a/core/test/gather/mock-driver.js +++ b/core/test/gather/mock-driver.js @@ -5,7 +5,7 @@ */ /** - * @fileoverview Mock fraggle rock driver for testing. + * @fileoverview Mock driver for testing. */ import jestMock from 'jest-mock'; @@ -26,7 +26,7 @@ import {NetworkMonitor} from '../../gather/driver/network-monitor.js'; function createMockSession() { return { setTargetInfo: fnAny(), - sendCommand: createMockSendCommandFn({useSessionId: false}), + sendCommand: createMockSendCommandFn(), setNextProtocolTimeout: fnAny(), hasNextProtocolTimeout: fnAny(), getNextProtocolTimeout: fnAny(), @@ -52,7 +52,7 @@ function createMockCdpSession(sessionId = 'DEFAULT_ID') { return { id: () => sessionId, - send: createMockSendCommandFn({useSessionId: false}), + send: createMockSendCommandFn(), once: createMockOnceFn(), on: createMockOnFn(), off: fnAny(), diff --git a/core/test/scenarios/__snapshots__/api-test-pptr.js.snap b/core/test/scenarios/__snapshots__/api-test-pptr.js.snap index 01fd365db366..8082112fd641 100644 --- a/core/test/scenarios/__snapshots__/api-test-pptr.js.snap +++ b/core/test/scenarios/__snapshots__/api-test-pptr.js.snap @@ -1,6 +1,6 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`Fraggle Rock API navigation should compute both snapshot & timespan results 1`] = ` +exports[`Individual modes API navigation should compute both snapshot & timespan results 1`] = ` Array [ "accesskeys", "aria-allowed-attr", @@ -171,7 +171,7 @@ Array [ ] `; -exports[`Fraggle Rock API navigation should compute results with callback requestor 1`] = ` +exports[`Individual modes API navigation should compute results with callback requestor 1`] = ` Array [ "accesskeys", "aria-allowed-attr", @@ -342,7 +342,7 @@ Array [ ] `; -exports[`Fraggle Rock API snapshot should compute accessibility results on the page as-is 1`] = ` +exports[`Individual modes API snapshot should compute accessibility results on the page as-is 1`] = ` Array [ "accesskeys", "aria-allowed-attr", @@ -435,7 +435,7 @@ Array [ ] `; -exports[`Fraggle Rock API startTimespan should compute ConsoleMessage results across a span of time 1`] = ` +exports[`Individual modes API startTimespan should compute ConsoleMessage results across a span of time 1`] = ` Array [ "bf-cache", "bootup-time", @@ -486,7 +486,7 @@ Array [ ] `; -exports[`Fraggle Rock API startTimespan should compute ConsoleMessage results across a span of time 2`] = ` +exports[`Individual modes API startTimespan should compute ConsoleMessage results across a span of time 2`] = ` Array [ "non-composited-animations", "preload-fonts", @@ -496,7 +496,7 @@ Array [ ] `; -exports[`Fraggle Rock API startTimespan should compute results from timespan after page load 1`] = ` +exports[`Individual modes API startTimespan should compute results from timespan after page load 1`] = ` Array [ "bf-cache", "bootup-time", @@ -547,7 +547,7 @@ Array [ ] `; -exports[`Fraggle Rock API startTimespan should compute results from timespan after page load 2`] = ` +exports[`Individual modes API startTimespan should compute results from timespan after page load 2`] = ` Array [ "duplicated-javascript", "efficient-animated-content", diff --git a/core/test/scenarios/api-test-pptr.js b/core/test/scenarios/api-test-pptr.js index 05ee289ec985..b293ee81d286 100644 --- a/core/test/scenarios/api-test-pptr.js +++ b/core/test/scenarios/api-test-pptr.js @@ -11,7 +11,7 @@ import {createTestState, getAuditsBreakdown} from './pptr-test-utils.js'; import {LH_ROOT} from '../../../root.js'; import {TargetManager} from '../../gather/driver/target-manager.js'; -describe('Fraggle Rock API', function() { +describe('Individual modes API', function() { // eslint-disable-next-line no-invalid-this this.timeout(120_000); diff --git a/core/test/scripts/run-mocha-tests.js b/core/test/scripts/run-mocha-tests.js index 538eeea9690a..eb237a07567f 100644 --- a/core/test/scripts/run-mocha-tests.js +++ b/core/test/scripts/run-mocha-tests.js @@ -75,7 +75,6 @@ const testsToIsolate = new Set([ 'core/test/gather/snapshot-runner-test.js', 'core/test/gather/timespan-runner-test.js', 'core/test/user-flow-test.js', - 'core/test/legacy/gather/gather-runner-test.js', 'core/test/gather/gatherers/dobetterweb/response-compression-test.js', 'core/test/gather/gatherers/script-elements-test.js', 'core/test/runner-test.js', @@ -90,7 +89,6 @@ const testsToIsolate = new Set([ 'flow-report/test/flow-report-pptr-test.ts', 'cli/test/cli/bin-test.js', 'cli/test/cli/run-test.js', - 'core/test/legacy/config/config-test.js', 'core/test/config/config-test.js', 'core/test/lib/emulation-test.js', 'core/test/lib/sentry-test.js', diff --git a/tsconfig.json b/tsconfig.json index 23fc56e66184..a4e6f4bc39f7 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -41,7 +41,6 @@ "core/test/config/budget-test.js", "core/test/config/config-helpers-test.js", "core/test/config/config-plugin-test.js", - "core/test/legacy/config/config-test.js", "core/test/config/default-config-test.js", "core/test/gather/driver/wait-for-condition-test.js", "core/test/gather/gatherers/accessibility-test.js", diff --git a/types/gatherer.d.ts b/types/gatherer.d.ts index 76edc0d2370d..7c065214e139 100644 --- a/types/gatherer.d.ts +++ b/types/gatherer.d.ts @@ -57,8 +57,8 @@ declare module Gatherer { gatherMode: GatherMode; /** The connection to the page being analyzed. */ driver: Driver; - /** The Puppeteer page handle. Will be undefined in legacy navigation mode. */ - page?: Puppeteer.Page; + /** The Puppeteer page handle. */ + page: Puppeteer.Page; /** The set of base artifacts that are always collected. */ baseArtifacts: BaseArtifacts; /** The cached results of computed artifacts. */ diff --git a/types/lhr/settings.d.ts b/types/lhr/settings.d.ts index 746215953e18..d0ea4a1d8344 100644 --- a/types/lhr/settings.d.ts +++ b/types/lhr/settings.d.ts @@ -71,7 +71,7 @@ export type ScreenEmulationSettings = { disableStorageReset?: boolean; /** Flag indicating that Lighthouse should pause after page load to wait for the user's permission to continue the audit. */ debugNavigation?: boolean; - /** If set to true, will skip the initial navigation to about:blank. This option is ignored when using the legacy navigation runner. */ + /** If set to true, will skip the initial navigation to about:blank. */ skipAboutBlank?: boolean; /** If set to true, gatherers should avoid any behavior that may be destructive to the page state. (e.g. extra navigations, resizing the viewport) */ usePassiveGathering?: boolean;