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(config): special case full-page-screenshot audit in filtering #11829

Merged
merged 9 commits into from
Dec 16, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions lighthouse-core/config/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -656,6 +656,16 @@ class Config {
}
});

// The `full-page-screenshot` audit belongs to no category, but we still want to include
// it (unless explictly excluded) because there are audits in every category that can use it.
if (settings.onlyCategories) {
adamraine marked this conversation as resolved.
Show resolved Hide resolved
const explicitlyExcludesFullPageScreenshot =
settings.skipAudits && settings.skipAudits.includes('full-page-screenshot');
if (!explicitlyExcludesFullPageScreenshot) {
includedAudits.add('full-page-screenshot');
}
}

return {categories, requestedAuditNames: includedAudits};
}

Expand Down
36 changes: 32 additions & 4 deletions lighthouse-core/test/config/config-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -663,8 +663,11 @@ describe('Config', () => {
});

assert.ok(config.audits.length, 'inherited audits by extension');
assert.equal(config.audits.length, origConfig.categories.performance.auditRefs.length + 1);
// +1 for `is-on-https`, +1 for `full-page-screenshot`.
assert.equal(config.audits.length, origConfig.categories.performance.auditRefs.length + 2);
assert.equal(config.passes.length, 1, 'filtered out passes');
assert.ok(config.audits.find(a => a.implementation.meta.id === 'is-on-https'));
assert.ok(config.audits.find(a => a.implementation.meta.id === 'full-page-screenshot'));
});

it('warns for invalid filters', () => {
Expand Down Expand Up @@ -1138,9 +1141,11 @@ describe('Config', () => {
};
const config = new Config(extended);
const selectedCategory = origConfig.categories.performance;
const auditCount = Object.keys(selectedCategory.auditRefs).length;
// +1 for `full-page-screenshot`.
const auditCount = Object.keys(selectedCategory.auditRefs).length + 1;

assert.equal(config.audits.length, auditCount, '# of audits match category list');
assert.ok(config.audits.find(a => a.implementation.meta.id === 'full-page-screenshot'));
});

it('should only run specified audits', () => {
Expand All @@ -1165,9 +1170,12 @@ describe('Config', () => {
};
const config = new Config(extended);
const selectedCategory = origConfig.categories.performance;
const auditCount = Object.keys(selectedCategory.auditRefs).length + 1;
// +1 for `service-worker`, +1 for `full-page-screenshot`.
const auditCount = Object.keys(selectedCategory.auditRefs).length + 2;
assert.equal(config.passes.length, 2, 'incorrect # of passes');
assert.equal(config.audits.length, auditCount, 'audit filtering failed');
assert.ok(config.audits.find(a => a.implementation.meta.id === 'service-worker'));
assert.ok(config.audits.find(a => a.implementation.meta.id === 'full-page-screenshot'));
});

it('should support redundant filtering', () => {
Expand All @@ -1180,9 +1188,29 @@ describe('Config', () => {
};
const config = new Config(extended);
const selectedCategory = origConfig.categories.pwa;
const auditCount = Object.keys(selectedCategory.auditRefs).length;
// +1 for `full-page-screenshot`.
const auditCount = Object.keys(selectedCategory.auditRefs).length + 1;
assert.equal(config.passes.length, 3, 'incorrect # of passes');
assert.equal(config.audits.length, auditCount, 'audit filtering failed');
assert.ok(config.audits.find(a => a.implementation.meta.id === 'full-page-screenshot'));
});

it('should keep uncategorized audits even if onlyCategories is set', () => {
assert.ok(origConfig.audits.includes('full-page-screenshot'));
// full-page-screenshot does not belong to a category.
const matchCategories = Object.values(origConfig.categories).filter(cat =>
cat.auditRefs.find(ref => ref.id === 'full-page-screenshot'));
assert.equal(matchCategories.length, 0);

const extended = {
extends: 'lighthouse:default',
settings: {
onlyCategories: ['accessibility'],
},
};
const config = new Config(extended);

assert.ok(config.audits.find(a => a.implementation.meta.id === 'full-page-screenshot'));
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,5 @@ Tests that exporting works.

++++++++ testExportJson

# of audits (json): 153
# of audits (json): 154

Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ Generate report: enabled visible
=============== Audits run ===============
apple-touch-icon
content-width
full-page-screenshot
installable-manifest
maskable-icon
pwa-cross-browser
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ Gathering setup: Accessibility
Gathering setup: TraceElements
Gathering setup: InspectorIssues
Gathering setup: SourceMaps
Gathering setup: FullPageScreenshot
Beginning devtoolsLog and trace
Getting browser version
Loading page & waiting for onload
Expand Down Expand Up @@ -86,6 +87,7 @@ Gathering in-page: Accessibility
Gathering in-page: TraceElements
Gathering in-page: InspectorIssues
Gathering in-page: SourceMaps
Gathering in-page: FullPageScreenshot
Gathering trace
Gathering devtoolsLog & network records
Running afterPass methods
Expand Down Expand Up @@ -116,6 +118,7 @@ Gathering: Accessibility
Gathering: TraceElements
Gathering: InspectorIssues
Gathering: SourceMaps
Gathering: FullPageScreenshot
Populate base artifacts
Get webapp manifest
Collect stacks
Expand Down Expand Up @@ -239,6 +242,7 @@ Auditing: Page has valid source maps
Auditing: Preload Largest Contentful Paint image
Computing artifact: LanternLargestContentfulPaint
Computing artifact: LanternFirstContentfulPaint
Auditing: Full-page screenshot
Auditing: Site works cross-browser
Auditing: Page transitions don't feel like they block on the network
Auditing: Each page has a URL
Expand Down Expand Up @@ -455,6 +459,7 @@ font-display: pass
font-size: fail
form-field-multiple-labels: notApplicable
frame-title: notApplicable
full-page-screenshot: informative
geolocation-on-start: pass
gpt-bids-parallel: notApplicable
heading-order: notApplicable
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ first-contentful-paint
first-cpu-idle
first-meaningful-paint
font-display
full-page-screenshot
interactive
largest-contentful-paint
largest-contentful-paint-element
Expand Down