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

core(lightwallet): add path to budget.json #9453

Merged
merged 12 commits into from
Aug 16, 2019
1 change: 1 addition & 0 deletions lighthouse-cli/test/smokehouse/perf/perf-config.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ const perfConfig = {

// A mixture of under, over, and meeting budget to exercise all paths.
budgets: [{
path: '/',
resourceCounts: [
{resourceType: 'total', budget: 8},
{resourceType: 'stylesheet', budget: 1}, // meets budget
Expand Down
13 changes: 11 additions & 2 deletions lighthouse-core/audits/performance-budget.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@

const Audit = require('./audit.js');
const ResourceSummary = require('../computed/resource-summary.js');
const MainResource = require('../computed/main-resource.js');
const Budget = require('../config/budget.js');
const i18n = require('../lib/i18n/i18n.js');

const UIStrings = {
Expand Down Expand Up @@ -121,7 +123,14 @@ class ResourceBudget extends Audit {
static async audit(artifacts, context) {
const devtoolsLog = artifacts.devtoolsLogs[Audit.DEFAULT_PASS];
const summary = await ResourceSummary.request({devtoolsLog, URL: artifacts.URL}, context);
const budget = context.settings.budgets ? context.settings.budgets[0] : undefined;
const mainResource = await MainResource.request({URL: artifacts.URL, devtoolsLog}, context);
// Clones budget so that the user-supplied version is not mutated.
/** @type {Array<LH.Budget>} */
const budgets = Array.from(context.settings.budgets || []);
// Applies the LAST matching budget
const budget = budgets ? budgets.reverse().find((b) => {
return Budget.urlMatchesPattern(mainResource.url, b.path);
}) : undefined;

if (!budget) {
return {
Expand All @@ -130,7 +139,7 @@ class ResourceBudget extends Audit {
};
}

/** @type { LH.Audit.Details.Table['headings'] } */
/** @type {LH.Audit.Details.Table['headings']} */
const headers = [
{key: 'label', itemType: 'text', text: str_(i18n.UIStrings.columnResourceType)},
{key: 'requestCount', itemType: 'numeric', text: str_(i18n.UIStrings.columnRequests)},
Expand Down
96 changes: 95 additions & 1 deletion lighthouse-core/config/budget.js
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,98 @@ class Budget {
};
}

/**
* @param {unknown} path
* @param {string} error
*/
static throwInvalidPathError(path, error) {
throw new Error(`Invalid path ${path}. ${error}\n` +
`'Path' should be specified using the 'robots.txt' format.\n` +
`Learn more about the 'robots.txt' format here:\n` +
`https://developers.google.com/search/reference/robots_txt#url-matching-based-on-path-values`);
}


/**
* Validates that path is either: a) undefined or ) properly formed.
* Verifies the quantity and location of the two robot.txt regex characters: $, *
* @param {unknown} path
* @return {undefined|string}
*/
static validatePath(path) {
if (path === undefined) {
return undefined;
} else if (typeof path !== 'string') {
this.throwInvalidPathError(path, `Path should be a string.`);
return;
} else if (!path.startsWith('/')) {
this.throwInvalidPathError(path, `Path should start with '/'.`);
} else if ((path.match(/\*/g) || []).length > 1) {
this.throwInvalidPathError(path, `Path should only contain one '*'.`);
} else if ((path.match(/\$/g) || []).length > 1) {
this.throwInvalidPathError(path, `Path should only contain one '$' character.`);
} else if (path.includes('$') && !path.endsWith('$')) {
this.throwInvalidPathError(path, `'$' character should only occur at end of path.`);
}
return path;
}

/**
* Determines whether a URL matches against a robots.txt-style "path".
* Pattern should use the robots.txt format. E.g. "/*-article.html" or "/". Reference:
* https://developers.google.com/search/reference/robots_txt#url-matching-based-on-path-values
* @param {string} url
* @param {string=} pattern
* @return {boolean}
*/
static urlMatchesPattern(url, pattern = '/') {
const urlObj = new URL(url);
const urlPath = urlObj.pathname + urlObj.search;

const hasWildcard = pattern.includes('*');
const hasDollarSign = pattern.includes('$');

/**
* There are 4 different cases of path strings.
* Paths should have already been validated with #validatePath.
*
* Case #1: No special characters
* Example: "/cat"
* Behavior: URL should start with given pattern.
*/
if (!hasWildcard && !hasDollarSign) {
return urlPath.startsWith(pattern);
/**
* Case #2: $ only
* Example: "/js$"
* Behavior: URL should be identical to pattern.
*/
} else if (!hasWildcard && hasDollarSign) {
return urlPath === pattern.slice(0, -1);
/**
* Case #3: * only
* Example: "/vendor*chunk"
* Behavior: URL should start with the string pattern that comes before the wildcard
* & later in the string contain the string pattern that comes after the wildcard.
*/
} else if (hasWildcard && !hasDollarSign) {
const [beforeWildcard, afterWildcard] = pattern.split('*');
const remainingUrl = urlPath.slice(beforeWildcard.length);
return urlPath.startsWith(beforeWildcard) && remainingUrl.includes(afterWildcard);
/**
* Case #4: $ and *
* Example: "/vendor*chunk.js$"
* Behavior: URL should start with the string pattern that comes before the wildcard
* & later in the string end with the string pattern that comes after the wildcard.
*/
} else if (hasWildcard && hasDollarSign) {
const [beforeWildcard, afterWildcard] = pattern.split('*');
const urlEnd = urlPath.slice(beforeWildcard.length);
return urlPath.startsWith(beforeWildcard) && urlEnd.endsWith(afterWildcard.slice(0, -1));
}
return false;
}

/**
* @param {Record<string, unknown>} timingBudget
* @return {LH.Budget.TimingBudget}
Expand Down Expand Up @@ -147,9 +239,11 @@ class Budget {
/** @type {LH.Budget} */
const budget = {};

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

budget.path = Budget.validatePath(path);

if (isArrayOfUnknownObjects(resourceSizes)) {
budget.resourceSizes = resourceSizes.map(Budget.validateResourceBudget);
Budget.assertNoDuplicateStrings(budget.resourceSizes.map(r => r.resourceType),
Expand Down
81 changes: 59 additions & 22 deletions lighthouse-core/test/audits/performance-budget-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,15 @@ describe('Performance: Resource budgets audit', () => {
{url: 'http://third-party.com/file.jpg', resourceType: 'Image', transferSize: 70},
]),
},
URL: {requestedURL: 'http://example.com', finalURL: 'http://example.com'},
URL: {requestedUrl: 'http://example.com', finalUrl: 'http://example.com'},
};
context = {computedCache: new Map(), settings: {}};
});

describe('with a budget.json', () => {
beforeEach(() => {
context.settings.budgets = [{
path: '/',
khempenius marked this conversation as resolved.
Show resolved Hide resolved
resourceSizes: [
{
resourceType: 'script',
Expand Down Expand Up @@ -86,6 +87,7 @@ describe('Performance: Resource budgets audit', () => {

it('convert budgets from kilobytes to bytes during calculations', async () => {
context.settings.budgets = [{
path: '/',
resourceSizes: [
{
resourceType: 'document',
Expand All @@ -98,13 +100,21 @@ describe('Performance: Resource budgets audit', () => {
});
});

it('does not mutate the budget config', async () => {
const configBefore = JSON.parse(JSON.stringify(context.settings.budgets));
await ResourceBudgetAudit.audit(artifacts, context);
const configAfter = JSON.parse(JSON.stringify(context.settings.budgets));
expect(configBefore).toEqual(configAfter);
});

it('only includes rows for resource types with budgets', async () => {
const result = await ResourceBudgetAudit.audit(artifacts, context);
expect(result.details.items).toHaveLength(2);
});

it('sorts rows by descending file size overage', async () => {
context.settings.budgets = [{
path: '/',
resourceSizes: [
{
resourceType: 'document',
Expand All @@ -126,27 +136,54 @@ describe('Performance: Resource budgets audit', () => {
expect(item.size).toBeGreaterThanOrEqual(items[index + 1].size);
});
});

it('uses the first budget in budgets', async () => {
context.settings.budgets = [{
resourceSizes: [
{
resourceType: 'image',
budget: 0,
},
],
},
{
resourceSizes: [
{
resourceType: 'script',
budget: 0,
},
],
},
];
const result = await ResourceBudgetAudit.audit(artifacts, context);
expect(result.details.items[0].resourceType).toBe('image');
describe('budget path', () => {
it('applies the last matching budget', async () => {
context.settings.budgets = [{
path: '/',
resourceSizes: [
{
resourceType: 'script',
budget: 0,
},
],
},
{
path: '/file.html',
resourceSizes: [
{
resourceType: 'image',
budget: 0,
},
],
},
{
path: '/not-a-match',
resourceSizes: [
{
resourceType: 'document',
budget: 0,
},
],
},
];
const result = await ResourceBudgetAudit.audit(artifacts, context);
expect(result.details.items[0].resourceType).toBe('image');
});
it('returns "audit does not apply" if no budget matches', async () => {
context.settings.budgets = [{
path: '/not-a-match',
resourceSizes: [
{
resourceType: 'script',
budget: 0,
},
],
},
];
const result = await ResourceBudgetAudit.audit(artifacts, context);
expect(result.details).toBeUndefined();
expect(result.notApplicable).toBe(true);
});
});
});

Expand Down
Loading