core: cap byte-efficiency-audit scores to a max of 1 #14921
Conversation
| // `wastedMs` may be negative, if making the opportunity change could be detrimental. | ||
| // This is useful information in the LHR and should be preserved. |
There was a problem hiding this comment.
added this comment because my first thought was "why isn't wastedMs being passed through a Math.max(0, wastedMs)?", but it's actually very useful information to know that the opportunity would be negative, so I wanted to make that explicit to future readers of the code. Happy to move/bikeshed.
| assert.ok(negativeResult.numericValue < 0); | ||
| assert.equal(negativeResult.numericValue, negativeResult.details.overallSavingsMs); |
There was a problem hiding this comment.
explicit test asserting that we want the negative result in the LHR even though it won't appear in the HTML report once it's tucked into the passing audits section (displayValue is used then)
| @@ -398,4 +409,19 @@ describe('Byte efficiency base audit', () => { | |||
| const result = await MockAudit.audit(artifacts, {settings, computedCache}); | |||
| expect(result.details.overallSavingsMs).toBeCloseTo(575, 1); | |||
| }); | |||
|
|
|||
| describe('#scoreForWastedMs', () => { | |||
There was a problem hiding this comment.
scoreForWastedMs is imported and used independently of audits extending ByteEfficiencyAudit in at least prioritize-lcp-image, so it felt worth it to test separately as well.
(there's one wastedMs assertion for every segment and every control point in the piecewise scoring function)
Sometimes enacting an opportunity could actually regress performance and its audit could measure a negative
wastedMs/overallSavingsMs. It turns outbyte-efficiency-auditdoesn't have any guard against this case and gives those results ascoregreater than 1, which triggers an out-of-range score error, and so what should be a passing result turns into an audit error.This PR caps the
byte-efficiency-auditscore so negative savings still max out at a score of 1 and returns in a valid audit result.For context, this doesn't happen often, e.g. ~0.09% for
prioritize-lcp-imagein the last HTTP Archive run and similar rates for the otherbyte-efficiency-audits, but obviously important to get right.