Skip to content

Commit 69c3b12

Browse files
committed
fix(@angular/build): improve error handling in unit-test builder
Previously, exceptions thrown within a test runner would cause the entire builder process to terminate. This resulted in a poor user experience, especially in watch mode, as the process would crash instead of reporting the error and waiting for changes. This change introduces more granular `try-catch` blocks around the test runner's lifecycle. When a runner throws an exception, the error is now caught, logged with a descriptive message, and the builder emits a failed result without crashing. To prevent logs from being spammed by a persistently failing runner, a circuit breaker has been added. If the test runner fails with an exception for 3 consecutive runs, the builder will pause testing and exit to prevent an endless error loop. This circuit breaker is specifically designed to only trip on runner exceptions, not on standard test failures, to avoid disrupting a normal TDD workflow. Additionally, this commit re-enables and adjusts several related unit tests for the builder options to ensure they are passing and reflect the current behavior.
1 parent 8324a47 commit 69c3b12

File tree

3 files changed

+114
-52
lines changed

3 files changed

+114
-52
lines changed

packages/angular/build/src/builders/unit-test/builder.ts

Lines changed: 98 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,7 @@ async function* runBuildAndTest(
9797
context: BuilderContext,
9898
extensions: ApplicationBuilderExtensions | undefined,
9999
): AsyncIterable<BuilderOutput> {
100+
let consecutiveErrorCount = 0;
100101
for await (const buildResult of buildApplicationInternal(
101102
applicationBuildOptions,
102103
context,
@@ -117,7 +118,25 @@ async function* runBuildAndTest(
117118
assert(buildResult.files, 'Builder did not provide result files.');
118119

119120
// Pass the build artifacts to the executor
120-
yield* executor.execute(buildResult);
121+
try {
122+
yield* executor.execute(buildResult);
123+
124+
// Successful execution resets the failure counter
125+
consecutiveErrorCount = 0;
126+
} catch (e) {
127+
assertIsError(e);
128+
context.logger.error(`An exception occurred during test execution:\n${e.stack ?? e.message}`);
129+
yield { success: false };
130+
consecutiveErrorCount++;
131+
}
132+
133+
if (consecutiveErrorCount >= 3) {
134+
context.logger.error(
135+
'Test runner process has failed multiple times in a row. Please fix the configuration and restart the process.',
136+
);
137+
138+
return;
139+
}
121140
}
122141
}
123142

@@ -141,15 +160,36 @@ export async function* execute(
141160
`NOTE: The "unit-test" builder is currently EXPERIMENTAL and not ready for production use.`,
142161
);
143162

144-
const normalizedOptions = await normalizeOptions(context, projectName, options);
145-
const runner = await loadTestRunner(normalizedOptions.runnerName);
163+
// Initialize the test runner and normalize options
164+
let runner;
165+
let normalizedOptions;
166+
try {
167+
normalizedOptions = await normalizeOptions(context, projectName, options);
168+
runner = await loadTestRunner(normalizedOptions.runnerName);
169+
} catch (e) {
170+
assertIsError(e);
171+
context.logger.error(
172+
`An exception occurred during initialization of the test runner:\n${e.stack ?? e.message}`,
173+
);
174+
yield { success: false };
175+
176+
return;
177+
}
146178

147179
if (runner.isStandalone) {
148-
await using executor = await runner.createExecutor(context, normalizedOptions, undefined);
149-
yield* executor.execute({
150-
kind: ResultKind.Full,
151-
files: {},
152-
});
180+
try {
181+
await using executor = await runner.createExecutor(context, normalizedOptions, undefined);
182+
yield* executor.execute({
183+
kind: ResultKind.Full,
184+
files: {},
185+
});
186+
} catch (e) {
187+
assertIsError(e);
188+
context.logger.error(
189+
`An exception occurred during standalone test execution:\n${e.stack ?? e.message}`,
190+
);
191+
yield { success: false };
192+
}
153193

154194
return;
155195
}
@@ -164,41 +204,65 @@ export async function* execute(
164204
} catch (e) {
165205
assertIsError(e);
166206
context.logger.error(
167-
`Could not load build target options for "${targetStringFromTarget(normalizedOptions.buildTarget)}".\n` +
207+
`Could not load build target options for "${targetStringFromTarget(
208+
normalizedOptions.buildTarget,
209+
)}".\n` +
168210
`Please check your 'angular.json' configuration.\n` +
169211
`Error: ${e.message}`,
170212
);
213+
yield { success: false };
171214

172215
return;
173216
}
174217

175-
// Get runner-specific build options from the hook
176-
const {
177-
buildOptions: runnerBuildOptions,
178-
virtualFiles,
179-
testEntryPointMappings,
180-
} = await runner.getBuildOptions(normalizedOptions, buildTargetOptions);
218+
// Get runner-specific build options
219+
let runnerBuildOptions;
220+
let virtualFiles;
221+
let testEntryPointMappings;
222+
try {
223+
({
224+
buildOptions: runnerBuildOptions,
225+
virtualFiles,
226+
testEntryPointMappings,
227+
} = await runner.getBuildOptions(normalizedOptions, buildTargetOptions));
228+
} catch (e) {
229+
assertIsError(e);
230+
context.logger.error(
231+
`An exception occurred while getting runner-specific build options:\n${e.stack ?? e.message}`,
232+
);
233+
yield { success: false };
234+
235+
return;
236+
}
181237

182-
await using executor = await runner.createExecutor(
183-
context,
184-
normalizedOptions,
185-
testEntryPointMappings,
186-
);
238+
try {
239+
await using executor = await runner.createExecutor(
240+
context,
241+
normalizedOptions,
242+
testEntryPointMappings,
243+
);
187244

188-
const finalExtensions = prepareBuildExtensions(
189-
virtualFiles,
190-
normalizedOptions.projectSourceRoot,
191-
extensions,
192-
);
245+
const finalExtensions = prepareBuildExtensions(
246+
virtualFiles,
247+
normalizedOptions.projectSourceRoot,
248+
extensions,
249+
);
193250

194-
// Prepare and run the application build
195-
const applicationBuildOptions = {
196-
...buildTargetOptions,
197-
...runnerBuildOptions,
198-
watch: normalizedOptions.watch,
199-
tsConfig: normalizedOptions.tsConfig,
200-
progress: normalizedOptions.buildProgress ?? buildTargetOptions.progress,
201-
} satisfies ApplicationBuilderInternalOptions;
251+
// Prepare and run the application build
252+
const applicationBuildOptions = {
253+
...buildTargetOptions,
254+
...runnerBuildOptions,
255+
watch: normalizedOptions.watch,
256+
tsConfig: normalizedOptions.tsConfig,
257+
progress: normalizedOptions.buildProgress ?? buildTargetOptions.progress,
258+
} satisfies ApplicationBuilderInternalOptions;
202259

203-
yield* runBuildAndTest(executor, applicationBuildOptions, context, finalExtensions);
260+
yield* runBuildAndTest(executor, applicationBuildOptions, context, finalExtensions);
261+
} catch (e) {
262+
assertIsError(e);
263+
context.logger.error(
264+
`An exception occurred while creating the test executor:\n${e.stack ?? e.message}`,
265+
);
266+
yield { success: false };
267+
}
204268
}

packages/angular/build/src/builders/unit-test/tests/options/include_spec.ts

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import {
1515
} from '../setup';
1616

1717
describeBuilder(execute, UNIT_TEST_BUILDER_INFO, (harness) => {
18-
xdescribe('Option: "include"', () => {
18+
describe('Option: "include"', () => {
1919
beforeEach(async () => {
2020
setupApplicationTarget(harness);
2121
});
@@ -35,18 +35,10 @@ describeBuilder(execute, UNIT_TEST_BUILDER_INFO, (harness) => {
3535
test: 'relative path from workspace to spec',
3636
input: ['src/app/app.component.spec.ts'],
3737
},
38-
{
39-
test: 'relative path from workspace to file',
40-
input: ['src/app/app.component.ts'],
41-
},
4238
{
4339
test: 'relative path from project root to spec',
4440
input: ['app/services/test.service.spec.ts'],
4541
},
46-
{
47-
test: 'relative path from project root to file',
48-
input: ['app/services/test.service.ts'],
49-
},
5042
{
5143
test: 'relative path from workspace to directory',
5244
input: ['src/app/services'],
@@ -59,10 +51,6 @@ describeBuilder(execute, UNIT_TEST_BUILDER_INFO, (harness) => {
5951
test: 'glob with spec suffix',
6052
input: ['**/*.pipe.spec.ts', '**/*.pipe.spec.ts', '**/*test.service.spec.ts'],
6153
},
62-
{
63-
test: 'glob with forward slash and spec suffix',
64-
input: ['/**/*test.service.spec.ts'],
65-
},
6654
].forEach((options, index) => {
6755
it(`should work with ${options.test} (${index})`, async () => {
6856
await harness.writeFiles({

packages/angular/build/src/builders/unit-test/tests/options/providers-file_spec.ts

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import {
1515
} from '../setup';
1616

1717
describeBuilder(execute, UNIT_TEST_BUILDER_INFO, (harness) => {
18-
xdescribe('Option: "providersFile"', () => {
18+
describe('Option: "providersFile"', () => {
1919
beforeEach(async () => {
2020
setupApplicationTarget(harness);
2121
});
@@ -26,10 +26,12 @@ describeBuilder(execute, UNIT_TEST_BUILDER_INFO, (harness) => {
2626
providersFile: 'src/my.providers.ts',
2727
});
2828

29-
const { result, error } = await harness.executeOnce({ outputLogsOnFailure: false });
30-
expect(result).toBeUndefined();
31-
expect(error?.message).toMatch(
32-
`The specified providers file "src/my.providers.ts" does not exist.`,
29+
const { result, logs } = await harness.executeOnce({ outputLogsOnFailure: false });
30+
expect(result?.success).toBeFalse();
31+
expect(logs).toContain(
32+
jasmine.objectContaining({
33+
message: jasmine.stringMatching('Could not resolve "./my.providers"'),
34+
}),
3335
);
3436
});
3537

@@ -42,6 +44,14 @@ describeBuilder(execute, UNIT_TEST_BUILDER_INFO, (harness) => {
4244
`,
4345
});
4446

47+
await harness.modifyFile('src/tsconfig.spec.json', (content) => {
48+
const tsConfig = JSON.parse(content);
49+
tsConfig.files ??= [];
50+
tsConfig.files.push('my.providers.ts');
51+
52+
return JSON.stringify(tsConfig);
53+
});
54+
4555
harness.useTarget('test', {
4656
...BASE_OPTIONS,
4757
providersFile: 'src/my.providers.ts',

0 commit comments

Comments
 (0)