core(config): special case full-page-screenshot audit in filtering

[connorjclark]

not pretty. potentially better than alternative (?), which is to ditch the FPS audit and make it a property of the LHR... (lhr.fullPageScreenshot).

why: CDT uses onlyCategories, which means this audit is always skipped because it belongs to no category :(

[patrickhulce]

Just throwing this alternative out there, put it in all of them just hidden? :)

[brendankenny]

potentially better than alternative (?), which is to ditch the FPS audit and make it a property of the LHR... (lhr.fullPageScreenshot).

The property could go wherever feels good. The reasoning is that the desired behavior is basically exactly the same as stackPacks. Gathered and audited regardless of which audits are run or filtered out, and the results aren't used as an audit in the report, they're used to provide extra information in a bunch of other audits.

Just throwing this alternative out there, put it in all of them just hidden? :)

We could revive @adrianaixba's hidden audit PR :)

[connorjclark]

We could revive @adrianaixba's hidden audit PR :)

I'd prefer this, but how much work does that entail? If we are pressed for time this PR seems fine to me, for now.

[patrickhulce]

I'd prefer this, but how much work does that entail? If we are pressed for time this PR seems fine to me, for now.

Probably not worth it, IMO, but seemed like you were searching for alternatives :)

I'd say we can just explore the lhr.fullPageScreenshot style for v8 and this works until then.

[benschwarz]

We use onlyAudits and onlyCategories quite a bit. FPS is one of the audits that's regularly skipped by our test verification/scheduler as it caused PROTOCOL_TIMEOUT or crashed Chrome for certain sites.

Are you saying that it'd not be configurable to opt out of it running?

[patrickhulce]

@benschwarz you can still use skipAudits to opt-out of it, see https://github.com/GoogleChrome/lighthouse/pull/11829/files#diff-185377a98c18b94b4fb8a22d45b34794522f1cc2edfcc2d730432dc5f9e47d57R665-R667

[paulirish]

here i wrote you a test :)

    it('should keep uncategorized audits even if onlyCategories is set', () => {
      assert.ok(origConfig.audits.includes('full-page-screenshot'));
      // f-p-s is non-categorized
      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.path.includes('full-page-screenshot')));
    });

[adamraine]

Overall LGTM

[paulirish]

coolio. @brendankenny is gonna write up an issue exploring our options for where we can take this.