diff --git a/lighthouse-core/config/config.js b/lighthouse-core/config/config.js index 9f363c455e55..fc170e7481d4 100644 --- a/lighthouse-core/config/config.js +++ b/lighthouse-core/config/config.js @@ -381,9 +381,17 @@ class Config { * @param {!Array=} skipAuditIds * @return {!Object}>} */ - static filterCategoriesAndAudits(oldCategories, categoryIds = [], auditIds = [], - skipAuditIds = []) { + static filterCategoriesAndAudits(oldCategories, categoryIds, auditIds, skipAuditIds) { + if (auditIds && skipAuditIds) { + throw new Error('Cannot set both skipAudits and onlyAudits'); + } + const categories = {}; + const filterByIncludedCategory = !!categoryIds; + const filterByIncludedAudit = !!auditIds; + categoryIds = categoryIds || []; + auditIds = auditIds || []; + skipAuditIds = skipAuditIds || []; // warn if the category is not found categoryIds.forEach(categoryId => { @@ -400,10 +408,6 @@ class Config { return audits.find(candidate => candidate.id === auditId); }); - if (skipAuditIds.includes(auditId) && auditIds.includes(auditId)) { - log.warn('config', `audit '${auditId}' in 'onlyAudits' was also found in 'skipAudits'`); - } - if (!foundCategory) { const parentKeyName = skipAuditIds.includes(auditId) ? 'skipAudits' : 'onlyAudits'; log.warn('config', `unrecognized audit in '${parentKeyName}': ${auditId}`); @@ -418,8 +422,17 @@ class Config { Object.keys(oldCategories).forEach(categoryId => { const category = deepClone(oldCategories[categoryId]); - // filter to the audit whitelist if we didn't include the whole category - if (!categoryIds.includes(categoryId)) { + if (filterByIncludedCategory && filterByIncludedAudit) { + // If we're filtering to the category and audit whitelist, include the union of the two + if (!categoryIds.includes(categoryId)) { + category.audits = category.audits.filter(audit => auditIds.includes(audit.id)); + } + } else if (filterByIncludedCategory) { + // If we're filtering to just the category whitelist and the category is not included, skip it + if (!categoryIds.includes(categoryId)) { + return; + } + } else if (filterByIncludedAudit) { category.audits = category.audits.filter(audit => auditIds.includes(audit.id)); } diff --git a/lighthouse-core/test/config/config-test.js b/lighthouse-core/test/config/config-test.js index 4355df28f40d..b1710e30f26f 100644 --- a/lighthouse-core/test/config/config-test.js +++ b/lighthouse-core/test/config/config-test.js @@ -296,7 +296,6 @@ describe('Config', () => { settings: { onlyCategories: ['needed-category'], onlyAudits: ['color-contrast'], - skipAudits: ['first-meaningful-paint'], }, passes: [ {recordTrace: true, gatherers: []}, @@ -329,14 +328,54 @@ describe('Config', () => { }, }); - assert.ok(config.audits.length, 2); - assert.equal(config.passes.length, 2); + assert.equal(config.audits.length, 3, 'reduces audit count'); + assert.equal(config.passes.length, 2, 'preserves both passes'); assert.ok(config.passes[0].recordTrace, 'preserves recordTrace pass'); assert.ok(!config.categories['unused-category'], 'removes unused categories'); - assert.equal(config.categories['needed-category'].audits.length, 1); + assert.equal(config.categories['needed-category'].audits.length, 2); assert.equal(config.categories['other-category'].audits.length, 1); }); + it('filters the config w/ skipAudits', () => { + const config = new Config({ + settings: { + skipAudits: ['first-meaningful-paint'], + }, + passes: [ + {recordTrace: true, gatherers: []}, + {passName: 'a11y', gatherers: ['accessibility']}, + ], + audits: [ + 'accessibility/color-contrast', + 'first-meaningful-paint', + 'first-interactive', + 'estimated-input-latency', + ], + categories: { + 'needed-category': { + audits: [ + {id: 'first-meaningful-paint'}, + {id: 'first-interactive'}, + {id: 'color-contrast'}, + ], + }, + 'other-category': { + audits: [ + {id: 'color-contrast'}, + {id: 'estimated-input-latency'}, + ], + }, + }, + }); + + assert.equal(config.audits.length, 3, 'skips the FMP audit'); + assert.equal(config.passes.length, 2, 'preserves both passes'); + assert.ok(config.passes[0].recordTrace, 'preserves recordTrace pass'); + assert.equal(config.categories['needed-category'].audits.length, 2, + 'removes skipped audit from category'); + }); + + it('filtering filters out traces when not needed', () => { const warnings = []; const saveWarning = evt => warnings.push(evt); @@ -378,13 +417,24 @@ describe('Config', () => { settings: { onlyCategories: ['performance', 'missing-category'], onlyAudits: ['first-interactive', 'missing-audit'], - skipAudits: ['first-interactive'], }, }); log.events.removeListener('warning', saveWarning); assert.ok(config, 'failed to generate config'); - assert.equal(warnings.length, 4, 'did not warn enough'); + assert.equal(warnings.length, 3, 'did not warn enough'); + }); + + it('throws for invalid use of skipAudits and onlyAudits', () => { + assert.throws(() => { + new Config({ + extends: true, + settings: { + onlyAudits: ['first-meaningful-paint'], + skipAudits: ['first-meaningful-paint'], + } + }); + }); }); describe('artifact loading', () => {