-
Notifications
You must be signed in to change notification settings - Fork 9.6k
core(byte-efficiency): mark n/a if no network records in timespan #12839
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
Conversation
patrickhulce
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice work!
|
|
||
| // TODO(FR-COMPAT): Reduce this number by handling the error, making N/A, or removing timespan support. | ||
| expect(erroredAudits.length).toMatchInlineSnapshot(`14`); | ||
| expect(erroredAudits.length).toMatchInlineSnapshot(`1`); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice! what's the final one, #12827?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
long-tasks. It's not a byte efficiency audit but requests the page dependency graph, which throws an error when there are no network records. It requires some additional investigation before we decide to just mark as N/A.
lighthouse-core/audits/byte-efficiency/byte-efficiency-audit.js
Outdated
Show resolved
Hide resolved
Co-authored-by: Patrick Hulce <[email protected]>
This resolves most of the errors we were seeing for a timespan run after page load.
The solution is to mark all BE audits N/A if there were no network records in timespan mode. Conceptually, this makes some sense since ms savings are calculated from throughput in timespan mode.
Possible improvement:
Not all BE audits require network records. It was
LoadSimulator, used to calculate throughput savings for all BE audits, which requires network records. Instead of always returning N/A, we could get throughput from the settings when using DT or simulated throttling, but not provided throttling. Provided throttling would need to be N/A without network records in timespan mode.