Skip to content

Commit

Permalink
core: remove legacy runner (#15253)
Browse files Browse the repository at this point in the history
  • Loading branch information
adamraine committed Jul 14, 2023
1 parent 3cd03a3 commit 1ef1faf
Show file tree
Hide file tree
Showing 57 changed files with 507 additions and 6,808 deletions.
5 changes: 1 addition & 4 deletions build/build-bundle.js
Original file line number Diff line number Diff line change
Expand Up @@ -110,10 +110,7 @@ async function buildBundle(entryPath, distPath, opts = {minify: true}) {
}).join(',\n');

/** @type {Record<string, string>} */
const shimsObj = {
[require.resolve('../core/legacy/gather/connections/cri.js')]:
'export const CriConnection = {}',
};
const shimsObj = {};

const modulesToIgnore = [
'puppeteer-core',
Expand Down
5 changes: 0 additions & 5 deletions cli/cli-flags.js
Original file line number Diff line number Diff line change
Expand Up @@ -117,11 +117,6 @@ function getYargsParser(manualArgv) {
type: 'boolean',
describe: 'Pause after page load to wait for permission to continue the run, evaluate `continueLighthouseRun` in the console to continue.',
},
'legacy-navigation': {
type: 'boolean',
default: false,
describe: '[DEPRECATED] Use the legacy navigation runner to gather results. Only use this if you are using a pre-10.0 custom Lighthouse config, or if Lighthouse unexpectedly fails after updating to 10.0. Please file a bug if you need this flag for Lighthouse to work.',
},
'additional-trace-categories': {
type: 'string',
describe: 'Additional categories to capture with the trace (comma-delimited).',
Expand Down
13 changes: 3 additions & 10 deletions cli/run.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import log from 'lighthouse-logger';
import open from 'open';

import * as Printer from './printer.js';
import lighthouse, {legacyNavigation} from '../core/index.js';
import lighthouse from '../core/index.js';
import {getLhrFilenamePrefix} from '../report/generator/file-namer.js';
import * as assetSaver from '../core/lib/asset-saver.js';
import UrlUtils from '../core/lib/url-utils.js';
Expand Down Expand Up @@ -238,16 +238,9 @@ async function runLighthouse(url, flags, config) {
flags.port = launchedChrome.port;
}

if (flags.legacyNavigation) {
log.warn('CLI', 'Legacy navigation CLI is deprecated');
flags.channel = 'legacy-navigation-cli';
} else if (!flags.channel) {
flags.channel = 'cli';
}
flags.channel = 'cli';

const runnerResult = flags.legacyNavigation ?
await legacyNavigation(url, flags, config) :
await lighthouse(url, flags, config);
const runnerResult = await lighthouse(url, flags, config);

// If in gatherMode only, there will be no runnerResult.
if (runnerResult) {
Expand Down
20 changes: 0 additions & 20 deletions cli/test/cli/__snapshots__/cli-flags-test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,6 @@ Object {
],
"chromeIgnoreDefaultFlags": false,
"hostname": "127.0.0.1",
"legacy-navigation": false,
"legacyNavigation": false,
"list-all-audits": false,
"list-locales": false,
"list-trace-categories": false,
Expand Down Expand Up @@ -56,8 +54,6 @@ Object {
"chromeFlags": "",
"chromeIgnoreDefaultFlags": false,
"hostname": "127.0.0.1",
"legacy-navigation": false,
"legacyNavigation": false,
"list-all-audits": false,
"list-locales": false,
"list-trace-categories": false,
Expand Down Expand Up @@ -112,8 +108,6 @@ Object {
"foo": "bar",
},
"hostname": "127.0.0.1",
"legacy-navigation": false,
"legacyNavigation": false,
"list-all-audits": false,
"list-locales": false,
"list-trace-categories": false,
Expand Down Expand Up @@ -152,8 +146,6 @@ Object {
"x-men": "wolverine",
},
"hostname": "127.0.0.1",
"legacy-navigation": false,
"legacyNavigation": false,
"list-all-audits": false,
"list-locales": false,
"list-trace-categories": false,
Expand Down Expand Up @@ -184,8 +176,6 @@ Object {
"chromeFlags": "",
"chromeIgnoreDefaultFlags": false,
"hostname": "127.0.0.1",
"legacy-navigation": false,
"legacyNavigation": false,
"list-all-audits": false,
"list-locales": false,
"list-trace-categories": false,
Expand Down Expand Up @@ -230,8 +220,6 @@ Object {
"chromeFlags": "",
"chromeIgnoreDefaultFlags": false,
"hostname": "127.0.0.1",
"legacy-navigation": false,
"legacyNavigation": false,
"list-all-audits": false,
"list-locales": false,
"list-trace-categories": false,
Expand Down Expand Up @@ -276,8 +264,6 @@ Object {
"chromeFlags": "",
"chromeIgnoreDefaultFlags": false,
"hostname": "127.0.0.1",
"legacy-navigation": false,
"legacyNavigation": false,
"list-all-audits": false,
"list-locales": false,
"list-trace-categories": false,
Expand Down Expand Up @@ -322,8 +308,6 @@ Object {
"chromeFlags": "",
"chromeIgnoreDefaultFlags": false,
"hostname": "127.0.0.1",
"legacy-navigation": false,
"legacyNavigation": false,
"list-all-audits": false,
"list-locales": false,
"list-trace-categories": false,
Expand Down Expand Up @@ -375,8 +359,6 @@ Object {
"cli-flags-path": "__REPLACED__/cli/test/fixtures/cli-flags-path-inline-budgets.json",
"cliFlagsPath": "__REPLACED__/cli/test/fixtures/cli-flags-path-inline-budgets.json",
"hostname": "127.0.0.1",
"legacy-navigation": false,
"legacyNavigation": false,
"list-all-audits": false,
"list-locales": false,
"list-trace-categories": false,
Expand Down Expand Up @@ -419,8 +401,6 @@ Object {
"X-Men": "wolverine",
},
"hostname": "127.0.0.1",
"legacy-navigation": false,
"legacyNavigation": false,
"list-all-audits": false,
"list-locales": false,
"list-trace-categories": false,
Expand Down
1 change: 0 additions & 1 deletion cli/test/cli/bin-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@ beforeEach(async () => {
view: false,
verbose: false,
quiet: false,
legacyNavigation: false,
port: 0,
hostname: '',
// Command modes
Expand Down
17 changes: 1 addition & 16 deletions clients/devtools/devtools-entry.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,11 @@ import {Buffer} from 'buffer';

import log from 'lighthouse-logger';

import lighthouse, {legacyNavigation, navigation, startTimespan, snapshot} from '../../core/index.js';
import {RawConnection} from '../../core/legacy/gather/connections/raw.js';
import lighthouse, {navigation, startTimespan, snapshot} from '../../core/index.js';
import {lookupLocale} from '../../core/lib/i18n/i18n.js';
import {registerLocaleData, getCanonicalLocales} from '../../shared/localization/format.js';
import * as constants from '../../core/config/constants.js';

/** @typedef {import('../../core/legacy/gather/connections/connection.js')} Connection */

// Rollup seems to overlook some references to `Buffer`, so it must be made explicit.
// (`parseSourceMapFromDataUrl` breaks without this)
/** @type {BufferConstructor} */
Expand Down Expand Up @@ -56,14 +53,6 @@ function createConfig(categoryIDs, device) {
};
}

/**
* @param {import('../../core/legacy/gather/connections/raw.js').Port} port
* @return {RawConnection}
*/
function setUpWorkerConnection(port) {
return new RawConnection(port);
}

/** @param {(status: [string, string, string]) => void} listenCallback */
function listenForStatus(listenCallback) {
log.events.addListener('status', listenCallback);
Expand Down Expand Up @@ -111,10 +100,6 @@ if (typeof self !== 'undefined') {
// TODO: refactor and delete `global.isDevtools`.
global.isDevtools = true;

// @ts-expect-error
self.setUpWorkerConnection = setUpWorkerConnection;
// @ts-expect-error
self.runLighthouse = legacyNavigation;
// @ts-expect-error
self.runLighthouseNavigation = runLighthouseNavigation;
// @ts-expect-error
Expand Down
20 changes: 7 additions & 13 deletions clients/lightrider/lightrider-entry.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,15 +26,13 @@ const LR_PRESETS = {
desktop: desktopConfig,
};

/** @typedef {import('../../core/legacy/gather/connections/connection.js').Connection} Connection */

// Rollup seems to overlook some references to `Buffer`, so it must be made explicit.
// (`parseSourceMapFromDataUrl` breaks without this)
/** @type {BufferConstructor} */
globalThis.Buffer = Buffer;

/**
* @param {Connection} connection
* @param {any} connection
* @return {Promise<LH.Puppeteer.Page>}
*/
async function getPageFromConnection(connection) {
Expand All @@ -43,7 +41,6 @@ async function getPageFromConnection(connection) {
await connection.sendCommand('Target.getTargetInfo', undefined);
const {frameTree} = await connection.sendCommand('Page.getFrameTree', undefined);

// @ts-expect-error Hack to access the WRS/SRS transport layer.
const channel = connection.channel_ || connection.rootSessionConnection_;
const transport = channel.root_.transport_;

Expand Down Expand Up @@ -72,10 +69,10 @@ async function getPageFromConnection(connection) {
* Run lighthouse for connection and provide similar results as in CLI.
*
* If configOverride is provided, lrDevice and categoryIDs are ignored.
* @param {Connection} connection
* @param {any} connection
* @param {string} url
* @param {LH.Flags} flags Lighthouse flags
* @param {{lrDevice?: 'desktop'|'mobile', categoryIDs?: Array<string>, logAssets: boolean, configOverride?: LH.Config, useFraggleRock?: boolean}} lrOpts Options coming from Lightrider
* @param {{lrDevice?: 'desktop'|'mobile', categoryIDs?: Array<string>, logAssets: boolean, configOverride?: LH.Config}} lrOpts Options coming from Lightrider
* @return {Promise<string>}
*/
async function runLighthouseInLR(connection, url, flags, lrOpts) {
Expand All @@ -101,13 +98,8 @@ async function runLighthouseInLR(connection, url, flags, lrOpts) {
}

try {
let runnerResult;
if (lrOpts.useFraggleRock) {
const page = await getPageFromConnection(connection);
runnerResult = await lighthouse(url, flags, config, page);
} else {
runnerResult = await api.legacyNavigation(url, flags, config, connection);
}
const page = await runLighthouseInLR.getPageFromConnection(connection);
const runnerResult = await lighthouse(url, flags, config, page);

if (!runnerResult) throw new Error('Lighthouse finished without a runnerResult');

Expand Down Expand Up @@ -164,6 +156,8 @@ if (typeof window !== 'undefined') {

const {computeBenchmarkIndex} = pageFunctions;

runLighthouseInLR.getPageFromConnection = getPageFromConnection;

export {
runLighthouseInLR,
api,
Expand Down
40 changes: 19 additions & 21 deletions clients/test/lightrider/lightrider-entry-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,23 +11,33 @@ import jestMock from 'jest-mock';
import {runLighthouseInLR} from '../../lightrider/lightrider-entry.js';
import {Runner} from '../../../core/runner.js';
import {LighthouseError} from '../../../core/lib/lh-error.js';
import {createMockPage} from '../../../core/test/gather/mock-driver.js';

describe('lightrider-entry', () => {
let mockConnection;

beforeEach(() => {
mockConnection = {
connect: jestMock.fn(),
disconnect: jestMock.fn(),
sendCommand: jestMock.fn(),
on: jestMock.fn(),
};

runLighthouseInLR.getPageFromConnection = async (connection) => {
await connection.connect();
return createMockPage();
};
});

describe('#runLighthouseInLR', () => {
it('returns a runtimeError LHR when lighthouse throws a runtimeError', async () => {
const connectionError = new LighthouseError(
LighthouseError.errors.FAILED_DOCUMENT_REQUEST,
{errorDetails: 'Bad connection req'}
);
assert.strictEqual(connectionError.lhrRuntimeError, true);
const mockConnection = {
async connect() {
throw connectionError;
},
async disconnect() {},
async sendCommand() {},
on() {},
};
mockConnection.connect.mockRejectedValue(connectionError);
const url = 'https://example.com';
const output = 'json';

Expand All @@ -43,14 +53,7 @@ describe('lightrider-entry', () => {
const errorMsg = 'Errors are the best!';
const connectionError = new Error(errorMsg);
assert.strictEqual(connectionError.lhrRuntimeError, undefined);
const mockConnection = {
async connect() {
throw connectionError;
},
async disconnect() {},
async sendCommand() {},
on() {},
};
mockConnection.connect.mockRejectedValue(connectionError);
const url = 'https://example.com';
const output = 'json';

Expand All @@ -63,7 +66,6 @@ describe('lightrider-entry', () => {
it('specifies the channel as lr', async () => {
const runStub = jestMock.spyOn(Runner, 'gather');

const mockConnection = {};
const url = 'https://example.com';

await runLighthouseInLR(mockConnection, url, {}, {});
Expand All @@ -76,7 +78,6 @@ describe('lightrider-entry', () => {
it('uses the desktop config preset when device is desktop', async () => {
const runStub = jestMock.spyOn(Runner, 'gather');

const mockConnection = {};
const url = 'https://example.com';

const lrDevice = 'desktop';
Expand All @@ -90,7 +91,6 @@ describe('lightrider-entry', () => {
it('uses the mobile config preset when device is mobile', async () => {
const runStub = jestMock.spyOn(Runner, 'gather');

const mockConnection = {};
const url = 'https://example.com';

const lrDevice = 'mobile';
Expand All @@ -104,7 +104,6 @@ describe('lightrider-entry', () => {
it('overrides the default config when one is provided', async () => {
const runStub = jestMock.spyOn(Runner, 'gather');

const mockConnection = {};
const url = 'https://example.com';

const configOverride = {
Expand Down Expand Up @@ -138,7 +137,6 @@ describe('lightrider-entry', () => {
},
}));

const mockConnection = {};
const url = 'https://example.com';
const lrFlags = {
logAssets: true,
Expand Down
13 changes: 3 additions & 10 deletions core/config/config-helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import {getModuleDirectory} from '../../esm-utils.js';

const require = createRequire(import.meta.url);

/** @typedef {typeof import('../gather/gatherers/gatherer.js').Gatherer} GathererConstructor */
/** @typedef {typeof import('../gather/base-gatherer.js').default} GathererConstructor */
/** @typedef {typeof import('../audits/audit.js')['Audit']} Audit */
/** @typedef {InstanceType<GathererConstructor>} Gatherer */

Expand Down Expand Up @@ -160,7 +160,7 @@ function mergeConfigFragmentArrayByKey(baseArray, extensionArray, keyFn) {
* - {instance: myGathererInstance}
*
* @param {LH.Config.GathererJson} gatherer
* @return {{instance?: Gatherer, implementation?: GathererConstructor, path?: string}} passes
* @return {{instance?: Gatherer, implementation?: GathererConstructor, path?: string}}
*/
function expandGathererShorthand(gatherer) {
if (typeof gatherer === 'string') {
Expand All @@ -178,7 +178,7 @@ function expandGathererShorthand(gatherer) {
} else if (typeof gatherer === 'function') {
// just GathererConstructor
return {implementation: gatherer};
} else if (gatherer && typeof gatherer.beforePass === 'function') {
} else if (gatherer && typeof gatherer.getArtifact === 'function') {
// just GathererInstance
return {instance: gatherer};
} else {
Expand Down Expand Up @@ -595,13 +595,6 @@ function deepCloneConfigJson(json) {

// Copy arrays that could contain non-serializable properties to allow for programmatic
// injection of audit and gatherer implementations.
if (Array.isArray(cloned.passes) && Array.isArray(json.passes)) {
for (let i = 0; i < cloned.passes.length; i++) {
const pass = cloned.passes[i];
pass.gatherers = (json.passes[i].gatherers || []).map(gatherer => shallowClone(gatherer));
}
}

if (Array.isArray(json.audits)) {
cloned.audits = json.audits.map(audit => shallowClone(audit));
}
Expand Down
Loading

0 comments on commit 1ef1faf

Please sign in to comment.