feat(core): change to execute all plugins before throwing on failed (#…

…275)

Changed executePlugins() logic to run all plugins, even if one or more
plugins have failed. Then console.error the failed plugin errors, and
throw.
Closes #159

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[] = [

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<PluginReport>[]));
 
   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<PluginReport>) => result.value);
 }
 
 function auditOutputsCorrelateWithPluginOutput(

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';

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<FileResult>[];
 
 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<FileResult>) => {
+    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<FileResult>(
+    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 {

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<FileResult>,
+    ];
+    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');
   });
 });
 

packages/utils/src/lib/log-results.ts

@@ -0,0 +1,40 @@
+import {
+  isPromiseFulfilledResult,
+  isPromiseRejectedResult,
+} from './promise-result';
+
+export function logMultipleResults<T>(
+  results: PromiseSettledResult<T>[],
+  messagePrefix: string,
+  succeededCallback?: (result: PromiseFulfilledResult<T>) => 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<unknown> | PromiseRejectedResult,
+>(results: T[], logMessage: string, callback: (result: T) => void): void {
+  if (results.length) {
+    console.log(logMessage);
+    results.forEach(callback);
+  }
+}