Skip to content

Commit

Permalink
fix(integ-runner): catch errors that happen during snapshot generation
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
rix0rrr committed May 27, 2022
1 parent 7779531 commit fb10189
Show file tree
Hide file tree
Showing 2 changed files with 64 additions and 21 deletions.
76 changes: 58 additions & 18 deletions packages/@aws-cdk/integ-runner/lib/runner/runner-base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand Down Expand Up @@ -133,22 +183,12 @@ export abstract class IntegRunner {
private legacyContext?: Record<string, any>;

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({
Expand All @@ -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();
Expand Down
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -86,9 +86,12 @@ export function integTestWorker(request: IntegTestBatchRequest): IntegTestWorker
*/
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();

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,
Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit fb10189

Please sign in to comment.