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

misc: remove residual references to legacy #15292

Merged
merged 2 commits into from
Jul 26, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 2 additions & 5 deletions cli/test/fixtures/dobetterweb/dbw_tester.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,7 @@

(function() {

const params = new URLSearchParams(location.search);

if (location.search === '' || params.has('deprecations')) {
window.webkitStorageInfo.PERSISTENT; // FAIL(deprecations)
Copy link
Member Author

@adamraine adamraine Jul 20, 2023

Choose a reason for hiding this comment

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

Drive-by: webkitStorageInfo was removed (no longer just a deprecation) which was causing an extra error to appear in errors-in-console. We were not catching this before because we were not asserting a specific length for the number of errors before, but we are in this PR.

We had a note to remove when M110 hit stable as well.

}
// Just here to block rendering for now...
// Feel free to add code here if it's useful for the test.

})();
30 changes: 7 additions & 23 deletions cli/test/smokehouse/test-definitions/dobetterweb.js
Original file line number Diff line number Diff line change
Expand Up @@ -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': {
Expand Down Expand Up @@ -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: {
Expand Down
8 changes: 4 additions & 4 deletions core/config/validation.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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[]} */
Expand Down Expand Up @@ -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<string>} */
const artifactIds = new Set();
Expand Down Expand Up @@ -303,7 +303,7 @@ export {
isValidArtifactDependency,
assertValidPluginName,
assertValidArtifact,
assertValidFRNavigations,
assertValidNavigations,
assertValidAudit,
assertValidCategories,
assertValidSettings,
Expand Down
1 change: 0 additions & 1 deletion core/gather/driver/network-monitor.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ class NetworkMonitor extends NetworkMonitorEventEmitter {
/** @type {Array<LH.Crdp.Page.Frame>} */
_frameNavigations = [];

// TODO(FR-COMPAT): switch to real TargetManager when legacy removed.
/** @param {LH.Gatherer.Driver['targetManager']} targetManager */
constructor(targetManager) {
super();
Expand Down
5 changes: 2 additions & 3 deletions core/gather/gatherers/scripts.js
Original file line number Diff line number Diff line change
Expand Up @@ -91,9 +91,8 @@ class Scripts extends BaseGatherer {

session.off('Debugger.scriptParsed', this.onScriptParsed);

// Without this line the Debugger domain will be off in FR runner,
// because only the legacy gatherer has special handling for multiple,
// overlapped enabled/disable calls.
// Without this line the Debugger domain will be off due
// to overlapped enabled/disable calls in other gatherers.
await session.sendCommand('Debugger.enable');

// If run on a mobile device, be sensitive to memory limitations and only
Expand Down
10 changes: 5 additions & 5 deletions core/test/config/config-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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 () => {
Expand Down
2 changes: 1 addition & 1 deletion core/test/config/filters-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
8 changes: 4 additions & 4 deletions core/test/config/validation-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -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<LH.Config.NavigationDefn>} */
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');
Expand All @@ -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/);
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
<meta name="viewport" content="width=device-width, initial-scale=1.0, minimum-scale=1.0">
</head>
<body>
Hello, Fraggle Rock!
Hello, User Flows!

This page has some basic violations when the page has loaded.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
<meta name="viewport" content="width=device-width, initial-scale=1.0, minimum-scale=1.0">
</head>
<body>
This page links to the normal fraggle rock index page.
This page links to the normal user flow index page.

<a href="/?redirect=/index.html">Link to index</a>
</body>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
<meta name="viewport" content="width=device-width, initial-scale=1.0, minimum-scale=1.0">
</head>
<body>
Hello, Fraggle Rock!
Hello, User Flows!

This page has no accessibility violations until the mouse is clicked.

Expand Down
6 changes: 3 additions & 3 deletions core/test/gather/driver/target-manager-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand All @@ -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')
Expand Down Expand Up @@ -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')
Expand Down
3 changes: 3 additions & 0 deletions core/test/gather/gatherers/css-usage-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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(),
Expand Down
40 changes: 10 additions & 30 deletions core/test/gather/mock-commands.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Copy link
Member Author

Choose a reason for hiding this comment

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

useSessionId always needs to be false now that we removed the legacy runner, so we can just get rid of the options entirely

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
Expand All @@ -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);
Expand Down Expand Up @@ -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]);
},
});

Expand Down
6 changes: 3 additions & 3 deletions core/test/gather/mock-driver.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
*/

/**
* @fileoverview Mock fraggle rock driver for testing.
* @fileoverview Mock driver for testing.
*/

import jestMock from 'jest-mock';
Expand All @@ -25,7 +25,7 @@ import {fnAny} from '../test-utils.js';
function createMockSession() {
return {
setTargetInfo: fnAny(),
sendCommand: createMockSendCommandFn({useSessionId: false}),
sendCommand: createMockSendCommandFn(),
setNextProtocolTimeout: fnAny(),
hasNextProtocolTimeout: fnAny(),
getNextProtocolTimeout: fnAny(),
Expand All @@ -51,7 +51,7 @@ function createMockCdpSession(sessionId = 'DEFAULT_ID') {

return {
id: () => sessionId,
send: createMockSendCommandFn({useSessionId: false}),
send: createMockSendCommandFn(),
once: createMockOnceFn(),
on: createMockOnFn(),
off: fnAny(),
Expand Down
Loading