Skip to content

Commit

Permalink
pr
Browse files Browse the repository at this point in the history
  • Loading branch information
connorjclark committed Jun 3, 2024
1 parent 34316d5 commit bf7b5d2
Show file tree
Hide file tree
Showing 4 changed files with 9 additions and 28 deletions.
3 changes: 3 additions & 0 deletions .github/workflows/unit.yml
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,9 @@ jobs:
- name: yarn unit
if: ${{ matrix.node != env.LATEST_NODE }}
run: yarn unit:ci
- name: yarn unit-lantern-trace
if: ${{ matrix.node != env.LATEST_NODE }}
run: yarn unit-lantern-trace:ci

# Only gather coverage on latest node, where c8 is the most accurate.
- name: yarn unit:cicoverage
Expand Down
5 changes: 3 additions & 2 deletions core/lib/lantern/page-dependency-graph.js
Original file line number Diff line number Diff line change
Expand Up @@ -664,10 +664,10 @@ class PageDependencyGraph {
}

const timing = request.args.data.timing ? {
...request.args.data.timing,
// These two timings are not included in the trace.
workerFetchStart: -1,
workerRespondWithSettled: -1,
...request.args.data.timing,
} : undefined;

const networkRequestTime = timing ?
Expand All @@ -691,7 +691,8 @@ class PageDependencyGraph {
// There are some minor differences in the fields, accounted for here.
// Most importantly, there seems to be fewer frames in the trace than the equivalent
// events over the CDP. This results in less accuracy in determining the initiator request,
// which means less edges in the graph, which mean worse results. Should fix.
// which means less edges in the graph, which mean worse results.
// TODO: Should fix in Chromium.
/** @type {Lantern.NetworkRequest['initiator']} */
const initiator = request.args.data.initiator ?? {type: 'other'};
if (request.args.data.stackTrace) {
Expand Down
24 changes: 0 additions & 24 deletions core/test/computed/metrics/largest-contentful-paint-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@ import {getURLArtifactFromDevtoolsLog, readJson} from '../../test-utils.js';

const trace = readJson('../../fixtures/artifacts/paul/trace.json', import.meta);
const devtoolsLog = readJson('../../fixtures/artifacts/paul/devtoolslog.json', import.meta);
const invalidTrace = readJson('../../fixtures/traces/progressive-app-m60.json', import.meta);
const invalidDevtoolsLog = readJson('../../fixtures/traces/progressive-app-m60.devtools.log.json', import.meta);

describe('Metrics: LCP', () => {
const gatherContext = {gatherMode: 'navigation'};
Expand Down Expand Up @@ -49,26 +47,4 @@ Object {
}
`);
});

['provided', 'simulate'].forEach(throttlingMethod => {
it(`should fail to compute a value for old trace (${throttlingMethod})`, async () => {
// TODO(15841): fails... but instead w/ "No frames found in trace data" (not NO_LCP).
// ... handle error when calling code uses lantern? or update expected error? idk
if (throttlingMethod === 'simulate' && process.env.INTERNAL_LANTERN_USE_TRACE !== undefined) {
return;
}

const settings = {throttlingMethod};
const context = {settings, computedCache: new Map()};
const URL = getURLArtifactFromDevtoolsLog(invalidDevtoolsLog);
const resultPromise = LargestContentfulPaint.request(
{gatherContext, trace: invalidTrace, devtoolsLog: invalidDevtoolsLog, settings, URL},
context
);
await expect(resultPromise).rejects.toMatchObject({
code: 'NO_LCP',
friendlyMessage: expect.toBeDisplayString(/The page did not display content.*NO_LCP/),
});
});
});
});
5 changes: 3 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -57,12 +57,13 @@
"unit-viewer": "yarn mocha --testMatch viewer/**/*-test.js",
"unit-flow": "bash flow-report/test/run-flow-report-tests.sh",
"unit": "yarn unit-flow && yarn mocha && yarn unit-lantern-trace",
"unit:ci": "NODE_OPTIONS=--max-old-space-size=8192 npm run unit && NODE_OPTIONS=--max-old-space-size=8192 npm run unit-lantern-trace",
"unit:ci": "NODE_OPTIONS=--max-old-space-size=8192 npm run unit",
"unit-lantern-trace:ci": "NODE_OPTIONS=--max-old-space-size=8192 npm run unit-lantern-trace",
"core-unit": "yarn unit-core",
"cli-unit": "yarn unit-cli",
"viewer-unit": "yarn unit-viewer",
"watch": "yarn unit-core --watch",
"unit:cicoverage": "yarn c8 --all yarn unit:ci",
"unit:cicoverage": "yarn c8 --all yarn unit:ci && yarn c8 --all yarn unit-lantern-trace:ci",
"coverage": "yarn unit:cicoverage && c8 report --reporter html",
"coverage:smoke": "yarn c8 yarn smoke -j=1 && c8 report --reporter html",
"devtools": "bash core/scripts/roll-to-devtools.sh",
Expand Down

0 comments on commit bf7b5d2

Please sign in to comment.