From fb10189e8de24522cc9004d6d4e6bf625d07e30b Mon Sep 17 00:00:00 2001 From: Rico Huijbers Date: Fri, 27 May 2022 15:00:45 +0200 Subject: [PATCH] fix(integ-runner): catch errors that happen during snapshot generation The constructor of `IntegSnapshotRunner` calls `loadManifest()`, which on my computer happened to fail and stopped the entire test suite because this error happened outside the `try/catch` block. Move it inside the try/catch block, needed a bit of refactoring to make sure we could still get at the test name. --- .../integ-runner/lib/runner/runner-base.ts | 76 ++++++++++++++----- .../lib/workers/extract/extract_worker.ts | 9 ++- 2 files changed, 64 insertions(+), 21 deletions(-) diff --git a/packages/@aws-cdk/integ-runner/lib/runner/runner-base.ts b/packages/@aws-cdk/integ-runner/lib/runner/runner-base.ts index 6ebdce2eea40c..b60b1ce799359 100644 --- a/packages/@aws-cdk/integ-runner/lib/runner/runner-base.ts +++ b/packages/@aws-cdk/integ-runner/lib/runner/runner-base.ts @@ -57,10 +57,60 @@ export interface IntegRunnerOptions { readonly cdk?: ICdk; } +/** + * The different components of a test name + */ +export interface TestNameParts { + /** + * (Absolute) path to the file + */ + readonly fileName: string; + + /** + * Directory the test is in + */ + readonly directory: string; + + /** + * Display name for the test + */ + readonly testName: string; + + /** + * Full path of the snapshot directory for this test + */ + readonly snapshotDir: string; +} + /** * Represents an Integration test runner */ export abstract class IntegRunner { + /** + * Parse a test name into parts + */ + public static testNameFromFile(fileName: string, rootDirectory: string): TestNameParts { + // Make absolute + fileName = path.resolve(fileName); + const parsed = path.parse(fileName); + + const baseTestName = parsed.name.slice(6); // Leave name without 'integ.' and '.ts' + + // if we are running in a package directory then juse use the fileName + // as the testname, but if we are running in a parent directory with + // multiple packages then use the directory/filename as the testname + const testName = parsed.dir === path.join(rootDirectory, 'test') + ? baseTestName + : path.join(path.relative(rootDirectory, parsed.dir), baseTestName); + + return { + fileName, + testName, + directory: parsed.dir, + snapshotDir: path.resolve(parsed.dir, `${baseTestName}.integ.snapshot`), + }; + } + /** * The directory where the snapshot will be stored */ @@ -133,22 +183,12 @@ export abstract class IntegRunner { private legacyContext?: Record; constructor(options: IntegRunnerOptions) { - const parsed = path.parse(options.fileName); - this.directory = parsed.dir; - const testName = parsed.name.slice(6); - - // if we are running in a package directory then juse use the fileName - // as the testname, but if we are running in a parent directory with - // multiple packages then use the directory/filename as the testname - if (parsed.dir === 'test') { - this.testName = testName; - } else { - const relativePath = path.relative(options.directory, parsed.dir); - this.testName = `${relativePath ? relativePath + '/' : ''}${parsed.name}`; - } - this.snapshotDir = path.join(this.directory, `${testName}.integ.snapshot`); - this.relativeSnapshotDir = `${testName}.integ.snapshot`; - this.sourceFilePath = path.join(this.directory, parsed.base); + const parsed = IntegRunner.testNameFromFile(options.fileName, options.directory); + this.directory = parsed.directory; + this.testName = parsed.testName; + this.snapshotDir = parsed.snapshotDir; + this.relativeSnapshotDir = path.relative(this.directory, this.snapshotDir); + this.sourceFilePath = parsed.fileName; this.cdkContextPath = path.join(this.directory, 'cdk.context.json'); this.cdk = options.cdk ?? new CdkCliWrapper({ @@ -157,8 +197,8 @@ export abstract class IntegRunner { ...options.env, }, }); - this.cdkOutDir = options.integOutDir ?? `${CDK_OUTDIR_PREFIX}.${testName}`; - this.cdkApp = `node ${parsed.base}`; + this.cdkOutDir = options.integOutDir ?? `${CDK_OUTDIR_PREFIX}.${this.testName}`; + this.cdkApp = `node ${this.sourceFilePath}`; this.profile = options.profile; if (this.hasSnapshot()) { this.expectedTestSuite = this.loadManifest(); diff --git a/packages/@aws-cdk/integ-runner/lib/workers/extract/extract_worker.ts b/packages/@aws-cdk/integ-runner/lib/workers/extract/extract_worker.ts index 957341647e90f..7bd8919094dcc 100644 --- a/packages/@aws-cdk/integ-runner/lib/workers/extract/extract_worker.ts +++ b/packages/@aws-cdk/integ-runner/lib/workers/extract/extract_worker.ts @@ -1,5 +1,5 @@ import * as workerpool from 'workerpool'; -import { IntegSnapshotRunner, IntegTestRunner } from '../../runner'; +import { IntegRunner, IntegSnapshotRunner, IntegTestRunner } from '../../runner'; import { IntegTestConfig } from '../../runner/integration-tests'; import { DiagnosticReason, IntegTestWorkerConfig, SnapshotVerificationOptions, Diagnostic, formatAssertionResults } from '../common'; import { IntegTestBatchRequest } from '../integ-test-worker'; @@ -86,9 +86,12 @@ export function integTestWorker(request: IntegTestBatchRequest): IntegTestWorker */ export function snapshotTestWorker(test: IntegTestConfig, options: SnapshotVerificationOptions = {}): IntegTestWorkerConfig[] { const failedTests = new Array(); - const runner = new IntegSnapshotRunner({ fileName: test.fileName, directory: test.directory }); const start = Date.now(); + + const testName = IntegRunner.testNameFromFile(test.fileName, test.directory); + try { + const runner = new IntegSnapshotRunner({ fileName: test.fileName, directory: test.directory }); if (!runner.hasSnapshot()) { workerpool.workerEmit({ reason: DiagnosticReason.NO_SNAPSHOT, @@ -122,7 +125,7 @@ export function snapshotTestWorker(test: IntegTestConfig, options: SnapshotVerif failedTests.push(test); workerpool.workerEmit({ message: e.message, - testName: runner.testName, + testName: testName.testName, reason: DiagnosticReason.SNAPSHOT_FAILED, duration: (Date.now() - start) / 1000, } as Diagnostic);