From b8e359f55dce79ac736fe4ab78ccf02efc9c14aa Mon Sep 17 00:00:00 2001 From: Katie Hempenius Date: Thu, 18 Apr 2019 21:41:22 -0400 Subject: [PATCH 1/9] Add --budgets-path & Budgets constructor --- .../test/cli/__snapshots__/index-test.js.snap | 2 + lighthouse-core/config/budgets.js | 118 ++++++++++++++++++ lighthouse-core/config/constants.js | 1 + lighthouse-core/test/config/budgets-test.js | 99 +++++++++++++++ .../test/results/artifacts/artifacts.json | 1 + lighthouse-core/test/results/sample_v2.json | 1 + types/budgets.d.ts | 35 ++++++ types/externs.d.ts | 2 + 8 files changed, 259 insertions(+) create mode 100644 lighthouse-core/config/budgets.js create mode 100644 lighthouse-core/test/config/budgets-test.js create mode 100644 types/budgets.d.ts diff --git a/lighthouse-cli/test/cli/__snapshots__/index-test.js.snap b/lighthouse-cli/test/cli/__snapshots__/index-test.js.snap index 593c3c48d078..4a8496fc5fb4 100644 --- a/lighthouse-cli/test/cli/__snapshots__/index-test.js.snap +++ b/lighthouse-cli/test/cli/__snapshots__/index-test.js.snap @@ -1198,6 +1198,7 @@ Object { "additionalTraceCategories": null, "auditMode": false, "blockedUrlPatterns": null, + "budgetsPath": null, "channel": "cli", "disableStorageReset": false, "emulatedFormFactor": "mobile", @@ -1327,6 +1328,7 @@ Object { "additionalTraceCategories": null, "auditMode": true, "blockedUrlPatterns": null, + "budgetsPath": null, "channel": "cli", "disableStorageReset": false, "emulatedFormFactor": "mobile", diff --git a/lighthouse-core/config/budgets.js b/lighthouse-core/config/budgets.js new file mode 100644 index 000000000000..2f30b0db0c7d --- /dev/null +++ b/lighthouse-core/config/budgets.js @@ -0,0 +1,118 @@ +/** + * @license Copyright 2016 Google Inc. All Rights Reserved. + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. + */ +'use strict'; + +class Budgets { + /** + * @param {LH.Budgets.ResourceBudget} resourceBudget + * @return {LH.Budgets.ResourceBudget} + */ + static validateResourceBudget(resourceBudget) { + const validResourceTypes = [ + 'total', + 'document', + 'script', + 'stylesheet', + 'image', + 'media', + 'font', + 'other', + 'thirdParty', + ]; + if (!validResourceTypes.includes(resourceBudget.resourceType)) { + throw new Error(`Invalid resource type: ${resourceBudget.resourceType}. \n` + + `Valid resource types are: ${ validResourceTypes.join(', ') }`); + } + if (isNaN(resourceBudget.budget)) { + throw new Error('Invalid budget: ${resourceBudget.budget}'); + } + return resourceBudget; + } + + /** + * @param {LH.Budgets.TimingBudget} timingBudget + * @return {LH.Budgets.TimingBudget} + */ + static validateTimingBudget(timingBudget) { + const validTimingMetrics = [ + 'firstContentfulPaint', + 'firstCpuIdle', + 'timeToInteractive', + 'firstMeaningfulPaint', + 'estimaatedInputLatency', + ]; + if (!validTimingMetrics.includes(timingBudget.metric)) { + throw new Error(`Invalid timing metric: ${timingBudget.metric}. \n` + + `Valid timing metrics are: ${validTimingMetrics.join(', ')}`); + } + if (isNaN(timingBudget.budget)) { + throw new Error('Invalid budget: ${timingBudget.budget}'); + } + if (timingBudget.tolerance !== undefined && isNaN(timingBudget.tolerance)) { + throw new Error('Invalid tolerance: ${timingBudget.tolerance}'); + } + return timingBudget; + } + + /** + * @constructor + * @implements {LH.Budgets.Json} + * @param {LH.Budgets.Json} budgetsJSON + */ + constructor(budgetsJSON) { + /** @type {Array} */ + this.budgets = []; + + budgetsJSON.budgets.forEach((b) => { + /** @type {LH.Budgets.Budget} */ + const budget = {}; + const validBudgetProperties = ['resourceSizes', 'resourceCounts', 'timings']; + for (const prop in b) { + if (!validBudgetProperties.includes(prop)) { + throw new Error(`Unsupported budget property: ${prop}. \n` + + `Valid properties are: ${validBudgetProperties.join(', ')}`); + } + } + if (b.resourceSizes !== undefined) { + budget.resourceSizes = b.resourceSizes.map((r) => { + return Budgets.validateResourceBudget({ + resourceType: r.resourceType, + budget: r.budget, + }); + }); + } + + if (b.resourceCounts !== undefined) { + budget.resourceCounts = b.resourceCounts.map((r) => { + return Budgets.validateResourceBudget({ + resourceType: r.resourceType, + budget: r.budget, + }); + }); + } + + if (b.timings !== undefined) { + budget.timings = b.timings.map((t) => { + if (t.tolerance !== undefined) { + return Budgets.validateTimingBudget({ + metric: t.metric, + budget: t.budget, + tolerance: t.tolerance, + }); + } else { + return Budgets.validateTimingBudget({ + metric: t.metric, + budget: t.budget, + }); + } + }); + } + this.budgets.push(budget); + }); + } +} + +module.exports = Budgets; diff --git a/lighthouse-core/config/constants.js b/lighthouse-core/config/constants.js index 331975751ece..b0e0b58faeae 100644 --- a/lighthouse-core/config/constants.js +++ b/lighthouse-core/config/constants.js @@ -52,6 +52,7 @@ const defaultSettings = { disableStorageReset: false, emulatedFormFactor: 'mobile', channel: 'node', + budgetsPath: null, // the following settings have no defaults but we still want ensure that `key in settings` // in config will work in a typechecked way diff --git a/lighthouse-core/test/config/budgets-test.js b/lighthouse-core/test/config/budgets-test.js new file mode 100644 index 000000000000..e4843e0e813d --- /dev/null +++ b/lighthouse-core/test/config/budgets-test.js @@ -0,0 +1,99 @@ +/** + * @license Copyright 2016 Google Inc. All Rights Reserved. + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. + */ +'use strict'; + +const Budgets = require('../../config/budgets'); +const assert = require('assert'); +/* eslint-env jest */ + +describe('Budgets', () => { + let budgetsJson; + beforeEach(() => { + budgetsJson = { + budgets: [{ + resourceSizes: [{ + resourceType: 'script', + budget: 123, + }, + { + resourceType: 'image', + budget: 456, + }], + resourceCounts: [{ + resourceType: 'total', + budget: 100, + }, + { + resourceType: 'thirdParty', + budget: 10, + }], + timings: [{ + metric: 'timeToInteractive', + budget: 2000, + tolerance: 1000, + }, + { + metric: 'firstContentfulPaint', + budget: 1000, + tolerance: 500, + }], + }, + { + resourceSizes: [ + { + resourceType: 'script', + budget: 1000, + }, + ], + }], + }; + }); + it('initializes correctly', () => { + const budgets = new Budgets(budgetsJson); + assert.equal(budgets.budgets.length, 2); + + assert.equal(budgets.budgets[0].resourceSizes.length, 2); + assert.equal(budgets.budgets[0].resourceSizes[0].resourceType, 'script'); + assert.equal(budgets.budgets[0].resourceSizes[0].budget, 123); + + assert.equal(budgets.budgets[0].resourceCounts.length, 2); + assert.equal(budgets.budgets[0].resourceCounts[0].resourceType, 'total'); + assert.equal(budgets.budgets[0].resourceCounts[0].budget, 100); + + assert.equal(budgets.budgets[0].timings.length, 2); + assert.equal(budgets.budgets[0].timings[1].metric, 'firstContentfulPaint'); + assert.equal(budgets.budgets[0].timings[1].budget, 1000); + assert.equal(budgets.budgets[0].timings[1].tolerance, 500); + }); + it('throws error if an unsupported budget property is used', () => { + budgetsJson.budgets[0].sizes = []; + assert.throws(_ => new Budgets(budgetsJson), /Unsupported budget property/); + }); + describe('resource budget validation', () => { + it('throws when an invalid resource type is supplied', () => { + budgetsJson.budgets[0].resourceSizes[0].resourceType = 'movies'; + assert.throws(_ => new Budgets(budgetsJson), /Invalid resource type/); + }); + it('throws when an invalid budget is supplied', () => { + budgetsJson.budgets[0].resourceSizes[0].budget = '100 MB'; + assert.throws(_ => new Budgets(budgetsJson), /Invalid budget/); + }); + }); + describe('timing budget validation', () => { + it('throws when an invalid metric is supplied', () => { + budgetsJson.budgets[0].timings[0].metric = 'lastMeaningfulPaint'; + assert.throws(_ => new Budgets(budgetsJson), /Invalid timing metric/); + }); + it('throws when an invalid budget is supplied', () => { + budgetsJson.budgets[0].timings[0].budget = '100KB'; + assert.throws(_ => new Budgets(budgetsJson), /Invalid budget/); + }); + it('throws when an invalid tolerance is supplied', () => { + budgetsJson.budgets[0].timings[0].tolerance = '100ms'; + assert.throws(_ => new Budgets(budgetsJson), /Invalid tolerance/); + }); + }); +}); diff --git a/lighthouse-core/test/results/artifacts/artifacts.json b/lighthouse-core/test/results/artifacts/artifacts.json index 0aef7b229206..7b573e5b76e6 100644 --- a/lighthouse-core/test/results/artifacts/artifacts.json +++ b/lighthouse-core/test/results/artifacts/artifacts.json @@ -29,6 +29,7 @@ "disableStorageReset": false, "emulatedFormFactor": "mobile", "channel": "cli", + "budgetsPath": null, "locale": "en-US", "blockedUrlPatterns": null, "additionalTraceCategories": null, diff --git a/lighthouse-core/test/results/sample_v2.json b/lighthouse-core/test/results/sample_v2.json index b9e493af335c..608c071eb830 100644 --- a/lighthouse-core/test/results/sample_v2.json +++ b/lighthouse-core/test/results/sample_v2.json @@ -2963,6 +2963,7 @@ "disableStorageReset": false, "emulatedFormFactor": "mobile", "channel": "cli", + "budgetsPath": null, "locale": "en-US", "blockedUrlPatterns": null, "additionalTraceCategories": null, diff --git a/types/budgets.d.ts b/types/budgets.d.ts new file mode 100644 index 000000000000..ea29fcb0a29d --- /dev/null +++ b/types/budgets.d.ts @@ -0,0 +1,35 @@ +/** + * @license Copyright 2018 Google Inc. All Rights Reserved. + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. + */ + +declare global { + module LH { + module Budgets { + export interface Json { + budgets: Array; + } + + export interface Budget { + resourceCounts?: Array; + resourceSizes?: Array; + timings?: Array; + } + + export interface ResourceBudget { + resourceType: string; + budget: number; + } + + export interface TimingBudget { + metric: string; + budget: number; + tolerance?: number; + } + } + } +} + +// empty export to keep file a module +export { } diff --git a/types/externs.d.ts b/types/externs.d.ts index 8de82bbb0252..d3a2f6c59a88 100644 --- a/types/externs.d.ts +++ b/types/externs.d.ts @@ -119,6 +119,8 @@ declare global { channel?: string /** Precomputed lantern estimates to use instead of observed analysis. */ precomputedLanternData?: PrecomputedLanternData | null; + // Flag indicating the path to the budgets.json file for LightWallet + budgetsPath?: string | null; } /** From 133a45f3596040e47d3f6627ccb499c8cd01bcc9 Mon Sep 17 00:00:00 2001 From: Katie Hempenius Date: Fri, 19 Apr 2019 11:48:25 -0400 Subject: [PATCH 2/9] Update budgets validation --- lighthouse-core/config/budgets.js | 73 ++++++++++++--------- lighthouse-core/test/config/budgets-test.js | 18 ++++- types/budgets.d.ts | 2 +- 3 files changed, 59 insertions(+), 34 deletions(-) diff --git a/lighthouse-core/config/budgets.js b/lighthouse-core/config/budgets.js index 2f30b0db0c7d..9ce7ed6ed411 100644 --- a/lighthouse-core/config/budgets.js +++ b/lighthouse-core/config/budgets.js @@ -1,16 +1,33 @@ /** - * @license Copyright 2016 Google Inc. All Rights Reserved. + * @license Copyright 2019 Google Inc. All Rights Reserved. * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. */ 'use strict'; class Budgets { +/** + * Asserts that obj has no own properties, throwing a nice error message if it does. + * Plugin and object name are included for nicer logging. + * @param {Record} obj + * @param {string} objectName + */ + static assertNoExcessProperties(obj, objectName) { + const invalidKeys = Object.keys(obj); + if (invalidKeys.length > 0) { + const keys = invalidKeys.join(', '); + throw new Error(`${objectName} has unrecognized properties: [${keys}]`); + } + } + /** * @param {LH.Budgets.ResourceBudget} resourceBudget * @return {LH.Budgets.ResourceBudget} */ static validateResourceBudget(resourceBudget) { + const {resourceType, budget, ...invalidRest} = resourceBudget; + Budgets.assertNoExcessProperties(invalidRest, 'Resource Budget'); + const validResourceTypes = [ 'total', 'document', @@ -29,7 +46,10 @@ class Budgets { if (isNaN(resourceBudget.budget)) { throw new Error('Invalid budget: ${resourceBudget.budget}'); } - return resourceBudget; + return { + resourceType, + budget, + }; } /** @@ -37,6 +57,9 @@ class Budgets { * @return {LH.Budgets.TimingBudget} */ static validateTimingBudget(timingBudget) { + const {metric, budget, tolerance, ...invalidRest} = timingBudget; + Budgets.assertNoExcessProperties(invalidRest, 'Timing Budget'); + const validTimingMetrics = [ 'firstContentfulPaint', 'firstCpuIdle', @@ -54,7 +77,11 @@ class Budgets { if (timingBudget.tolerance !== undefined && isNaN(timingBudget.tolerance)) { throw new Error('Invalid tolerance: ${timingBudget.tolerance}'); } - return timingBudget; + return { + metric, + budget, + tolerance, + }; } /** @@ -69,48 +96,32 @@ class Budgets { budgetsJSON.budgets.forEach((b) => { /** @type {LH.Budgets.Budget} */ const budget = {}; - const validBudgetProperties = ['resourceSizes', 'resourceCounts', 'timings']; - for (const prop in b) { - if (!validBudgetProperties.includes(prop)) { - throw new Error(`Unsupported budget property: ${prop}. \n` + - `Valid properties are: ${validBudgetProperties.join(', ')}`); - } - } + + const {resourceSizes, resourceCounts, timings, ...invalidRest} = b; + Budgets.assertNoExcessProperties(invalidRest, 'Budget'); + if (b.resourceSizes !== undefined) { budget.resourceSizes = b.resourceSizes.map((r) => { - return Budgets.validateResourceBudget({ - resourceType: r.resourceType, - budget: r.budget, - }); + return Budgets.validateResourceBudget(r); }); } if (b.resourceCounts !== undefined) { budget.resourceCounts = b.resourceCounts.map((r) => { - return Budgets.validateResourceBudget({ - resourceType: r.resourceType, - budget: r.budget, - }); + return Budgets.validateResourceBudget(r); }); } if (b.timings !== undefined) { budget.timings = b.timings.map((t) => { - if (t.tolerance !== undefined) { - return Budgets.validateTimingBudget({ - metric: t.metric, - budget: t.budget, - tolerance: t.tolerance, - }); - } else { - return Budgets.validateTimingBudget({ - metric: t.metric, - budget: t.budget, - }); - } + return Budgets.validateTimingBudget(t); }); } - this.budgets.push(budget); + this.budgets.push({ + resourceSizes, + resourceCounts, + timings, + }); }); } } diff --git a/lighthouse-core/test/config/budgets-test.js b/lighthouse-core/test/config/budgets-test.js index e4843e0e813d..30f752ac01d2 100644 --- a/lighthouse-core/test/config/budgets-test.js +++ b/lighthouse-core/test/config/budgets-test.js @@ -1,5 +1,5 @@ /** - * @license Copyright 2016 Google Inc. All Rights Reserved. + * @license Copyright 2019 Google Inc. All Rights Reserved. * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. */ @@ -55,22 +55,28 @@ describe('Budgets', () => { const budgets = new Budgets(budgetsJson); assert.equal(budgets.budgets.length, 2); + // Sets resources sizes correctly assert.equal(budgets.budgets[0].resourceSizes.length, 2); assert.equal(budgets.budgets[0].resourceSizes[0].resourceType, 'script'); assert.equal(budgets.budgets[0].resourceSizes[0].budget, 123); + // Sets resource counts correctly assert.equal(budgets.budgets[0].resourceCounts.length, 2); assert.equal(budgets.budgets[0].resourceCounts[0].resourceType, 'total'); assert.equal(budgets.budgets[0].resourceCounts[0].budget, 100); + // Sets timings correctly assert.equal(budgets.budgets[0].timings.length, 2); assert.equal(budgets.budgets[0].timings[1].metric, 'firstContentfulPaint'); assert.equal(budgets.budgets[0].timings[1].budget, 1000); assert.equal(budgets.budgets[0].timings[1].tolerance, 500); + + // Does not set unsupplied budgets + assert.equal(budgets.budgets[1].timings, null); }); it('throws error if an unsupported budget property is used', () => { budgetsJson.budgets[0].sizes = []; - assert.throws(_ => new Budgets(budgetsJson), /Unsupported budget property/); + assert.throws(_ => new Budgets(budgetsJson), /[sizes]/); }); describe('resource budget validation', () => { it('throws when an invalid resource type is supplied', () => { @@ -81,6 +87,10 @@ describe('Budgets', () => { budgetsJson.budgets[0].resourceSizes[0].budget = '100 MB'; assert.throws(_ => new Budgets(budgetsJson), /Invalid budget/); }); + it('throws when an invalid property is supplied', () => { + budgetsJson.budgets[0].resourceSizes[0].browser = 'Chrome'; + assert.throws(_ => new Budgets(budgetsJson), /[browser]/); + }); }); describe('timing budget validation', () => { it('throws when an invalid metric is supplied', () => { @@ -95,5 +105,9 @@ describe('Budgets', () => { budgetsJson.budgets[0].timings[0].tolerance = '100ms'; assert.throws(_ => new Budgets(budgetsJson), /Invalid tolerance/); }); + it('throws when an invalid property is supplied', () => { + budgetsJson.budgets[0].timings[0].device = 'Phone'; + assert.throws(_ => new Budgets(budgetsJson), /[device]/); + }); }); }); diff --git a/types/budgets.d.ts b/types/budgets.d.ts index ea29fcb0a29d..76e5454b96d2 100644 --- a/types/budgets.d.ts +++ b/types/budgets.d.ts @@ -1,5 +1,5 @@ /** - * @license Copyright 2018 Google Inc. All Rights Reserved. + * @license Copyright 2019 Google Inc. All Rights Reserved. * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. */ From f680fb647f19966025b857b6cfa9a15ba9fff248 Mon Sep 17 00:00:00 2001 From: Katie Hempenius Date: Fri, 19 Apr 2019 13:11:07 -0400 Subject: [PATCH 3/9] Add Budgets to SharedFlagSettings --- lighthouse-cli/bin.js | 13 +++++++++++++ .../test/cli/__snapshots__/index-test.js.snap | 4 ++-- lighthouse-cli/test/cli/bin-test.js | 11 +++++++++++ lighthouse-core/config/constants.js | 2 +- lighthouse-core/runner.js | 2 ++ lighthouse-core/test/results/sample_v2.json | 2 +- types/externs.d.ts | 6 ++++-- 7 files changed, 34 insertions(+), 6 deletions(-) diff --git a/lighthouse-cli/bin.js b/lighthouse-cli/bin.js index b442e85496f8..588a2d77c688 100644 --- a/lighthouse-cli/bin.js +++ b/lighthouse-cli/bin.js @@ -26,6 +26,7 @@ const printer = require('./printer.js'); const getFlags = require('./cli-flags.js').getFlags; const runLighthouse = require('./run.js').runLighthouse; const generateConfig = require('../lighthouse-core/index.js').generateConfig; +const Budgets = require('../lighthouse-core/config/budgets.js'); const log = require('lighthouse-logger'); const pkg = require('../package.json'); @@ -77,6 +78,18 @@ async function begin() { configJson = require(`../lighthouse-core/config/${cliFlags.preset}-config.js`); } + if (cliFlags.budgetsPath) { + cliFlags.budgetsPath = path.resolve(process.cwd(), cliFlags.budgetsPath); + try { + const budgetsJsonStr = fs.readFileSync(cliFlags.budgetsPath, 'utf8'); + /** @type {LH.Budgets.Json} */ + const budgets = new Budgets(JSON.parse(budgetsJsonStr)); + cliFlags.budgetsJSON = budgets; + } catch (err) { + throw new Error(err); + } + } + // set logging preferences cliFlags.logLevel = 'info'; if (cliFlags.verbose) { diff --git a/lighthouse-cli/test/cli/__snapshots__/index-test.js.snap b/lighthouse-cli/test/cli/__snapshots__/index-test.js.snap index 4a8496fc5fb4..4c7d308eb8f8 100644 --- a/lighthouse-cli/test/cli/__snapshots__/index-test.js.snap +++ b/lighthouse-cli/test/cli/__snapshots__/index-test.js.snap @@ -1198,7 +1198,7 @@ Object { "additionalTraceCategories": null, "auditMode": false, "blockedUrlPatterns": null, - "budgetsPath": null, + "budgetsJSON": null, "channel": "cli", "disableStorageReset": false, "emulatedFormFactor": "mobile", @@ -1328,7 +1328,7 @@ Object { "additionalTraceCategories": null, "auditMode": true, "blockedUrlPatterns": null, - "budgetsPath": null, + "budgetsJSON": null, "channel": "cli", "disableStorageReset": false, "emulatedFormFactor": "mobile", diff --git a/lighthouse-cli/test/cli/bin-test.js b/lighthouse-cli/test/cli/bin-test.js index 70ef3e9544dd..8a7f3b4aaf00 100644 --- a/lighthouse-cli/test/cli/bin-test.js +++ b/lighthouse-cli/test/cli/bin-test.js @@ -99,6 +99,17 @@ describe('CLI bin', function() { }); }); + describe('budgets', () => { + it('should load the config from the path', async () => { + const budgetsPath = require.resolve('../../../lighthouse-core/test/fixtures/budgets.json'); + cliFlags = {...cliFlags, budgetsPath: budgetsPath}; + const actualBudgetsJSON = require(budgetsPath); + await bin.begin(); + + expect(getRunLighthouseArgs()[1].budgetsJSON).toEqual(actualBudgetsJSON); + }); + }); + describe('logging', () => { it('should have info by default', async () => { await bin.begin(); diff --git a/lighthouse-core/config/constants.js b/lighthouse-core/config/constants.js index b0e0b58faeae..e527112a3332 100644 --- a/lighthouse-core/config/constants.js +++ b/lighthouse-core/config/constants.js @@ -52,7 +52,7 @@ const defaultSettings = { disableStorageReset: false, emulatedFormFactor: 'mobile', channel: 'node', - budgetsPath: null, + budgetsJSON: null, // the following settings have no defaults but we still want ensure that `key in settings` // in config will work in a typechecked way diff --git a/lighthouse-core/runner.js b/lighthouse-core/runner.js index 1cbbf12ce1cd..bcfe24039679 100644 --- a/lighthouse-core/runner.js +++ b/lighthouse-core/runner.js @@ -234,6 +234,8 @@ class Runner { gatherMode: undefined, auditMode: undefined, output: undefined, + budgetsPath: undefined, + budgetsJSON: undefined, }; const normalizedGatherSettings = Object.assign({}, artifacts.settings, overrides); const normalizedAuditSettings = Object.assign({}, settings, overrides); diff --git a/lighthouse-core/test/results/sample_v2.json b/lighthouse-core/test/results/sample_v2.json index 608c071eb830..d5294f589ac6 100644 --- a/lighthouse-core/test/results/sample_v2.json +++ b/lighthouse-core/test/results/sample_v2.json @@ -2963,7 +2963,7 @@ "disableStorageReset": false, "emulatedFormFactor": "mobile", "channel": "cli", - "budgetsPath": null, + "budgetsJSON": null, "locale": "en-US", "blockedUrlPatterns": null, "additionalTraceCategories": null, diff --git a/types/externs.d.ts b/types/externs.d.ts index d3a2f6c59a88..35fcaaa9d1e9 100644 --- a/types/externs.d.ts +++ b/types/externs.d.ts @@ -119,8 +119,8 @@ declare global { channel?: string /** Precomputed lantern estimates to use instead of observed analysis. */ precomputedLanternData?: PrecomputedLanternData | null; - // Flag indicating the path to the budgets.json file for LightWallet - budgetsPath?: string | null; + /** The budgets.json object for LightWallet. */ + budgetsJSON?: Budgets.Json | null; } /** @@ -171,6 +171,8 @@ declare global { precomputedLanternDataPath?: string; /** Path to the file where precomputed lantern data should be written to. */ lanternDataOutputPath?: string; + /** Path to the budgets.json file for LightWallet */ + budgetsPath?: string | null; // The following are given defaults in cli-flags, so are not optional like in Flags or SharedFlagsSettings. output: OutputMode[]; From bea7d9fb8fbd7f9e73551e755999d69a9657d150 Mon Sep 17 00:00:00 2001 From: Katie Hempenius Date: Fri, 19 Apr 2019 13:18:55 -0400 Subject: [PATCH 4/9] Add budgets.json test fixture --- lighthouse-core/test/fixtures/budgets.json | 47 ++++++++++++++++++++++ 1 file changed, 47 insertions(+) create mode 100644 lighthouse-core/test/fixtures/budgets.json diff --git a/lighthouse-core/test/fixtures/budgets.json b/lighthouse-core/test/fixtures/budgets.json new file mode 100644 index 000000000000..8af6cbfe43eb --- /dev/null +++ b/lighthouse-core/test/fixtures/budgets.json @@ -0,0 +1,47 @@ +{ + "budgets": [ + { + "resourceSizes": [ + { + "resourceType": "script", + "budget": 125 + }, + { + "resourceType": "image", + "budget": 300 + }, + { + "resourceType": "total", + "budget": 500 + }, + { + "resourceType": "thirdParty", + "budget": 200 + } + ], + "timings": [ + { + "metric": "timeToInteractive", + "budget": 5000, + "tolerance": 1000 + }, + { + "metric": "firstMeaningfulPaint", + "budget": 2000 + } + ] + }, + { + "resourceCounts": [ + { + "resourceType": "total", + "budget": 100 + }, + { + "resourceType": "thirdParty", + "budget": 0 + } + ] + } + ] +} \ No newline at end of file From 6f5df6f1bfe9f71f7876439bccf367ac23926821 Mon Sep 17 00:00:00 2001 From: Katie Hempenius Date: Fri, 19 Apr 2019 14:38:23 -0400 Subject: [PATCH 5/9] LightWallet: use snakecase for ids, add links to docs --- lighthouse-core/config/budgets.js | 14 ++++++++------ lighthouse-core/test/config/budgets-test.js | 8 ++++---- lighthouse-core/test/fixtures/budgets.json | 8 ++++---- types/budgets.d.ts | 1 + 4 files changed, 17 insertions(+), 14 deletions(-) diff --git a/lighthouse-core/config/budgets.js b/lighthouse-core/config/budgets.js index 9ce7ed6ed411..4e4087f6c5e5 100644 --- a/lighthouse-core/config/budgets.js +++ b/lighthouse-core/config/budgets.js @@ -37,7 +37,7 @@ class Budgets { 'media', 'font', 'other', - 'thirdParty', + 'third-party', ]; if (!validResourceTypes.includes(resourceBudget.resourceType)) { throw new Error(`Invalid resource type: ${resourceBudget.resourceType}. \n` + @@ -61,11 +61,11 @@ class Budgets { Budgets.assertNoExcessProperties(invalidRest, 'Timing Budget'); const validTimingMetrics = [ - 'firstContentfulPaint', - 'firstCpuIdle', - 'timeToInteractive', - 'firstMeaningfulPaint', - 'estimaatedInputLatency', + 'first-contentful-paint', + 'first-cpu-idle', + 'time-to-interactive', + 'first-meaningful-paint', + 'estimated-input-latency', ]; if (!validTimingMetrics.includes(timingBudget.metric)) { throw new Error(`Invalid timing metric: ${timingBudget.metric}. \n` + @@ -84,6 +84,8 @@ class Budgets { }; } + // More info on the Budgets format: + // https://github.com/GoogleChrome/lighthouse/issues/6053#issuecomment-428385930 /** * @constructor * @implements {LH.Budgets.Json} diff --git a/lighthouse-core/test/config/budgets-test.js b/lighthouse-core/test/config/budgets-test.js index 30f752ac01d2..86fa2914e88e 100644 --- a/lighthouse-core/test/config/budgets-test.js +++ b/lighthouse-core/test/config/budgets-test.js @@ -27,16 +27,16 @@ describe('Budgets', () => { budget: 100, }, { - resourceType: 'thirdParty', + resourceType: 'third-party', budget: 10, }], timings: [{ - metric: 'timeToInteractive', + metric: 'time-to-interactive', budget: 2000, tolerance: 1000, }, { - metric: 'firstContentfulPaint', + metric: 'first-contentful-paint', budget: 1000, tolerance: 500, }], @@ -67,7 +67,7 @@ describe('Budgets', () => { // Sets timings correctly assert.equal(budgets.budgets[0].timings.length, 2); - assert.equal(budgets.budgets[0].timings[1].metric, 'firstContentfulPaint'); + assert.equal(budgets.budgets[0].timings[1].metric, 'first-contentful-paint'); assert.equal(budgets.budgets[0].timings[1].budget, 1000); assert.equal(budgets.budgets[0].timings[1].tolerance, 500); diff --git a/lighthouse-core/test/fixtures/budgets.json b/lighthouse-core/test/fixtures/budgets.json index 8af6cbfe43eb..878ee6461a99 100644 --- a/lighthouse-core/test/fixtures/budgets.json +++ b/lighthouse-core/test/fixtures/budgets.json @@ -15,18 +15,18 @@ "budget": 500 }, { - "resourceType": "thirdParty", + "resourceType": "third-party", "budget": 200 } ], "timings": [ { - "metric": "timeToInteractive", + "metric": "time-to-interactive", "budget": 5000, "tolerance": 1000 }, { - "metric": "firstMeaningfulPaint", + "metric": "first-meaningful-paint", "budget": 2000 } ] @@ -38,7 +38,7 @@ "budget": 100 }, { - "resourceType": "thirdParty", + "resourceType": "third-party", "budget": 0 } ] diff --git a/types/budgets.d.ts b/types/budgets.d.ts index 76e5454b96d2..1b501fa1c346 100644 --- a/types/budgets.d.ts +++ b/types/budgets.d.ts @@ -7,6 +7,7 @@ declare global { module LH { module Budgets { + // More info: https://github.com/GoogleChrome/lighthouse/issues/6053#issuecomment-428385930 export interface Json { budgets: Array; } From 49275daefdc0a3314257f517f456ba12c46a2d61 Mon Sep 17 00:00:00 2001 From: Katie Hempenius Date: Sat, 20 Apr 2019 12:56:25 -0400 Subject: [PATCH 6/9] Change budgets.json to array; code cleanup --- lighthouse-cli/bin.js | 4 +- lighthouse-cli/cli-flags.js | 4 +- lighthouse-cli/test/cli/bin-test.js | 4 +- lighthouse-core/config/budgets.js | 33 ++--- lighthouse-core/test/config/budgets-test.js | 126 +++++++++--------- lighthouse-core/test/fixtures/budgets.json | 47 ------- .../test/fixtures/simple-budgets.json | 45 +++++++ types/{budgets.d.ts => budgets-json.d.ts} | 8 +- types/externs.d.ts | 2 +- 9 files changed, 134 insertions(+), 139 deletions(-) delete mode 100644 lighthouse-core/test/fixtures/budgets.json create mode 100644 lighthouse-core/test/fixtures/simple-budgets.json rename types/{budgets.d.ts => budgets-json.d.ts} (84%) diff --git a/lighthouse-cli/bin.js b/lighthouse-cli/bin.js index 588a2d77c688..333ce768a0f8 100644 --- a/lighthouse-cli/bin.js +++ b/lighthouse-cli/bin.js @@ -82,8 +82,8 @@ async function begin() { cliFlags.budgetsPath = path.resolve(process.cwd(), cliFlags.budgetsPath); try { const budgetsJsonStr = fs.readFileSync(cliFlags.budgetsPath, 'utf8'); - /** @type {LH.Budgets.Json} */ - const budgets = new Budgets(JSON.parse(budgetsJsonStr)); + /** @type {Array} */ + const budgets = Budgets.parseBudgets(JSON.parse(budgetsJsonStr)); cliFlags.budgetsJSON = budgets; } catch (err) { throw new Error(err); diff --git a/lighthouse-cli/cli-flags.js b/lighthouse-cli/cli-flags.js index 61bcdccfcc6f..2396d16c86b9 100644 --- a/lighthouse-cli/cli-flags.js +++ b/lighthouse-cli/cli-flags.js @@ -59,7 +59,7 @@ function getFlags(manualArgv) { 'save-assets', 'list-all-audits', 'list-trace-categories', 'print-config', 'additional-trace-categories', 'config-path', 'preset', 'chrome-flags', 'port', 'hostname', 'emulated-form-factor', 'max-wait-for-load', 'enable-error-reporting', 'gather-mode', 'audit-mode', - 'only-audits', 'only-categories', 'skip-audits', + 'only-audits', 'only-categories', 'skip-audits', 'budgets-path', ], 'Configuration:') .describe({ @@ -88,6 +88,7 @@ function getFlags(manualArgv) { 'Additional categories to capture with the trace (comma-delimited).', 'config-path': `The path to the config JSON. An example config file: lighthouse-core/config/lr-desktop-config.js`, + 'budgets-path': `The path to the budgets.json file for LightWallet.`, 'preset': `Use a built-in configuration. WARNING: If the --config-path flag is provided, this preset will be ignored.`, 'chrome-flags': @@ -141,6 +142,7 @@ function getFlags(manualArgv) { .string('channel') .string('precomputedLanternDataPath') .string('lanternDataOutputPath') + .string('budgetsPath') // default values .default('chrome-flags', '') diff --git a/lighthouse-cli/test/cli/bin-test.js b/lighthouse-cli/test/cli/bin-test.js index 8a7f3b4aaf00..d377ef951492 100644 --- a/lighthouse-cli/test/cli/bin-test.js +++ b/lighthouse-cli/test/cli/bin-test.js @@ -101,8 +101,8 @@ describe('CLI bin', function() { describe('budgets', () => { it('should load the config from the path', async () => { - const budgetsPath = require.resolve('../../../lighthouse-core/test/fixtures/budgets.json'); - cliFlags = {...cliFlags, budgetsPath: budgetsPath}; + const budgetsPath = '../../../lighthouse-core/test/fixtures/simple-budgets.json'; + cliFlags = {...cliFlags, budgetsPath: require.resolve(budgetsPath)}; const actualBudgetsJSON = require(budgetsPath); await bin.begin(); diff --git a/lighthouse-core/config/budgets.js b/lighthouse-core/config/budgets.js index 4e4087f6c5e5..89090d1117b5 100644 --- a/lighthouse-core/config/budgets.js +++ b/lighthouse-core/config/budgets.js @@ -21,8 +21,8 @@ class Budgets { } /** - * @param {LH.Budgets.ResourceBudget} resourceBudget - * @return {LH.Budgets.ResourceBudget} + * @param {LH.BudgetsJSON.ResourceBudget} resourceBudget + * @return {LH.BudgetsJSON.ResourceBudget} */ static validateResourceBudget(resourceBudget) { const {resourceType, budget, ...invalidRest} = resourceBudget; @@ -53,8 +53,8 @@ class Budgets { } /** - * @param {LH.Budgets.TimingBudget} timingBudget - * @return {LH.Budgets.TimingBudget} + * @param {LH.BudgetsJSON.TimingBudget} timingBudget + * @return {LH.BudgetsJSON.TimingBudget} */ static validateTimingBudget(timingBudget) { const {metric, budget, tolerance, ...invalidRest} = timingBudget; @@ -84,19 +84,19 @@ class Budgets { }; } - // More info on the Budgets format: - // https://github.com/GoogleChrome/lighthouse/issues/6053#issuecomment-428385930 + /** More info on the Budgets format: + * https://github.com/GoogleChrome/lighthouse/issues/6053#issuecomment-428385930 + * / /** - * @constructor - * @implements {LH.Budgets.Json} - * @param {LH.Budgets.Json} budgetsJSON - */ - constructor(budgetsJSON) { - /** @type {Array} */ - this.budgets = []; + * @param {Array} budgetsJSON + * @return {Array} + */ + static parseBudgets(budgetsJSON) { + /** @type {Array} */ + const budgets = []; - budgetsJSON.budgets.forEach((b) => { - /** @type {LH.Budgets.Budget} */ + budgetsJSON.forEach((b) => { + /** @type {LH.BudgetsJSON.Budget} */ const budget = {}; const {resourceSizes, resourceCounts, timings, ...invalidRest} = b; @@ -119,12 +119,13 @@ class Budgets { return Budgets.validateTimingBudget(t); }); } - this.budgets.push({ + budgets.push({ resourceSizes, resourceCounts, timings, }); }); + return budgets; } } diff --git a/lighthouse-core/test/config/budgets-test.js b/lighthouse-core/test/config/budgets-test.js index 86fa2914e88e..93fa4ab44684 100644 --- a/lighthouse-core/test/config/budgets-test.js +++ b/lighthouse-core/test/config/budgets-test.js @@ -12,102 +12,100 @@ const assert = require('assert'); describe('Budgets', () => { let budgetsJson; beforeEach(() => { - budgetsJson = { - budgets: [{ - resourceSizes: [{ - resourceType: 'script', - budget: 123, - }, - { - resourceType: 'image', - budget: 456, - }], - resourceCounts: [{ - resourceType: 'total', - budget: 100, - }, - { - resourceType: 'third-party', - budget: 10, - }], - timings: [{ - metric: 'time-to-interactive', - budget: 2000, - tolerance: 1000, - }, - { - metric: 'first-contentful-paint', - budget: 1000, - tolerance: 500, - }], + budgetsJson = [{ + resourceSizes: [{ + resourceType: 'script', + budget: 123, + }, + { + resourceType: 'image', + budget: 456, + }], + resourceCounts: [{ + resourceType: 'total', + budget: 100, + }, + { + resourceType: 'third-party', + budget: 10, + }], + timings: [{ + metric: 'time-to-interactive', + budget: 2000, + tolerance: 1000, }, { - resourceSizes: [ - { - resourceType: 'script', - budget: 1000, - }, - ], + metric: 'first-contentful-paint', + budget: 1000, + tolerance: 500, }], - }; + }, + { + resourceSizes: [ + { + resourceType: 'script', + budget: 1000, + }, + ], + }]; }); it('initializes correctly', () => { - const budgets = new Budgets(budgetsJson); - assert.equal(budgets.budgets.length, 2); + const budgets = Budgets.parseBudgets(budgetsJson); + assert.equal(budgets.length, 2); // Sets resources sizes correctly - assert.equal(budgets.budgets[0].resourceSizes.length, 2); - assert.equal(budgets.budgets[0].resourceSizes[0].resourceType, 'script'); - assert.equal(budgets.budgets[0].resourceSizes[0].budget, 123); + assert.equal(budgets[0].resourceSizes.length, 2); + assert.equal(budgets[0].resourceSizes[0].resourceType, 'script'); + assert.equal(budgets[0].resourceSizes[0].budget, 123); // Sets resource counts correctly - assert.equal(budgets.budgets[0].resourceCounts.length, 2); - assert.equal(budgets.budgets[0].resourceCounts[0].resourceType, 'total'); - assert.equal(budgets.budgets[0].resourceCounts[0].budget, 100); + assert.equal(budgets[0].resourceCounts.length, 2); + assert.equal(budgets[0].resourceCounts[0].resourceType, 'total'); + assert.equal(budgets[0].resourceCounts[0].budget, 100); // Sets timings correctly - assert.equal(budgets.budgets[0].timings.length, 2); - assert.equal(budgets.budgets[0].timings[1].metric, 'first-contentful-paint'); - assert.equal(budgets.budgets[0].timings[1].budget, 1000); - assert.equal(budgets.budgets[0].timings[1].tolerance, 500); + assert.equal(budgets[0].timings.length, 2); + assert.equal(budgets[0].timings[1].metric, 'first-contentful-paint'); + assert.equal(budgets[0].timings[1].budget, 1000); + assert.equal(budgets[0].timings[1].tolerance, 500); // Does not set unsupplied budgets - assert.equal(budgets.budgets[1].timings, null); + assert.equal(budgets[1].timings, null); }); it('throws error if an unsupported budget property is used', () => { - budgetsJson.budgets[0].sizes = []; - assert.throws(_ => new Budgets(budgetsJson), /[sizes]/); + budgetsJson[0].sizes = []; + assert.throws(_ => Budgets.parseBudgets(budgetsJson), /[sizes]/); }); describe('resource budget validation', () => { it('throws when an invalid resource type is supplied', () => { - budgetsJson.budgets[0].resourceSizes[0].resourceType = 'movies'; - assert.throws(_ => new Budgets(budgetsJson), /Invalid resource type/); + budgetsJson[0].resourceSizes[0].resourceType = 'movies'; + assert.throws(_ => Budgets.parseBudgets(budgetsJson), /Invalid resource type/); }); it('throws when an invalid budget is supplied', () => { - budgetsJson.budgets[0].resourceSizes[0].budget = '100 MB'; - assert.throws(_ => new Budgets(budgetsJson), /Invalid budget/); + budgetsJson[0].resourceSizes[0].budget = '100 MB'; + assert.throws(_ => Budgets.parseBudgets(budgetsJson), /Invalid budget/); }); it('throws when an invalid property is supplied', () => { - budgetsJson.budgets[0].resourceSizes[0].browser = 'Chrome'; - assert.throws(_ => new Budgets(budgetsJson), /[browser]/); + budgetsJson[0].resourceSizes[0].browser = 'Chrome'; + assert.throws(_ => Budgets.parseBudgets(budgetsJson), /[browser]/); }); }); describe('timing budget validation', () => { it('throws when an invalid metric is supplied', () => { - budgetsJson.budgets[0].timings[0].metric = 'lastMeaningfulPaint'; - assert.throws(_ => new Budgets(budgetsJson), /Invalid timing metric/); + budgetsJson[0].timings[0].metric = 'lastMeaningfulPaint'; + assert.throws(_ => Budgets.parseBudgets(budgetsJson), /Invalid timing metric/); }); it('throws when an invalid budget is supplied', () => { - budgetsJson.budgets[0].timings[0].budget = '100KB'; - assert.throws(_ => new Budgets(budgetsJson), /Invalid budget/); + budgetsJson[0].timings[0].budget = '100KB'; + assert.throws(_ => Budgets.parseBudgets(budgetsJson), /Invalid budget/); }); it('throws when an invalid tolerance is supplied', () => { - budgetsJson.budgets[0].timings[0].tolerance = '100ms'; - assert.throws(_ => new Budgets(budgetsJson), /Invalid tolerance/); + budgetsJson[0].timings[0].tolerance = '100ms'; + assert.throws(_ => Budgets.parseBudgets(budgetsJson), /Invalid tolerance/); }); it('throws when an invalid property is supplied', () => { - budgetsJson.budgets[0].timings[0].device = 'Phone'; - assert.throws(_ => new Budgets(budgetsJson), /[device]/); + budgetsJson[0].timings[0].device = 'Phone'; + assert.throws(_ => Budgets.parseBudgets(budgetsJson), /[device]/); }); }); }); diff --git a/lighthouse-core/test/fixtures/budgets.json b/lighthouse-core/test/fixtures/budgets.json deleted file mode 100644 index 878ee6461a99..000000000000 --- a/lighthouse-core/test/fixtures/budgets.json +++ /dev/null @@ -1,47 +0,0 @@ -{ - "budgets": [ - { - "resourceSizes": [ - { - "resourceType": "script", - "budget": 125 - }, - { - "resourceType": "image", - "budget": 300 - }, - { - "resourceType": "total", - "budget": 500 - }, - { - "resourceType": "third-party", - "budget": 200 - } - ], - "timings": [ - { - "metric": "time-to-interactive", - "budget": 5000, - "tolerance": 1000 - }, - { - "metric": "first-meaningful-paint", - "budget": 2000 - } - ] - }, - { - "resourceCounts": [ - { - "resourceType": "total", - "budget": 100 - }, - { - "resourceType": "third-party", - "budget": 0 - } - ] - } - ] -} \ No newline at end of file diff --git a/lighthouse-core/test/fixtures/simple-budgets.json b/lighthouse-core/test/fixtures/simple-budgets.json new file mode 100644 index 000000000000..07fe2c73e8e2 --- /dev/null +++ b/lighthouse-core/test/fixtures/simple-budgets.json @@ -0,0 +1,45 @@ +[ + { + "resourceSizes": [ + { + "resourceType": "script", + "budget": 125 + }, + { + "resourceType": "image", + "budget": 300 + }, + { + "resourceType": "total", + "budget": 500 + }, + { + "resourceType": "third-party", + "budget": 200 + } + ], + "timings": [ + { + "metric": "time-to-interactive", + "budget": 5000, + "tolerance": 1000 + }, + { + "metric": "first-meaningful-paint", + "budget": 2000 + } + ] + }, + { + "resourceCounts": [ + { + "resourceType": "total", + "budget": 100 + }, + { + "resourceType": "third-party", + "budget": 0 + } + ] + } +] \ No newline at end of file diff --git a/types/budgets.d.ts b/types/budgets-json.d.ts similarity index 84% rename from types/budgets.d.ts rename to types/budgets-json.d.ts index 1b501fa1c346..37681bb9636b 100644 --- a/types/budgets.d.ts +++ b/types/budgets-json.d.ts @@ -6,12 +6,8 @@ declare global { module LH { - module Budgets { - // More info: https://github.com/GoogleChrome/lighthouse/issues/6053#issuecomment-428385930 - export interface Json { - budgets: Array; - } - + module BudgetsJSON { + /** More info: https://github.com/GoogleChrome/lighthouse/issues/6053#issuecomment-428385930 */ export interface Budget { resourceCounts?: Array; resourceSizes?: Array; diff --git a/types/externs.d.ts b/types/externs.d.ts index 35fcaaa9d1e9..f688ba676208 100644 --- a/types/externs.d.ts +++ b/types/externs.d.ts @@ -120,7 +120,7 @@ declare global { /** Precomputed lantern estimates to use instead of observed analysis. */ precomputedLanternData?: PrecomputedLanternData | null; /** The budgets.json object for LightWallet. */ - budgetsJSON?: Budgets.Json | null; + budgetsJSON?: Array | null; } /** From bab413dc4a9a7d42a782c9f3b867f61dc51eb885 Mon Sep 17 00:00:00 2001 From: Katie Hempenius Date: Mon, 22 Apr 2019 14:00:24 -0400 Subject: [PATCH 7/9] Initialize budgets in config.js --- lighthouse-cli/bin.js | 6 ++---- lighthouse-core/config/budgets.js | 2 +- lighthouse-core/config/config.js | 4 ++++ lighthouse-core/test/config/budgets-test.js | 18 +++++++++--------- lighthouse-core/test/config/config-test.js | 5 +++++ 5 files changed, 21 insertions(+), 14 deletions(-) diff --git a/lighthouse-cli/bin.js b/lighthouse-cli/bin.js index 333ce768a0f8..66d81ce9dc07 100644 --- a/lighthouse-cli/bin.js +++ b/lighthouse-cli/bin.js @@ -26,7 +26,6 @@ const printer = require('./printer.js'); const getFlags = require('./cli-flags.js').getFlags; const runLighthouse = require('./run.js').runLighthouse; const generateConfig = require('../lighthouse-core/index.js').generateConfig; -const Budgets = require('../lighthouse-core/config/budgets.js'); const log = require('lighthouse-logger'); const pkg = require('../package.json'); @@ -81,10 +80,9 @@ async function begin() { if (cliFlags.budgetsPath) { cliFlags.budgetsPath = path.resolve(process.cwd(), cliFlags.budgetsPath); try { - const budgetsJsonStr = fs.readFileSync(cliFlags.budgetsPath, 'utf8'); /** @type {Array} */ - const budgets = Budgets.parseBudgets(JSON.parse(budgetsJsonStr)); - cliFlags.budgetsJSON = budgets; + const budgetsJsonStr = JSON.parse(fs.readFileSync(cliFlags.budgetsPath, 'utf8')); + cliFlags.budgetsJSON = budgetsJsonStr; } catch (err) { throw new Error(err); } diff --git a/lighthouse-core/config/budgets.js b/lighthouse-core/config/budgets.js index 89090d1117b5..b28cbbe4150e 100644 --- a/lighthouse-core/config/budgets.js +++ b/lighthouse-core/config/budgets.js @@ -91,7 +91,7 @@ class Budgets { * @param {Array} budgetsJSON * @return {Array} */ - static parseBudgets(budgetsJSON) { + static initializeBudgets(budgetsJSON) { /** @type {Array} */ const budgets = []; diff --git a/lighthouse-core/config/config.js b/lighthouse-core/config/config.js index b7fb5f7b9101..be2296a58246 100644 --- a/lighthouse-core/config/config.js +++ b/lighthouse-core/config/config.js @@ -17,6 +17,7 @@ const path = require('path'); const Audit = require('../audits/audit.js'); const Runner = require('../runner.js'); const ConfigPlugin = require('./config-plugin.js'); +const Budgets = require('./budgets.js'); /** @typedef {typeof import('../gather/gatherers/gatherer.js')} GathererConstructor */ /** @typedef {InstanceType} Gatherer */ @@ -512,6 +513,9 @@ class Config { // Override any applicable settings with CLI flags const settingsWithFlags = merge(settingWithDefaults || {}, cleanFlagsForSettings(flags), true); + if (settingsWithFlags.budgetsJSON) { + settingsWithFlags.budgetsJSON = Budgets.initializeBudgets(settingsWithFlags.budgetsJSON); + } // Locale is special and comes only from flags/settings/lookupLocale. settingsWithFlags.locale = locale; diff --git a/lighthouse-core/test/config/budgets-test.js b/lighthouse-core/test/config/budgets-test.js index 93fa4ab44684..8f33fd791d26 100644 --- a/lighthouse-core/test/config/budgets-test.js +++ b/lighthouse-core/test/config/budgets-test.js @@ -50,7 +50,7 @@ describe('Budgets', () => { }]; }); it('initializes correctly', () => { - const budgets = Budgets.parseBudgets(budgetsJson); + const budgets = Budgets.initializeBudgets(budgetsJson); assert.equal(budgets.length, 2); // Sets resources sizes correctly @@ -74,38 +74,38 @@ describe('Budgets', () => { }); it('throws error if an unsupported budget property is used', () => { budgetsJson[0].sizes = []; - assert.throws(_ => Budgets.parseBudgets(budgetsJson), /[sizes]/); + assert.throws(_ => Budgets.initializeBudgets(budgetsJson), /[sizes]/); }); describe('resource budget validation', () => { it('throws when an invalid resource type is supplied', () => { budgetsJson[0].resourceSizes[0].resourceType = 'movies'; - assert.throws(_ => Budgets.parseBudgets(budgetsJson), /Invalid resource type/); + assert.throws(_ => Budgets.initializeBudgets(budgetsJson), /Invalid resource type/); }); it('throws when an invalid budget is supplied', () => { budgetsJson[0].resourceSizes[0].budget = '100 MB'; - assert.throws(_ => Budgets.parseBudgets(budgetsJson), /Invalid budget/); + assert.throws(_ => Budgets.initializeBudgets(budgetsJson), /Invalid budget/); }); it('throws when an invalid property is supplied', () => { budgetsJson[0].resourceSizes[0].browser = 'Chrome'; - assert.throws(_ => Budgets.parseBudgets(budgetsJson), /[browser]/); + assert.throws(_ => Budgets.initializeBudgets(budgetsJson), /[browser]/); }); }); describe('timing budget validation', () => { it('throws when an invalid metric is supplied', () => { budgetsJson[0].timings[0].metric = 'lastMeaningfulPaint'; - assert.throws(_ => Budgets.parseBudgets(budgetsJson), /Invalid timing metric/); + assert.throws(_ => Budgets.initializeBudgets(budgetsJson), /Invalid timing metric/); }); it('throws when an invalid budget is supplied', () => { budgetsJson[0].timings[0].budget = '100KB'; - assert.throws(_ => Budgets.parseBudgets(budgetsJson), /Invalid budget/); + assert.throws(_ => Budgets.initializeBudgets(budgetsJson), /Invalid budget/); }); it('throws when an invalid tolerance is supplied', () => { budgetsJson[0].timings[0].tolerance = '100ms'; - assert.throws(_ => Budgets.parseBudgets(budgetsJson), /Invalid tolerance/); + assert.throws(_ => Budgets.initializeBudgets(budgetsJson), /Invalid tolerance/); }); it('throws when an invalid property is supplied', () => { budgetsJson[0].timings[0].device = 'Phone'; - assert.throws(_ => Budgets.parseBudgets(budgetsJson), /[device]/); + assert.throws(_ => Budgets.initializeBudgets(budgetsJson), /[device]/); }); }); }); diff --git a/lighthouse-core/test/config/config-test.js b/lighthouse-core/test/config/config-test.js index efeefb736485..11ff2e939bbf 100644 --- a/lighthouse-core/test/config/config-test.js +++ b/lighthouse-core/test/config/config-test.js @@ -741,6 +741,11 @@ describe('Config', () => { assert.strictEqual(config.categories['lighthouse-plugin-simple'].title, 'Simple'); }); + it('should initialize budgetsJSON', () => { + assert.throws(() => new Config({settings: {budgetsJSON: ['invalid123']}}), + /Budget has unrecognized properties/); + }); + it('should load plugins from the config and from passed-in flags', () => { const baseConfigJson = { audits: ['installable-manifest'], From 616c71aec122d41fc2344eea427dab3860ce8370 Mon Sep 17 00:00:00 2001 From: Katie Hempenius Date: Tue, 23 Apr 2019 00:40:34 -0400 Subject: [PATCH 8/9] Rename "budgets" to "budget" --- lighthouse-cli/bin.js | 14 +-- lighthouse-cli/cli-flags.js | 6 +- .../test/cli/__snapshots__/index-test.js.snap | 4 +- lighthouse-cli/test/cli/bin-test.js | 10 +- .../config/{budgets.js => budget.js} | 40 +++--- lighthouse-core/config/config.js | 6 +- lighthouse-core/config/constants.js | 2 +- lighthouse-core/runner.js | 3 +- .../{budgets-test.js => budget-test.js} | 119 ++++++++++-------- lighthouse-core/test/config/config-test.js | 25 +++- ...simple-budgets.json => simple-budget.json} | 2 +- .../test/results/artifacts/artifacts.json | 28 ++++- lighthouse-core/test/results/sample_v2.json | 2 +- types/budget.d.ts | 46 +++++++ types/budgets-json.d.ts | 32 ----- types/externs.d.ts | 8 +- 16 files changed, 208 insertions(+), 139 deletions(-) rename lighthouse-core/config/{budgets.js => budget.js} (78%) rename lighthouse-core/test/config/{budgets-test.js => budget-test.js} (53%) rename lighthouse-core/test/fixtures/{simple-budgets.json => simple-budget.json} (95%) create mode 100644 types/budget.d.ts delete mode 100644 types/budgets-json.d.ts diff --git a/lighthouse-cli/bin.js b/lighthouse-cli/bin.js index 66d81ce9dc07..b1b7eaec782b 100644 --- a/lighthouse-cli/bin.js +++ b/lighthouse-cli/bin.js @@ -77,15 +77,11 @@ async function begin() { configJson = require(`../lighthouse-core/config/${cliFlags.preset}-config.js`); } - if (cliFlags.budgetsPath) { - cliFlags.budgetsPath = path.resolve(process.cwd(), cliFlags.budgetsPath); - try { - /** @type {Array} */ - const budgetsJsonStr = JSON.parse(fs.readFileSync(cliFlags.budgetsPath, 'utf8')); - cliFlags.budgetsJSON = budgetsJsonStr; - } catch (err) { - throw new Error(err); - } + if (cliFlags.budgetPath) { + cliFlags.budgetPath = path.resolve(process.cwd(), cliFlags.budgetPath); + /** @type {Array} */ + const parsedBudget = JSON.parse(fs.readFileSync(cliFlags.budgetPath, 'utf8')); + cliFlags.budget = parsedBudget; } // set logging preferences diff --git a/lighthouse-cli/cli-flags.js b/lighthouse-cli/cli-flags.js index 2396d16c86b9..c24354cae69c 100644 --- a/lighthouse-cli/cli-flags.js +++ b/lighthouse-cli/cli-flags.js @@ -59,7 +59,7 @@ function getFlags(manualArgv) { 'save-assets', 'list-all-audits', 'list-trace-categories', 'print-config', 'additional-trace-categories', 'config-path', 'preset', 'chrome-flags', 'port', 'hostname', 'emulated-form-factor', 'max-wait-for-load', 'enable-error-reporting', 'gather-mode', 'audit-mode', - 'only-audits', 'only-categories', 'skip-audits', 'budgets-path', + 'only-audits', 'only-categories', 'skip-audits', 'budget-path', ], 'Configuration:') .describe({ @@ -88,7 +88,7 @@ function getFlags(manualArgv) { 'Additional categories to capture with the trace (comma-delimited).', 'config-path': `The path to the config JSON. An example config file: lighthouse-core/config/lr-desktop-config.js`, - 'budgets-path': `The path to the budgets.json file for LightWallet.`, + 'budget-path': `The path to the budget.json file for LightWallet.`, 'preset': `Use a built-in configuration. WARNING: If the --config-path flag is provided, this preset will be ignored.`, 'chrome-flags': @@ -142,7 +142,7 @@ function getFlags(manualArgv) { .string('channel') .string('precomputedLanternDataPath') .string('lanternDataOutputPath') - .string('budgetsPath') + .string('budgetPath') // default values .default('chrome-flags', '') diff --git a/lighthouse-cli/test/cli/__snapshots__/index-test.js.snap b/lighthouse-cli/test/cli/__snapshots__/index-test.js.snap index 4c7d308eb8f8..3a64035b6006 100644 --- a/lighthouse-cli/test/cli/__snapshots__/index-test.js.snap +++ b/lighthouse-cli/test/cli/__snapshots__/index-test.js.snap @@ -1198,7 +1198,7 @@ Object { "additionalTraceCategories": null, "auditMode": false, "blockedUrlPatterns": null, - "budgetsJSON": null, + "budget": null, "channel": "cli", "disableStorageReset": false, "emulatedFormFactor": "mobile", @@ -1328,7 +1328,7 @@ Object { "additionalTraceCategories": null, "auditMode": true, "blockedUrlPatterns": null, - "budgetsJSON": null, + "budget": null, "channel": "cli", "disableStorageReset": false, "emulatedFormFactor": "mobile", diff --git a/lighthouse-cli/test/cli/bin-test.js b/lighthouse-cli/test/cli/bin-test.js index d377ef951492..c7e9a61306d2 100644 --- a/lighthouse-cli/test/cli/bin-test.js +++ b/lighthouse-cli/test/cli/bin-test.js @@ -99,14 +99,14 @@ describe('CLI bin', function() { }); }); - describe('budgets', () => { + describe('budget', () => { it('should load the config from the path', async () => { - const budgetsPath = '../../../lighthouse-core/test/fixtures/simple-budgets.json'; - cliFlags = {...cliFlags, budgetsPath: require.resolve(budgetsPath)}; - const actualBudgetsJSON = require(budgetsPath); + const budgetPath = '../../../lighthouse-core/test/fixtures/simple-budget.json'; + cliFlags = {...cliFlags, budgetPath: require.resolve(budgetPath)}; + const budgetFile = require(budgetPath); await bin.begin(); - expect(getRunLighthouseArgs()[1].budgetsJSON).toEqual(actualBudgetsJSON); + expect(getRunLighthouseArgs()[1].budget).toEqual(budgetFile); }); }); diff --git a/lighthouse-core/config/budgets.js b/lighthouse-core/config/budget.js similarity index 78% rename from lighthouse-core/config/budgets.js rename to lighthouse-core/config/budget.js index b28cbbe4150e..6dc9c14a466e 100644 --- a/lighthouse-core/config/budgets.js +++ b/lighthouse-core/config/budget.js @@ -5,7 +5,7 @@ */ 'use strict'; -class Budgets { +class Budget { /** * Asserts that obj has no own properties, throwing a nice error message if it does. * Plugin and object name are included for nicer logging. @@ -21,12 +21,12 @@ class Budgets { } /** - * @param {LH.BudgetsJSON.ResourceBudget} resourceBudget - * @return {LH.BudgetsJSON.ResourceBudget} + * @param {LH.Budget.ResourceBudget} resourceBudget + * @return {LH.Budget.ResourceBudget} */ static validateResourceBudget(resourceBudget) { const {resourceType, budget, ...invalidRest} = resourceBudget; - Budgets.assertNoExcessProperties(invalidRest, 'Resource Budget'); + Budget.assertNoExcessProperties(invalidRest, 'Resource Budget'); const validResourceTypes = [ 'total', @@ -53,17 +53,17 @@ class Budgets { } /** - * @param {LH.BudgetsJSON.TimingBudget} timingBudget - * @return {LH.BudgetsJSON.TimingBudget} + * @param {LH.Budget.TimingBudget} timingBudget + * @return {LH.Budget.TimingBudget} */ static validateTimingBudget(timingBudget) { const {metric, budget, tolerance, ...invalidRest} = timingBudget; - Budgets.assertNoExcessProperties(invalidRest, 'Timing Budget'); + Budget.assertNoExcessProperties(invalidRest, 'Timing Budget'); const validTimingMetrics = [ 'first-contentful-paint', 'first-cpu-idle', - 'time-to-interactive', + 'interactive', 'first-meaningful-paint', 'estimated-input-latency', ]; @@ -84,39 +84,39 @@ class Budgets { }; } - /** More info on the Budgets format: + /** More info on the Budget format: * https://github.com/GoogleChrome/lighthouse/issues/6053#issuecomment-428385930 * / /** - * @param {Array} budgetsJSON - * @return {Array} + * @param {Array} budgetArr + * @return {Array} */ - static initializeBudgets(budgetsJSON) { - /** @type {Array} */ + static initializeBudget(budgetArr) { + /** @type {Array} */ const budgets = []; - budgetsJSON.forEach((b) => { - /** @type {LH.BudgetsJSON.Budget} */ + budgetArr.forEach((b) => { + /** @type {LH.Budget.Budget} */ const budget = {}; const {resourceSizes, resourceCounts, timings, ...invalidRest} = b; - Budgets.assertNoExcessProperties(invalidRest, 'Budget'); + Budget.assertNoExcessProperties(invalidRest, 'Budget'); if (b.resourceSizes !== undefined) { budget.resourceSizes = b.resourceSizes.map((r) => { - return Budgets.validateResourceBudget(r); + return Budget.validateResourceBudget(r); }); } if (b.resourceCounts !== undefined) { budget.resourceCounts = b.resourceCounts.map((r) => { - return Budgets.validateResourceBudget(r); + return Budget.validateResourceBudget(r); }); } if (b.timings !== undefined) { budget.timings = b.timings.map((t) => { - return Budgets.validateTimingBudget(t); + return Budget.validateTimingBudget(t); }); } budgets.push({ @@ -129,4 +129,4 @@ class Budgets { } } -module.exports = Budgets; +module.exports = Budget; diff --git a/lighthouse-core/config/config.js b/lighthouse-core/config/config.js index be2296a58246..49fc46061c69 100644 --- a/lighthouse-core/config/config.js +++ b/lighthouse-core/config/config.js @@ -17,7 +17,7 @@ const path = require('path'); const Audit = require('../audits/audit.js'); const Runner = require('../runner.js'); const ConfigPlugin = require('./config-plugin.js'); -const Budgets = require('./budgets.js'); +const Budget = require('./budget.js'); /** @typedef {typeof import('../gather/gatherers/gatherer.js')} GathererConstructor */ /** @typedef {InstanceType} Gatherer */ @@ -513,8 +513,8 @@ class Config { // Override any applicable settings with CLI flags const settingsWithFlags = merge(settingWithDefaults || {}, cleanFlagsForSettings(flags), true); - if (settingsWithFlags.budgetsJSON) { - settingsWithFlags.budgetsJSON = Budgets.initializeBudgets(settingsWithFlags.budgetsJSON); + if (settingsWithFlags.budget) { + settingsWithFlags.budget = Budget.initializeBudget(settingsWithFlags.budget); } // Locale is special and comes only from flags/settings/lookupLocale. settingsWithFlags.locale = locale; diff --git a/lighthouse-core/config/constants.js b/lighthouse-core/config/constants.js index e527112a3332..2045bce5dc0d 100644 --- a/lighthouse-core/config/constants.js +++ b/lighthouse-core/config/constants.js @@ -52,10 +52,10 @@ const defaultSettings = { disableStorageReset: false, emulatedFormFactor: 'mobile', channel: 'node', - budgetsJSON: null, // the following settings have no defaults but we still want ensure that `key in settings` // in config will work in a typechecked way + budget: null, locale: 'en-US', // actual default determined by Config using lib/i18n blockedUrlPatterns: null, additionalTraceCategories: null, diff --git a/lighthouse-core/runner.js b/lighthouse-core/runner.js index bcfe24039679..dac3af242eb4 100644 --- a/lighthouse-core/runner.js +++ b/lighthouse-core/runner.js @@ -234,8 +234,7 @@ class Runner { gatherMode: undefined, auditMode: undefined, output: undefined, - budgetsPath: undefined, - budgetsJSON: undefined, + budget: undefined, }; const normalizedGatherSettings = Object.assign({}, artifacts.settings, overrides); const normalizedAuditSettings = Object.assign({}, settings, overrides); diff --git a/lighthouse-core/test/config/budgets-test.js b/lighthouse-core/test/config/budget-test.js similarity index 53% rename from lighthouse-core/test/config/budgets-test.js rename to lighthouse-core/test/config/budget-test.js index 8f33fd791d26..79e2c9fab3a0 100644 --- a/lighthouse-core/test/config/budgets-test.js +++ b/lighthouse-core/test/config/budget-test.js @@ -5,52 +5,60 @@ */ 'use strict'; -const Budgets = require('../../config/budgets'); +const Budget = require('../../config/budget.js'); const assert = require('assert'); /* eslint-env jest */ -describe('Budgets', () => { - let budgetsJson; +describe('Budget', () => { + let budget; beforeEach(() => { - budgetsJson = [{ - resourceSizes: [{ - resourceType: 'script', - budget: 123, - }, + budget = [ { - resourceType: 'image', - budget: 456, - }], - resourceCounts: [{ - resourceType: 'total', - budget: 100, + resourceSizes: [ + { + resourceType: 'script', + budget: 123, + }, + { + resourceType: 'image', + budget: 456, + }, + ], + resourceCounts: [ + { + resourceType: 'total', + budget: 100, + }, + { + resourceType: 'third-party', + budget: 10, + }, + ], + timings: [ + { + metric: 'interactive', + budget: 2000, + tolerance: 1000, + }, + { + metric: 'first-contentful-paint', + budget: 1000, + tolerance: 500, + }, + ], }, { - resourceType: 'third-party', - budget: 10, - }], - timings: [{ - metric: 'time-to-interactive', - budget: 2000, - tolerance: 1000, + resourceSizes: [ + { + resourceType: 'script', + budget: 1000, + }, + ], }, - { - metric: 'first-contentful-paint', - budget: 1000, - tolerance: 500, - }], - }, - { - resourceSizes: [ - { - resourceType: 'script', - budget: 1000, - }, - ], - }]; + ]; }); it('initializes correctly', () => { - const budgets = Budgets.initializeBudgets(budgetsJson); + const budgets = Budget.initializeBudget(budget); assert.equal(budgets.length, 2); // Sets resources sizes correctly @@ -72,40 +80,47 @@ describe('Budgets', () => { // Does not set unsupplied budgets assert.equal(budgets[1].timings, null); }); + it('throws error if an unsupported budget property is used', () => { - budgetsJson[0].sizes = []; - assert.throws(_ => Budgets.initializeBudgets(budgetsJson), /[sizes]/); + budget[0].sizes = []; + assert.throws(_ => Budget.initializeBudget(budget), /[sizes]/); }); + describe('resource budget validation', () => { it('throws when an invalid resource type is supplied', () => { - budgetsJson[0].resourceSizes[0].resourceType = 'movies'; - assert.throws(_ => Budgets.initializeBudgets(budgetsJson), /Invalid resource type/); + budget[0].resourceSizes[0].resourceType = 'movies'; + assert.throws(_ => Budget.initializeBudget(budget), /Invalid resource type/); }); + it('throws when an invalid budget is supplied', () => { - budgetsJson[0].resourceSizes[0].budget = '100 MB'; - assert.throws(_ => Budgets.initializeBudgets(budgetsJson), /Invalid budget/); + budget[0].resourceSizes[0].budget = '100 MB'; + assert.throws(_ => Budget.initializeBudget(budget), /Invalid budget/); }); + it('throws when an invalid property is supplied', () => { - budgetsJson[0].resourceSizes[0].browser = 'Chrome'; - assert.throws(_ => Budgets.initializeBudgets(budgetsJson), /[browser]/); + budget[0].resourceSizes[0].browser = 'Chrome'; + assert.throws(_ => Budget.initializeBudget(budget), /[browser]/); }); }); describe('timing budget validation', () => { it('throws when an invalid metric is supplied', () => { - budgetsJson[0].timings[0].metric = 'lastMeaningfulPaint'; - assert.throws(_ => Budgets.initializeBudgets(budgetsJson), /Invalid timing metric/); + budget[0].timings[0].metric = 'lastMeaningfulPaint'; + assert.throws(_ => Budget.initializeBudget(budget), /Invalid timing metric/); }); + it('throws when an invalid budget is supplied', () => { - budgetsJson[0].timings[0].budget = '100KB'; - assert.throws(_ => Budgets.initializeBudgets(budgetsJson), /Invalid budget/); + budget[0].timings[0].budget = '100KB'; + assert.throws(_ => Budget.initializeBudget(budget), /Invalid budget/); }); + it('throws when an invalid tolerance is supplied', () => { - budgetsJson[0].timings[0].tolerance = '100ms'; - assert.throws(_ => Budgets.initializeBudgets(budgetsJson), /Invalid tolerance/); + budget[0].timings[0].tolerance = '100ms'; + assert.throws(_ => Budget.initializeBudget(budget), /Invalid tolerance/); }); + it('throws when an invalid property is supplied', () => { - budgetsJson[0].timings[0].device = 'Phone'; - assert.throws(_ => Budgets.initializeBudgets(budgetsJson), /[device]/); + budget[0].timings[0].device = 'Phone'; + assert.throws(_ => Budget.initializeBudget(budget), /[device]/); }); }); }); diff --git a/lighthouse-core/test/config/config-test.js b/lighthouse-core/test/config/config-test.js index 11ff2e939bbf..d39d2797c2db 100644 --- a/lighthouse-core/test/config/config-test.js +++ b/lighthouse-core/test/config/config-test.js @@ -741,9 +741,28 @@ describe('Config', () => { assert.strictEqual(config.categories['lighthouse-plugin-simple'].title, 'Simple'); }); - it('should initialize budgetsJSON', () => { - assert.throws(() => new Config({settings: {budgetsJSON: ['invalid123']}}), - /Budget has unrecognized properties/); + describe('budget setting', () => { + it('should be initialized', () => { + const configJson = { + settings: { + budget: [{ + resourceCounts: [{ + resourceType: 'image', + budget: 500, + }], + }], + }, + }; + const config = new Config(configJson); + assert.equal(config.settings.budget[0].resourceCounts.length, 1); + assert.equal(config.settings.budget[0].resourceCounts[0].resourceType, 'image'); + assert.equal(config.settings.budget[0].resourceCounts[0].budget, 500); + }); + + it('should throw when provided an invalid budget', () => { + assert.throws(() => new Config({settings: {budget: ['invalid123']}}), + /Budget has unrecognized properties/); + }); }); it('should load plugins from the config and from passed-in flags', () => { diff --git a/lighthouse-core/test/fixtures/simple-budgets.json b/lighthouse-core/test/fixtures/simple-budget.json similarity index 95% rename from lighthouse-core/test/fixtures/simple-budgets.json rename to lighthouse-core/test/fixtures/simple-budget.json index 07fe2c73e8e2..c72f39a632a4 100644 --- a/lighthouse-core/test/fixtures/simple-budgets.json +++ b/lighthouse-core/test/fixtures/simple-budget.json @@ -20,7 +20,7 @@ ], "timings": [ { - "metric": "time-to-interactive", + "metric": "interactive", "budget": 5000, "tolerance": 1000 }, diff --git a/lighthouse-core/test/results/artifacts/artifacts.json b/lighthouse-core/test/results/artifacts/artifacts.json index 7b573e5b76e6..3afca4abf5f5 100644 --- a/lighthouse-core/test/results/artifacts/artifacts.json +++ b/lighthouse-core/test/results/artifacts/artifacts.json @@ -29,7 +29,33 @@ "disableStorageReset": false, "emulatedFormFactor": "mobile", "channel": "cli", - "budgetsPath": null, + "budget": [ + { + "resourceSizes": [ + { + "resourceType": "script", + "budget": 125 + }, + { + "resourceType": "total", + "budget": 500 + } + ], + "timings": [ + { + "metric": "interactive", + "budget": 5000, + "tolerance": 1000 + } + ], + "resourceCounts": [ + { + "resourceType": "third-party", + "budget": 0 + } + ] + } + ], "locale": "en-US", "blockedUrlPatterns": null, "additionalTraceCategories": null, diff --git a/lighthouse-core/test/results/sample_v2.json b/lighthouse-core/test/results/sample_v2.json index d5294f589ac6..491fa0d206fc 100644 --- a/lighthouse-core/test/results/sample_v2.json +++ b/lighthouse-core/test/results/sample_v2.json @@ -2963,7 +2963,7 @@ "disableStorageReset": false, "emulatedFormFactor": "mobile", "channel": "cli", - "budgetsJSON": null, + "budget": null, "locale": "en-US", "blockedUrlPatterns": null, "additionalTraceCategories": null, diff --git a/types/budget.d.ts b/types/budget.d.ts new file mode 100644 index 000000000000..581e1da767b1 --- /dev/null +++ b/types/budget.d.ts @@ -0,0 +1,46 @@ +/** + * @license Copyright 2019 Google Inc. All Rights Reserved. + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. + */ + +declare global { + module LH { + module Budget { + /** More info: https://github.com/GoogleChrome/lighthouse/issues/6053#issuecomment-428385930 */ + export interface Budget { + /** Budgets based on resource count */ + resourceCounts ? : Array < ResourceBudget > ; + /** Budgets based on resource size */ + resourceSizes ? : Array < ResourceBudget > ; + /** Budgets based on timing metrics */ + timings ? : Array < TimingBudget > ; + } + + export interface ResourceBudget { + /** The resource type that a budget applies to */ + resourceType: ResourceType; + /** Budget for resource. Depending on context, this is either the count or size (KB) of a resource */ + budget: number; + } + + export interface TimingBudget { + /** The type of timing metric */ + metric: TimingMetric; + /** Budget for timing measurement, in milliseconds */ + budget: number; + /** Tolerance, i.e. buffer, to apply to a timing budget. Units: milliseconds. */ + tolerance ? : number; + } + + /** Supported timing metrics */ + export type TimingMetric = 'first-contentful-paint' | 'first-cpu-idle' | 'interactive' | 'first-meaningful-paint' | 'estimated-input-latency'; + + /** Supported resource types */ + export type ResourceType = 'stylesheet' | 'image' | 'media' | 'font' | 'script' | 'document' | 'other'; + } + } +} + +// empty export to keep file a module +export {} \ No newline at end of file diff --git a/types/budgets-json.d.ts b/types/budgets-json.d.ts deleted file mode 100644 index 37681bb9636b..000000000000 --- a/types/budgets-json.d.ts +++ /dev/null @@ -1,32 +0,0 @@ -/** - * @license Copyright 2019 Google Inc. All Rights Reserved. - * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 - * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. - */ - -declare global { - module LH { - module BudgetsJSON { - /** More info: https://github.com/GoogleChrome/lighthouse/issues/6053#issuecomment-428385930 */ - export interface Budget { - resourceCounts?: Array; - resourceSizes?: Array; - timings?: Array; - } - - export interface ResourceBudget { - resourceType: string; - budget: number; - } - - export interface TimingBudget { - metric: string; - budget: number; - tolerance?: number; - } - } - } -} - -// empty export to keep file a module -export { } diff --git a/types/externs.d.ts b/types/externs.d.ts index f688ba676208..0c91bf601f7c 100644 --- a/types/externs.d.ts +++ b/types/externs.d.ts @@ -119,8 +119,8 @@ declare global { channel?: string /** Precomputed lantern estimates to use instead of observed analysis. */ precomputedLanternData?: PrecomputedLanternData | null; - /** The budgets.json object for LightWallet. */ - budgetsJSON?: Array | null; + /** The budget.json object for LightWallet. */ + budget?: Array | null; } /** @@ -171,8 +171,8 @@ declare global { precomputedLanternDataPath?: string; /** Path to the file where precomputed lantern data should be written to. */ lanternDataOutputPath?: string; - /** Path to the budgets.json file for LightWallet */ - budgetsPath?: string | null; + /** Path to the budget.json file for LightWallet */ + budgetPath?: string | null; // The following are given defaults in cli-flags, so are not optional like in Flags or SharedFlagsSettings. output: OutputMode[]; From c9663225568c7479b58eb3b8646f6149febba187 Mon Sep 17 00:00:00 2001 From: Brendan Kenny Date: Wed, 24 Apr 2019 13:44:55 -0700 Subject: [PATCH 9/9] formatting, interface nesting tweaks --- lighthouse-cli/bin.js | 4 +- .../test/cli/__snapshots__/index-test.js.snap | 4 +- lighthouse-cli/test/cli/bin-test.js | 2 +- lighthouse-core/config/budget.js | 39 ++++++----- lighthouse-core/config/config.js | 4 +- lighthouse-core/config/constants.js | 2 +- lighthouse-core/runner.js | 2 +- lighthouse-core/test/config/budget-test.js | 2 + lighthouse-core/test/config/config-test.js | 10 +-- .../test/fixtures/simple-budget.json | 2 +- .../test/results/artifacts/artifacts.json | 2 +- lighthouse-core/test/results/sample_v2.json | 2 +- types/budget.d.ts | 65 ++++++++++--------- types/externs.d.ts | 4 +- 14 files changed, 74 insertions(+), 70 deletions(-) diff --git a/lighthouse-cli/bin.js b/lighthouse-cli/bin.js index b1b7eaec782b..5fe06032f71a 100644 --- a/lighthouse-cli/bin.js +++ b/lighthouse-cli/bin.js @@ -79,9 +79,9 @@ async function begin() { if (cliFlags.budgetPath) { cliFlags.budgetPath = path.resolve(process.cwd(), cliFlags.budgetPath); - /** @type {Array} */ + /** @type {Array} */ const parsedBudget = JSON.parse(fs.readFileSync(cliFlags.budgetPath, 'utf8')); - cliFlags.budget = parsedBudget; + cliFlags.budgets = parsedBudget; } // set logging preferences diff --git a/lighthouse-cli/test/cli/__snapshots__/index-test.js.snap b/lighthouse-cli/test/cli/__snapshots__/index-test.js.snap index 3a64035b6006..dd4636eb9237 100644 --- a/lighthouse-cli/test/cli/__snapshots__/index-test.js.snap +++ b/lighthouse-cli/test/cli/__snapshots__/index-test.js.snap @@ -1198,7 +1198,7 @@ Object { "additionalTraceCategories": null, "auditMode": false, "blockedUrlPatterns": null, - "budget": null, + "budgets": null, "channel": "cli", "disableStorageReset": false, "emulatedFormFactor": "mobile", @@ -1328,7 +1328,7 @@ Object { "additionalTraceCategories": null, "auditMode": true, "blockedUrlPatterns": null, - "budget": null, + "budgets": null, "channel": "cli", "disableStorageReset": false, "emulatedFormFactor": "mobile", diff --git a/lighthouse-cli/test/cli/bin-test.js b/lighthouse-cli/test/cli/bin-test.js index c7e9a61306d2..b53983926ab7 100644 --- a/lighthouse-cli/test/cli/bin-test.js +++ b/lighthouse-cli/test/cli/bin-test.js @@ -106,7 +106,7 @@ describe('CLI bin', function() { const budgetFile = require(budgetPath); await bin.begin(); - expect(getRunLighthouseArgs()[1].budget).toEqual(budgetFile); + expect(getRunLighthouseArgs()[1].budgets).toEqual(budgetFile); }); }); diff --git a/lighthouse-core/config/budget.js b/lighthouse-core/config/budget.js index 6dc9c14a466e..820dda302f4b 100644 --- a/lighthouse-core/config/budget.js +++ b/lighthouse-core/config/budget.js @@ -6,12 +6,12 @@ 'use strict'; class Budget { -/** - * Asserts that obj has no own properties, throwing a nice error message if it does. - * Plugin and object name are included for nicer logging. - * @param {Record} obj - * @param {string} objectName - */ + /** + * Asserts that obj has no own properties, throwing a nice error message if it does. + * Plugin and object name are included for nicer logging. + * @param {Record} obj + * @param {string} objectName + */ static assertNoExcessProperties(obj, objectName) { const invalidKeys = Object.keys(obj); if (invalidKeys.length > 0) { @@ -21,9 +21,9 @@ class Budget { } /** - * @param {LH.Budget.ResourceBudget} resourceBudget - * @return {LH.Budget.ResourceBudget} - */ + * @param {LH.Budget.ResourceBudget} resourceBudget + * @return {LH.Budget.ResourceBudget} + */ static validateResourceBudget(resourceBudget) { const {resourceType, budget, ...invalidRest} = resourceBudget; Budget.assertNoExcessProperties(invalidRest, 'Resource Budget'); @@ -53,9 +53,9 @@ class Budget { } /** - * @param {LH.Budget.TimingBudget} timingBudget - * @return {LH.Budget.TimingBudget} - */ + * @param {LH.Budget.TimingBudget} timingBudget + * @return {LH.Budget.TimingBudget} + */ static validateTimingBudget(timingBudget) { const {metric, budget, tolerance, ...invalidRest} = timingBudget; Budget.assertNoExcessProperties(invalidRest, 'Timing Budget'); @@ -84,19 +84,18 @@ class Budget { }; } - /** More info on the Budget format: - * https://github.com/GoogleChrome/lighthouse/issues/6053#issuecomment-428385930 - * / /** - * @param {Array} budgetArr - * @return {Array} - */ + * More info on the Budget format: + * https://github.com/GoogleChrome/lighthouse/issues/6053#issuecomment-428385930 + * @param {Array} budgetArr + * @return {Array} + */ static initializeBudget(budgetArr) { - /** @type {Array} */ + /** @type {Array} */ const budgets = []; budgetArr.forEach((b) => { - /** @type {LH.Budget.Budget} */ + /** @type {LH.Budget} */ const budget = {}; const {resourceSizes, resourceCounts, timings, ...invalidRest} = b; diff --git a/lighthouse-core/config/config.js b/lighthouse-core/config/config.js index 49fc46061c69..a828f3abf19d 100644 --- a/lighthouse-core/config/config.js +++ b/lighthouse-core/config/config.js @@ -513,8 +513,8 @@ class Config { // Override any applicable settings with CLI flags const settingsWithFlags = merge(settingWithDefaults || {}, cleanFlagsForSettings(flags), true); - if (settingsWithFlags.budget) { - settingsWithFlags.budget = Budget.initializeBudget(settingsWithFlags.budget); + if (settingsWithFlags.budgets) { + settingsWithFlags.budgets = Budget.initializeBudget(settingsWithFlags.budgets); } // Locale is special and comes only from flags/settings/lookupLocale. settingsWithFlags.locale = locale; diff --git a/lighthouse-core/config/constants.js b/lighthouse-core/config/constants.js index 2045bce5dc0d..9f5ce283b313 100644 --- a/lighthouse-core/config/constants.js +++ b/lighthouse-core/config/constants.js @@ -55,7 +55,7 @@ const defaultSettings = { // the following settings have no defaults but we still want ensure that `key in settings` // in config will work in a typechecked way - budget: null, + budgets: null, locale: 'en-US', // actual default determined by Config using lib/i18n blockedUrlPatterns: null, additionalTraceCategories: null, diff --git a/lighthouse-core/runner.js b/lighthouse-core/runner.js index dac3af242eb4..1504607bc99c 100644 --- a/lighthouse-core/runner.js +++ b/lighthouse-core/runner.js @@ -234,7 +234,7 @@ class Runner { gatherMode: undefined, auditMode: undefined, output: undefined, - budget: undefined, + budgets: undefined, }; const normalizedGatherSettings = Object.assign({}, artifacts.settings, overrides); const normalizedAuditSettings = Object.assign({}, settings, overrides); diff --git a/lighthouse-core/test/config/budget-test.js b/lighthouse-core/test/config/budget-test.js index 79e2c9fab3a0..7a56d587b49a 100644 --- a/lighthouse-core/test/config/budget-test.js +++ b/lighthouse-core/test/config/budget-test.js @@ -57,6 +57,7 @@ describe('Budget', () => { }, ]; }); + it('initializes correctly', () => { const budgets = Budget.initializeBudget(budget); assert.equal(budgets.length, 2); @@ -102,6 +103,7 @@ describe('Budget', () => { assert.throws(_ => Budget.initializeBudget(budget), /[browser]/); }); }); + describe('timing budget validation', () => { it('throws when an invalid metric is supplied', () => { budget[0].timings[0].metric = 'lastMeaningfulPaint'; diff --git a/lighthouse-core/test/config/config-test.js b/lighthouse-core/test/config/config-test.js index d39d2797c2db..c3c29045b09f 100644 --- a/lighthouse-core/test/config/config-test.js +++ b/lighthouse-core/test/config/config-test.js @@ -745,7 +745,7 @@ describe('Config', () => { it('should be initialized', () => { const configJson = { settings: { - budget: [{ + budgets: [{ resourceCounts: [{ resourceType: 'image', budget: 500, @@ -754,13 +754,13 @@ describe('Config', () => { }, }; const config = new Config(configJson); - assert.equal(config.settings.budget[0].resourceCounts.length, 1); - assert.equal(config.settings.budget[0].resourceCounts[0].resourceType, 'image'); - assert.equal(config.settings.budget[0].resourceCounts[0].budget, 500); + assert.equal(config.settings.budgets[0].resourceCounts.length, 1); + assert.equal(config.settings.budgets[0].resourceCounts[0].resourceType, 'image'); + assert.equal(config.settings.budgets[0].resourceCounts[0].budget, 500); }); it('should throw when provided an invalid budget', () => { - assert.throws(() => new Config({settings: {budget: ['invalid123']}}), + assert.throws(() => new Config({settings: {budgets: ['invalid123']}}), /Budget has unrecognized properties/); }); }); diff --git a/lighthouse-core/test/fixtures/simple-budget.json b/lighthouse-core/test/fixtures/simple-budget.json index c72f39a632a4..e8f3030e67b4 100644 --- a/lighthouse-core/test/fixtures/simple-budget.json +++ b/lighthouse-core/test/fixtures/simple-budget.json @@ -42,4 +42,4 @@ } ] } -] \ No newline at end of file +] diff --git a/lighthouse-core/test/results/artifacts/artifacts.json b/lighthouse-core/test/results/artifacts/artifacts.json index 3afca4abf5f5..c5a3d08948bc 100644 --- a/lighthouse-core/test/results/artifacts/artifacts.json +++ b/lighthouse-core/test/results/artifacts/artifacts.json @@ -29,7 +29,7 @@ "disableStorageReset": false, "emulatedFormFactor": "mobile", "channel": "cli", - "budget": [ + "budgets": [ { "resourceSizes": [ { diff --git a/lighthouse-core/test/results/sample_v2.json b/lighthouse-core/test/results/sample_v2.json index 491fa0d206fc..ff31c029d842 100644 --- a/lighthouse-core/test/results/sample_v2.json +++ b/lighthouse-core/test/results/sample_v2.json @@ -2963,7 +2963,7 @@ "disableStorageReset": false, "emulatedFormFactor": "mobile", "channel": "cli", - "budget": null, + "budgets": null, "locale": "en-US", "blockedUrlPatterns": null, "additionalTraceCategories": null, diff --git a/types/budget.d.ts b/types/budget.d.ts index 581e1da767b1..b1da9e8de36c 100644 --- a/types/budget.d.ts +++ b/types/budget.d.ts @@ -5,42 +5,45 @@ */ declare global { - module LH { - module Budget { - /** More info: https://github.com/GoogleChrome/lighthouse/issues/6053#issuecomment-428385930 */ - export interface Budget { - /** Budgets based on resource count */ - resourceCounts ? : Array < ResourceBudget > ; - /** Budgets based on resource size */ - resourceSizes ? : Array < ResourceBudget > ; - /** Budgets based on timing metrics */ - timings ? : Array < TimingBudget > ; - } + module LH { + /** + * The performance budget interface. + * More info: https://github.com/GoogleChrome/lighthouse/issues/6053#issuecomment-428385930 + */ + export interface Budget { + /** Budgets based on resource count. */ + resourceCounts?: Array; + /** Budgets based on resource size. */ + resourceSizes?: Array; + /** Budgets based on timing metrics. */ + timings?: Array ; + } - export interface ResourceBudget { - /** The resource type that a budget applies to */ - resourceType: ResourceType; - /** Budget for resource. Depending on context, this is either the count or size (KB) of a resource */ - budget: number; - } + module Budget { + export interface ResourceBudget { + /** The resource type that a budget applies to. */ + resourceType: ResourceType; + /** Budget for resource. Depending on context, this is either the count or size (KB) of a resource. */ + budget: number; + } - export interface TimingBudget { - /** The type of timing metric */ - metric: TimingMetric; - /** Budget for timing measurement, in milliseconds */ - budget: number; - /** Tolerance, i.e. buffer, to apply to a timing budget. Units: milliseconds. */ - tolerance ? : number; - } + export interface TimingBudget { + /** The type of timing metric. */ + metric: TimingMetric; + /** Budget for timing measurement, in milliseconds. */ + budget: number; + /** Tolerance, i.e. buffer, to apply to a timing budget. Units: milliseconds. */ + tolerance?: number; + } - /** Supported timing metrics */ - export type TimingMetric = 'first-contentful-paint' | 'first-cpu-idle' | 'interactive' | 'first-meaningful-paint' | 'estimated-input-latency'; + /** Supported timing metrics. */ + export type TimingMetric = 'first-contentful-paint' | 'first-cpu-idle' | 'interactive' | 'first-meaningful-paint' | 'estimated-input-latency'; - /** Supported resource types */ - export type ResourceType = 'stylesheet' | 'image' | 'media' | 'font' | 'script' | 'document' | 'other'; - } + /** Supported resource types. */ + export type ResourceType = 'stylesheet' | 'image' | 'media' | 'font' | 'script' | 'document' | 'other'; } + } } // empty export to keep file a module -export {} \ No newline at end of file +export {} diff --git a/types/externs.d.ts b/types/externs.d.ts index 0c91bf601f7c..0b7780e13e8d 100644 --- a/types/externs.d.ts +++ b/types/externs.d.ts @@ -120,7 +120,7 @@ declare global { /** Precomputed lantern estimates to use instead of observed analysis. */ precomputedLanternData?: PrecomputedLanternData | null; /** The budget.json object for LightWallet. */ - budget?: Array | null; + budgets?: Array | null; } /** @@ -171,7 +171,7 @@ declare global { precomputedLanternDataPath?: string; /** Path to the file where precomputed lantern data should be written to. */ lanternDataOutputPath?: string; - /** Path to the budget.json file for LightWallet */ + /** Path to the budget.json file for LightWallet. */ budgetPath?: string | null; // The following are given defaults in cli-flags, so are not optional like in Flags or SharedFlagsSettings.