Repository: GoogleChrome/lighthouse
Pull request #8915 (2019-05-09 20:55:58)

core(budget): throw error on duplicate budget types
something missed in #8727


Patch:
diff --git a/docs/plugins.md b/docs/plugins.md
index 2ef0cc331..f778be202 100644
--- a/docs/plugins.md
+++ b/docs/plugins.md
@@ -350,5 +350,5 @@ Most artifacts will try to represent as truthfully as possible what was observed
 
 ## Examples
 
-- [Google AdSpeed Insights](https://github.com/googleads/ad-speed-insights) - a well-written, but complex, plugin
+- [Publisher Ads Audits](https://github.com/googleads/pub-ads-lighthouse-plugin) - a well-written, but complex, plugin
 - [Lighthouse Plugin Recipe](./recipes/lighthouse-plugin-example)
diff --git a/lighthouse-core/config/budget.js b/lighthouse-core/config/budget.js
index e788e8f49..0f5d1553a 100644
--- a/lighthouse-core/config/budget.js
+++ b/lighthouse-core/config/budget.js
@@ -46,6 +46,22 @@ class Budget {
     }
   }
 
+  /**
+   * Asserts that `strings` has no duplicate strings in it, throwing an error if
+   * it does. `arrayName` is included for nicer logging.
+   * @param {Array<string>} strings
+   * @param {string} arrayName
+   */
+  static assertNoDuplicateStrings(strings, arrayName) {
+    const foundStrings = new Set();
+    for (const string of strings) {
+      if (foundStrings.has(string)) {
+        throw new Error(`${arrayName} has duplicate entry of type '${string}'`);
+      }
+      foundStrings.add(string);
+    }
+  }
+
   /**
    * @param {Record<string, unknown>} resourceBudget
    * @return {LH.Budget.ResourceBudget}
@@ -94,7 +110,7 @@ class Budget {
       'first-cpu-idle',
       'interactive',
       'first-meaningful-paint',
-      'estimated-input-latency',
+      'max-potential-fid',
     ];
     // Assume metric is an allowed string, throw if not.
     if (!validTimingMetrics.includes(/** @type {LH.Budget.TimingMetric} */ (metric))) {
@@ -136,18 +152,24 @@ class Budget {
 
       if (isArrayOfUnknownObjects(resourceSizes)) {
         budget.resourceSizes = resourceSizes.map(Budget.validateResourceBudget);
+        Budget.assertNoDuplicateStrings(budget.resourceSizes.map(r => r.resourceType),
+          `budgets[${index}].resourceSizes`);
       } else if (resourceSizes !== undefined) {
         throw new Error(`Invalid resourceSizes entry in budget at index ${index}`);
       }
 
       if (isArrayOfUnknownObjects(resourceCounts)) {
         budget.resourceCounts = resourceCounts.map(Budget.validateResourceBudget);
+        Budget.assertNoDuplicateStrings(budget.resourceCounts.map(r => r.resourceType),
+          `budgets[${index}].resourceCounts`);
       } else if (resourceCounts !== undefined) {
         throw new Error(`Invalid resourceCounts entry in budget at index ${index}`);
       }
 
       if (isArrayOfUnknownObjects(timings)) {
         budget.timings = timings.map(Budget.validateTimingBudget);
+        Budget.assertNoDuplicateStrings(budget.timings.map(r => r.metric),
+          `budgets[${index}].timings`);
       } else if (timings !== undefined) {
         throw new Error(`Invalid timings entry in budget at index ${index}`);
       }
diff --git a/types/budget.d.ts b/types/budget.d.ts
index fa8632f1b..4c1d058b2 100644
--- a/types/budget.d.ts
+++ b/types/budget.d.ts
@@ -37,7 +37,7 @@ declare global {
       }
 
       /** Supported timing metrics. */
-      export type TimingMetric = 'first-contentful-paint' | 'first-cpu-idle' | 'interactive' | 'first-meaningful-paint' | 'estimated-input-latency';
+      export type TimingMetric = 'first-contentful-paint' | 'first-cpu-idle' | 'interactive' | 'first-meaningful-paint' | 'max-potential-fid';
 
       /** Supported values for the resourceType property of a ResourceBudget. */
       export type ResourceType = 'stylesheet' | 'image' | 'media' | 'font' | 'script' | 'document' | 'other' | 'total' | 'third-party';


Test patch:
diff --git a/lighthouse-cli/test/smokehouse/perf/perf-config.js b/lighthouse-cli/test/smokehouse/perf/perf-config.js
index 6b22341a9..f253a0502 100644
--- a/lighthouse-cli/test/smokehouse/perf/perf-config.js
+++ b/lighthouse-cli/test/smokehouse/perf/perf-config.js
@@ -41,7 +41,7 @@ const perfConfig = {
         {metric: 'first-cpu-idle', budget: 2000, tolerance: 100},
         {metric: 'interactive', budget: 2000, tolerance: 100},
         {metric: 'first-meaningful-paint', budget: 2000, tolerance: 100},
-        {metric: 'estimated-input-latency', budget: 2000, tolerance: 100},
+        {metric: 'max-potential-fid', budget: 2000, tolerance: 100},
       ],
     }],
   },
diff --git a/lighthouse-core/test/config/budget-test.js b/lighthouse-core/test/config/budget-test.js
index 19de4fb00..20b4843e8 100644
--- a/lighthouse-core/test/config/budget-test.js
+++ b/lighthouse-core/test/config/budget-test.js
@@ -146,6 +146,18 @@ describe('Budget', () => {
       assert.throws(_ => Budget.initializeBudget(budget),
         /Resource Budget has unrecognized properties: \[browser\]/);
     });
+
+    it('throws when a duplicate resourceType is specified in resourceSizes', () => {
+      budget[1].resourceSizes.push({resourceType: 'script', budget: 100});
+      assert.throws(_ => Budget.initializeBudget(budget),
+        /budgets\[1\]\.resourceSizes has duplicate entry of type 'script'/);
+    });
+
+    it('throws when a duplicate resourceType is specified in resourceCounts', () => {
+      budget[0].resourceCounts.push({resourceType: 'third-party', budget: 100});
+      assert.throws(_ => Budget.initializeBudget(budget),
+        /budgets\[0\]\.resourceCounts has duplicate entry of type 'third-party'/);
+    });
   });
 
   describe('timing budget validation', () => {
@@ -172,5 +184,11 @@ describe('Budget', () => {
       assert.throws(_ => Budget.initializeBudget(budget),
         /Timing Budget has unrecognized properties: \[device, location\]/);
     });
+
+    it('throws when a duplicate metric type is specified in timings', () => {
+      budget[0].timings.push({metric: 'interactive', budget: 1000});
+      assert.throws(_ => Budget.initializeBudget(budget),
+        /budgets\[0\]\.timings has duplicate entry of type 'interactive'/);
+    });
   });
 });
diff --git a/lighthouse-core/test/results/sample-config.js b/lighthouse-core/test/results/sample-config.js
index 9f8144dbb..8f63818ce 100644
--- a/lighthouse-core/test/results/sample-config.js
+++ b/lighthouse-core/test/results/sample-config.js
@@ -42,7 +42,7 @@ const budgetedConfig = {
         {metric: 'first-cpu-idle', budget: 2900, tolerance: 100},
         {metric: 'interactive', budget: 2900, tolerance: 100},
         {metric: 'first-meaningful-paint', budget: 2000, tolerance: 100},
-        {metric: 'estimated-input-latency', budget: 100, tolerance: 100},
+        {metric: 'max-potential-fid', budget: 100, tolerance: 100},
       ],
     }],
   },
diff --git a/lighthouse-core/test/results/sample_v2.json b/lighthouse-core/test/results/sample_v2.json
index 19844e8e8..89e0bd6c6 100644
--- a/lighthouse-core/test/results/sample_v2.json
+++ b/lighthouse-core/test/results/sample_v2.json
@@ -3290,7 +3290,7 @@
             "tolerance": 100
           },
           {
-            "metric": "estimated-input-latency",
+            "metric": "max-potential-fid",
             "budget": 100,
             "tolerance": 100
           }