From 7bea6528a52fc2be2dcf30639823e451578238a3 Mon Sep 17 00:00:00 2001
From: Brendan Kenny <bckenny@gmail.com>
Date: Wed, 22 Mar 2023 16:08:17 -0500
Subject: [PATCH] core: cap byte-efficiency-audit scores to a max of 1

---
 .../byte-efficiency/byte-efficiency-audit.js  |  6 ++++-
 .../byte-efficiency-audit-test.js             | 26 +++++++++++++++++++
 2 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/core/audits/byte-efficiency/byte-efficiency-audit.js b/core/audits/byte-efficiency/byte-efficiency-audit.js
index f086d8f6ec95..edf308fc5336 100644
--- a/core/audits/byte-efficiency/byte-efficiency-audit.js
+++ b/core/audits/byte-efficiency/byte-efficiency-audit.js
@@ -39,12 +39,14 @@ const WASTED_MS_FOR_SCORE_OF_ZERO = 5000;
 class ByteEfficiencyAudit extends Audit {
   /**
    * Creates a score based on the wastedMs value using linear interpolation between control points.
+   * A negative wastedMs is scored as 1, assuming time is not being wasted with respect to the
+   * opportunity being measured.
    *
    * @param {number} wastedMs
    * @return {number}
    */
   static scoreForWastedMs(wastedMs) {
-    if (wastedMs === 0) {
+    if (wastedMs <= 0) {
       return 1;
     } else if (wastedMs < WASTED_MS_FOR_AVERAGE) {
       return linearInterpolation(0, 1, WASTED_MS_FOR_AVERAGE, 0.75, wastedMs);
@@ -214,6 +216,8 @@ class ByteEfficiencyAudit extends Audit {
 
     const wastedBytes = results.reduce((sum, item) => sum + item.wastedBytes, 0);
 
+    // `wastedMs` may be negative, if making the opportunity change could be detrimental.
+    // This is useful information in the LHR and should be preserved.
     let wastedMs;
     if (gatherContext.gatherMode === 'navigation') {
       if (!graph) throw Error('Page dependency graph should always be computed in navigation mode');
diff --git a/core/test/audits/byte-efficiency/byte-efficiency-audit-test.js b/core/test/audits/byte-efficiency/byte-efficiency-audit-test.js
index 79fb9c83e8a6..bd1550ce8bcb 100644
--- a/core/test/audits/byte-efficiency/byte-efficiency-audit-test.js
+++ b/core/test/audits/byte-efficiency/byte-efficiency-audit-test.js
@@ -137,6 +137,17 @@ describe('Byte efficiency base audit', () => {
     assert.ok(failingResult.score < 0.5, 'scores failing wastedMs');
   });
 
+  it('should score negative wastedMs as perfect', () => {
+    const negativeResult = ByteEfficiencyAudit.createAuditProduct({
+      headings: baseHeadings,
+      items: [{url: 'http://example.com/', wastedBytes: -1 * 1000}],
+    }, graph, simulator, {gatherMode: 'timespan'});
+
+    assert.equal(negativeResult.score, 1);
+    assert.ok(negativeResult.numericValue < 0);
+    assert.equal(negativeResult.numericValue, negativeResult.details.overallSavingsMs);
+  });
+
   it('should throw on invalid graph', () => {
     assert.throws(() => {
       ByteEfficiencyAudit.createAuditProduct({
@@ -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', () => {
+    it('scores wastedMs values', () => {
+      expect(ByteEfficiencyAudit.scoreForWastedMs(-50)).toBe(1);
+      expect(ByteEfficiencyAudit.scoreForWastedMs(0)).toBe(1);
+      expect(ByteEfficiencyAudit.scoreForWastedMs(240)).toBe(0.8);
+      expect(ByteEfficiencyAudit.scoreForWastedMs(300)).toBe(0.75);
+      expect(ByteEfficiencyAudit.scoreForWastedMs(390)).toBe(0.7);
+      expect(ByteEfficiencyAudit.scoreForWastedMs(750)).toBe(0.5);
+      expect(ByteEfficiencyAudit.scoreForWastedMs(1_175)).toBe(0.45);
+      expect(ByteEfficiencyAudit.scoreForWastedMs(5_000)).toBe(0);
+      expect(ByteEfficiencyAudit.scoreForWastedMs(10_000)).toBe(0);
+      expect(ByteEfficiencyAudit.scoreForWastedMs(Number.MAX_VALUE)).toBe(0);
+    });
+  });
 });