From 66c41126daf11f1db74f381ea09c586df8ae9f8e Mon Sep 17 00:00:00 2001 From: hanna-skryl Date: Thu, 19 Sep 2024 16:52:32 -0400 Subject: [PATCH] refactor(cli): improve plugin filter option validation --- .../validate-plugin-filter-options.utils.ts | 24 +++--- ...e-plugin-filter-options.utils.unit.test.ts | 75 +++++++++++++++---- 2 files changed, 69 insertions(+), 30 deletions(-) diff --git a/packages/cli/src/lib/implementation/validate-plugin-filter-options.utils.ts b/packages/cli/src/lib/implementation/validate-plugin-filter-options.utils.ts index f47376bd1..358d969d6 100644 --- a/packages/cli/src/lib/implementation/validate-plugin-filter-options.utils.ts +++ b/packages/cli/src/lib/implementation/validate-plugin-filter-options.utils.ts @@ -1,4 +1,3 @@ -import { yellow } from 'ansis'; import type { CategoryConfig, PluginConfig } from '@code-pushup/models'; import { filterItemRefsBy, ui } from '@code-pushup/utils'; @@ -28,15 +27,13 @@ export function validatePluginFilterOption( ? pluginsToFilterSet.has(plugin) : !pluginsToFilterSet.has(plugin); - if (missingPlugins.length > 0 && verbose) { - ui().logger.info( - `${yellow( - '⚠', - )} The --${filterOption} argument references plugins with "${missingPlugins.join( - '", "', - )}" slugs, but no such plugins are present in the configuration. Expected one of the following plugin slugs: "${plugins - .map(({ slug }) => slug) - .join('", "')}".`, + if (missingPlugins.length > 0) { + ui().logger.warning( + `The --${filterOption} argument references ${ + missingPlugins.length === 1 ? 'a plugin that does' : 'plugins that do' + } not exist: ${missingPlugins.join(', ')}. The valid plugin ${ + plugins.length === 1 ? 'slug is' : 'slugs are' + } ${plugins.map(({ slug }) => slug).join(', ')}.`, ); } @@ -45,10 +42,9 @@ export function validatePluginFilterOption( filterFunction(plugin), ).map(({ slug }) => slug); ui().logger.info( - `The --${filterOption} argument removed categories with "${removedCategorySlugs.join( - '", "', - )}" slugs. - `, + `The --${filterOption} argument removed the following categories: ${removedCategorySlugs.join( + ', ', + )}.`, ); } } diff --git a/packages/cli/src/lib/implementation/validate-plugin-filter-options.utils.unit.test.ts b/packages/cli/src/lib/implementation/validate-plugin-filter-options.utils.unit.test.ts index 56bfb6717..099896b46 100644 --- a/packages/cli/src/lib/implementation/validate-plugin-filter-options.utils.unit.test.ts +++ b/packages/cli/src/lib/implementation/validate-plugin-filter-options.utils.unit.test.ts @@ -6,7 +6,22 @@ import { validatePluginFilterOption } from './validate-plugin-filter-options.uti describe('validatePluginFilterOption', () => { describe('onlyPlugins', () => { - it('should warn if onlyPlugins option contains non-existing plugin', () => { + it('should log a warning if the onlyPlugins argument contains multiple nonexistent plugins', () => { + validatePluginFilterOption( + 'onlyPlugins', + { + plugins: [{ slug: 'plugin1', audits: [{}] }] as PluginConfig[], + categories: [], + }, + { pluginsToFilter: ['plugin1', 'plugin3', 'plugin4'] }, + ); + const logs = getLogMessages(ui().logger); + expect(logs[0]).toContain( + 'The --onlyPlugins argument references plugins that do not exist: plugin3, plugin4.', + ); + }); + + it('should log a warning if the onlyPlugins argument contains one nonexistent plugin', () => { validatePluginFilterOption( 'onlyPlugins', { @@ -15,31 +30,44 @@ describe('validatePluginFilterOption', () => { ] as PluginConfig[], categories: [], }, - { - pluginsToFilter: ['plugin1', 'plugin3', 'plugin4'], - verbose: true, - }, + { pluginsToFilter: ['plugin1', 'plugin2'] }, ); const logs = getLogMessages(ui().logger); expect(logs[0]).toContain( - 'The --onlyPlugins argument references plugins with "plugin3", "plugin4" slugs', + 'The --onlyPlugins argument references a plugin that does not exist: plugin2.', ); }); - it('should not log if onlyPlugins option contains only existing plugins', () => { + it('should include all valid plugin slugs in a warning', () => { validatePluginFilterOption( 'onlyPlugins', { plugins: [ { slug: 'plugin1', audits: [{ slug: 'a1-p1' }] }, { slug: 'plugin2', audits: [{ slug: 'a1-p2' }] }, + { slug: 'plugin3', audits: [{ slug: 'a1-p3' }] }, ] as PluginConfig[], categories: [], }, + { pluginsToFilter: ['plugin4'] }, + ); + const logs = getLogMessages(ui().logger); + expect(logs[0]).toContain( + 'The valid plugin slugs are plugin1, plugin2, plugin3.', + ); + }); + + it('should not log anything if the onlyPlugins argument contains only valid plugins', () => { + validatePluginFilterOption( + 'onlyPlugins', { - pluginsToFilter: ['plugin1'], - verbose: true, + plugins: [ + { slug: 'plugin1', audits: [{ slug: 'a1-p1' }] }, + { slug: 'plugin2', audits: [{ slug: 'a1-p2' }] }, + ] as PluginConfig[], + categories: [], }, + { pluginsToFilter: ['plugin1'] }, ); expect(getLogMessages(ui().logger)).toHaveLength(0); }); @@ -65,12 +93,13 @@ describe('validatePluginFilterOption', () => { ); expect(getLogMessages(ui().logger)).toHaveLength(1); expect(getLogMessages(ui().logger)[0]).toContain( - 'The --onlyPlugins argument removed categories with "c1", "c3" slugs', + 'The --onlyPlugins argument removed the following categories: c1, c3', ); }); }); + describe('skipPlugins', () => { - it('should warn if skipPlugins option contains non-existing plugin', () => { + it('should log a warning if the skipPlugins argument contains multiple nonexistent plugins', () => { validatePluginFilterOption( 'skipPlugins', { @@ -79,18 +108,32 @@ describe('validatePluginFilterOption', () => { ] as PluginConfig[], categories: [], }, + { pluginsToFilter: ['plugin1', 'plugin3', 'plugin4'] }, + ); + const logs = getLogMessages(ui().logger); + expect(logs[0]).toContain( + 'The --skipPlugins argument references plugins that do not exist: plugin3, plugin4.', + ); + }); + + it('should log a warning if the skipPlugins argument contains one nonexistent plugin', () => { + validatePluginFilterOption( + 'skipPlugins', { - pluginsToFilter: ['plugin1', 'plugin3', 'plugin4'], - verbose: true, + plugins: [ + { slug: 'plugin1', audits: [{ slug: 'a1' }] }, + ] as PluginConfig[], + categories: [], }, + { pluginsToFilter: ['plugin1', 'plugin2'] }, ); const logs = getLogMessages(ui().logger); expect(logs[0]).toContain( - 'The --skipPlugins argument references plugins with "plugin3", "plugin4" slugs', + 'The --skipPlugins argument references a plugin that does not exist: plugin2.', ); }); - it('should not log if skipPlugins option contains only existing plugins', () => { + it('should not log anything if the skipPlugins argument contains only valid plugins', () => { validatePluginFilterOption( 'skipPlugins', { @@ -129,7 +172,7 @@ describe('validatePluginFilterOption', () => { ); expect(getLogMessages(ui().logger)).toHaveLength(1); expect(getLogMessages(ui().logger)[0]).toContain( - 'The --skipPlugins argument removed categories with "c1", "c3" slugs', + 'The --skipPlugins argument removed the following categories: c1, c3.', ); }); });