chore(integ-runner): --inspect-failures option (#20399)

When the integ runner fails to validate a snapshot, add an option
to retain the tempdir with the "new actual" snapshot so that a human
can go look around in it.

Also make the `--verbose` flag print out a command that will reproduce
the running of the CDK app directly from the command-line, so that
it can be debugged more easily.


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

Author: rix0rrr

packages/@aws-cdk/integ-runner/lib/cli.ts

@@ -27,7 +27,9 @@ async function main() {
     .options('max-workers', { type: 'number', desc: 'The max number of workerpool workers to use when running integration tests in parallel', default: 16 })
     .options('exclude', { type: 'boolean', desc: 'All tests should be run, except for the list of tests provided', default: false })
     .options('from-file', { type: 'string', desc: 'Import tests to include or exclude from a file' })
+    .option('inspect-failures', { type: 'boolean', desc: 'Keep the integ test cloud assembly if a failure occurs for inspection', default: false })
     .option('disable-update-workflow', { type: 'boolean', default: false, desc: 'If this is "true" then the stack update workflow will be disabled' })
+    .strict()
     .argv;
 
   const pool = workerpool.pool(path.join(__dirname, '../lib/workers/extract/index.js'), {
@@ -70,7 +72,10 @@ async function main() {
     // always run snapshot tests, but if '--force' is passed then
     // run integration tests on all failed tests, not just those that
     // failed snapshot tests
-    failedSnapshots = await runSnapshotTests(pool, testsFromArgs);
+    failedSnapshots = await runSnapshotTests(pool, testsFromArgs, {
+      retain: argv['inspect-failures'],
+      verbose: argv.verbose,
+    });
     for (const failure of failedSnapshots) {
       destructiveChanges.push(...failure.destructiveChanges ?? []);
     }

packages/@aws-cdk/integ-runner/lib/runner/runner-base.ts

@@ -129,6 +129,8 @@ export abstract class IntegRunner {
 
   protected readonly profile?: string;
 
+  protected readonly cdkExecutable: string;
+
   protected _destructiveChanges?: DestructiveChange[];
   private legacyContext?: Record<string, any>;
 
@@ -150,8 +152,10 @@ export abstract class IntegRunner {
     this.relativeSnapshotDir = `${testName}.integ.snapshot`;
     this.sourceFilePath = path.join(this.directory, parsed.base);
     this.cdkContextPath = path.join(this.directory, 'cdk.context.json');
+
+    this.cdkExecutable = require.resolve('aws-cdk/bin/cdk');
     this.cdk = options.cdk ?? new CdkCliWrapper({
-      cdkExecutable: require.resolve('aws-cdk/bin/cdk'),
+      cdkExecutable: this.cdkExecutable,
       directory: this.directory,
       env: {
         ...options.env,

packages/@aws-cdk/integ-runner/lib/runner/snapshot-test-runner.ts

@@ -2,7 +2,7 @@ import * as path from 'path';
 import { Writable, WritableOptions } from 'stream';
 import { StringDecoder, NodeStringDecoder } from 'string_decoder';
 import { diffTemplate, formatDifferences, ResourceDifference, ResourceImpact } from '@aws-cdk/cloudformation-diff';
-import { Diagnostic, DiagnosticReason, DestructiveChange } from '../workers/common';
+import { Diagnostic, DiagnosticReason, DestructiveChange, SnapshotVerificationOptions } from '../workers/common';
 import { canonicalizeTemplate } from './private/canonicalize-assets';
 import { AssemblyManifestReader } from './private/cloud-assembly';
 import { IntegRunnerOptions, IntegRunner, DEFAULT_SYNTH_OPTIONS } from './runner-base';
@@ -22,7 +22,8 @@ export class IntegSnapshotRunner extends IntegRunner {
    *
    * @returns any diagnostics and any destructive changes
    */
-  public testSnapshot(): { diagnostics: Diagnostic[], destructiveChanges: DestructiveChange[] } {
+  public testSnapshot(options: SnapshotVerificationOptions = {}): { diagnostics: Diagnostic[], destructiveChanges: DestructiveChange[] } {
+    let doClean = true;
     try {
       // read the existing snapshot
       const expectedStacks = this.readAssembly(this.snapshotDir);
@@ -39,17 +40,19 @@ export class IntegSnapshotRunner extends IntegRunner {
       // the cdkOutDir exists already, but for some reason generateActualSnapshot
       // generates an incorrect snapshot and I have no idea why so synth again here
       // to produce the "correct" snapshot
+      const env = {
+        ...DEFAULT_SYNTH_OPTIONS.env,
+        CDK_CONTEXT_JSON: JSON.stringify(this.getContext()),
+      };
       this.cdk.synthFast({
         execCmd: this.cdkApp.split(' '),
-        env: {
-          ...DEFAULT_SYNTH_OPTIONS.env,
-          CDK_CONTEXT_JSON: JSON.stringify(this.getContext()),
-        },
+        env,
         output: this.cdkOutDir,
       });
 
       // read the "actual" snapshot
-      const actualStacks = this.readAssembly(path.join(this.directory, this.cdkOutDir));
+      const actualDir = path.join(this.directory, this.cdkOutDir);
+      const actualStacks = this.readAssembly(actualDir);
       // only diff stacks that are part of the test case
       const actualStacksToDiff: Record<string, any> = {};
       for (const [stackName, template] of Object.entries(actualStacks)) {
@@ -60,11 +63,45 @@ export class IntegSnapshotRunner extends IntegRunner {
 
       // diff the existing snapshot (expected) with the integration test (actual)
       const diagnostics = this.diffAssembly(expectedStacksToDiff, actualStacksToDiff);
+
+      if (diagnostics.diagnostics.length) {
+        // Attach additional messages to the first diagnostic
+        const additionalMessages: string[] = [];
+
+        if (options.retain) {
+          additionalMessages.push(
+            `(Failure retained) Expected: ${path.relative(process.cwd(), this.snapshotDir)}`,
+            `                   Actual:   ${path.relative(process.cwd(), actualDir)}`,
+          ),
+          doClean = false;
+        }
+
+        if (options.verbose) {
+          // Show the command necessary to repro this
+          const envSet = Object.entries(env)
+            .filter(([k, _]) => k !== 'CDK_CONTEXT_JSON')
+            .map(([k, v]) => `${k}='${v}'`);
+          const envCmd = envSet.length > 0 ? ['env', ...envSet] : [];
+
+          additionalMessages.push(
+            'Repro:',
+            `  ${[...envCmd, 'cdk synth', `-a '${this.cdkApp}'`, `-o '${this.cdkOutDir}'`, ...Object.entries(this.getContext()).flatMap(([k, v]) => typeof v !== 'object' ? [`-c '${k}=${v}'`] : [])].join(' ')}`,
+          );
+        }
+
+        diagnostics.diagnostics[0] = {
+          ...diagnostics.diagnostics[0],
+          additionalMessages,
+        };
+      }
+
       return diagnostics;
     } catch (e) {
       throw e;
     } finally {
-      this.cleanup();
+      if (doClean) {
+        this.cleanup();
+      }
     }
   }
 

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

@@ -89,6 +89,22 @@ export interface IntegRunnerMetrics {
   readonly profile?: string;
 }
 
+export interface SnapshotVerificationOptions {
+  /**
+   * Retain failed snapshot comparisons
+   *
+   * @default false
+   */
+  readonly retain?: boolean;
+
+  /**
+   * Verbose mode
+   *
+   * @default false
+   */
+  readonly verbose?: boolean;
+}
+
 /**
  * Integration test results
  */
@@ -208,6 +224,11 @@ export interface Diagnostic {
    * The reason for the diagnostic
    */
   readonly reason: DiagnosticReason;
+
+  /**
+   * Additional messages to print
+   */
+  readonly additionalMessages?: string[];
 }
 
 export function printSummary(total: number, failed: number): void {
@@ -251,4 +272,7 @@ export function printResults(diagnostic: Diagnostic): void {
     case DiagnosticReason.ASSERTION_FAILED:
       logger.error('   %s - Assertions Failed! %s\n%s', diagnostic.testName, chalk.gray(`${diagnostic.duration}s`), diagnostic.message);
   }
+  for (const addl of diagnostic.additionalMessages ?? []) {
+    logger.print(`      ${addl}`);
+  }
 }

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

@@ -1,7 +1,7 @@
 import * as workerpool from 'workerpool';
 import { IntegSnapshotRunner, IntegTestRunner } from '../../runner';
 import { IntegTestConfig } from '../../runner/integration-tests';
-import { DiagnosticReason, IntegTestWorkerConfig, formatAssertionResults } from '../common';
+import { DiagnosticReason, IntegTestWorkerConfig, SnapshotVerificationOptions, Diagnostic, formatAssertionResults } from '../common';
 import { IntegTestBatchRequest } from '../integ-test-worker';
 
 /**
@@ -84,7 +84,7 @@ export function integTestWorker(request: IntegTestBatchRequest): IntegTestWorker
  * if there is an existing snapshot, and if there is will
  * check if there are any changes
  */
-export function snapshotTestWorker(test: IntegTestConfig): IntegTestWorkerConfig[] {
+export function snapshotTestWorker(test: IntegTestConfig, options: SnapshotVerificationOptions = {}): IntegTestWorkerConfig[] {
   const failedTests = new Array<IntegTestWorkerConfig>();
   const runner = new IntegSnapshotRunner({ fileName: test.fileName, directory: test.directory });
   const start = Date.now();
@@ -98,12 +98,12 @@ export function snapshotTestWorker(test: IntegTestConfig): IntegTestWorkerConfig
       });
       failedTests.push(test);
     } else {
-      const { diagnostics, destructiveChanges } = runner.testSnapshot();
+      const { diagnostics, destructiveChanges } = runner.testSnapshot(options);
       if (diagnostics.length > 0) {
         diagnostics.forEach(diagnostic => workerpool.workerEmit({
           ...diagnostic,
           duration: (Date.now() - start) / 1000,
-        }));
+        } as Diagnostic));
         failedTests.push({
           fileName: test.fileName,
           directory: test.directory,
@@ -115,7 +115,7 @@ export function snapshotTestWorker(test: IntegTestConfig): IntegTestWorkerConfig
           testName: runner.testName,
           message: 'Success',
           duration: (Date.now() - start) / 1000,
-        });
+        } as Diagnostic);
       }
     }
   } catch (e) {
@@ -125,7 +125,7 @@ export function snapshotTestWorker(test: IntegTestConfig): IntegTestWorkerConfig
       testName: runner.testName,
       reason: DiagnosticReason.SNAPSHOT_FAILED,
       duration: (Date.now() - start) / 1000,
-    });
+    } as Diagnostic);
   }
 
   return failedTests;

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

@@ -2,18 +2,22 @@ import * as workerpool from 'workerpool';
 import * as logger from '../logger';
 import { IntegTestConfig } from '../runner/integration-tests';
 import { flatten } from '../utils';
-import { printSummary, printResults, IntegTestWorkerConfig } from './common';
+import { printSummary, printResults, IntegTestWorkerConfig, SnapshotVerificationOptions } from './common';
 
 /**
  * Run Snapshot tests
  * First batch up the tests. By default there will be 3 tests per batch.
  * Use a workerpool to run the batches in parallel.
  */
-export async function runSnapshotTests(pool: workerpool.WorkerPool, tests: IntegTestConfig[]): Promise<IntegTestWorkerConfig[]> {
+export async function runSnapshotTests(
+  pool: workerpool.WorkerPool,
+  tests: IntegTestConfig[],
+  options: SnapshotVerificationOptions,
+): Promise<IntegTestWorkerConfig[]> {
   logger.highlight('\nVerifying integration test snapshots...\n');
 
   const failedTests: IntegTestWorkerConfig[][] = await Promise.all(
-    tests.map((test) => pool.exec('snapshotTestWorker', [test], {
+    tests.map((test) => pool.exec('snapshotTestWorker', [test, options], {
       on: printResults,
     })),
   );

packages/@aws-cdk/integ-runner/test/workers/integ-worker.test.ts

@@ -147,7 +147,7 @@ describe('test runner', () => {
       ]),
     ]));
 
-    expect(results.length).toEqual(0);
+    expect(results).toEqual([]);
   });
 
   test('deploy failed', () => {