feat(config): add support for audit blacklisting in isolation (#2539)

* test(config): Add assertions for just skipAudits

* fix: add support for skipAudits used in isolation (#2540)

* throw on both skipAudits + onlyAudits

lighthouse-core/config/config.js

@@ -381,9 +381,17 @@ class Config {
    * @param {!Array<string>=} skipAuditIds
    * @return {!Object<string, {audits: !Array<{id: string}>}>}
    */
-  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));
       }
 

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', () => {