Closed connorjclark closed 4 weeks ago
Had to exclude many more Lighthouse unit tests from running with the Lantern trace option. It'll be a mix of more trace updates and hunting down differences in the graph construction. I think this PR should still land because the lantern unit tests are all in good order, and otherwise it'll just get too large to review.
i see debugNormalizeRequests ... can you update the PR description to note how you're developing against this impl? like.. fresh traces or fixture ones.. also are you invoking just this with a trace or always as part of the LH run?
It's either. When a test fails using the trace backend, I enabled this debug method and copied the network requests when CDP was used, then when trace is used, and diff'd them. This method helped remove non-logical differences from the comparison (order of properties, slight rounding discrepancies, removing object cycles, etc).
I added a comment.
so far the test skips from what I've checked seem to be
1) lacking feature parity in createTestTrace 2) bug in redirect stuff in createGraphFromTrace 3) could be other real bugs
This PR does not change the default behavior of Lighthouse.
The most interesting part of this PR is
createGraphFromTrace
- given a trace (and TraceEngine result), produces the network records from the trace. Also handles making the CPU nodes too and returns the graph.In Lighthouse, CDP is still used for the network portion of the graph, except in the presence of
INTERNAL_LANTERN_USE_TRACE=1
env variable. This includes the lantern smoke tests. The CI scripts have been modified minimally to run both variants. When we switch to using the trace totally, this internal control should be removed.Note that the unit tests in
core/test/lib/lantern
only use the trace to build the graph -INTERNAL_LANTERN_USE_TRACE
is only for Lighthouse tests.The only change in these Lantern unit test results is
core/test/lib/lantern/metrics/lantern-largest-contentful-paint-test.js
- a minor difference in the results due to missing requests in the trace for pre-flight CORs, which doesn't actually impact the simulation except for having one less sample when estimating the RTT/throughput. Adding these requests to the trace is non-trivial and this change in results does not yet warrant further work.ref https://github.com/GoogleChrome/lighthouse/issues/15841