Skip to content

Commit

Permalink
core: round metric savings to remove false precision (#15721)
Browse files Browse the repository at this point in the history
  • Loading branch information
adamraine authored Jan 4, 2024
1 parent dd8feb7 commit 4e4a835
Show file tree
Hide file tree
Showing 4 changed files with 73 additions and 26 deletions.
43 changes: 41 additions & 2 deletions core/audits/audit.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,15 @@ import {Util} from '../../shared/util.js';

const DEFAULT_PASS = 'defaultPass';

/** @type {LH.Audit.MetricSavings} */
const METRIC_SAVINGS_PRECISION = {
FCP: 50,
LCP: 50,
INP: 50,
TBT: 50,
CLS: 0.001,
};

/**
* @typedef TableOptions
* @property {number=} wastedMs
Expand Down Expand Up @@ -339,6 +348,33 @@ class Audit {
return score;
}

/**
* @param {LH.Audit.MetricSavings|undefined} metricSavings
* @return {LH.Audit.MetricSavings|undefined}
*/
static _quantizeMetricSavings(metricSavings) {
if (!metricSavings) return;

/** @type {LH.Audit.MetricSavings} */
const normalizedMetricSavings = {...metricSavings};

for (const key of /** @type {Array<LH.Result.MetricAcronym>} */ (Object.keys(metricSavings))) {
let value = metricSavings[key];
if (value === undefined) continue;

value = Math.max(value, 0);

const precision = METRIC_SAVINGS_PRECISION[key];
if (precision !== undefined) {
value = Math.round(value / precision) * precision;
}

normalizedMetricSavings[key] = value;
}

return normalizedMetricSavings;
}

/**
* @param {typeof Audit} audit
* @param {string | LH.IcuMessage} errorMessage
Expand Down Expand Up @@ -378,10 +414,13 @@ class Audit {
scoreDisplayMode = product.scoreDisplayMode;
}

const metricSavings = Audit._quantizeMetricSavings(product.metricSavings);
const hasSomeSavings = Object.values(metricSavings || {}).some(v => v);

if (scoreDisplayMode === Audit.SCORING_MODES.METRIC_SAVINGS) {
if (score && score >= Util.PASS_THRESHOLD) {
score = 1;
} else if (Object.values(product.metricSavings || {}).some(v => v)) {
} else if (hasSomeSavings) {
score = 0;
} else {
score = 0.5;
Expand Down Expand Up @@ -419,7 +458,7 @@ class Audit {
errorStack: product.errorStack,
warnings: product.warnings,
scoringOptions: product.scoringOptions,
metricSavings: product.metricSavings,
metricSavings,

details: product.details,
guidanceLevel: audit.meta.guidanceLevel,
Expand Down
8 changes: 8 additions & 0 deletions core/test/audits/audit-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,14 @@ describe('Audit', () => {
assert.strictEqual(auditResult.score, 0.3);
});

it('quantizes metric savings', () => {
const auditResult = Audit.generateAuditResult(PassOrFailAudit, {
score: 0,
metricSavings: {LCP: 0.1, FCP: 149, CLS: 0.0015, TBT: -100},
});
assert.deepStrictEqual(auditResult.metricSavings, {LCP: 0, FCP: 150, CLS: 0.002, TBT: 0});
});

it('chooses the title if score is passing', () => {
const auditResult = Audit.generateAuditResult(PassOrFailAudit, {score: 1});
assert.strictEqual(auditResult.score, 1);
Expand Down
26 changes: 13 additions & 13 deletions core/test/fixtures/user-flows/reports/sample-flow-result.json
Original file line number Diff line number Diff line change
Expand Up @@ -529,7 +529,7 @@
"numericUnit": "millisecond",
"displayValue": "1.0 s",
"metricSavings": {
"TBT": 143
"TBT": 150
},
"details": {
"type": "table",
Expand Down Expand Up @@ -594,7 +594,7 @@
"numericUnit": "millisecond",
"displayValue": "0.3 s",
"metricSavings": {
"TBT": 142.50196940171634
"TBT": 150
},
"details": {
"type": "table",
Expand Down Expand Up @@ -1744,7 +1744,7 @@
"scoreDisplayMode": "informative",
"displayValue": "5 elements found",
"metricSavings": {
"CLS": 0.002631263732910156
"CLS": 0.003
},
"details": {
"type": "table",
Expand Down Expand Up @@ -1869,7 +1869,7 @@
"scoreDisplayMode": "informative",
"displayValue": "3 long tasks found",
"metricSavings": {
"TBT": 143
"TBT": 150
},
"details": {
"type": "table",
Expand Down Expand Up @@ -8996,7 +8996,7 @@
"numericUnit": "millisecond",
"displayValue": "0.9 s",
"metricSavings": {
"TBT": 122.83299999999986
"TBT": 100
},
"details": {
"type": "table",
Expand Down Expand Up @@ -9066,7 +9066,7 @@
"numericUnit": "millisecond",
"displayValue": "0.4 s",
"metricSavings": {
"TBT": 106.76186411159928
"TBT": 100
},
"details": {
"type": "table",
Expand Down Expand Up @@ -9874,7 +9874,7 @@
"scoreDisplayMode": "metricSavings",
"displayValue": "1 element found",
"metricSavings": {
"CLS": 0.13125
"CLS": 0.131
},
"details": {
"type": "table",
Expand Down Expand Up @@ -9923,7 +9923,7 @@
"scoreDisplayMode": "informative",
"displayValue": "1 long task found",
"metricSavings": {
"TBT": 122.83299999999986
"TBT": 100
},
"details": {
"type": "table",
Expand Down Expand Up @@ -10713,7 +10713,7 @@
"scoreDisplayMode": "informative",
"displayValue": "60 ms spent on event 'keypress'",
"metricSavings": {
"INP": 64
"INP": 50
},
"details": {
"type": "list",
Expand Down Expand Up @@ -18485,7 +18485,7 @@
"numericUnit": "millisecond",
"displayValue": "0.5 s",
"metricSavings": {
"TBT": 13
"TBT": 0
},
"details": {
"type": "table",
Expand Down Expand Up @@ -18550,7 +18550,7 @@
"numericUnit": "millisecond",
"displayValue": "0.2 s",
"metricSavings": {
"TBT": 25.85177364864864
"TBT": 50
},
"details": {
"type": "table",
Expand Down Expand Up @@ -19676,7 +19676,7 @@
"scoreDisplayMode": "informative",
"displayValue": "2 long tasks found",
"metricSavings": {
"TBT": 13
"TBT": 0
},
"details": {
"type": "table",
Expand Down Expand Up @@ -20988,7 +20988,7 @@
"warnings": [],
"metricSavings": {
"FCP": 0,
"LCP": 330
"LCP": 350
},
"details": {
"type": "opportunity",
Expand Down
22 changes: 11 additions & 11 deletions core/test/results/sample_v2.json
Original file line number Diff line number Diff line change
Expand Up @@ -390,8 +390,8 @@
"numericUnit": "millisecond",
"displayValue": "Root document took 570 ms",
"metricSavings": {
"FCP": 468.46799999999996,
"LCP": 468.46799999999996
"FCP": 450,
"LCP": 450
},
"details": {
"type": "opportunity",
Expand Down Expand Up @@ -1106,7 +1106,7 @@
"numericUnit": "millisecond",
"displayValue": "2.2 s",
"metricSavings": {
"TBT": 1220.691999999999
"TBT": 1200
},
"details": {
"type": "table",
Expand Down Expand Up @@ -1171,7 +1171,7 @@
"numericUnit": "millisecond",
"displayValue": "1.3 s",
"metricSavings": {
"TBT": 1209.4020894768312
"TBT": 1200
},
"details": {
"type": "table",
Expand Down Expand Up @@ -2349,7 +2349,7 @@
"scoreDisplayMode": "informative",
"displayValue": "Third-party code blocked the main thread for 0 ms",
"metricSavings": {
"TBT": 23.424291876693196
"TBT": 0
},
"details": {
"type": "table",
Expand Down Expand Up @@ -2430,7 +2430,7 @@
"scoreDisplayMode": "metricSavings",
"displayValue": "13,320 ms",
"metricSavings": {
"LCP": 10819.961
"LCP": 10800
},
"details": {
"type": "list",
Expand Down Expand Up @@ -2530,7 +2530,7 @@
"scoreDisplayMode": "metricSavings",
"displayValue": "5 elements found",
"metricSavings": {
"CLS": 0.13570762803819444
"CLS": 0.136
},
"details": {
"type": "table",
Expand Down Expand Up @@ -2655,7 +2655,7 @@
"scoreDisplayMode": "informative",
"displayValue": "2 long tasks found",
"metricSavings": {
"TBT": 1220.691999999999
"TBT": 1200
},
"details": {
"type": "table",
Expand Down Expand Up @@ -4609,7 +4609,7 @@
"warnings": [],
"metricSavings": {
"FCP": 0,
"LCP": 610
"LCP": 600
},
"details": {
"type": "opportunity",
Expand Down Expand Up @@ -4919,7 +4919,7 @@
"displayValue": "Potential savings of 143 KiB",
"metricSavings": {
"FCP": 150,
"LCP": 1060
"LCP": 1050
},
"details": {
"type": "opportunity",
Expand Down Expand Up @@ -5195,7 +5195,7 @@
"numericUnit": "element",
"displayValue": "153 elements",
"metricSavings": {
"TBT": 1
"TBT": 0
},
"details": {
"type": "table",
Expand Down

0 comments on commit 4e4a835

Please sign in to comment.