diff --git a/packages/core/src/lib/implementation/execute-plugin.integration.test.ts b/packages/core/src/lib/implementation/execute-plugin.integration.test.ts index cda58837c..837bcb52e 100644 --- a/packages/core/src/lib/implementation/execute-plugin.integration.test.ts +++ b/packages/core/src/lib/implementation/execute-plugin.integration.test.ts @@ -1,4 +1,4 @@ -import { describe, expect, it } from 'vitest'; +import { SpyInstance, describe, expect, it } from 'vitest'; import { AuditOutputs, OnProgress, @@ -105,6 +105,16 @@ describe('executePlugin', () => { }); describe('executePlugins', () => { + let errorSpy: SpyInstance; + + beforeEach(() => { + errorSpy = vi.spyOn(console, 'error'); + }); + + afterEach(() => { + errorSpy.mockRestore(); + }); + it('should work with valid plugins', async () => { const plugins = [validPluginCfg, validPluginCfg2]; const pluginResult = await executePlugins(plugins, DEFAULT_OPTIONS); @@ -129,6 +139,32 @@ describe('executePlugins', () => { ).rejects.toThrow('Audit metadata not found for slug mock-audit-slug'); }); + it('should log invalid plugin errors and throw', async () => { + const pluginConfig = { + ...validPluginCfg, + runner: vi.fn().mockRejectedValue('plugin 1 error'), + }; + const pluginConfig2 = { + ...validPluginCfg2, + runner: vi.fn().mockResolvedValue([]), + }; + const pluginConfig3 = { + ...validPluginCfg, + runner: vi.fn().mockRejectedValue('plugin 3 error'), + }; + const plugins = [pluginConfig, pluginConfig2, pluginConfig3]; + await expect(() => + executePlugins(plugins, DEFAULT_OPTIONS), + ).rejects.toThrow( + 'Plugins failed: 2 errors: plugin 1 error, plugin 3 error', + ); + expect(errorSpy).toHaveBeenCalledWith('plugin 1 error'); + expect(errorSpy).toHaveBeenCalledWith('plugin 3 error'); + expect(pluginConfig.runner).toHaveBeenCalled(); + expect(pluginConfig2.runner).toHaveBeenCalled(); + expect(pluginConfig3.runner).toHaveBeenCalled(); + }); + it('should use outputTransform if provided', async () => { const processRunner = validPluginCfg.runner as RunnerConfig; const plugins: PluginConfig[] = [ diff --git a/packages/core/src/lib/implementation/execute-plugin.ts b/packages/core/src/lib/implementation/execute-plugin.ts index c0ec49336..bfe064253 100644 --- a/packages/core/src/lib/implementation/execute-plugin.ts +++ b/packages/core/src/lib/implementation/execute-plugin.ts @@ -9,7 +9,12 @@ import { PluginReport, auditOutputsSchema, } from '@code-pushup/models'; -import { getProgressBar } from '@code-pushup/utils'; +import { + getProgressBar, + isPromiseFulfilledResult, + isPromiseRejectedResult, + logMultipleResults, +} from '@code-pushup/utils'; import { executeRunnerConfig, executeRunnerFunction } from './runner'; /** @@ -123,15 +128,41 @@ export async function executePlugins( progressBar?.updateTitle(`Executing ${chalk.bold(pluginCfg.title)}`); - const pluginReport = await executePlugin(pluginCfg); - progressBar?.incrementInSteps(plugins.length); - - return outputs.concat(pluginReport); - }, Promise.resolve([] as PluginReport[])); + try { + const pluginReport = await executePlugin(pluginCfg); + progressBar?.incrementInSteps(plugins.length); + return outputs.concat(Promise.resolve(pluginReport)); + } catch (e: unknown) { + progressBar?.incrementInSteps(plugins.length); + return outputs.concat( + Promise.reject(e instanceof Error ? e.message : String(e)), + ); + } + }, Promise.resolve([] as Promise[])); progressBar?.endProgress('Done running plugins'); - return pluginsResult; + const errorsCallback = ({ reason }: PromiseRejectedResult) => + console.error(reason); + const results = await Promise.allSettled(pluginsResult); + + logMultipleResults(results, 'Plugins', undefined, errorsCallback); + + // TODO: add groupBy method for promiseSettledResult by status, #287 + + const failedResults = results.filter(isPromiseRejectedResult); + if (failedResults.length) { + const errorMessages = failedResults + .map(({ reason }: PromiseRejectedResult) => reason) + .join(', '); + throw new Error( + `Plugins failed: ${failedResults.length} errors: ${errorMessages}`, + ); + } + + return results + .filter(isPromiseFulfilledResult) + .map((result: PromiseFulfilledResult) => result.value); } function auditOutputsCorrelateWithPluginOutput( diff --git a/packages/utils/src/index.ts b/packages/utils/src/index.ts index 896d75449..c179a1995 100644 --- a/packages/utils/src/index.ts +++ b/packages/utils/src/index.ts @@ -43,3 +43,8 @@ export { slugify, } from './lib/transformation'; export { NEW_LINE } from './lib/md'; +export { logMultipleResults } from './lib/log-results'; +export { + isPromiseFulfilledResult, + isPromiseRejectedResult, +} from './lib/promise-result'; diff --git a/packages/utils/src/lib/file-system.ts b/packages/utils/src/lib/file-system.ts index bfb6112f0..1faa83410 100644 --- a/packages/utils/src/lib/file-system.ts +++ b/packages/utils/src/lib/file-system.ts @@ -1,6 +1,7 @@ import { type Options, bundleRequire } from 'bundle-require'; import chalk from 'chalk'; import { mkdir, readFile } from 'fs/promises'; +import { logMultipleResults } from './log-results'; import { formatBytes } from './report'; export function toUnixPath( @@ -42,35 +43,26 @@ export type FileResult = readonly [string] | readonly [string, number]; export type MultipleFileResults = PromiseSettledResult[]; export function logMultipleFileResults( - persistResult: MultipleFileResults, + fileResults: MultipleFileResults, messagePrefix: string, -) { - const succeededPersistedResults = persistResult.filter( - (result): result is PromiseFulfilledResult<[string, number]> => - result.status === 'fulfilled', - ); - - if (succeededPersistedResults.length) { - console.log(`${messagePrefix} successfully: `); - succeededPersistedResults.forEach(res => { - const [fileName, size] = res.value; - console.log( - `- ${chalk.bold(fileName)}` + - (size ? ` (${chalk.gray(formatBytes(size))})` : ''), - ); - }); - } +): void { + const succeededCallback = (result: PromiseFulfilledResult) => { + const [fileName, size] = result.value; + console.log( + `- ${chalk.bold(fileName)}` + + (size ? ` (${chalk.gray(formatBytes(size))})` : ''), + ); + }; + const failedCallback = (result: PromiseRejectedResult) => { + console.log(`- ${chalk.bold(result.reason)}`); + }; - const failedPersistedResults = persistResult.filter( - (result): result is PromiseRejectedResult => result.status === 'rejected', + logMultipleResults( + fileResults, + messagePrefix, + succeededCallback, + failedCallback, ); - - if (failedPersistedResults.length) { - console.log(`${messagePrefix} failed: `); - failedPersistedResults.forEach(result => { - console.log(`- ${chalk.bold(result.reason)}`); - }); - } } export class NoExportError extends Error { diff --git a/packages/utils/src/lib/file-system.unit.test.ts b/packages/utils/src/lib/file-system.unit.test.ts index 60db3557d..d51ed2307 100644 --- a/packages/utils/src/lib/file-system.unit.test.ts +++ b/packages/utils/src/lib/file-system.unit.test.ts @@ -4,14 +4,15 @@ import { join } from 'path'; import process from 'process'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { MEMFS_VOLUME } from '@code-pushup/models/testing'; -import { mockConsole, unmockConsole } from '../../test/console.mock'; import { + FileResult, NoExportError, ensureDirectoryExists, importEsmModule, logMultipleFileResults, toUnixPath, } from './file-system'; +import * as logResults from './log-results'; // Mock file system API's vi.mock('fs', async () => { @@ -63,70 +64,32 @@ describe('ensureDirectoryExists', () => { }); describe('logMultipleFileResults', () => { - let logs: string[]; - const setupConsole = async () => { - logs = []; - mockConsole(msg => logs.push(msg)); - }; - const teardownConsole = async () => { - logs = []; - unmockConsole(); - }; - - beforeEach(async () => { - logs = []; - setupConsole(); - }); - afterEach(() => { - teardownConsole(); + vi.restoreAllMocks(); }); - it('should log reports correctly`', async () => { - logMultipleFileResults( - [{ status: 'fulfilled', value: ['out.json'] }], - 'Uploaded reports', + it('should call logMultipleResults with the correct arguments', () => { + const logMultipleResultsSpy = vi.spyOn( + logResults, + 'logMultipleResults' as never, ); - expect(logs).toHaveLength(2); - expect(logs[0]).toContain('Uploaded reports successfully: '); - expect(logs[1]).toContain('- out.json'); - }); - - it('should log report sizes correctly`', async () => { - logMultipleFileResults( - [{ status: 'fulfilled', value: ['out.json', 10000] }], - 'Generated reports', + const persistResult = [ + { + status: 'fulfilled', + value: ['out.json', 10000], + } as PromiseFulfilledResult, + ]; + const messagePrefix = 'Generated reports'; + + logMultipleFileResults(persistResult, messagePrefix); + + expect(logMultipleResultsSpy).toHaveBeenCalled(); + expect(logMultipleResultsSpy).toHaveBeenCalledWith( + persistResult, + messagePrefix, + expect.any(Function), + expect.any(Function), ); - expect(logs).toHaveLength(2); - expect(logs[0]).toContain('Generated reports successfully: '); - expect(logs[1]).toContain('- out.json (9.77 kB)'); - }); - - it('should log fails correctly`', async () => { - logMultipleFileResults( - [{ status: 'rejected', reason: 'fail' }], - 'Generated reports', - ); - expect(logs).toHaveLength(2); - - expect(logs).toContain('Generated reports failed: '); - expect(logs).toContain('- fail'); - }); - - it('should log report sizes and fails correctly`', async () => { - logMultipleFileResults( - [ - { status: 'fulfilled', value: ['out.json', 10000] }, - { status: 'rejected', reason: 'fail' }, - ], - 'Generated reports', - ); - expect(logs).toHaveLength(4); - expect(logs).toContain('Generated reports successfully: '); - expect(logs).toContain('- out.json (9.77 kB)'); - - expect(logs).toContain('Generated reports failed: '); - expect(logs).toContain('- fail'); }); }); diff --git a/packages/utils/src/lib/log-results.ts b/packages/utils/src/lib/log-results.ts new file mode 100644 index 000000000..dc3805460 --- /dev/null +++ b/packages/utils/src/lib/log-results.ts @@ -0,0 +1,40 @@ +import { + isPromiseFulfilledResult, + isPromiseRejectedResult, +} from './promise-result'; + +export function logMultipleResults( + results: PromiseSettledResult[], + messagePrefix: string, + succeededCallback?: (result: PromiseFulfilledResult) => void, + failedCallback?: (result: PromiseRejectedResult) => void, +) { + if (succeededCallback) { + const succeededResults = results.filter(isPromiseFulfilledResult); + + logPromiseResults( + succeededResults, + `${messagePrefix} successfully: `, + succeededCallback, + ); + } + + if (failedCallback) { + const failedResults = results.filter(isPromiseRejectedResult); + + logPromiseResults( + failedResults, + `${messagePrefix} failed: `, + failedCallback, + ); + } +} + +export function logPromiseResults< + T extends PromiseFulfilledResult | PromiseRejectedResult, +>(results: T[], logMessage: string, callback: (result: T) => void): void { + if (results.length) { + console.log(logMessage); + results.forEach(callback); + } +} diff --git a/packages/utils/src/lib/log-results.unit.test.ts b/packages/utils/src/lib/log-results.unit.test.ts new file mode 100644 index 000000000..333684847 --- /dev/null +++ b/packages/utils/src/lib/log-results.unit.test.ts @@ -0,0 +1,129 @@ +import chalk from 'chalk'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { mockConsole, unmockConsole } from '../../test'; +import { FileResult } from './file-system'; +import { logMultipleResults, logPromiseResults } from './log-results'; +import { formatBytes } from './report'; + +const succeededCallback = (result: PromiseFulfilledResult) => { + const [fileName, size] = result.value; + console.log( + `- ${chalk.bold(fileName)}` + + (size ? ` (${chalk.gray(formatBytes(size))})` : ''), + ); +}; + +const failedCallback = (result: PromiseRejectedResult) => { + console.log(`- ${chalk.bold(result.reason)}`); +}; + +describe('logMultipleResults', () => { + const succeededCallbackMock = vi.fn().mockImplementation(succeededCallback); + const failedCallbackMock = vi.fn().mockImplementation(failedCallback); + + beforeEach(() => { + succeededCallbackMock.mockClear(); + failedCallbackMock.mockClear(); + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + it('should call logPromiseResults with successfull plugin result', async () => { + logMultipleResults( + [ + { + status: 'fulfilled', + value: ['out.json', 10000], + } as PromiseFulfilledResult, + ], + 'Generated reports', + succeededCallbackMock, + failedCallbackMock, + ); + + expect(succeededCallbackMock).toHaveBeenCalled(); + expect(failedCallbackMock).not.toHaveBeenCalled(); + }); + + it('should call logPromiseResults with failed plugin result', async () => { + logMultipleResults( + [{ status: 'rejected', reason: 'fail' } as PromiseRejectedResult], + 'Generated reports', + succeededCallbackMock, + failedCallbackMock, + ); + + expect(failedCallbackMock).toHaveBeenCalled(); + expect(succeededCallbackMock).not.toHaveBeenCalled(); + }); + + it('should call logPromiseResults twice', async () => { + logMultipleResults( + [ + { + status: 'fulfilled', + value: ['out.json', 10000], + } as PromiseFulfilledResult, + { status: 'rejected', reason: 'fail' } as PromiseRejectedResult, + ], + 'Generated reports', + succeededCallbackMock, + failedCallbackMock, + ); + + expect(succeededCallbackMock).toHaveBeenCalled(); + expect(failedCallbackMock).toHaveBeenCalled(); + }); +}); + +describe('logPromiseResults', () => { + let logs: string[]; + const setupConsole = async () => { + logs = []; + mockConsole(msg => logs.push(msg)); + }; + const teardownConsole = async () => { + logs = []; + unmockConsole(); + }; + + beforeEach(async () => { + logs = []; + setupConsole(); + }); + + afterEach(() => { + teardownConsole(); + }); + + it('should log on success', async () => { + logPromiseResults( + [ + { + status: 'fulfilled', + value: ['out.json'], + } as PromiseFulfilledResult, + ], + 'Uploaded reports successfully: ', + succeededCallback, + ); + + expect(logs).toHaveLength(2); + expect(logs[0]).toContain('Uploaded reports successfully: '); + expect(logs[1]).toContain('- out.json'); + }); + + it('should log on fail', async () => { + logPromiseResults( + [{ status: 'rejected', reason: 'fail' } as PromiseRejectedResult], + 'Generated reports failed: ', + failedCallback, + ); + + expect(logs).toHaveLength(2); + expect(logs[0]).toContain('Generated reports failed: '); + expect(logs[1]).toContain('- fail'); + }); +}); diff --git a/packages/utils/src/lib/promise-result.ts b/packages/utils/src/lib/promise-result.ts new file mode 100644 index 000000000..ad79d21e9 --- /dev/null +++ b/packages/utils/src/lib/promise-result.ts @@ -0,0 +1,11 @@ +export function isPromiseFulfilledResult( + result: PromiseSettledResult, +): result is PromiseFulfilledResult { + return result.status === 'fulfilled'; +} + +export function isPromiseRejectedResult( + result: PromiseSettledResult, +): result is PromiseRejectedResult { + return result.status === 'rejected'; +} diff --git a/packages/utils/src/lib/promise-result.unit.test.ts b/packages/utils/src/lib/promise-result.unit.test.ts new file mode 100644 index 000000000..9d80e8808 --- /dev/null +++ b/packages/utils/src/lib/promise-result.unit.test.ts @@ -0,0 +1,25 @@ +import { describe } from 'vitest'; +import { + isPromiseFulfilledResult, + isPromiseRejectedResult, +} from './promise-result'; + +describe('promise-result', () => { + it('should get fulfilled result', () => { + const result = { + status: 'fulfilled', + value: 'value', + } as PromiseSettledResult; + expect(isPromiseFulfilledResult(result)).toBe(true); + expect(isPromiseRejectedResult(result)).toBe(false); + }); + + it('should get rejected result', () => { + const result = { + status: 'rejected', + reason: 'reason', + } as PromiseSettledResult; + expect(isPromiseFulfilledResult(result)).toBe(false); + expect(isPromiseRejectedResult(result)).toBe(true); + }); +});