From e271079d7a92604bb23f3cdefb5f550a3368bffc Mon Sep 17 00:00:00 2001 From: Patrick Hulce Date: Wed, 28 Jul 2021 11:35:04 -0500 Subject: [PATCH 1/2] core(fr): add config extension support --- lighthouse-core/config/config-helpers.js | 36 +++++ lighthouse-core/fraggle-rock/config/config.js | 36 ++++- .../fraggle-rock/config/default-config.js | 8 +- .../test/config/config-helpers-test.js | 39 ++++++ .../test/fraggle-rock/config/config-test.js | 124 +++++++++++++++++- 5 files changed, 238 insertions(+), 5 deletions(-) diff --git a/lighthouse-core/config/config-helpers.js b/lighthouse-core/config/config-helpers.js index 18a47299094d..0a580e52a600 100644 --- a/lighthouse-core/config/config-helpers.js +++ b/lighthouse-core/config/config-helpers.js @@ -93,6 +93,41 @@ function _mergeConfigFragment(base, extension, overwriteArrays = false) { */ const mergeConfigFragment = _mergeConfigFragment; +/** + * Merge an array of items by a caller-defined key. `mergeConfigFragment` is used to merge any items + * with a matching key. + * + * @template T + * @param {Array|null|undefined} baseArray + * @param {Array|null|undefined} extensionArray + * @param {(item: T) => string} keyFn + * @return {Array} + */ +function mergeConfigFragmentArrayByKey(baseArray, extensionArray, keyFn) { + /** @type {Map} */ + const itemsByKey = new Map(); + const mergedArray = baseArray || []; + for (let i = 0; i < mergedArray.length; i++) { + const item = mergedArray[i]; + itemsByKey.set(keyFn(item), {index: i, item}); + } + + for (const item of extensionArray || []) { + const baseItemEntry = itemsByKey.get(keyFn(item)); + if (baseItemEntry) { + const baseItem = baseItemEntry.item; + const merged = typeof item === 'object' && typeof baseItem === 'object' ? + mergeConfigFragment(baseItem, item, true) : + item; + mergedArray[baseItemEntry.index] = merged; + } else { + mergedArray.push(item); + } + } + + return mergedArray; +} + /** * Validate the settings after they've been built * @param {LH.Config.Settings} settings @@ -558,6 +593,7 @@ module.exports = { deepCloneConfigJson, mergeOptionsOfItems, mergeConfigFragment, + mergeConfigFragmentArrayByKey, resolveSettings, resolveGathererToDefn, resolveAuditsToDefns, diff --git a/lighthouse-core/fraggle-rock/config/config.js b/lighthouse-core/fraggle-rock/config/config.js index c5f57d9bcad7..ed8eb804b544 100644 --- a/lighthouse-core/fraggle-rock/config/config.js +++ b/lighthouse-core/fraggle-rock/config/config.js @@ -23,6 +23,8 @@ const { resolveSettings, resolveAuditsToDefns, resolveGathererToDefn, + mergeConfigFragment, + mergeConfigFragmentArrayByKey, } = require('../../config/config-helpers.js'); const defaultConfigPath = path.join(__dirname, './default-config.js'); @@ -53,6 +55,35 @@ function resolveWorkingCopy(configJSON, context) { }; } +/** + * @param {LH.Config.Json} configJSON + * @return {LH.Config.Json} + */ +function resolveExtensions(configJSON) { + if (!configJSON.extends) return configJSON; + + if (configJSON.extends !== 'lighthouse:default') { + throw new Error('`lighthouse:default` is the only valid extension method.'); + } + + const {artifacts, navigations, ...extensionJSON} = configJSON; + const defaultClone = deepCloneConfigJson(defaultConfig); + const mergedConfig = mergeConfigFragment(defaultClone, extensionJSON); + + mergedConfig.artifacts = mergeConfigFragmentArrayByKey( + defaultClone.artifacts, + artifacts, + artifact => artifact.id + ); + mergedConfig.navigations = mergeConfigFragmentArrayByKey( + defaultClone.navigations, + navigations, + navigation => navigation.id + ); + + return mergedConfig; +} + /** * Looks up the required artifact IDs for each dependency, throwing if no earlier artifact satisfies the dependency. * @@ -170,9 +201,10 @@ function initializeConfig(configJSON, context) { const status = {msg: 'Initialize config', id: 'lh:config'}; log.time(status, 'verbose'); - const {configWorkingCopy, configDir} = resolveWorkingCopy(configJSON, context); + let {configWorkingCopy, configDir} = resolveWorkingCopy(configJSON, context); // eslint-disable-line prefer-const + + configWorkingCopy = resolveExtensions(configWorkingCopy); - // TODO(FR-COMPAT): handle config extension // TODO(FR-COMPAT): handle config plugins const settings = resolveSettings(configWorkingCopy.settings || {}, context.settingsOverrides); diff --git a/lighthouse-core/fraggle-rock/config/default-config.js b/lighthouse-core/fraggle-rock/config/default-config.js index 3ba94899047b..d62f16efab1b 100644 --- a/lighthouse-core/fraggle-rock/config/default-config.js +++ b/lighthouse-core/fraggle-rock/config/default-config.js @@ -191,7 +191,13 @@ const defaultConfig = { }, ], settings: legacyDefaultConfig.settings, - audits: [...(legacyDefaultConfig.audits || []), ...frAudits], + audits: [ + ...(legacyDefaultConfig.audits || []).map(audit => { + if (typeof audit === 'string') return {path: audit}; + return audit; + }), + ...frAudits, + ], categories: mergeCategories(), groups: legacyDefaultConfig.groups, }; diff --git a/lighthouse-core/test/config/config-helpers-test.js b/lighthouse-core/test/config/config-helpers-test.js index 8713b36ade10..a3cd6bc620b4 100644 --- a/lighthouse-core/test/config/config-helpers-test.js +++ b/lighthouse-core/test/config/config-helpers-test.js @@ -16,6 +16,7 @@ const { resolveAuditsToDefns, resolveModulePath, mergeConfigFragment, + mergeConfigFragmentArrayByKey, } = require('../../config/config-helpers.js'); const Runner = require('../../runner.js'); const Gatherer = require('../../gather/gatherers/gatherer.js'); @@ -83,6 +84,44 @@ describe('.mergeConfigFragment', () => { }); }); +describe('.mergeConfigFragmentArrayByKey', () => { + it('should use mergeConfigFragment to merge items', () => { + const base = [{a: 1, b: 'yes', c: true}]; + const extension = [{a: 2, c: false, d: 123}]; + const merged = mergeConfigFragmentArrayByKey(base, extension, () => 'key'); + expect(merged).toBe(base); + expect(merged).toEqual([{a: 2, b: 'yes', c: false, d: 123}]); + }); + + it('should merge by the keyFn', () => { + const base = [{id: 'a', value: 1}, {id: 'b', value: 2}]; + const extension = [{id: 'b', value: 1}, {id: 'a', value: 2}, {id: 'c'}]; + const merged = mergeConfigFragmentArrayByKey(base, extension, item => item.id); + expect(merged).toEqual([{id: 'a', value: 2}, {id: 'b', value: 1}, {id: 'c'}]); + }); + + it('should merge recursively', () => { + const base = [{foo: {bar: 1}}]; + const extension = [{foo: {baz: 2, bam: 3}}]; + const merged = mergeConfigFragmentArrayByKey(base, extension, () => 'key'); + expect(merged).toEqual([{foo: {bar: 1, baz: 2, bam: 3}}]); + }); + + it('should handle null items in base', () => { + const base = [null]; + const extension = [{x: 1}]; + const merged = mergeConfigFragmentArrayByKey(base, extension, () => ''); + expect(merged).toEqual([{x: 1}]); + }); + + it('should handle undefined items in extension', () => { + const base = [{x: 1}]; + const extension = [undefined]; + const merged = mergeConfigFragmentArrayByKey(base, extension, () => ''); + expect(merged).toEqual([undefined]); + }); +}); + describe('.deepClone', () => { it('should clone things deeply', () => { const input = {a: {b: {c: 1}}}; diff --git a/lighthouse-core/test/fraggle-rock/config/config-test.js b/lighthouse-core/test/fraggle-rock/config/config-test.js index eaad3086fd66..0c940c4b15ef 100644 --- a/lighthouse-core/test/fraggle-rock/config/config-test.js +++ b/lighthouse-core/test/fraggle-rock/config/config-test.js @@ -5,6 +5,7 @@ */ 'use strict'; +const BaseAudit = require('../../../audits/audit.js'); const BaseGatherer = require('../../../fraggle-rock/gather/base-gatherer.js'); const {initializeConfig} = require('../../../fraggle-rock/config/config.js'); @@ -262,9 +263,128 @@ describe('Fraggle Rock Config', () => { }); }); - it.todo('should support extension'); + describe('.resolveExtensions', () => { + /** @type {LH.Config.Json} */ + let extensionConfig; + + beforeEach(() => { + const gatherer = new BaseGatherer(); + gatherer.meta = {supportedModes: ['navigation']}; + + class ExtraAudit extends BaseAudit { + static get meta() { + return { + id: 'extra-audit', + title: 'Extra', + description: 'Extra', + requiredArtifacts: /** @type {*} */ (['ExtraArtifact']), + }; + } + + /** @return {LH.Audit.Product} */ + static audit() { + throw new Error('Unimplemented'); + } + } + + extensionConfig = { + extends: 'lighthouse:default', + artifacts: [ + {id: 'ExtraArtifact', gatherer: {instance: gatherer}}, + ], + navigations: [ + {id: 'default', artifacts: ['ExtraArtifact']}, + ], + audits: [ + {implementation: ExtraAudit}, + ], + categories: { + performance: { + title: 'Performance', + auditRefs: [ + {id: 'extra-audit', weight: 0}, + ], + }, + }, + }; + }); + + it('should do nothing when not extending', () => { + const {config} = initializeConfig({ + artifacts: [ + {id: 'Accessibility', gatherer: 'accessibility'}, + ], + navigations: [ + {id: 'default', artifacts: ['Accessibility']}, + ], + }, {gatherMode: 'navigation'}); + + expect(config).toMatchObject({ + audits: null, + groups: null, + artifacts: [ + {id: 'Accessibility'}, + ], + navigations: [ + {id: 'default', artifacts: [{id: 'Accessibility'}]}, + ], + }); + }); + + it('should extend the default config', () => { + const gatherMode = 'navigation'; + const {config} = initializeConfig({extends: 'lighthouse:default'}, {gatherMode}); + if (!config.artifacts) throw new Error(`No artifacts created`); + if (!config.audits) throw new Error(`No audits created`); + + + const hasAccessibilityArtifact = config.artifacts.some(a => a.id === 'Accessibility'); + if (!hasAccessibilityArtifact) expect(config.artifacts).toContain('Accessibility'); + + const hasAccessibilityAudit = config.audits. + some(a => a.implementation.meta.id === 'color-contrast'); + if (!hasAccessibilityAudit) expect(config.audits).toContain('color-contrast'); + + expect(config.categories).toHaveProperty('performance'); + }); + + it('should merge in artifacts', () => { + const {config} = initializeConfig(extensionConfig, {gatherMode: 'navigation'}); + if (!config.artifacts) throw new Error(`No artifacts created`); + + const hasExtraArtifact = config.artifacts.some(a => a.id === 'ExtraArtifact'); + if (!hasExtraArtifact) expect(config.artifacts).toContain('ExtraArtifact'); + }); + + it('should merge in navigations', () => { + const {config} = initializeConfig(extensionConfig, {gatherMode: 'navigation'}); + if (!config.navigations) throw new Error(`No navigations created`); + + expect(config.navigations).toHaveLength(1); + const hasNavigation = config.navigations[0].artifacts. + some(a => a.id === 'ExtraArtifact'); + if (!hasNavigation) expect(config.navigations[0].artifacts).toContain('ExtraArtifact'); + }); + + it('should merge in audits', () => { + const {config} = initializeConfig(extensionConfig, {gatherMode: 'navigation'}); + if (!config.audits) throw new Error(`No audits created`); + + const hasExtraAudit = config.audits. + some(a => a.implementation.meta.id === 'extra-audit'); + if (!hasExtraAudit) expect(config.audits).toContain('extra-audit'); + }); + + it('should merge in categories', () => { + const {config} = initializeConfig(extensionConfig, {gatherMode: 'navigation'}); + if (!config.categories) throw new Error(`No categories created`); + + const hasCategory = config.categories.performance.auditRefs.some(a => a.id === 'extra-audit'); + if (!hasCategory) expect(config.categories.performance.auditRefs).toContain('extra-audit'); + }); + }); + it.todo('should support plugins'); it.todo('should adjust default pass options for throttling method'); it.todo('should validate audit/gatherer interdependencies'); - it.todo('should validate gatherers do not support all 3 modes'); }); From be484cacc194be14f001d0a8e69b35a073f11e2f Mon Sep 17 00:00:00 2001 From: Patrick Hulce Date: Wed, 28 Jul 2021 11:38:08 -0500 Subject: [PATCH 2/2] add filtering test --- .../test/fraggle-rock/config/config-test.js | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/lighthouse-core/test/fraggle-rock/config/config-test.js b/lighthouse-core/test/fraggle-rock/config/config-test.js index 0c940c4b15ef..3d73b666ef4c 100644 --- a/lighthouse-core/test/fraggle-rock/config/config-test.js +++ b/lighthouse-core/test/fraggle-rock/config/config-test.js @@ -331,13 +331,15 @@ describe('Fraggle Rock Config', () => { }); }); - it('should extend the default config', () => { + it('should extend the default config with filters', () => { const gatherMode = 'navigation'; - const {config} = initializeConfig({extends: 'lighthouse:default'}, {gatherMode}); + const {config} = initializeConfig({ + extends: 'lighthouse:default', + settings: {onlyCategories: ['accessibility']}, + }, {gatherMode}); if (!config.artifacts) throw new Error(`No artifacts created`); if (!config.audits) throw new Error(`No audits created`); - const hasAccessibilityArtifact = config.artifacts.some(a => a.id === 'Accessibility'); if (!hasAccessibilityArtifact) expect(config.artifacts).toContain('Accessibility'); @@ -345,7 +347,8 @@ describe('Fraggle Rock Config', () => { some(a => a.implementation.meta.id === 'color-contrast'); if (!hasAccessibilityAudit) expect(config.audits).toContain('color-contrast'); - expect(config.categories).toHaveProperty('performance'); + expect(config.categories).toHaveProperty('accessibility'); + expect(config.categories).not.toHaveProperty('performance'); }); it('should merge in artifacts', () => {