Skip to content

Commit f1a3a7d

Browse files
authored
fix(integ-runner): assembly execution errors don't provide meaningful information (#716)
This PR improves error formatting in the `integ-runner` including the actual cause of a command line execution failure and improving readability through colors and indentation. Relates to #715 ### Before <img width="1197" height="99" alt="image" src="https://github.com/user-attachments/assets/2abdb7d6-39a8-4109-a025-5002547c71ad" /> Note that the original error can be extracted from double verbose logs (running with `-vv`). But it's hidden within a wall of text: <img width="1197" height="158" alt="image" src="https://github.com/user-attachments/assets/d5cfe578-e49b-408e-90d0-e21000685ed1" /> ### After <img width="1197" height="121" alt="image" src="https://github.com/user-attachments/assets/10331faa-05d1-4bc8-a26b-226331ee359c" /> --- By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license
1 parent 6beac2b commit f1a3a7d

File tree

3 files changed

+40
-11
lines changed

3 files changed

+40
-11
lines changed

packages/@aws-cdk/integ-runner/lib/workers/common.ts

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -286,30 +286,37 @@ export function printResults(diagnostic: Diagnostic): void {
286286
logger.success(' UNCHANGED %s %s', diagnostic.testName, chalk.gray(`${diagnostic.duration}s`));
287287
break;
288288
case DiagnosticReason.TEST_SUCCESS:
289-
logger.success(' SUCCESS %s %s\n ', diagnostic.testName, chalk.gray(`${diagnostic.duration}s`), diagnostic.message);
289+
logger.success(' SUCCESS %s %s\n ', diagnostic.testName, chalk.gray(`${diagnostic.duration}s`));
290290
break;
291291
case DiagnosticReason.NO_SNAPSHOT:
292292
logger.error(' NEW %s %s', diagnostic.testName, chalk.gray(`${diagnostic.duration}s`));
293293
break;
294294
case DiagnosticReason.SNAPSHOT_FAILED:
295-
logger.error(' CHANGED %s %s\n %s', diagnostic.testName, chalk.gray(`${diagnostic.duration}s`), diagnostic.message);
295+
logger.error(' CHANGED %s %s\n%s', diagnostic.testName, chalk.gray(`${diagnostic.duration}s`), indentLines(diagnostic.message, 6));
296296
break;
297297
case DiagnosticReason.SNAPSHOT_ERROR:
298298
case DiagnosticReason.TEST_ERROR:
299-
logger.error(' ERROR %s %s\n %s', diagnostic.testName, chalk.gray(`${diagnostic.duration}s`), diagnostic.message);
299+
logger.error(' ERROR %s %s\n%s', diagnostic.testName, chalk.gray(`${diagnostic.duration}s`), indentLines(diagnostic.message, 6));
300300
break;
301301
case DiagnosticReason.TEST_FAILED:
302-
logger.error(' FAILED %s %s\n %s', diagnostic.testName, chalk.gray(`${diagnostic.duration}s`), diagnostic.message);
302+
logger.error(' FAILED %s %s\n%s', diagnostic.testName, chalk.gray(`${diagnostic.duration}s`), indentLines(diagnostic.message, 6));
303303
break;
304304
case DiagnosticReason.ASSERTION_FAILED:
305-
logger.error(' ASSERT %s %s\n %s', diagnostic.testName, chalk.gray(`${diagnostic.duration}s`), diagnostic.message);
305+
logger.error(' ASSERT %s %s\n%s', diagnostic.testName, chalk.gray(`${diagnostic.duration}s`), indentLines(diagnostic.message, 6));
306306
break;
307307
}
308308
for (const addl of diagnostic.additionalMessages ?? []) {
309309
logger.print(` ${addl}`);
310310
}
311311
}
312312

313+
/**
314+
* Takes a multiline string and indents every line with the same number of spaces.
315+
*/
316+
function indentLines(message: string, count = 2): string {
317+
return message.split('\n').map(line => ' '.repeat(count) + line).join('\n');
318+
}
319+
313320
export function printLaggards(testNames: Set<string>) {
314321
const parts = [
315322
' ',
@@ -319,3 +326,14 @@ export function printLaggards(testNames: Set<string>) {
319326

320327
logger.print(chalk.grey(parts.filter(x => x).join(' ')));
321328
}
329+
330+
export function formatError(error: any): string {
331+
const name = error.name || 'Error';
332+
const message = error.message || String(error);
333+
334+
if (error.cause) {
335+
return `${name}: ${message}\n${chalk.gray('Cause: ' + formatError(error.cause))}`;
336+
}
337+
338+
return `${name}: ${message}`;
339+
}

packages/@aws-cdk/integ-runner/lib/workers/extract/extract_worker.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { IntegSnapshotRunner, IntegTestRunner } from '../../runner';
33
import type { IntegTestInfo } from '../../runner/integration-tests';
44
import { IntegTest } from '../../runner/integration-tests';
55
import type { IntegTestWorkerConfig, SnapshotVerificationOptions, Diagnostic } from '../common';
6-
import { DiagnosticReason, formatAssertionResults } from '../common';
6+
import { DiagnosticReason, formatAssertionResults, formatError } from '../common';
77
import type { IntegTestBatchRequest } from '../integ-test-worker';
88
import type { IntegWatchOptions } from '../integ-watch-worker';
99

@@ -73,7 +73,7 @@ export async function integTestWorker(request: IntegTestBatchRequest): Promise<I
7373
workerpool.workerEmit({
7474
reason: DiagnosticReason.TEST_FAILED,
7575
testName: `${runner.testName}-${testCaseName} (${request.profile}/${request.region})`,
76-
message: `Integration test failed: ${e}`,
76+
message: `Integration test failed: ${formatError(e)}`,
7777
duration: (Date.now() - start) / 1000,
7878
});
7979
}
@@ -83,7 +83,7 @@ export async function integTestWorker(request: IntegTestBatchRequest): Promise<I
8383
workerpool.workerEmit({
8484
reason: DiagnosticReason.TEST_ERROR,
8585
testName: `${testInfo.fileName} (${request.profile}/${request.region})`,
86-
message: `Error during integration test: ${e}`,
86+
message: `Error during integration test: ${formatError(e)}`,
8787
duration: (Date.now() - start) / 1000,
8888
});
8989
}

packages/@aws-cdk/toolkit-lib/lib/api/cloud-assembly/private/exec.ts

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import * as child_process from 'node:child_process';
22
// eslint-disable-next-line @typescript-eslint/no-require-imports
33
import split = require('split2');
4-
import { ToolkitError } from '../../../toolkit/toolkit-error';
4+
import { AssemblyError } from '../../../toolkit/toolkit-error';
55

66
type EventPublisher = (event: 'open' | 'data_stdout' | 'data_stderr' | 'close', line: string) => void;
77

@@ -45,16 +45,27 @@ export async function execInChildProcess(commandAndArgs: string, options: ExecOp
4545
return;
4646
}
4747
});
48+
49+
const stderr = new Array<string>();
50+
4851
proc.stdout.pipe(split()).on('data', (line) => eventPublisher('data_stdout', line));
49-
proc.stderr.pipe(split()).on('data', (line) => eventPublisher('data_stderr', line));
52+
proc.stderr.pipe(split()).on('data', (line) => {
53+
stderr.push(line);
54+
return eventPublisher('data_stderr', line);
55+
});
5056

5157
proc.on('error', fail);
5258

5359
proc.on('exit', code => {
5460
if (code === 0) {
5561
return ok();
5662
} else {
57-
return fail(new ToolkitError(`${commandAndArgs}: Subprocess exited with error ${code}`));
63+
let cause: Error | undefined;
64+
if (stderr.length) {
65+
cause = new Error(stderr.join('\n'));
66+
cause.name = 'ExecutionError';
67+
}
68+
return fail(AssemblyError.withCause(`${commandAndArgs}: Subprocess exited with error ${code}`, cause));
5869
}
5970
});
6071
});

0 commit comments

Comments
 (0)