Skip to content

Commit 32a6ef5

Browse files
MishaSeredenkoPushBasedMishaSeredenkoPushBased
authored
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
1 parent be60a65 commit 32a6ef5

9 files changed

+326
-94
lines changed

‎packages/core/src/lib/implementation/execute-plugin.integration.test.ts

+37-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { describe, expect, it } from 'vitest';
1+
import { SpyInstance, describe, expect, it } from 'vitest';
22
import {
33
AuditOutputs,
44
OnProgress,
@@ -105,6 +105,16 @@ describe('executePlugin', () => {
105105
});
106106

107107
describe('executePlugins', () => {
108+
let errorSpy: SpyInstance;
109+
110+
beforeEach(() => {
111+
errorSpy = vi.spyOn(console, 'error');
112+
});
113+
114+
afterEach(() => {
115+
errorSpy.mockRestore();
116+
});
117+
108118
it('should work with valid plugins', async () => {
109119
const plugins = [validPluginCfg, validPluginCfg2];
110120
const pluginResult = await executePlugins(plugins, DEFAULT_OPTIONS);
@@ -129,6 +139,32 @@ describe('executePlugins', () => {
129139
).rejects.toThrow('Audit metadata not found for slug mock-audit-slug');
130140
});
131141

142+
it('should log invalid plugin errors and throw', async () => {
143+
const pluginConfig = {
144+
...validPluginCfg,
145+
runner: vi.fn().mockRejectedValue('plugin 1 error'),
146+
};
147+
const pluginConfig2 = {
148+
...validPluginCfg2,
149+
runner: vi.fn().mockResolvedValue([]),
150+
};
151+
const pluginConfig3 = {
152+
...validPluginCfg,
153+
runner: vi.fn().mockRejectedValue('plugin 3 error'),
154+
};
155+
const plugins = [pluginConfig, pluginConfig2, pluginConfig3];
156+
await expect(() =>
157+
executePlugins(plugins, DEFAULT_OPTIONS),
158+
).rejects.toThrow(
159+
'Plugins failed: 2 errors: plugin 1 error, plugin 3 error',
160+
);
161+
expect(errorSpy).toHaveBeenCalledWith('plugin 1 error');
162+
expect(errorSpy).toHaveBeenCalledWith('plugin 3 error');
163+
expect(pluginConfig.runner).toHaveBeenCalled();
164+
expect(pluginConfig2.runner).toHaveBeenCalled();
165+
expect(pluginConfig3.runner).toHaveBeenCalled();
166+
});
167+
132168
it('should use outputTransform if provided', async () => {
133169
const processRunner = validPluginCfg.runner as RunnerConfig;
134170
const plugins: PluginConfig[] = [

‎packages/core/src/lib/implementation/execute-plugin.ts

+38-7
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,12 @@ import {
99
PluginReport,
1010
auditOutputsSchema,
1111
} from '@code-pushup/models';
12-
import { getProgressBar } from '@code-pushup/utils';
12+
import {
13+
getProgressBar,
14+
isPromiseFulfilledResult,
15+
isPromiseRejectedResult,
16+
logMultipleResults,
17+
} from '@code-pushup/utils';
1318
import { executeRunnerConfig, executeRunnerFunction } from './runner';
1419

1520
/**
@@ -123,15 +128,41 @@ export async function executePlugins(
123128

124129
progressBar?.updateTitle(`Executing ${chalk.bold(pluginCfg.title)}`);
125130

126-
const pluginReport = await executePlugin(pluginCfg);
127-
progressBar?.incrementInSteps(plugins.length);
128-
129-
return outputs.concat(pluginReport);
130-
}, Promise.resolve([] as PluginReport[]));
131+
try {
132+
const pluginReport = await executePlugin(pluginCfg);
133+
progressBar?.incrementInSteps(plugins.length);
134+
return outputs.concat(Promise.resolve(pluginReport));
135+
} catch (e: unknown) {
136+
progressBar?.incrementInSteps(plugins.length);
137+
return outputs.concat(
138+
Promise.reject(e instanceof Error ? e.message : String(e)),
139+
);
140+
}
141+
}, Promise.resolve([] as Promise<PluginReport>[]));
131142

132143
progressBar?.endProgress('Done running plugins');
133144

134-
return pluginsResult;
145+
const errorsCallback = ({ reason }: PromiseRejectedResult) =>
146+
console.error(reason);
147+
const results = await Promise.allSettled(pluginsResult);
148+
149+
logMultipleResults(results, 'Plugins', undefined, errorsCallback);
150+
151+
// TODO: add groupBy method for promiseSettledResult by status, #287
152+
153+
const failedResults = results.filter(isPromiseRejectedResult);
154+
if (failedResults.length) {
155+
const errorMessages = failedResults
156+
.map(({ reason }: PromiseRejectedResult) => reason)
157+
.join(', ');
158+
throw new Error(
159+
`Plugins failed: ${failedResults.length} errors: ${errorMessages}`,
160+
);
161+
}
162+
163+
return results
164+
.filter(isPromiseFulfilledResult)
165+
.map((result: PromiseFulfilledResult<PluginReport>) => result.value);
135166
}
136167

137168
function auditOutputsCorrelateWithPluginOutput(

‎packages/utils/src/index.ts

+5
Original file line numberDiff line numberDiff line change
@@ -43,3 +43,8 @@ export {
4343
slugify,
4444
} from './lib/transformation';
4545
export { NEW_LINE } from './lib/md';
46+
export { logMultipleResults } from './lib/log-results';
47+
export {
48+
isPromiseFulfilledResult,
49+
isPromiseRejectedResult,
50+
} from './lib/promise-result';

‎packages/utils/src/lib/file-system.ts

+18-26
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { type Options, bundleRequire } from 'bundle-require';
22
import chalk from 'chalk';
33
import { mkdir, readFile } from 'fs/promises';
4+
import { logMultipleResults } from './log-results';
45
import { formatBytes } from './report';
56

67
export function toUnixPath(
@@ -42,35 +43,26 @@ export type FileResult = readonly [string] | readonly [string, number];
4243
export type MultipleFileResults = PromiseSettledResult<FileResult>[];
4344

4445
export function logMultipleFileResults(
45-
persistResult: MultipleFileResults,
46+
fileResults: MultipleFileResults,
4647
messagePrefix: string,
47-
) {
48-
const succeededPersistedResults = persistResult.filter(
49-
(result): result is PromiseFulfilledResult<[string, number]> =>
50-
result.status === 'fulfilled',
51-
);
52-
53-
if (succeededPersistedResults.length) {
54-
console.log(`${messagePrefix} successfully: `);
55-
succeededPersistedResults.forEach(res => {
56-
const [fileName, size] = res.value;
57-
console.log(
58-
`- ${chalk.bold(fileName)}` +
59-
(size ? ` (${chalk.gray(formatBytes(size))})` : ''),
60-
);
61-
});
62-
}
48+
): void {
49+
const succeededCallback = (result: PromiseFulfilledResult<FileResult>) => {
50+
const [fileName, size] = result.value;
51+
console.log(
52+
`- ${chalk.bold(fileName)}` +
53+
(size ? ` (${chalk.gray(formatBytes(size))})` : ''),
54+
);
55+
};
56+
const failedCallback = (result: PromiseRejectedResult) => {
57+
console.log(`- ${chalk.bold(result.reason)}`);
58+
};
6359

64-
const failedPersistedResults = persistResult.filter(
65-
(result): result is PromiseRejectedResult => result.status === 'rejected',
60+
logMultipleResults<FileResult>(
61+
fileResults,
62+
messagePrefix,
63+
succeededCallback,
64+
failedCallback,
6665
);
67-
68-
if (failedPersistedResults.length) {
69-
console.log(`${messagePrefix} failed: `);
70-
failedPersistedResults.forEach(result => {
71-
console.log(`- ${chalk.bold(result.reason)}`);
72-
});
73-
}
7466
}
7567

7668
export class NoExportError extends Error {

‎packages/utils/src/lib/file-system.unit.test.ts

+23-60
Original file line numberDiff line numberDiff line change
@@ -4,14 +4,15 @@ import { join } from 'path';
44
import process from 'process';
55
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
66
import { MEMFS_VOLUME } from '@code-pushup/models/testing';
7-
import { mockConsole, unmockConsole } from '../../test/console.mock';
87
import {
8+
FileResult,
99
NoExportError,
1010
ensureDirectoryExists,
1111
importEsmModule,
1212
logMultipleFileResults,
1313
toUnixPath,
1414
} from './file-system';
15+
import * as logResults from './log-results';
1516

1617
// Mock file system API's
1718
vi.mock('fs', async () => {
@@ -63,70 +64,32 @@ describe('ensureDirectoryExists', () => {
6364
});
6465

6566
describe('logMultipleFileResults', () => {
66-
let logs: string[];
67-
const setupConsole = async () => {
68-
logs = [];
69-
mockConsole(msg => logs.push(msg));
70-
};
71-
const teardownConsole = async () => {
72-
logs = [];
73-
unmockConsole();
74-
};
75-
76-
beforeEach(async () => {
77-
logs = [];
78-
setupConsole();
79-
});
80-
8167
afterEach(() => {
82-
teardownConsole();
68+
vi.restoreAllMocks();
8369
});
8470

85-
it('should log reports correctly`', async () => {
86-
logMultipleFileResults(
87-
[{ status: 'fulfilled', value: ['out.json'] }],
88-
'Uploaded reports',
71+
it('should call logMultipleResults with the correct arguments', () => {
72+
const logMultipleResultsSpy = vi.spyOn(
73+
logResults,
74+
'logMultipleResults' as never,
8975
);
90-
expect(logs).toHaveLength(2);
91-
expect(logs[0]).toContain('Uploaded reports successfully: ');
92-
expect(logs[1]).toContain('- out.json');
93-
});
94-
95-
it('should log report sizes correctly`', async () => {
96-
logMultipleFileResults(
97-
[{ status: 'fulfilled', value: ['out.json', 10000] }],
98-
'Generated reports',
76+
const persistResult = [
77+
{
78+
status: 'fulfilled',
79+
value: ['out.json', 10000],
80+
} as PromiseFulfilledResult<FileResult>,
81+
];
82+
const messagePrefix = 'Generated reports';
83+
84+
logMultipleFileResults(persistResult, messagePrefix);
85+
86+
expect(logMultipleResultsSpy).toHaveBeenCalled();
87+
expect(logMultipleResultsSpy).toHaveBeenCalledWith(
88+
persistResult,
89+
messagePrefix,
90+
expect.any(Function),
91+
expect.any(Function),
9992
);
100-
expect(logs).toHaveLength(2);
101-
expect(logs[0]).toContain('Generated reports successfully: ');
102-
expect(logs[1]).toContain('- out.json (9.77 kB)');
103-
});
104-
105-
it('should log fails correctly`', async () => {
106-
logMultipleFileResults(
107-
[{ status: 'rejected', reason: 'fail' }],
108-
'Generated reports',
109-
);
110-
expect(logs).toHaveLength(2);
111-
112-
expect(logs).toContain('Generated reports failed: ');
113-
expect(logs).toContain('- fail');
114-
});
115-
116-
it('should log report sizes and fails correctly`', async () => {
117-
logMultipleFileResults(
118-
[
119-
{ status: 'fulfilled', value: ['out.json', 10000] },
120-
{ status: 'rejected', reason: 'fail' },
121-
],
122-
'Generated reports',
123-
);
124-
expect(logs).toHaveLength(4);
125-
expect(logs).toContain('Generated reports successfully: ');
126-
expect(logs).toContain('- out.json (9.77 kB)');
127-
128-
expect(logs).toContain('Generated reports failed: ');
129-
expect(logs).toContain('- fail');
13093
});
13194
});
13295

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

+40
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
import {
2+
isPromiseFulfilledResult,
3+
isPromiseRejectedResult,
4+
} from './promise-result';
5+
6+
export function logMultipleResults<T>(
7+
results: PromiseSettledResult<T>[],
8+
messagePrefix: string,
9+
succeededCallback?: (result: PromiseFulfilledResult<T>) => void,
10+
failedCallback?: (result: PromiseRejectedResult) => void,
11+
) {
12+
if (succeededCallback) {
13+
const succeededResults = results.filter(isPromiseFulfilledResult);
14+
15+
logPromiseResults(
16+
succeededResults,
17+
`${messagePrefix} successfully: `,
18+
succeededCallback,
19+
);
20+
}
21+
22+
if (failedCallback) {
23+
const failedResults = results.filter(isPromiseRejectedResult);
24+
25+
logPromiseResults(
26+
failedResults,
27+
`${messagePrefix} failed: `,
28+
failedCallback,
29+
);
30+
}
31+
}
32+
33+
export function logPromiseResults<
34+
T extends PromiseFulfilledResult<unknown> | PromiseRejectedResult,
35+
>(results: T[], logMessage: string, callback: (result: T) => void): void {
36+
if (results.length) {
37+
console.log(logMessage);
38+
results.forEach(callback);
39+
}
40+
}

0 commit comments

Comments
 (0)