From 1dca7bfa529fd870eb7c49f2fdc946ab87ff9993 Mon Sep 17 00:00:00 2001 From: Adam Raine Date: Tue, 2 Jan 2024 13:31:50 -0500 Subject: [PATCH 1/7] core: normalize metric savings --- core/audits/audit.js | 38 ++++++++++++++++++++++++++++++++++++-- 1 file changed, 36 insertions(+), 2 deletions(-) diff --git a/core/audits/audit.js b/core/audits/audit.js index 7b6766e9fb7d..03d8cd963def 100644 --- a/core/audits/audit.js +++ b/core/audits/audit.js @@ -10,6 +10,14 @@ import {Util} from '../../shared/util.js'; const DEFAULT_PASS = 'defaultPass'; +/** @type {LH.Audit.MetricSavings} */ +const METRIC_SAVINGS_PRECISION = { + FCP: 100, + LCP: 100, + TBT: 100, + CLS: 0.001, +}; + /** * @typedef TableOptions * @property {number=} wastedMs @@ -339,6 +347,30 @@ class Audit { return score; } + /** + * @param {LH.Audit.MetricSavings|undefined} metricSavings + * @return {LH.Audit.MetricSavings|undefined} + */ + static _normalizeMetricSavings(metricSavings) { + if (!metricSavings) return; + + /** @type {LH.Audit.MetricSavings} */ + const normalizedMetricSavings = {...metricSavings}; + + // eslint-disable-next-line max-len + for (const key of /** @type {Array} */ (Object.keys(metricSavings))) { + const value = metricSavings[key]; + if (value === undefined) continue; + + const precision = METRIC_SAVINGS_PRECISION[key]; + if (precision === undefined) continue; + + normalizedMetricSavings[key] = Math.floor(value / precision) * precision; + } + + return normalizedMetricSavings; + } + /** * @param {typeof Audit} audit * @param {string | LH.IcuMessage} errorMessage @@ -378,10 +410,12 @@ class Audit { scoreDisplayMode = product.scoreDisplayMode; } + const metricSavings = Audit._normalizeMetricSavings(product.metricSavings); + 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 (Object.values(metricSavings || {}).some(v => v)) { score = 0; } else { score = 0.5; @@ -419,7 +453,7 @@ class Audit { errorStack: product.errorStack, warnings: product.warnings, scoringOptions: product.scoringOptions, - metricSavings: product.metricSavings, + metricSavings, details: product.details, guidanceLevel: audit.meta.guidanceLevel, From d6cea5d33ed30d7aead603e2ae85528cac88d69e Mon Sep 17 00:00:00 2001 From: Adam Raine Date: Tue, 2 Jan 2024 13:38:48 -0500 Subject: [PATCH 2/7] test --- core/test/audits/audit-test.js | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/core/test/audits/audit-test.js b/core/test/audits/audit-test.js index 7ebdac4d2273..86c37d2a33f9 100644 --- a/core/test/audits/audit-test.js +++ b/core/test/audits/audit-test.js @@ -178,6 +178,14 @@ describe('Audit', () => { assert.strictEqual(auditResult.score, 0.3); }); + it('normalizes metric savings', () => { + const auditResult = Audit.generateAuditResult(PassOrFailAudit, { + score: 0, + metricSavings: {LCP: 0.1, FCP: 199}, + }); + assert.deepStrictEqual(auditResult.metricSavings, {LCP: 0, FCP: 100}); + }); + it('chooses the title if score is passing', () => { const auditResult = Audit.generateAuditResult(PassOrFailAudit, {score: 1}); assert.strictEqual(auditResult.score, 1); From f632bb0eb2c3de4748debe2ce3a4f11377512f50 Mon Sep 17 00:00:00 2001 From: Adam Raine Date: Tue, 2 Jan 2024 14:09:03 -0500 Subject: [PATCH 3/7] sample --- .../reports/sample-flow-result.json | 24 ++++++++-------- core/test/results/sample_v2.json | 28 +++++++++---------- 2 files changed, 26 insertions(+), 26 deletions(-) diff --git a/core/test/fixtures/user-flows/reports/sample-flow-result.json b/core/test/fixtures/user-flows/reports/sample-flow-result.json index 9a8b521e7062..966b278713da 100644 --- a/core/test/fixtures/user-flows/reports/sample-flow-result.json +++ b/core/test/fixtures/user-flows/reports/sample-flow-result.json @@ -529,7 +529,7 @@ "numericUnit": "millisecond", "displayValue": "1.0 s", "metricSavings": { - "TBT": 143 + "TBT": 100 }, "details": { "type": "table", @@ -594,7 +594,7 @@ "numericUnit": "millisecond", "displayValue": "0.3 s", "metricSavings": { - "TBT": 142.50196940171634 + "TBT": 100 }, "details": { "type": "table", @@ -1744,7 +1744,7 @@ "scoreDisplayMode": "informative", "displayValue": "5 elements found", "metricSavings": { - "CLS": 0.002631263732910156 + "CLS": 0.002 }, "details": { "type": "table", @@ -1869,7 +1869,7 @@ "scoreDisplayMode": "informative", "displayValue": "3 long tasks found", "metricSavings": { - "TBT": 143 + "TBT": 100 }, "details": { "type": "table", @@ -8996,7 +8996,7 @@ "numericUnit": "millisecond", "displayValue": "0.9 s", "metricSavings": { - "TBT": 122.83299999999986 + "TBT": 100 }, "details": { "type": "table", @@ -9066,7 +9066,7 @@ "numericUnit": "millisecond", "displayValue": "0.4 s", "metricSavings": { - "TBT": 106.76186411159928 + "TBT": 100 }, "details": { "type": "table", @@ -9874,7 +9874,7 @@ "scoreDisplayMode": "metricSavings", "displayValue": "1 element found", "metricSavings": { - "CLS": 0.13125 + "CLS": 0.131 }, "details": { "type": "table", @@ -9923,7 +9923,7 @@ "scoreDisplayMode": "informative", "displayValue": "1 long task found", "metricSavings": { - "TBT": 122.83299999999986 + "TBT": 100 }, "details": { "type": "table", @@ -18485,7 +18485,7 @@ "numericUnit": "millisecond", "displayValue": "0.5 s", "metricSavings": { - "TBT": 13 + "TBT": 0 }, "details": { "type": "table", @@ -18550,7 +18550,7 @@ "numericUnit": "millisecond", "displayValue": "0.2 s", "metricSavings": { - "TBT": 25.85177364864864 + "TBT": 0 }, "details": { "type": "table", @@ -19676,7 +19676,7 @@ "scoreDisplayMode": "informative", "displayValue": "2 long tasks found", "metricSavings": { - "TBT": 13 + "TBT": 0 }, "details": { "type": "table", @@ -20988,7 +20988,7 @@ "warnings": [], "metricSavings": { "FCP": 0, - "LCP": 330 + "LCP": 300 }, "details": { "type": "opportunity", diff --git a/core/test/results/sample_v2.json b/core/test/results/sample_v2.json index 5ec1b015912e..465aa64a90c7 100644 --- a/core/test/results/sample_v2.json +++ b/core/test/results/sample_v2.json @@ -390,8 +390,8 @@ "numericUnit": "millisecond", "displayValue": "Root document took 570 ms", "metricSavings": { - "FCP": 468.46799999999996, - "LCP": 468.46799999999996 + "FCP": 400, + "LCP": 400 }, "details": { "type": "opportunity", @@ -1106,7 +1106,7 @@ "numericUnit": "millisecond", "displayValue": "2.2 s", "metricSavings": { - "TBT": 1220.691999999999 + "TBT": 1200 }, "details": { "type": "table", @@ -1171,7 +1171,7 @@ "numericUnit": "millisecond", "displayValue": "1.3 s", "metricSavings": { - "TBT": 1209.4020894768312 + "TBT": 1200 }, "details": { "type": "table", @@ -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", @@ -2430,7 +2430,7 @@ "scoreDisplayMode": "metricSavings", "displayValue": "13,320 ms", "metricSavings": { - "LCP": 10819.961 + "LCP": 10800 }, "details": { "type": "list", @@ -2530,7 +2530,7 @@ "scoreDisplayMode": "metricSavings", "displayValue": "5 elements found", "metricSavings": { - "CLS": 0.13570762803819444 + "CLS": 0.135 }, "details": { "type": "table", @@ -2655,7 +2655,7 @@ "scoreDisplayMode": "informative", "displayValue": "2 long tasks found", "metricSavings": { - "TBT": 1220.691999999999 + "TBT": 1200 }, "details": { "type": "table", @@ -4609,7 +4609,7 @@ "warnings": [], "metricSavings": { "FCP": 0, - "LCP": 610 + "LCP": 600 }, "details": { "type": "opportunity", @@ -4768,7 +4768,7 @@ "warnings": [], "metricSavings": { "FCP": 0, - "LCP": 450 + "LCP": 400 }, "details": { "type": "opportunity", @@ -4918,8 +4918,8 @@ "numericUnit": "millisecond", "displayValue": "Potential savings of 143 KiB", "metricSavings": { - "FCP": 150, - "LCP": 1060 + "FCP": 100, + "LCP": 1000 }, "details": { "type": "opportunity", @@ -5096,7 +5096,7 @@ "displayValue": "Potential savings of 26 KiB", "metricSavings": { "FCP": 0, - "LCP": 150 + "LCP": 100 }, "details": { "type": "opportunity", @@ -5195,7 +5195,7 @@ "numericUnit": "element", "displayValue": "153 elements", "metricSavings": { - "TBT": 1 + "TBT": 0 }, "details": { "type": "table", From 68eb7ceef9b99afb9fedc256e03642c73e8dde0b Mon Sep 17 00:00:00 2001 From: Adam Raine Date: Tue, 2 Jan 2024 14:22:35 -0500 Subject: [PATCH 4/7] pr --- core/audits/audit.js | 8 ++++---- core/test/audits/audit-test.js | 4 ++-- .../user-flows/reports/sample-flow-result.json | 12 ++++++------ core/test/results/sample_v2.json | 14 +++++++------- 4 files changed, 19 insertions(+), 19 deletions(-) diff --git a/core/audits/audit.js b/core/audits/audit.js index 03d8cd963def..0ab0b27867d4 100644 --- a/core/audits/audit.js +++ b/core/audits/audit.js @@ -12,9 +12,9 @@ const DEFAULT_PASS = 'defaultPass'; /** @type {LH.Audit.MetricSavings} */ const METRIC_SAVINGS_PRECISION = { - FCP: 100, - LCP: 100, - TBT: 100, + FCP: 50, + LCP: 50, + TBT: 50, CLS: 0.001, }; @@ -365,7 +365,7 @@ class Audit { const precision = METRIC_SAVINGS_PRECISION[key]; if (precision === undefined) continue; - normalizedMetricSavings[key] = Math.floor(value / precision) * precision; + normalizedMetricSavings[key] = Math.round(value / precision) * precision; } return normalizedMetricSavings; diff --git a/core/test/audits/audit-test.js b/core/test/audits/audit-test.js index 86c37d2a33f9..1fcaf7c92aba 100644 --- a/core/test/audits/audit-test.js +++ b/core/test/audits/audit-test.js @@ -181,9 +181,9 @@ describe('Audit', () => { it('normalizes metric savings', () => { const auditResult = Audit.generateAuditResult(PassOrFailAudit, { score: 0, - metricSavings: {LCP: 0.1, FCP: 199}, + metricSavings: {LCP: 0.1, FCP: 149}, }); - assert.deepStrictEqual(auditResult.metricSavings, {LCP: 0, FCP: 100}); + assert.deepStrictEqual(auditResult.metricSavings, {LCP: 0, FCP: 150}); }); it('chooses the title if score is passing', () => { diff --git a/core/test/fixtures/user-flows/reports/sample-flow-result.json b/core/test/fixtures/user-flows/reports/sample-flow-result.json index 966b278713da..26bc6e297bfa 100644 --- a/core/test/fixtures/user-flows/reports/sample-flow-result.json +++ b/core/test/fixtures/user-flows/reports/sample-flow-result.json @@ -529,7 +529,7 @@ "numericUnit": "millisecond", "displayValue": "1.0 s", "metricSavings": { - "TBT": 100 + "TBT": 150 }, "details": { "type": "table", @@ -594,7 +594,7 @@ "numericUnit": "millisecond", "displayValue": "0.3 s", "metricSavings": { - "TBT": 100 + "TBT": 150 }, "details": { "type": "table", @@ -1744,7 +1744,7 @@ "scoreDisplayMode": "informative", "displayValue": "5 elements found", "metricSavings": { - "CLS": 0.002 + "CLS": 0.003 }, "details": { "type": "table", @@ -1869,7 +1869,7 @@ "scoreDisplayMode": "informative", "displayValue": "3 long tasks found", "metricSavings": { - "TBT": 100 + "TBT": 150 }, "details": { "type": "table", @@ -18550,7 +18550,7 @@ "numericUnit": "millisecond", "displayValue": "0.2 s", "metricSavings": { - "TBT": 0 + "TBT": 50 }, "details": { "type": "table", @@ -20988,7 +20988,7 @@ "warnings": [], "metricSavings": { "FCP": 0, - "LCP": 300 + "LCP": 350 }, "details": { "type": "opportunity", diff --git a/core/test/results/sample_v2.json b/core/test/results/sample_v2.json index 465aa64a90c7..b191b172caed 100644 --- a/core/test/results/sample_v2.json +++ b/core/test/results/sample_v2.json @@ -390,8 +390,8 @@ "numericUnit": "millisecond", "displayValue": "Root document took 570 ms", "metricSavings": { - "FCP": 400, - "LCP": 400 + "FCP": 450, + "LCP": 450 }, "details": { "type": "opportunity", @@ -2530,7 +2530,7 @@ "scoreDisplayMode": "metricSavings", "displayValue": "5 elements found", "metricSavings": { - "CLS": 0.135 + "CLS": 0.136 }, "details": { "type": "table", @@ -4768,7 +4768,7 @@ "warnings": [], "metricSavings": { "FCP": 0, - "LCP": 400 + "LCP": 450 }, "details": { "type": "opportunity", @@ -4918,8 +4918,8 @@ "numericUnit": "millisecond", "displayValue": "Potential savings of 143 KiB", "metricSavings": { - "FCP": 100, - "LCP": 1000 + "FCP": 150, + "LCP": 1050 }, "details": { "type": "opportunity", @@ -5096,7 +5096,7 @@ "displayValue": "Potential savings of 26 KiB", "metricSavings": { "FCP": 0, - "LCP": 100 + "LCP": 150 }, "details": { "type": "opportunity", From 9757c3afae6c59f8235ecdfc34ccdb88495723bf Mon Sep 17 00:00:00 2001 From: Adam Raine Date: Thu, 4 Jan 2024 09:40:34 -0800 Subject: [PATCH 5/7] inp --- core/audits/audit.js | 5 +++-- .../test/fixtures/user-flows/reports/sample-flow-result.json | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/core/audits/audit.js b/core/audits/audit.js index 0ab0b27867d4..f4f6fd6b0ccd 100644 --- a/core/audits/audit.js +++ b/core/audits/audit.js @@ -14,6 +14,7 @@ const DEFAULT_PASS = 'defaultPass'; const METRIC_SAVINGS_PRECISION = { FCP: 50, LCP: 50, + INP: 50, TBT: 50, CLS: 0.001, }; @@ -351,7 +352,7 @@ class Audit { * @param {LH.Audit.MetricSavings|undefined} metricSavings * @return {LH.Audit.MetricSavings|undefined} */ - static _normalizeMetricSavings(metricSavings) { + static _quantizeMetricSavings(metricSavings) { if (!metricSavings) return; /** @type {LH.Audit.MetricSavings} */ @@ -410,7 +411,7 @@ class Audit { scoreDisplayMode = product.scoreDisplayMode; } - const metricSavings = Audit._normalizeMetricSavings(product.metricSavings); + const metricSavings = Audit._quantizeMetricSavings(product.metricSavings); if (scoreDisplayMode === Audit.SCORING_MODES.METRIC_SAVINGS) { if (score && score >= Util.PASS_THRESHOLD) { diff --git a/core/test/fixtures/user-flows/reports/sample-flow-result.json b/core/test/fixtures/user-flows/reports/sample-flow-result.json index 26bc6e297bfa..536fce8debb9 100644 --- a/core/test/fixtures/user-flows/reports/sample-flow-result.json +++ b/core/test/fixtures/user-flows/reports/sample-flow-result.json @@ -10713,7 +10713,7 @@ "scoreDisplayMode": "informative", "displayValue": "60 ms spent on event 'keypress'", "metricSavings": { - "INP": 64 + "INP": 50 }, "details": { "type": "list", From 34b4779f8dc5e6ba4ae1021466b4fec96e20f76a Mon Sep 17 00:00:00 2001 From: Adam Raine Date: Thu, 4 Jan 2024 09:47:42 -0800 Subject: [PATCH 6/7] nits --- core/audits/audit.js | 6 ++++-- core/test/audits/audit-test.js | 6 +++--- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/core/audits/audit.js b/core/audits/audit.js index f4f6fd6b0ccd..f85d791a4b58 100644 --- a/core/audits/audit.js +++ b/core/audits/audit.js @@ -366,7 +366,8 @@ class Audit { const precision = METRIC_SAVINGS_PRECISION[key]; if (precision === undefined) continue; - normalizedMetricSavings[key] = Math.round(value / precision) * precision; + const roundedValue = Math.round(value / precision) * precision; + normalizedMetricSavings[key] = Math.max(roundedValue, 0); } return normalizedMetricSavings; @@ -412,11 +413,12 @@ class Audit { } 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(metricSavings || {}).some(v => v)) { + } else if (hasSomeSavings) { score = 0; } else { score = 0.5; diff --git a/core/test/audits/audit-test.js b/core/test/audits/audit-test.js index 1fcaf7c92aba..3edd756f44e5 100644 --- a/core/test/audits/audit-test.js +++ b/core/test/audits/audit-test.js @@ -178,12 +178,12 @@ describe('Audit', () => { assert.strictEqual(auditResult.score, 0.3); }); - it('normalizes metric savings', () => { + it('quantizes metric savings', () => { const auditResult = Audit.generateAuditResult(PassOrFailAudit, { score: 0, - metricSavings: {LCP: 0.1, FCP: 149}, + metricSavings: {LCP: 0.1, FCP: 149, CLS: 0.0015, TBT: -100}, }); - assert.deepStrictEqual(auditResult.metricSavings, {LCP: 0, FCP: 150}); + assert.deepStrictEqual(auditResult.metricSavings, {LCP: 0, FCP: 150, CLS: 0.002, TBT: 0}); }); it('chooses the title if score is passing', () => { From b83a126a02f9d6ebbb5ce691a4feb446ddf211de Mon Sep 17 00:00:00 2001 From: Adam Raine Date: Thu, 4 Jan 2024 09:53:57 -0800 Subject: [PATCH 7/7] nit logic --- core/audits/audit.js | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/core/audits/audit.js b/core/audits/audit.js index f85d791a4b58..eddbc74e565e 100644 --- a/core/audits/audit.js +++ b/core/audits/audit.js @@ -358,16 +358,18 @@ class Audit { /** @type {LH.Audit.MetricSavings} */ const normalizedMetricSavings = {...metricSavings}; - // eslint-disable-next-line max-len - for (const key of /** @type {Array} */ (Object.keys(metricSavings))) { - const value = metricSavings[key]; + for (const key of /** @type {Array} */ (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) continue; + if (precision !== undefined) { + value = Math.round(value / precision) * precision; + } - const roundedValue = Math.round(value / precision) * precision; - normalizedMetricSavings[key] = Math.max(roundedValue, 0); + normalizedMetricSavings[key] = value; } return normalizedMetricSavings;