From a035267a70773e6d41a47c43d39b150444e7cf97 Mon Sep 17 00:00:00 2001 From: Michael Date: Wed, 24 Apr 2024 00:11:52 +0200 Subject: [PATCH 1/4] refactor(core): add better error messages --- .../src/lib/implementation/execute-plugin.ts | 58 +++++++++++++------ .../execute-plugin.unit.test.ts | 12 ++-- 2 files changed, 44 insertions(+), 26 deletions(-) diff --git a/packages/core/src/lib/implementation/execute-plugin.ts b/packages/core/src/lib/implementation/execute-plugin.ts index a08b5f924..8315b8287 100644 --- a/packages/core/src/lib/implementation/execute-plugin.ts +++ b/packages/core/src/lib/implementation/execute-plugin.ts @@ -10,9 +10,11 @@ import { auditOutputsSchema, } from '@code-pushup/models'; import { + ProgressBar, getProgressBar, groupByStatus, logMultipleResults, + pluralizeToken, } from '@code-pushup/utils'; import { normalizeAuditOutputs } from '../normalize'; import { executeRunnerConfig, executeRunnerFunction } from './runner'; @@ -22,7 +24,7 @@ import { executeRunnerConfig, executeRunnerFunction } from './runner'; */ export class PluginOutputMissingAuditError extends Error { constructor(auditSlug: string) { - super(`Audit metadata not found for slug ${auditSlug}`); + super(`Audit metadata not found for slug ${chalk.bold(auditSlug)}`); } } @@ -69,7 +71,11 @@ export async function executePlugin( const { audits: unvalidatedAuditOutputs, ...executionMeta } = runnerResult; // validate auditOutputs - const auditOutputs = auditOutputsSchema.parse(unvalidatedAuditOutputs); + const result = auditOutputsSchema.safeParse(unvalidatedAuditOutputs); + if (!result.success) { + throw new Error(`THIS IS NOT WORKING!! ${result.error.message}`); + } + const auditOutputs = result.data; auditOutputsCorrelateWithPluginOutput(auditOutputs, pluginConfigAudits); const normalizedAuditOutputs = await normalizeAuditOutputs(auditOutputs); @@ -95,6 +101,28 @@ export async function executePlugin( }; } +const wrapProgress = async ( + pluginCfg: PluginConfig, + steps: number, + progressBar: ProgressBar | null, +) => { + progressBar?.updateTitle(`Executing ${chalk.bold(pluginCfg.title)}`); + try { + const pluginReport = await executePlugin(pluginCfg); + progressBar?.incrementInSteps(steps); + return pluginReport; + } catch (error) { + progressBar?.incrementInSteps(steps); + throw new Error( + error instanceof Error + ? `- Plugin ${chalk.bold(pluginCfg.title)} (${chalk.bold( + pluginCfg.slug, + )}) produced the following error:\n - ${error.message}` + : String(error), + ); + } +}; + /** * Execute multiple plugins and aggregates their output. * @public @@ -124,21 +152,10 @@ export async function executePlugins( const progressBar = progress ? getProgressBar('Run plugins') : null; - const pluginsResult = await plugins.reduce(async (acc, pluginCfg) => { - progressBar?.updateTitle(`Executing ${chalk.bold(pluginCfg.title)}`); - - try { - const pluginReport = await executePlugin(pluginCfg); - progressBar?.incrementInSteps(plugins.length); - return [...(await acc), Promise.resolve(pluginReport)]; - } catch (error) { - progressBar?.incrementInSteps(plugins.length); - return [ - ...(await acc), - Promise.reject(error instanceof Error ? error.message : String(error)), - ]; - } - }, Promise.resolve([] as Promise[])); + const pluginsResult = await plugins.reduce(async (acc, pluginCfg) => [ + ...(await acc), + wrapProgress(pluginCfg, plugins.length, progressBar), + ], Promise.resolve([] as Promise[])); progressBar?.endProgress('Done running plugins'); @@ -151,9 +168,12 @@ export async function executePlugins( if (rejected.length > 0) { const errorMessages = rejected .map(({ reason }) => String(reason)) - .join(', '); + .join('\n'); throw new Error( - `Plugins failed: ${rejected.length} errors: ${errorMessages}`, + `Executing ${pluralizeToken( + 'plugin', + rejected.length, + )} failed.\n\n${errorMessages}\n\n`, ); } diff --git a/packages/core/src/lib/implementation/execute-plugin.unit.test.ts b/packages/core/src/lib/implementation/execute-plugin.unit.test.ts index f28aaa265..293707fbf 100644 --- a/packages/core/src/lib/implementation/execute-plugin.unit.test.ts +++ b/packages/core/src/lib/implementation/execute-plugin.unit.test.ts @@ -86,7 +86,9 @@ describe('executePlugin', () => { }, ], }), - ).rejects.toThrow('The slug has to follow the pattern'); + ) + // @TODO improve error message. add explanation + .rejects.toThrow('The slug has to follow the pattern'); }); }); @@ -120,9 +122,7 @@ describe('executePlugins', () => { ] satisfies PluginConfig[], { progress: false }, ), - ).rejects.toThrow( - /Plugins failed: 1 errors:.*Audit metadata not found for slug node-version/, - ); + ).rejects.toThrow(/node-version/g); }); it('should print invalid plugin errors and throw', async () => { @@ -145,9 +145,7 @@ describe('executePlugins', () => { executePlugins([pluginConfig, pluginConfig2, pluginConfig3], { progress: false, }), - ).rejects.toThrow( - 'Plugins failed: 2 errors: Audit metadata not found for slug node-version, plugin 3 error', - ); + ).rejects.toThrow('Executing 2 plugins failed.'); const logs = getLogMessages(ui().logger); expect(logs[0]).toBe('[ yellow(warn) ] Plugins failed: '); expect(logs[1]).toBe( From 8abc91f225096ec599ee9eed0a87dd1f2a8a8505 Mon Sep 17 00:00:00 2001 From: Michael Date: Wed, 24 Apr 2024 00:19:55 +0200 Subject: [PATCH 2/4] refactor(core): add better error messages 2 --- packages/core/src/lib/implementation/execute-plugin.ts | 7 +++++-- .../src/lib/implementation/execute-plugin.unit.test.ts | 2 +- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/packages/core/src/lib/implementation/execute-plugin.ts b/packages/core/src/lib/implementation/execute-plugin.ts index 8315b8287..8c5552c3c 100644 --- a/packages/core/src/lib/implementation/execute-plugin.ts +++ b/packages/core/src/lib/implementation/execute-plugin.ts @@ -152,10 +152,13 @@ export async function executePlugins( const progressBar = progress ? getProgressBar('Run plugins') : null; - const pluginsResult = await plugins.reduce(async (acc, pluginCfg) => [ + const pluginsResult = await plugins.reduce( + async (acc, pluginCfg) => [ ...(await acc), wrapProgress(pluginCfg, plugins.length, progressBar), - ], Promise.resolve([] as Promise[])); + ], + Promise.resolve([] as Promise[]), + ); progressBar?.endProgress('Done running plugins'); diff --git a/packages/core/src/lib/implementation/execute-plugin.unit.test.ts b/packages/core/src/lib/implementation/execute-plugin.unit.test.ts index 293707fbf..d8e08e5ee 100644 --- a/packages/core/src/lib/implementation/execute-plugin.unit.test.ts +++ b/packages/core/src/lib/implementation/execute-plugin.unit.test.ts @@ -149,7 +149,7 @@ describe('executePlugins', () => { const logs = getLogMessages(ui().logger); expect(logs[0]).toBe('[ yellow(warn) ] Plugins failed: '); expect(logs[1]).toBe( - '[ yellow(warn) ] Audit metadata not found for slug node-version', + '[ yellow(warn) ] Error: Audit metadata not found for slug node-version', ); expect(pluginConfig.runner).toHaveBeenCalled(); From 808b1b64672f84fd6f2de6603452c7fc93eb57c7 Mon Sep 17 00:00:00 2001 From: Michael Date: Thu, 25 Apr 2024 12:02:59 +0200 Subject: [PATCH 3/4] test(core): add tests for executePlugin errors --- .../src/lib/implementation/execute-plugin.ts | 8 +- .../execute-plugin.unit.test.ts | 199 ++++++++++++++---- 2 files changed, 167 insertions(+), 40 deletions(-) diff --git a/packages/core/src/lib/implementation/execute-plugin.ts b/packages/core/src/lib/implementation/execute-plugin.ts index 8c5552c3c..b142c5ba0 100644 --- a/packages/core/src/lib/implementation/execute-plugin.ts +++ b/packages/core/src/lib/implementation/execute-plugin.ts @@ -24,7 +24,11 @@ import { executeRunnerConfig, executeRunnerFunction } from './runner'; */ export class PluginOutputMissingAuditError extends Error { constructor(auditSlug: string) { - super(`Audit metadata not found for slug ${chalk.bold(auditSlug)}`); + super( + `Audit metadata not present in plugin config. Missing slug: ${chalk.bold( + auditSlug, + )}`, + ); } } @@ -73,7 +77,7 @@ export async function executePlugin( // validate auditOutputs const result = auditOutputsSchema.safeParse(unvalidatedAuditOutputs); if (!result.success) { - throw new Error(`THIS IS NOT WORKING!! ${result.error.message}`); + throw new Error(`Audit output is invalid: ${result.error.message}`); } const auditOutputs = result.data; auditOutputsCorrelateWithPluginOutput(auditOutputs, pluginConfigAudits); diff --git a/packages/core/src/lib/implementation/execute-plugin.unit.test.ts b/packages/core/src/lib/implementation/execute-plugin.unit.test.ts index d8e08e5ee..a16b88e9e 100644 --- a/packages/core/src/lib/implementation/execute-plugin.unit.test.ts +++ b/packages/core/src/lib/implementation/execute-plugin.unit.test.ts @@ -1,3 +1,4 @@ +import chalk from 'chalk'; import { vol } from 'memfs'; import { describe, expect, it, vi } from 'vitest'; import { AuditOutputs, PluginConfig } from '@code-pushup/models'; @@ -65,7 +66,7 @@ describe('executePlugin', () => { ]); }); - it('should throw when plugin slug is invalid', async () => { + it('should throw when audit slug is invalid', async () => { await expect(() => executePlugin({ ...MINIMAL_PLUGIN_CONFIG_MOCK, @@ -74,21 +75,24 @@ describe('executePlugin', () => { ).rejects.toThrow(new PluginOutputMissingAuditError('node-version')); }); - it('should throw if invalid runnerOutput is produced', async () => { + it('should throw for missing audit', async () => { + const missingSlug = 'missing-audit-slug'; await expect(() => executePlugin({ ...MINIMAL_PLUGIN_CONFIG_MOCK, runner: () => [ { - slug: '-invalid-audit-slug', + slug: missingSlug, score: 0, value: 0, }, ], }), - ) - // @TODO improve error message. add explanation - .rejects.toThrow('The slug has to follow the pattern'); + ).rejects.toThrow( + `Audit metadata not present in plugin config. Missing slug: ${chalk.bold( + missingSlug, + )}`, + ); }); }); @@ -110,51 +114,170 @@ describe('executePlugins', () => { expect(pluginResult[0]?.audits[0]?.slug).toBe('node-version'); }); - it('should throw for invalid plugins', async () => { + it('should throw for invalid audit output', async () => { + const slug = 'simulate-invalid-audit-slug'; + const title = 'Simulate an invalid audit slug in outputs'; await expect(() => executePlugins( [ - MINIMAL_PLUGIN_CONFIG_MOCK, { ...MINIMAL_PLUGIN_CONFIG_MOCK, - audits: [{ slug: '-invalid-slug', title: 'Invalid audit' }], + slug, + title, + runner: () => [ + { + slug: 'invalid-audit-slug-', + score: 0.3, + value: 16, + displayValue: '16.0.0', + }, + ], }, ] satisfies PluginConfig[], { progress: false }, ), - ).rejects.toThrow(/node-version/g); + ).rejects + .toThrow(`Executing 1 plugin failed.\n\nError: - Plugin ${chalk.bold( + title, + )} (${chalk.bold(slug)}) produced the following error: + - Audit output is invalid: [ + { + "validation": "regex", + "code": "invalid_string", + "message": "The slug has to follow the pattern [0-9a-z] followed by multiple optional groups of -[0-9a-z]. e.g. my-slug", + "path": [ + 0, + "slug" + ] + } +] +`); }); - it('should print invalid plugin errors and throw', async () => { - const pluginConfig = { - ...MINIMAL_PLUGIN_CONFIG_MOCK, - runner: vi - .fn() - .mockRejectedValue('Audit metadata not found for slug node-version'), - }; - const pluginConfig2 = { - ...MINIMAL_PLUGIN_CONFIG_MOCK, - runner: vi.fn().mockResolvedValue([]), - }; - const pluginConfig3 = { - ...MINIMAL_PLUGIN_CONFIG_MOCK, - runner: vi.fn().mockRejectedValue('plugin 3 error'), - }; + it('should throw for one failing plugin', async () => { + const missingAuditSlug = 'missing-audit-slug'; + try { + const r = await executePlugins( + [ + { + ...MINIMAL_PLUGIN_CONFIG_MOCK, + slug: 'plg1', + title: 'plg1', + runner: () => [ + { + slug: missingAuditSlug + '-a', + score: 0.3, + value: 16, + displayValue: '16.0.0', + }, + ], + }, + ] satisfies PluginConfig[], + { progress: false }, + ); + } catch (e) { + const msg = (e as Error).message; + expect(msg).toMatch('Executing 1 plugin failed.\n\n'); + } + }); - await expect(() => - executePlugins([pluginConfig, pluginConfig2, pluginConfig3], { - progress: false, - }), - ).rejects.toThrow('Executing 2 plugins failed.'); - const logs = getLogMessages(ui().logger); - expect(logs[0]).toBe('[ yellow(warn) ] Plugins failed: '); - expect(logs[1]).toBe( - '[ yellow(warn) ] Error: Audit metadata not found for slug node-version', - ); + it('should throw for multiple failing plugins', async () => { + const missingAuditSlug = 'missing-audit-slug'; + try { + const r = await executePlugins( + [ + { + ...MINIMAL_PLUGIN_CONFIG_MOCK, + slug: 'plg1', + title: 'plg1', + runner: () => [ + { + slug: missingAuditSlug + '-a', + score: 0.3, + value: 16, + displayValue: '16.0.0', + }, + ], + }, + { + ...MINIMAL_PLUGIN_CONFIG_MOCK, + slug: 'plg2', + title: 'plg2', + runner: () => [ + { + slug: missingAuditSlug + '-b', + score: 0.3, + value: 16, + displayValue: '16.0.0', + }, + ], + }, + ] satisfies PluginConfig[], + { progress: false }, + ); + } catch (e) { + const msg = (e as Error).message; + expect(msg).toMatch('Executing 2 plugins failed.\n\n'); + } + }); - expect(pluginConfig.runner).toHaveBeenCalled(); - expect(pluginConfig2.runner).toHaveBeenCalled(); - expect(pluginConfig3.runner).toHaveBeenCalled(); + it('should throw with indentation in message', async () => { + const missingAuditSlug = 'missing-audit-slug'; + try { + const r = await executePlugins( + [ + { + ...MINIMAL_PLUGIN_CONFIG_MOCK, + slug: 'plg1', + title: 'plg1', + runner: () => [ + { + slug: missingAuditSlug + '-a', + score: 0.3, + value: 16, + displayValue: '16.0.0', + }, + ], + }, + { + ...MINIMAL_PLUGIN_CONFIG_MOCK, + slug: 'plg2', + title: 'plg2', + runner: () => [ + { + slug: missingAuditSlug + '-b', + score: 0.3, + value: 16, + displayValue: '16.0.0', + }, + ], + }, + ] satisfies PluginConfig[], + { progress: false }, + ); + } catch (e) { + const msg = (e as Error).message; + expect(msg).toMatch( + `Error: - Plugin ${chalk.bold('plg1')} (${chalk.bold( + 'plg1', + )}) produced the following error:\n`, + ); + expect(msg).toMatch( + ` - Audit metadata not present in plugin config. Missing slug: ${chalk.bold( + 'missing-audit-slug-a', + )}\n`, + ); + expect(msg).toMatch( + `Error: - Plugin ${chalk.bold('plg2')} (${chalk.bold( + 'plg2', + )}) produced the following error:\n`, + ); + expect(msg).toMatch( + ` - Audit metadata not present in plugin config. Missing slug: ${chalk.bold( + 'missing-audit-slug-b', + )}`, + ); + } }); it('should use outputTransform if provided', async () => { From ad8b233f1e8021f2ff18ee7b3dd11ca8c005cd13 Mon Sep 17 00:00:00 2001 From: Michael Date: Thu, 25 Apr 2024 12:11:36 +0200 Subject: [PATCH 4/4] test(core): fix lint --- .../execute-plugin.unit.test.ts | 77 +++++++------------ 1 file changed, 29 insertions(+), 48 deletions(-) diff --git a/packages/core/src/lib/implementation/execute-plugin.unit.test.ts b/packages/core/src/lib/implementation/execute-plugin.unit.test.ts index a16b88e9e..35d3087cd 100644 --- a/packages/core/src/lib/implementation/execute-plugin.unit.test.ts +++ b/packages/core/src/lib/implementation/execute-plugin.unit.test.ts @@ -1,13 +1,11 @@ import chalk from 'chalk'; import { vol } from 'memfs'; -import { describe, expect, it, vi } from 'vitest'; +import { describe, expect, it } from 'vitest'; import { AuditOutputs, PluginConfig } from '@code-pushup/models'; import { MEMFS_VOLUME, MINIMAL_PLUGIN_CONFIG_MOCK, - getLogMessages, } from '@code-pushup/test-utils'; -import { ui } from '@code-pushup/utils'; import { PluginOutputMissingAuditError, executePlugin, @@ -156,8 +154,8 @@ describe('executePlugins', () => { it('should throw for one failing plugin', async () => { const missingAuditSlug = 'missing-audit-slug'; - try { - const r = await executePlugins( + await expect(() => + executePlugins( [ { ...MINIMAL_PLUGIN_CONFIG_MOCK, @@ -165,7 +163,7 @@ describe('executePlugins', () => { title: 'plg1', runner: () => [ { - slug: missingAuditSlug + '-a', + slug: `${missingAuditSlug}-a`, score: 0.3, value: 16, displayValue: '16.0.0', @@ -174,17 +172,14 @@ describe('executePlugins', () => { }, ] satisfies PluginConfig[], { progress: false }, - ); - } catch (e) { - const msg = (e as Error).message; - expect(msg).toMatch('Executing 1 plugin failed.\n\n'); - } + ), + ).rejects.toThrow('Executing 1 plugin failed.\n\n'); }); it('should throw for multiple failing plugins', async () => { const missingAuditSlug = 'missing-audit-slug'; - try { - const r = await executePlugins( + await expect(() => + executePlugins( [ { ...MINIMAL_PLUGIN_CONFIG_MOCK, @@ -192,7 +187,7 @@ describe('executePlugins', () => { title: 'plg1', runner: () => [ { - slug: missingAuditSlug + '-a', + slug: `${missingAuditSlug}-a`, score: 0.3, value: 16, displayValue: '16.0.0', @@ -205,7 +200,7 @@ describe('executePlugins', () => { title: 'plg2', runner: () => [ { - slug: missingAuditSlug + '-b', + slug: `${missingAuditSlug}-b`, score: 0.3, value: 16, displayValue: '16.0.0', @@ -214,17 +209,15 @@ describe('executePlugins', () => { }, ] satisfies PluginConfig[], { progress: false }, - ); - } catch (e) { - const msg = (e as Error).message; - expect(msg).toMatch('Executing 2 plugins failed.\n\n'); - } + ), + ).rejects.toThrow('Executing 2 plugins failed.\n\n'); }); it('should throw with indentation in message', async () => { const missingAuditSlug = 'missing-audit-slug'; - try { - const r = await executePlugins( + + await expect(() => + executePlugins( [ { ...MINIMAL_PLUGIN_CONFIG_MOCK, @@ -232,7 +225,7 @@ describe('executePlugins', () => { title: 'plg1', runner: () => [ { - slug: missingAuditSlug + '-a', + slug: `${missingAuditSlug}-a`, score: 0.3, value: 16, displayValue: '16.0.0', @@ -245,7 +238,7 @@ describe('executePlugins', () => { title: 'plg2', runner: () => [ { - slug: missingAuditSlug + '-b', + slug: `${missingAuditSlug}-b`, score: 0.3, value: 16, displayValue: '16.0.0', @@ -254,30 +247,18 @@ describe('executePlugins', () => { }, ] satisfies PluginConfig[], { progress: false }, - ); - } catch (e) { - const msg = (e as Error).message; - expect(msg).toMatch( - `Error: - Plugin ${chalk.bold('plg1')} (${chalk.bold( - 'plg1', - )}) produced the following error:\n`, - ); - expect(msg).toMatch( - ` - Audit metadata not present in plugin config. Missing slug: ${chalk.bold( - 'missing-audit-slug-a', - )}\n`, - ); - expect(msg).toMatch( - `Error: - Plugin ${chalk.bold('plg2')} (${chalk.bold( - 'plg2', - )}) produced the following error:\n`, - ); - expect(msg).toMatch( - ` - Audit metadata not present in plugin config. Missing slug: ${chalk.bold( - 'missing-audit-slug-b', - )}`, - ); - } + ), + ).rejects.toThrow( + `Error: - Plugin ${chalk.bold('plg1')} (${chalk.bold( + 'plg1', + )}) produced the following error:\n - Audit metadata not present in plugin config. Missing slug: ${chalk.bold( + 'missing-audit-slug-a', + )}\nError: - Plugin ${chalk.bold('plg2')} (${chalk.bold( + 'plg2', + )}) produced the following error:\n - Audit metadata not present in plugin config. Missing slug: ${chalk.bold( + 'missing-audit-slug-b', + )}`, + ); }); it('should use outputTransform if provided', async () => {