Skip to content

Commit

Permalink
core(fr): filter configs by gather mode (#11941)
Browse files Browse the repository at this point in the history
  • Loading branch information
patrickhulce authored and paulirish committed Mar 23, 2021
1 parent 0a899ae commit c04126b
Show file tree
Hide file tree
Showing 10 changed files with 343 additions and 42 deletions.
25 changes: 5 additions & 20 deletions lighthouse-core/fraggle-rock/api.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,19 +7,11 @@

const Driver = require('./gather/driver.js');
const Runner = require('../runner.js');
const Config = require('../config/config.js');

/**
* @param {LH.Gatherer.GathererInstance|LH.Gatherer.FRGathererInstance} gatherer
* @return {gatherer is LH.Gatherer.FRGathererInstance}
*/
function isFRGatherer(gatherer) {
return 'meta' in gatherer;
}
const {initializeConfig} = require('./config/config.js');

/** @param {{page: import('puppeteer').Page, config?: LH.Config.Json}} options */
async function snapshot(options) {
const config = new Config(options.config);
const {config} = initializeConfig(options.config, {gatherMode: 'snapshot'});
const driver = new Driver(options.page);
await driver.connect();

Expand Down Expand Up @@ -47,20 +39,13 @@ async function snapshot(options) {
PageLoadError: null,
};

const gatherers = (config.passes || [])
.flatMap(pass => pass.gatherers);

/** @type {Partial<LH.GathererArtifacts>} */
const artifacts = {};

for (const {instance} of gatherers) {
if (!isFRGatherer(instance)) continue;
if (!instance.meta.supportedModes.includes('snapshot')) continue;

/** @type {keyof LH.GathererArtifacts} */
const artifactName = instance.name;
for (const {id, gatherer} of config.artifacts || []) {
const artifactName = /** @type {keyof LH.GathererArtifacts} */ (id);
const artifact = await Promise.resolve()
.then(() => instance.snapshot({gatherMode: 'snapshot', driver}))
.then(() => gatherer.instance.snapshot({gatherMode: 'snapshot', driver}))
.catch(err => err);

artifacts[artifactName] = artifact;
Expand Down
17 changes: 13 additions & 4 deletions lighthouse-core/fraggle-rock/config/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ const path = require('path');
const log = require('lighthouse-logger');
const Runner = require('../../runner.js');
const defaultConfig = require('./default-config.js');
const {isFRGathererDefn} = require('./validation.js');
const {filterConfigByGatherMode} = require('./filters.js');
const {
deepCloneConfigJson,
resolveSettings,
Expand Down Expand Up @@ -58,9 +60,14 @@ function resolveArtifactsToDefns(artifacts, configDir) {

const coreGathererList = Runner.getGathererList();
const artifactDefns = artifacts.map(artifactJson => {
const gatherer = resolveGathererToDefn(artifactJson.gatherer, coreGathererList, configDir);
if (!isFRGathererDefn(gatherer)) {
throw new Error(`${gatherer.instance.name} gatherer does not support Fraggle Rock`);
}

return {
id: artifactJson.id,
gatherer: resolveGathererToDefn(artifactJson.gatherer, coreGathererList, configDir),
gatherer,
};
});

Expand All @@ -70,7 +77,7 @@ function resolveArtifactsToDefns(artifacts, configDir) {

/**
* @param {LH.Config.Json|undefined} configJSON
* @param {{configPath?: string, settingsOverrides?: LH.SharedFlagsSettings}} context
* @param {{gatherMode: LH.Gatherer.GatherMode, configPath?: string, settingsOverrides?: LH.SharedFlagsSettings}} context
* @return {{config: LH.Config.FRConfig, warnings: string[]}}
*/
function initializeConfig(configJSON, context) {
Expand All @@ -87,18 +94,20 @@ function initializeConfig(configJSON, context) {
const artifacts = resolveArtifactsToDefns(configWorkingCopy.artifacts, configDir);

/** @type {LH.Config.FRConfig} */
const config = {
let config = {
artifacts,
audits: resolveAuditsToDefns(configWorkingCopy.audits, configDir),
categories: configWorkingCopy.categories || null,
groups: configWorkingCopy.groups || null,
settings,
};

// TODO(FR-COMPAT): filter config
// TODO(FR-COMPAT): validate navigations
// TODO(FR-COMPAT): validate audits
// TODO(FR-COMPAT): validate categories
// TODO(FR-COMPAT): filter config using onlyAudits/onlyCategories

config = filterConfigByGatherMode(config, context.gatherMode);

log.timeEnd(status);
return {config, warnings: []};
Expand Down
88 changes: 88 additions & 0 deletions lighthouse-core/fraggle-rock/config/filters.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
/**
* @license Copyright 2021 The Lighthouse Authors. All Rights Reserved.
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0
* Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License.
*/
'use strict';

/**
* Filters an array of artifacts down to the set that supports the specified gather mode.
*
* @param {LH.Config.FRConfig['artifacts']} artifacts
* @param {LH.Gatherer.GatherMode} mode
* @return {LH.Config.FRConfig['artifacts']}
*/
function filterArtifactsByGatherMode(artifacts, mode) {
if (!artifacts) return null;
return artifacts.filter(artifact => {
return artifact.gatherer.instance.meta.supportedModes.includes(mode);
});
}

/**
* Filters an array of audits down to the set that can be computed using only the specified artifacts.
*
* @param {LH.Config.FRConfig['audits']} audits
* @param {Array<LH.Config.ArtifactDefn>} availableArtifacts
* @return {LH.Config.FRConfig['audits']}
*/
function filterAuditsByAvailableArtifacts(audits, availableArtifacts) {
if (!audits) return null;

const availableAritfactIds = new Set(availableArtifacts.map(artifact => artifact.id));

return audits.filter(audit =>
audit.implementation.meta.requiredArtifacts.every(id => availableAritfactIds.has(id))
);
}

/**
* Filters a categories object and their auditRefs down to the set that can be computed using
* only the specified audits.
*
* @param {LH.Config.Config['categories']} categories
* @param {Array<LH.Config.AuditDefn>} availableAudits
* @return {LH.Config.Config['categories']}
*/
function filterCategoriesByAvailableAudits(categories, availableAudits) {
if (!categories) return categories;

const availableAuditIds = new Set(availableAudits.map(audit => audit.implementation.meta.id));

return Object.fromEntries(
Object.entries(categories).map(([categoryId, category]) => {
const filteredCategory = {
...category,
auditRefs: category.auditRefs.filter(ref => availableAuditIds.has(ref.id)),
};
return [categoryId, filteredCategory];
})
);
}

/**
* Filters a config's artifacts, audits, and categories down to the set that supports the specified gather mode.
*
* @param {LH.Config.FRConfig} config
* @param {LH.Gatherer.GatherMode} mode
* @return {LH.Config.FRConfig}
*/
function filterConfigByGatherMode(config, mode) {
const artifacts = filterArtifactsByGatherMode(config.artifacts, mode);
const audits = filterAuditsByAvailableArtifacts(config.audits, artifacts || []);
const categories = filterCategoriesByAvailableAudits(config.categories, audits || []);

return {
...config,
artifacts,
audits,
categories,
};
}

module.exports = {
filterConfigByGatherMode,
filterArtifactsByGatherMode,
filterAuditsByAvailableArtifacts,
filterCategoriesByAvailableAudits,
};
16 changes: 16 additions & 0 deletions lighthouse-core/fraggle-rock/config/validation.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
/**
* @license Copyright 2021 The Lighthouse Authors. All Rights Reserved.
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0
* Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License.
*/
'use strict';

/**
* @param {LH.Config.GathererDefn | LH.Config.FRGathererDefn} gathererDefn
* @return {gathererDefn is LH.Config.FRGathererDefn}
*/
function isFRGathererDefn(gathererDefn) {
return 'meta' in gathererDefn.instance;
}

module.exports = {isFRGathererDefn};
5 changes: 3 additions & 2 deletions lighthouse-core/runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,9 @@ const LHError = require('./lib/lh-error.js');

class Runner {
/**
* @param {(runnerData: {requestedUrl: string, config: Config, driverMock?: Driver}) => Promise<LH.Artifacts>} gatherFn
* @param {{config: Config, url?: string, driverMock?: Driver}} runOpts
* @template {LH.Config.Config | LH.Config.FRConfig} TConfig
* @param {(runnerData: {requestedUrl: string, config: TConfig, driverMock?: Driver}) => Promise<LH.Artifacts>} gatherFn
* @param {{config: TConfig, url?: string, driverMock?: Driver}} runOpts
* @return {Promise<LH.RunnerResult|undefined>}
*/
static async run(gatherFn, runOpts) {
Expand Down
13 changes: 8 additions & 5 deletions lighthouse-core/test/fraggle-rock/api-test-pptr.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,13 +66,16 @@ describe('Fraggle Rock API', () => {
const accessibility = lhr.categories.accessibility;
expect(accessibility.score).toBeLessThan(1);

const auditResults = accessibility.auditRefs.map(ref => lhr.audits[ref.id]);
const auditResults = Object.values(lhr.audits);
// TODO(FR-COMPAT): This assertion can be removed when full compatibility is reached.
expect(auditResults.length).toMatchInlineSnapshot(`58`);

const irrelevantDisplayModes = new Set(['notApplicable', 'manual']);
const applicableAudits = auditResults
.filter(audit => !irrelevantDisplayModes.has(audit.scoreDisplayMode));
const applicableAudits = auditResults.filter(
audit => !irrelevantDisplayModes.has(audit.scoreDisplayMode)
);

const erroredAudits = applicableAudits
.filter(audit => audit.score === null);
const erroredAudits = applicableAudits.filter(audit => audit.score === null);
expect(erroredAudits).toHaveLength(0);

const failedAuditIds = applicableAudits
Expand Down
49 changes: 39 additions & 10 deletions lighthouse-core/test/fraggle-rock/config/config-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,27 +5,31 @@
*/
'use strict';

const BaseGatherer = require('../../../fraggle-rock/gather/base-gatherer.js');
const {initializeConfig} = require('../../../fraggle-rock/config/config.js');

/* eslint-env jest */

describe('Fraggle Rock Config', () => {
const gatherMode = 'snapshot';

it('should throw if the config path is not absolute', () => {
const configFn = () => initializeConfig(undefined, {configPath: '../relative/path'});
const configFn = () =>
initializeConfig(undefined, {gatherMode, configPath: '../relative/path'});
expect(configFn).toThrow(/must be an absolute path/);
});

it('should not mutate the original input', () => {
const configJson = {artifacts: [{id: 'ImageElements', gatherer: 'image-elements'}]};
const {config} = initializeConfig(configJson, {});
expect(configJson).toEqual({artifacts: [{id: 'ImageElements', gatherer: 'image-elements'}]});
const configJson = {artifacts: [{id: 'Accessibility', gatherer: 'accessibility'}]};
const {config} = initializeConfig(configJson, {gatherMode});
expect(configJson).toEqual({artifacts: [{id: 'Accessibility', gatherer: 'accessibility'}]});
expect(config).not.toBe(configJson);
expect(config).not.toEqual(configJson);
expect(config.artifacts).toMatchObject([{gatherer: {path: 'image-elements'}}]);
expect(config.artifacts).toMatchObject([{gatherer: {path: 'accessibility'}}]);
});

it('should use default config when none passed in', () => {
const {config} = initializeConfig(undefined, {});
const {config} = initializeConfig(undefined, {gatherMode});
expect(config.settings).toMatchObject({formFactor: 'mobile'});
if (!config.audits) throw new Error('Did not define audits');
expect(config.audits.length).toBeGreaterThan(0);
Expand All @@ -34,7 +38,7 @@ describe('Fraggle Rock Config', () => {
it('should resolve settings with defaults', () => {
const {config} = initializeConfig(
{settings: {output: 'csv', maxWaitForFcp: 1234}},
{settingsOverrides: {maxWaitForFcp: 12345}}
{settingsOverrides: {maxWaitForFcp: 12345}, gatherMode}
);

expect(config.settings).toMatchObject({
Expand All @@ -45,11 +49,35 @@ describe('Fraggle Rock Config', () => {
});

it('should resolve artifact definitions', () => {
const configJson = {artifacts: [{id: 'Accessibility', gatherer: 'accessibility'}]};
const {config} = initializeConfig(configJson, {gatherMode});

expect(config).toMatchObject({
artifacts: [{id: 'Accessibility', gatherer: {path: 'accessibility'}}],
});
});

it('should throw on invalid artifact definitions', () => {
const configJson = {artifacts: [{id: 'ImageElements', gatherer: 'image-elements'}]};
const {config} = initializeConfig(configJson, {});
expect(() => initializeConfig(configJson, {gatherMode})).toThrow(/ImageElements gatherer/);
});

it('should filter configuration by gatherMode', () => {
const timespanGatherer = new BaseGatherer();
timespanGatherer.meta = {supportedModes: ['timespan']};

const configJson = {
artifacts: [
{id: 'Accessibility', gatherer: 'accessibility'},
{id: 'Timespan', gatherer: {instance: timespanGatherer}},
],
};

const {config} = initializeConfig(configJson, {gatherMode: 'snapshot'});
expect(config).toMatchObject({
artifacts: [{id: 'ImageElements', gatherer: {path: 'image-elements'}}],
artifacts: [
{id: 'Accessibility', gatherer: {path: 'accessibility'}},
],
});
});

Expand All @@ -59,6 +87,7 @@ describe('Fraggle Rock Config', () => {
it.todo('should adjust default pass options for throttling method');
it.todo('should normalize gatherer inputs');
it.todo('should require gatherers from their paths');
it.todo('should filter configuration');
it.todo('should filter configuration by inclusive settings');
it.todo('should filter configuration by exclusive settings');
it.todo('should validate audit/gatherer interdependencies');
});
Loading

0 comments on commit c04126b

Please sign in to comment.