Skip to content

Commit

Permalink
core(config): special case full-page-screenshot audit in filtering (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
connorjclark committed Dec 16, 2020
1 parent 9dbb0a5 commit 7978f63
Show file tree
Hide file tree
Showing 6 changed files with 50 additions and 5 deletions.
10 changes: 10 additions & 0 deletions lighthouse-core/config/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -710,6 +710,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) {
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 @@ -664,8 +664,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 @@ -1180,9 +1183,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 @@ -1207,9 +1212,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 @@ -1222,9 +1230,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 @@ -454,6 +458,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

0 comments on commit 7978f63

Please sign in to comment.