Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(lightwallet): Add --budgets-path & Budgets constructor #8427

Merged
merged 9 commits into from
Apr 26, 2019
Merged
11 changes: 11 additions & 0 deletions lighthouse-cli/bin.js
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,17 @@ async function begin() {
configJson = require(`../lighthouse-core/config/${cliFlags.preset}-config.js`);
}

if (cliFlags.budgetsPath) {
khempenius marked this conversation as resolved.
Show resolved Hide resolved
cliFlags.budgetsPath = path.resolve(process.cwd(), cliFlags.budgetsPath);
try {
khempenius marked this conversation as resolved.
Show resolved Hide resolved
/** @type {Array<LH.BudgetsJSON.Budget>} */
const budgetsJsonStr = JSON.parse(fs.readFileSync(cliFlags.budgetsPath, 'utf8'));
cliFlags.budgetsJSON = budgetsJsonStr;
} catch (err) {
throw new Error(err);
khempenius marked this conversation as resolved.
Show resolved Hide resolved
}
}

// set logging preferences
cliFlags.logLevel = 'info';
if (cliFlags.verbose) {
Expand Down
4 changes: 3 additions & 1 deletion lighthouse-cli/cli-flags.js
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down Expand Up @@ -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':
Expand Down Expand Up @@ -141,6 +142,7 @@ function getFlags(manualArgv) {
.string('channel')
.string('precomputedLanternDataPath')
.string('lanternDataOutputPath')
.string('budgetsPath')

// default values
.default('chrome-flags', '')
Expand Down
2 changes: 2 additions & 0 deletions lighthouse-cli/test/cli/__snapshots__/index-test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -1198,6 +1198,7 @@ Object {
"additionalTraceCategories": null,
"auditMode": false,
"blockedUrlPatterns": null,
"budgetsJSON": null,
"channel": "cli",
"disableStorageReset": false,
"emulatedFormFactor": "mobile",
Expand Down Expand Up @@ -1327,6 +1328,7 @@ Object {
"additionalTraceCategories": null,
"auditMode": true,
"blockedUrlPatterns": null,
"budgetsJSON": null,
"channel": "cli",
"disableStorageReset": false,
"emulatedFormFactor": "mobile",
Expand Down
11 changes: 11 additions & 0 deletions lighthouse-cli/test/cli/bin-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,17 @@ describe('CLI bin', function() {
});
});

describe('budgets', () => {
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);
await bin.begin();

expect(getRunLighthouseArgs()[1].budgetsJSON).toEqual(actualBudgetsJSON);
});
});

describe('logging', () => {
it('should have info by default', async () => {
await bin.begin();
Expand Down
132 changes: 132 additions & 0 deletions lighthouse-core/config/budgets.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,132 @@
/**
* @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<string, unknown>} 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.BudgetsJSON.ResourceBudget} resourceBudget
* @return {LH.BudgetsJSON.ResourceBudget}
*/
static validateResourceBudget(resourceBudget) {
const {resourceType, budget, ...invalidRest} = resourceBudget;
Budgets.assertNoExcessProperties(invalidRest, 'Resource Budget');

const validResourceTypes = [
'total',
'document',
'script',
'stylesheet',
'image',
'media',
'font',
'other',
'third-party',
];
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 {
resourceType,
budget,
};
}

/**
* @param {LH.BudgetsJSON.TimingBudget} timingBudget
* @return {LH.BudgetsJSON.TimingBudget}
*/
static validateTimingBudget(timingBudget) {
const {metric, budget, tolerance, ...invalidRest} = timingBudget;
Budgets.assertNoExcessProperties(invalidRest, 'Timing Budget');

const validTimingMetrics = [
'first-contentful-paint',
'first-cpu-idle',
'time-to-interactive',
khempenius marked this conversation as resolved.
Show resolved Hide resolved
'first-meaningful-paint',
'estimated-input-latency',
];
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 {
metric,
budget,
tolerance,
};
}

/** More info on the Budgets format:
* https://github.com/GoogleChrome/lighthouse/issues/6053#issuecomment-428385930
* /
/**
* @param {Array<LH.BudgetsJSON.Budget>} budgetsJSON
* @return {Array<LH.BudgetsJSON.Budget>}
*/
static initializeBudgets(budgetsJSON) {
/** @type {Array<LH.BudgetsJSON.Budget>} */
const budgets = [];

budgetsJSON.forEach((b) => {
/** @type {LH.BudgetsJSON.Budget} */
const budget = {};

const {resourceSizes, resourceCounts, timings, ...invalidRest} = b;
Budgets.assertNoExcessProperties(invalidRest, 'Budget');

if (b.resourceSizes !== undefined) {
budget.resourceSizes = b.resourceSizes.map((r) => {
return Budgets.validateResourceBudget(r);
});
}

if (b.resourceCounts !== undefined) {
budget.resourceCounts = b.resourceCounts.map((r) => {
return Budgets.validateResourceBudget(r);
});
}

if (b.timings !== undefined) {
budget.timings = b.timings.map((t) => {
return Budgets.validateTimingBudget(t);
});
}
budgets.push({
resourceSizes,
resourceCounts,
timings,
});
});
return budgets;
}
}

module.exports = Budgets;
4 changes: 4 additions & 0 deletions lighthouse-core/config/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -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<GathererConstructor>} Gatherer */
Expand Down Expand Up @@ -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;

Expand Down
1 change: 1 addition & 0 deletions lighthouse-core/config/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ const defaultSettings = {
disableStorageReset: false,
emulatedFormFactor: 'mobile',
channel: 'node',
budgetsJSON: null,
khempenius marked this conversation as resolved.
Show resolved Hide resolved

// the following settings have no defaults but we still want ensure that `key in settings`
// in config will work in a typechecked way
Expand Down
2 changes: 2 additions & 0 deletions lighthouse-core/runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,8 @@ class Runner {
gatherMode: undefined,
auditMode: undefined,
output: undefined,
budgetsPath: undefined,
khempenius marked this conversation as resolved.
Show resolved Hide resolved
budgetsJSON: undefined,
khempenius marked this conversation as resolved.
Show resolved Hide resolved
};
const normalizedGatherSettings = Object.assign({}, artifacts.settings, overrides);
const normalizedAuditSettings = Object.assign({}, settings, overrides);
Expand Down
111 changes: 111 additions & 0 deletions lighthouse-core/test/config/budgets-test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
/**
* @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';

const Budgets = require('../../config/budgets');
khempenius marked this conversation as resolved.
Show resolved Hide resolved
const assert = require('assert');
/* eslint-env jest */

describe('Budgets', () => {
let budgetsJson;
beforeEach(() => {
budgetsJson = [{
khempenius marked this conversation as resolved.
Show resolved Hide resolved
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,
}],
},
{
resourceSizes: [
{
resourceType: 'script',
budget: 1000,
},
],
}];
});
it('initializes correctly', () => {
khempenius marked this conversation as resolved.
Show resolved Hide resolved
const budgets = Budgets.initializeBudgets(budgetsJson);
assert.equal(budgets.length, 2);

// Sets resources sizes correctly
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[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[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[1].timings, null);
});
it('throws error if an unsupported budget property is used', () => {
budgetsJson[0].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.initializeBudgets(budgetsJson), /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/);
});
it('throws when an invalid property is supplied', () => {
budgetsJson[0].resourceSizes[0].browser = 'Chrome';
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.initializeBudgets(budgetsJson), /Invalid timing metric/);
});
it('throws when an invalid budget is supplied', () => {
budgetsJson[0].timings[0].budget = '100KB';
assert.throws(_ => Budgets.initializeBudgets(budgetsJson), /Invalid budget/);
});
it('throws when an invalid tolerance is supplied', () => {
budgetsJson[0].timings[0].tolerance = '100ms';
assert.throws(_ => Budgets.initializeBudgets(budgetsJson), /Invalid tolerance/);
});
it('throws when an invalid property is supplied', () => {
budgetsJson[0].timings[0].device = 'Phone';
assert.throws(_ => Budgets.initializeBudgets(budgetsJson), /[device]/);
});
});
});
5 changes: 5 additions & 0 deletions lighthouse-core/test/config/config-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -741,6 +741,11 @@ describe('Config', () => {
assert.strictEqual(config.categories['lighthouse-plugin-simple'].title, 'Simple');
});

it('should initialize budgetsJSON', () => {
khempenius marked this conversation as resolved.
Show resolved Hide resolved
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'],
Expand Down
Loading