From 7bef940c08d27d20f01b24d5fd19e3cd7abb340f Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Mon, 10 Jun 2024 15:24:34 -0700 Subject: [PATCH 1/2] tests(lantern): use TraceEngine directly in test fixtures --- .eslintrc.cjs | 2 +- core/computed/metrics/lantern-metric.js | 2 +- core/computed/page-dependency-graph.js | 5 +-- core/computed/trace-engine-result.js | 25 +++---------- .../lantern/trace-engine-computation-data.js | 36 ++++++++++--------- .../lib/lantern/metrics/metric-test-utils.js | 28 +++++++++------ types/artifacts.d.ts | 5 ++- 7 files changed, 48 insertions(+), 55 deletions(-) diff --git a/.eslintrc.cjs b/.eslintrc.cjs index ae8e16ac4841..e565301f019e 100644 --- a/.eslintrc.cjs +++ b/.eslintrc.cjs @@ -76,7 +76,7 @@ module.exports = { vars: 'all', args: 'after-used', argsIgnorePattern: '(^reject$|^_+$)', - varsIgnorePattern: '(^_$|^LH$|^Lantern$)', + varsIgnorePattern: '(^_$|^LH$|^Lantern$|^TraceEngine$)', }], 'no-cond-assign': 2, 'space-infix-ops': 2, diff --git a/core/computed/metrics/lantern-metric.js b/core/computed/metrics/lantern-metric.js index 869592d97323..f71e1819d1f3 100644 --- a/core/computed/metrics/lantern-metric.js +++ b/core/computed/metrics/lantern-metric.js @@ -39,7 +39,7 @@ async function getComputationDataParamsFromTrace(data, context) { const graph = await PageDependencyGraph.request({...data, fromTrace: true}, context); const traceEngineResult = await TraceEngineResult.request(data, context); - const processedNavigation = createProcessedNavigation(traceEngineResult); + const processedNavigation = createProcessedNavigation(traceEngineResult.data); const simulator = data.simulator || (await LoadSimulator.request(data, context)); return {simulator, graph, processedNavigation}; diff --git a/core/computed/page-dependency-graph.js b/core/computed/page-dependency-graph.js index 0bb46c373cbf..f6a38fda02da 100644 --- a/core/computed/page-dependency-graph.js +++ b/core/computed/page-dependency-graph.js @@ -29,9 +29,10 @@ class PageDependencyGraph { if (data.fromTrace) { const traceEngineResult = await TraceEngineResult.request({trace}, context); - const requests = TraceEngineComputationData.createNetworkRequests(trace, traceEngineResult); + const traceEngineData = traceEngineResult.data; + const requests = TraceEngineComputationData.createNetworkRequests(trace, traceEngineData); const graph = - TraceEngineComputationData.createGraph(requests, trace, traceEngineResult, URL); + TraceEngineComputationData.createGraph(requests, trace, traceEngineData, URL); return graph; } diff --git a/core/computed/trace-engine-result.js b/core/computed/trace-engine-result.js index 856db6b49bb7..b9f26e530e34 100644 --- a/core/computed/trace-engine-result.js +++ b/core/computed/trace-engine-result.js @@ -10,20 +10,6 @@ import {CumulativeLayoutShift} from './metrics/cumulative-layout-shift.js'; import {ProcessedTrace} from './processed-trace.js'; import * as LH from '../../types/lh.js'; -/** @typedef {typeof ENABLED_HANDLERS} EnabledHandlers */ - -const ENABLED_HANDLERS = { - AuctionWorklets: TraceEngine.TraceHandlers.AuctionWorklets, - Initiators: TraceEngine.TraceHandlers.Initiators, - LayoutShifts: TraceEngine.TraceHandlers.LayoutShifts, - NetworkRequests: TraceEngine.TraceHandlers.NetworkRequests, - Renderer: TraceEngine.TraceHandlers.Renderer, - Samples: TraceEngine.TraceHandlers.Samples, - Screenshots: TraceEngine.TraceHandlers.Screenshots, - PageLoadMetrics: TraceEngine.TraceHandlers.PageLoadMetrics, - Workers: TraceEngine.TraceHandlers.Workers, -}; - /** * @fileoverview Processes trace with the shared trace engine. */ @@ -33,15 +19,14 @@ class TraceEngineResult { * @return {Promise} */ static async runTraceEngine(traceEvents) { - const engine = new TraceEngine.TraceProcessor(ENABLED_HANDLERS); + const processor = TraceEngine.TraceProcessor.createWithAllHandlers(); // eslint-disable-next-line max-len - await engine.parse(/** @type {import('@paulirish/trace_engine').Types.TraceEvents.TraceEventData[]} */ ( + await processor.parse(/** @type {import('@paulirish/trace_engine').Types.TraceEvents.TraceEventData[]} */ ( traceEvents )); - // TODO: use TraceEngine.TraceProcessor.createWithAllHandlers above. - if (!engine.traceParsedData) throw new Error('No data'); - if (!engine.insights) throw new Error('No insights'); - return {data: engine.traceParsedData, insights: engine.insights}; + if (!processor.traceParsedData) throw new Error('No data'); + if (!processor.insights) throw new Error('No insights'); + return {data: processor.traceParsedData, insights: processor.insights}; } /** diff --git a/core/lib/lantern/trace-engine-computation-data.js b/core/lib/lantern/trace-engine-computation-data.js index 8d440d6204a3..1f0b44164b8f 100644 --- a/core/lib/lantern/trace-engine-computation-data.js +++ b/core/lib/lantern/trace-engine-computation-data.js @@ -4,6 +4,8 @@ * SPDX-License-Identifier: Apache-2.0 */ +import * as TraceEngine from '@paulirish/trace_engine'; + import * as Lantern from './types/lantern.js'; import {PageDependencyGraph} from './page-dependency-graph.js'; import {RESOURCE_TYPES} from '../network-request.js'; @@ -12,13 +14,13 @@ import {RESOURCE_TYPES} from '../network-request.js'; /** @typedef {import('@paulirish/trace_engine/models/trace/handlers/PageLoadMetricsHandler.js').MetricScore} MetricScore */ /** - * @param {LH.Artifacts.TraceEngineResult} traceEngineResult + * @param {TraceEngine.Handlers.Types.TraceParseData} traceEngineData * @return {Lantern.Simulation.ProcessedNavigation} */ -function createProcessedNavigation(traceEngineResult) { - const Meta = traceEngineResult.data.Meta; +function createProcessedNavigation(traceEngineData) { + const Meta = traceEngineData.Meta; const frameId = Meta.mainFrameId; - const scoresByNav = traceEngineResult.data.PageLoadMetrics.metricScoresByFrameId.get(frameId); + const scoresByNav = traceEngineData.PageLoadMetrics.metricScoresByFrameId.get(frameId); if (!scoresByNav) { throw new Error('missing metric scores for main frame'); } @@ -97,12 +99,12 @@ function findWorkerThreads(trace) { } /** - * @param {LH.Artifacts.TraceEngineResult} traceEngineResult + * @param {TraceEngine.Handlers.Types.TraceParseData} traceEngineData * @param {Map} workerThreads * @param {import('@paulirish/trace_engine/models/trace/types/TraceEvents.js').SyntheticNetworkRequest} request * @return {Lantern.NetworkRequest=} */ -function createLanternRequest(traceEngineResult, workerThreads, request) { +function createLanternRequest(traceEngineData, workerThreads, request) { if (request.args.data.connectionId === undefined || request.args.data.connectionReused === undefined) { throw new Error('Trace is too old'); @@ -134,7 +136,7 @@ function createLanternRequest(traceEngineResult, workerThreads, request) { // TraceEngine collects worker thread ids in a different manner than `workerThreads` does. // AFAIK these should be equivalent, but in case they are not let's also check this for now. - if (traceEngineResult.data.Workers.workerIdByThread.has(request.tid)) { + if (traceEngineData.Workers.workerIdByThread.has(request.tid)) { fromWorker = true; } @@ -295,16 +297,16 @@ function linkInitiators(lanternRequests) { /** * @param {LH.Trace} trace - * @param {LH.Artifacts.TraceEngineResult} traceEngineResult + * @param {TraceEngine.Handlers.Types.TraceParseData} traceEngineData * @return {Lantern.NetworkRequest[]} */ -function createNetworkRequests(trace, traceEngineResult) { +function createNetworkRequests(trace, traceEngineData) { const workerThreads = findWorkerThreads(trace); /** @type {Lantern.NetworkRequest[]} */ const lanternRequests = []; - for (const request of traceEngineResult.data.NetworkRequests.byTime) { - const lanternRequest = createLanternRequest(traceEngineResult, workerThreads, request); + for (const request of traceEngineData.NetworkRequests.byTime) { + const lanternRequest = createLanternRequest(traceEngineData, workerThreads, request); if (lanternRequest) { lanternRequests.push(lanternRequest); } @@ -389,11 +391,11 @@ function createNetworkRequests(trace, traceEngineResult) { /** * @param {LH.Trace} trace - * @param {LH.Artifacts.TraceEngineResult} traceEngineResult + * @param {TraceEngine.Handlers.Types.TraceParseData} traceEngineData * @return {LH.TraceEvent[]} */ -function collectMainThreadEvents(trace, traceEngineResult) { - const Meta = traceEngineResult.data.Meta; +function collectMainThreadEvents(trace, traceEngineData) { + const Meta = traceEngineData.Meta; const mainFramePids = Meta.mainFrameNavigations.length ? new Set(Meta.mainFrameNavigations.map(nav => nav.pid)) : Meta.topLevelRendererIds; @@ -430,11 +432,11 @@ function collectMainThreadEvents(trace, traceEngineResult) { /** * @param {Lantern.NetworkRequest[]} requests * @param {LH.Trace} trace - * @param {LH.Artifacts.TraceEngineResult} traceEngineResult + * @param {TraceEngine.Handlers.Types.TraceParseData} traceEngineData * @param {LH.Artifacts.URL=} URL */ -function createGraph(requests, trace, traceEngineResult, URL) { - const mainThreadEvents = collectMainThreadEvents(trace, traceEngineResult); +function createGraph(requests, trace, traceEngineData, URL) { + const mainThreadEvents = collectMainThreadEvents(trace, traceEngineData); // URL defines the initial request that the Lantern graph starts at (the root node) and the // main document request. These are equal if there are no redirects. diff --git a/core/test/lib/lantern/metrics/metric-test-utils.js b/core/test/lib/lantern/metrics/metric-test-utils.js index a6e850b2e5e2..0f1e56a2e4b6 100644 --- a/core/test/lib/lantern/metrics/metric-test-utils.js +++ b/core/test/lib/lantern/metrics/metric-test-utils.js @@ -4,15 +4,21 @@ * SPDX-License-Identifier: Apache-2.0 */ -import {TraceEngineResult} from '../../../../computed/trace-engine-result.js'; +import * as TraceEngine from '@paulirish/trace_engine'; + import {NetworkAnalyzer} from '../../../../lib/lantern/simulator/network-analyzer.js'; import {Simulator} from '../../../../lib/lantern/simulator/simulator.js'; import * as TraceEngineComputationData from '../../../../lib/lantern/trace-engine-computation-data.js'; -import * as Lantern from '../../../../lib/lantern/types/lantern.js'; - -/** @typedef {Lantern.NetworkRequest} NetworkRequest */ -// TODO(15841): remove usage of Lighthouse code to create test data +/** + * @param {TraceEngine.Types.TraceEvents.TraceEventData[]} traceEvents + */ +async function runTraceEngine(traceEvents) { + const processor = TraceEngine.Processor.TraceProcessor.createWithAllHandlers(); + await processor.parse(traceEvents); + if (!processor.traceParsedData) throw new Error('No data'); + return processor.traceParsedData; +} /** * @param {{trace: LH.Trace, settings?: LH.Config.Settings, URL?: LH.Artifacts.URL}} opts @@ -20,16 +26,16 @@ import * as Lantern from '../../../../lib/lantern/types/lantern.js'; async function getComputationDataFromFixture({trace, settings, URL}) { settings = settings ?? /** @type {LH.Config.Settings} */({}); if (!settings.throttlingMethod) settings.throttlingMethod = 'simulate'; - - const context = {settings, computedCache: new Map()}; - const traceEngineResult = await TraceEngineResult.request({trace}, context); - const requests = TraceEngineComputationData.createNetworkRequests(trace, traceEngineResult); + const traceEngineData = await runTraceEngine( + /** @type {TraceEngine.Types.TraceEvents.TraceEventData[]} */ (trace.traceEvents) + ); + const requests = TraceEngineComputationData.createNetworkRequests(trace, traceEngineData); const networkAnalysis = NetworkAnalyzer.analyze(requests); return { simulator: Simulator.createSimulator({...settings, networkAnalysis}), - graph: TraceEngineComputationData.createGraph(requests, trace, traceEngineResult, URL), - processedNavigation: TraceEngineComputationData.createProcessedNavigation(traceEngineResult), + graph: TraceEngineComputationData.createGraph(requests, trace, traceEngineData, URL), + processedNavigation: TraceEngineComputationData.createProcessedNavigation(traceEngineData), }; } diff --git a/types/artifacts.d.ts b/types/artifacts.d.ts index 798f74ba6b72..6580caeb3c92 100644 --- a/types/artifacts.d.ts +++ b/types/artifacts.d.ts @@ -16,7 +16,6 @@ import speedline from 'speedline-core'; import * as CDTSourceMap from '../core/lib/cdt/generated/SourceMap.js'; import {ArbitraryEqualityMap} from '../core/lib/arbitrary-equality-map.js'; import type { TaskNode as _TaskNode } from '../core/lib/tracehouse/main-thread-tasks.js'; -import type {EnabledHandlers} from '../core/computed/trace-engine-result.js'; import AuditDetails from './lhr/audit-details.js' import Config from './config.js'; import Gatherer from './gatherer.js'; @@ -511,8 +510,8 @@ declare module Artifacts { } interface TraceEngineResult { - data: TraceEngine.Handlers.Types.EnabledHandlerDataWithMeta; - insights: TraceEngine.Insights.Types.TraceInsightData; + data: TraceEngine.Handlers.Types.TraceParseData; + insights: TraceEngine.Insights.Types.TraceInsightData; } interface TraceEngineRootCauses { From be104a439063b0fb7bd56e264c08d7163853e7d5 Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Mon, 10 Jun 2024 15:30:59 -0700 Subject: [PATCH 2/2] fix ts --- core/gather/gatherers/root-causes.js | 1 - 1 file changed, 1 deletion(-) diff --git a/core/gather/gatherers/root-causes.js b/core/gather/gatherers/root-causes.js index d06f075430c1..5d6a310368ff 100644 --- a/core/gather/gatherers/root-causes.js +++ b/core/gather/gatherers/root-causes.js @@ -114,7 +114,6 @@ class RootCauses extends BaseGatherer { const rootCausesEngine = new TraceEngine.RootCauses(protocolInterface); const layoutShiftEvents = traceParsedData.LayoutShifts.clusters.flatMap(c => c.events); for (const event of layoutShiftEvents) { - // @ts-expect-error The data we do get in the trace processor is still enough here const r = await rootCausesEngine.layoutShifts.rootCausesForEvent(traceParsedData, event); if (!r) continue;