From cedfde8cb07eb879ee384bda93bba813ede91699 Mon Sep 17 00:00:00 2001 From: Rico Huijbers Date: Tue, 31 May 2022 16:32:21 +0200 Subject: [PATCH] fix(integ-runner): catch snapshot errors, treat `--from-file` as command-line (#20523) Snapshot errors --------------- 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. Same for the integ runner itself. Move it inside the try/catch block, needed a bit of refactoring to make sure we could still get at the test name. `--from-file` ------------- Instead of having `--from-file` require a JSON file with its own structure, interpret it as a text file which gets treated exactly the same as the `[TEST [..]]` arguments on the command line. This still allows for the `--exclude` behavior by setting that flag on the command-line. Also be very liberal on the pattern (file name or test name or display name) that we accept, encoded in the `IntegTest.matches()` class. Refactoring ----------- Moved the logic around determining test names and directories into a class (`IntegTest`) which is a convenience class on top of a static data record (`IntegTestInfo`). The split between data and logic is so that we can pass the data to worker threads where we can hydrate the helper class on top again. I tried to be consistent: in memory, all the fields are with respect to `process.cwd()`, so valid file paths in the current process. Only when they are passed to the CLI wrapper are the paths made relative to the CLI wrapper directory. Print snapshot validations that are running for a long time (1 minute). This helps diagnose what is stuck, if anything is. On my machine, it was tests using Docker because there was some issue with it, and this lost me a day. Also change the test reporting formatting slightly. -------------- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- packages/@aws-cdk/integ-runner/lib/cli.ts | 26 +-- .../lib/runner/integ-test-runner.ts | 24 ++- .../lib/runner/integration-tests.ts | 154 ++++++++++++++---- .../integ-runner/lib/runner/runner-base.ts | 72 +++----- .../lib/runner/snapshot-test-runner.ts | 4 +- packages/@aws-cdk/integ-runner/lib/utils.ts | 58 +++++++ .../integ-runner/lib/workers/common.ts | 43 ++++- .../lib/workers/extract/extract_worker.ts | 72 ++++---- .../lib/workers/integ-snapshot-worker.ts | 21 ++- .../lib/workers/integ-test-worker.ts | 4 +- .../test/runner/integ-test-runner.test.ts | 92 +++++++---- .../test/runner/integration-tests.test.ts | 55 ------- .../test/runner/snapshot-test-runner.test.ts | 40 +++-- .../test/workers/integ-worker.test.ts | 60 +++---- .../test/workers/mock-extract_worker.ts | 4 +- .../test/workers/snapshot-worker.test.ts | 6 +- 16 files changed, 453 insertions(+), 282 deletions(-) diff --git a/packages/@aws-cdk/integ-runner/lib/cli.ts b/packages/@aws-cdk/integ-runner/lib/cli.ts index 6014d4997378a..148d5e456f21c 100644 --- a/packages/@aws-cdk/integ-runner/lib/cli.ts +++ b/packages/@aws-cdk/integ-runner/lib/cli.ts @@ -1,9 +1,10 @@ // Exercise all integ stacks and if they deploy, update the expected synth files +import { promises as fs } from 'fs'; import * as path from 'path'; import * as chalk from 'chalk'; import * as workerpool from 'workerpool'; import * as logger from './logger'; -import { IntegrationTests, IntegTestConfig } from './runner/integration-tests'; +import { IntegrationTests, IntegTestInfo, IntegTest } from './runner/integration-tests'; import { runSnapshotTests, runIntegrationTests, IntegRunnerMetrics, IntegTestWorkerConfig, DestructiveChange } from './workers'; // https://github.com/yargs/yargs/issues/1929 @@ -25,8 +26,8 @@ async function main() { .options('directory', { type: 'string', default: 'test', desc: 'starting directory to discover integration tests. Tests will be discovered recursively from this directory' }) .options('profiles', { type: 'array', desc: 'list of AWS profiles to use. Tests will be run in parallel across each profile+regions', nargs: 1, default: [] }) .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' }) + .options('exclude', { type: 'boolean', desc: 'Run all tests in the directory, except the specified TESTs', default: false }) + .options('from-file', { type: 'string', desc: 'Read TEST names from a file (one TEST per line)' }) .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() @@ -39,7 +40,7 @@ async function main() { // list of integration tests that will be executed const testsToRun: IntegTestWorkerConfig[] = []; const destructiveChanges: DestructiveChange[] = []; - const testsFromArgs: IntegTestConfig[] = []; + const testsFromArgs: IntegTest[] = []; const parallelRegions = arrayFromYargs(argv['parallel-regions']); const testRegions: string[] = parallelRegions ?? ['us-east-1', 'us-east-2', 'us-west-2']; const profiles = arrayFromYargs(argv.profiles); @@ -56,19 +57,18 @@ async function main() { try { if (argv.list) { const tests = await new IntegrationTests(argv.directory).fromCliArgs(); - process.stdout.write(tests.map(t => t.fileName).join('\n') + '\n'); + process.stdout.write(tests.map(t => t.discoveryRelativeFileName).join('\n') + '\n'); return; } if (argv._.length > 0 && fromFile) { throw new Error('A list of tests cannot be provided if "--from-file" is provided'); - } else if (argv._.length === 0 && !fromFile) { - testsFromArgs.push(...(await new IntegrationTests(path.resolve(argv.directory)).fromCliArgs())); - } else if (fromFile) { - testsFromArgs.push(...(await new IntegrationTests(path.resolve(argv.directory)).fromFile(path.resolve(fromFile)))); - } else { - testsFromArgs.push(...(await new IntegrationTests(path.resolve(argv.directory)).fromCliArgs(argv._.map((x: any) => x.toString()), exclude))); } + const requestedTests = fromFile + ? (await fs.readFile(fromFile, { encoding: 'utf8' })).split('\n').filter(x => x) + : (argv._.length > 0 ? argv._ : undefined); // 'undefined' means no request + + testsFromArgs.push(...(await new IntegrationTests(path.resolve(argv.directory)).fromCliArgs(requestedTests, exclude))); // always run snapshot tests, but if '--force' is passed then // run integration tests on all failed tests, not just those that @@ -85,7 +85,7 @@ async function main() { } else { // if any of the test failed snapshot tests, keep those results // and merge with the rest of the tests from args - testsToRun.push(...mergeTests(testsFromArgs, failedSnapshots)); + testsToRun.push(...mergeTests(testsFromArgs.map(t => t.info), failedSnapshots)); } // run integration tests if `--update-on-failed` OR `--force` is used @@ -174,7 +174,7 @@ function arrayFromYargs(xs: string[]): string[] | undefined { * tests that failed snapshot tests. The failed snapshot tests have additional * information that we want to keep so this should override any test from args */ -function mergeTests(testFromArgs: IntegTestConfig[], failedSnapshotTests: IntegTestWorkerConfig[]): IntegTestWorkerConfig[] { +function mergeTests(testFromArgs: IntegTestInfo[], failedSnapshotTests: IntegTestWorkerConfig[]): IntegTestWorkerConfig[] { const failedTestNames = new Set(failedSnapshotTests.map(test => test.fileName)); const final: IntegTestWorkerConfig[] = failedSnapshotTests; final.push(...testFromArgs.filter(test => !failedTestNames.has(test.fileName))); diff --git a/packages/@aws-cdk/integ-runner/lib/runner/integ-test-runner.ts b/packages/@aws-cdk/integ-runner/lib/runner/integ-test-runner.ts index 3c325d34c3700..fe1e6cadf1e61 100644 --- a/packages/@aws-cdk/integ-runner/lib/runner/integ-test-runner.ts +++ b/packages/@aws-cdk/integ-runner/lib/runner/integ-test-runner.ts @@ -72,7 +72,8 @@ export class IntegTestRunner extends IntegRunner { * all branches and we then search for one that starts with `HEAD branch: ` */ private checkoutSnapshot(): void { - const cwd = path.dirname(this.snapshotDir); + const cwd = this.directory; + // https://git-scm.com/docs/git-merge-base let baseBranch: string | undefined = undefined; // try to find the base branch that the working branch was created from @@ -98,17 +99,19 @@ export class IntegTestRunner extends IntegRunner { // if we found the base branch then get the merge-base (most recent common commit) // and checkout the snapshot using that commit if (baseBranch) { + const relativeSnapshotDir = path.relative(this.directory, this.snapshotDir); + try { const base = exec(['git', 'merge-base', 'HEAD', baseBranch], { cwd, }); - exec(['git', 'checkout', base, '--', this.relativeSnapshotDir], { + exec(['git', 'checkout', base, '--', relativeSnapshotDir], { cwd, }); } catch (e) { logger.warning('%s\n%s', `Could not checkout snapshot directory ${this.snapshotDir} using these commands: `, - `git merge-base HEAD ${baseBranch} && git checkout {merge-base} -- ${this.relativeSnapshotDir}`, + `git merge-base HEAD ${baseBranch} && git checkout {merge-base} -- ${relativeSnapshotDir}`, ); logger.warning('error: %s', e); } @@ -129,6 +132,9 @@ export class IntegTestRunner extends IntegRunner { public runIntegTestCase(options: RunOptions): AssertionResults | undefined { let assertionResults: AssertionResults | undefined; const actualTestCase = this.actualTestSuite.testSuite[options.testCaseName]; + if (!actualTestCase) { + throw new Error(`Did not find test case name '${options.testCaseName}' in '${Object.keys(this.actualTestSuite.testSuite)}'`); + } const clean = options.clean ?? true; const updateWorkflowEnabled = (options.updateWorkflow ?? true) && (actualTestCase.stackUpdateWorkflow ?? true); @@ -151,7 +157,7 @@ export class IntegTestRunner extends IntegRunner { this.cdk.synthFast({ execCmd: this.cdkApp.split(' '), env, - output: this.cdkOutDir, + output: path.relative(this.directory, this.cdkOutDir), }); } // only create the snapshot if there are no assertion assertion results @@ -170,7 +176,7 @@ export class IntegTestRunner extends IntegRunner { all: true, force: true, app: this.cdkApp, - output: this.cdkOutDir, + output: path.relative(this.directory, this.cdkOutDir), ...actualTestCase.cdkCommandOptions?.destroy?.args, context: this.getContext(actualTestCase.cdkCommandOptions?.destroy?.args?.context), }); @@ -241,7 +247,7 @@ export class IntegTestRunner extends IntegRunner { stacks: expectedTestCase.stacks, ...expectedTestCase?.cdkCommandOptions?.deploy?.args, context: this.getContext(expectedTestCase?.cdkCommandOptions?.deploy?.args?.context), - app: this.relativeSnapshotDir, + app: path.relative(this.directory, this.snapshotDir), lookups: this.expectedTestSuite?.enableLookups, }); } @@ -255,9 +261,9 @@ export class IntegTestRunner extends IntegRunner { ...actualTestCase.assertionStack ? [actualTestCase.assertionStack] : [], ], rollback: false, - output: this.cdkOutDir, + output: path.relative(this.directory, this.cdkOutDir), ...actualTestCase?.cdkCommandOptions?.deploy?.args, - ...actualTestCase.assertionStack ? { outputsFile: path.join(this.cdkOutDir, 'assertion-results.json') } : undefined, + ...actualTestCase.assertionStack ? { outputsFile: path.relative(this.directory, path.join(this.cdkOutDir, 'assertion-results.json')) } : undefined, context: this.getContext(actualTestCase?.cdkCommandOptions?.deploy?.args?.context), app: this.cdkApp, }); @@ -270,7 +276,7 @@ export class IntegTestRunner extends IntegRunner { if (actualTestCase.assertionStack) { return this.processAssertionResults( - path.join(this.directory, this.cdkOutDir, 'assertion-results.json'), + path.join(this.cdkOutDir, 'assertion-results.json'), actualTestCase.assertionStack, ); } diff --git a/packages/@aws-cdk/integ-runner/lib/runner/integration-tests.ts b/packages/@aws-cdk/integ-runner/lib/runner/integration-tests.ts index 98511a76daff8..ac139187c500a 100644 --- a/packages/@aws-cdk/integ-runner/lib/runner/integration-tests.ts +++ b/packages/@aws-cdk/integ-runner/lib/runner/integration-tests.ts @@ -1,22 +1,118 @@ import * as path from 'path'; import * as fs from 'fs-extra'; +const CDK_OUTDIR_PREFIX = 'cdk-integ.out'; + /** * Represents a single integration test + * + * This type is a data-only structure, so it can trivially be passed to workers. + * Derived attributes are calculated using the `IntegTest` class. */ -export interface IntegTestConfig { +export interface IntegTestInfo { /** - * The name of the file that contains the - * integration tests. This will be in the format - * of integ.{test-name}.js + * Path to the file to run + * + * Path is relative to the current working directory. */ readonly fileName: string; /** - * The base directory where the tests are - * discovered from + * The root directory we discovered this test from + * + * Path is relative to the current working directory. */ - readonly directory: string; + readonly discoveryRoot: string; +} + +/** + * Derived information for IntegTests + */ +export class IntegTest { + /** + * The name of the file to run + * + * Path is relative to the current working directory. + */ + public readonly fileName: string; + + /** + * Relative path to the file to run + * + * Relative from the "discovery root". + */ + public readonly discoveryRelativeFileName: string; + + /** + * The absolute path to the file + */ + public readonly absoluteFileName: string; + + /** + * Directory the test is in + */ + public readonly directory: string; + + /** + * Display name for the test + * + * Depends on the discovery directory. + * + * Looks like `integ.mytest` or `package/test/integ.mytest`. + */ + public readonly testName: string; + + /** + * Path of the snapshot directory for this test + */ + public readonly snapshotDir: string; + + /** + * Path to the temporary output directory for this test + */ + public readonly temporaryOutputDir: string; + + constructor(public readonly info: IntegTestInfo) { + this.absoluteFileName = path.resolve(info.fileName); + this.fileName = path.relative(process.cwd(), info.fileName); + + const parsed = path.parse(this.fileName); + this.discoveryRelativeFileName = path.relative(info.discoveryRoot, info.fileName); + this.directory = parsed.dir; + + // if we are running in a package directory then just 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 + // + // Looks either like `integ.mytest` or `package/test/integ.mytest`. + const relDiscoveryRoot = path.relative(process.cwd(), info.discoveryRoot); + this.testName = this.directory === path.join(relDiscoveryRoot, 'test') || this.directory === path.join(relDiscoveryRoot) + ? parsed.name + : path.join(path.relative(this.info.discoveryRoot, parsed.dir), parsed.name); + + const nakedTestName = parsed.name.slice(6); // Leave name without 'integ.' and '.ts' + this.snapshotDir = path.join(this.directory, `${nakedTestName}.integ.snapshot`); + this.temporaryOutputDir = path.join(this.directory, `${CDK_OUTDIR_PREFIX}.${nakedTestName}`); + } + + /** + * Whether this test matches the user-given name + * + * We are very lenient here. A name matches if it matches: + * + * - The CWD-relative filename + * - The discovery root-relative filename + * - The suite name + * - The absolute filename + */ + public matches(name: string) { + return [ + this.fileName, + this.discoveryRelativeFileName, + this.testName, + this.absoluteFileName, + ].includes(name); + } } /** @@ -49,7 +145,7 @@ export class IntegrationTests { * Takes a file name of a file that contains a list of test * to either run or exclude and returns a list of Integration Tests to run */ - public async fromFile(fileName: string): Promise { + public async fromFile(fileName: string): Promise { const file: IntegrationTestFileConfig = JSON.parse(fs.readFileSync(fileName, { encoding: 'utf-8' })); const foundTests = await this.discover(); @@ -65,32 +161,27 @@ export class IntegrationTests { * If they have provided a test name that we don't find, then we write out that error message. * - If it is a list of tests to exclude, then we discover all available tests and filter out the tests that were provided by the user. */ - private filterTests(discoveredTests: IntegTestConfig[], requestedTests?: string[], exclude?: boolean): IntegTestConfig[] { - if (!requestedTests || requestedTests.length === 0) { + private filterTests(discoveredTests: IntegTest[], requestedTests?: string[], exclude?: boolean): IntegTest[] { + if (!requestedTests) { return discoveredTests; } - const all = discoveredTests.map(x => { - return path.relative(x.directory, x.fileName); - }); - let foundAll = true; - // Pare down found tests to filter + + const allTests = discoveredTests.filter(t => { - if (exclude) { - return (!requestedTests.includes(path.relative(t.directory, t.fileName))); - } - return (requestedTests.includes(path.relative(t.directory, t.fileName))); + const matches = requestedTests.some(pattern => t.matches(pattern)); + return matches !== !!exclude; // Looks weird but is equal to (matches && !exclude) || (!matches && exclude) }); + // If not excluding, all patterns must have matched at least one test if (!exclude) { - const selectedNames = allTests.map(t => path.relative(t.directory, t.fileName)); - for (const unmatched of requestedTests.filter(t => !selectedNames.includes(t))) { + const unmatchedPatterns = requestedTests.filter(pattern => !discoveredTests.some(t => t.matches(pattern))); + for (const unmatched of unmatchedPatterns) { process.stderr.write(`No such integ test: ${unmatched}\n`); - foundAll = false; } - } - if (!foundAll) { - process.stderr.write(`Available tests: ${all.join(' ')}\n`); - return []; + if (unmatchedPatterns.length > 0) { + process.stderr.write(`Available tests: ${discoveredTests.map(t => t.discoveryRelativeFileName).join(' ')}\n`); + return []; + } } return allTests; @@ -99,8 +190,11 @@ export class IntegrationTests { /** * Takes an optional list of tests to look for, otherwise * it will look for all tests from the directory + * + * @param tests Tests to include or exclude, undefined means include all tests. + * @param exclude Whether the 'tests' list is inclusive or exclusive (inclusive by default). */ - public async fromCliArgs(tests?: string[], exclude?: boolean): Promise { + public async fromCliArgs(tests?: string[], exclude?: boolean): Promise { const discoveredTests = await this.discover(); const allTests = this.filterTests(discoveredTests, tests, exclude); @@ -108,14 +202,14 @@ export class IntegrationTests { return allTests; } - private async discover(): Promise { + private async discover(): Promise { const files = await this.readTree(); const integs = files.filter(fileName => path.basename(fileName).startsWith('integ.') && path.basename(fileName).endsWith('.js')); return this.request(integs); } - private request(files: string[]): IntegTestConfig[] { - return files.map(fileName => { return { directory: this.directory, fileName }; }); + private request(files: string[]): IntegTest[] { + return files.map(fileName => new IntegTest({ discoveryRoot: this.directory, fileName })); } private async readTree(): Promise { 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..1a74184078769 100644 --- a/packages/@aws-cdk/integ-runner/lib/runner/runner-base.ts +++ b/packages/@aws-cdk/integ-runner/lib/runner/runner-base.ts @@ -6,9 +6,9 @@ import * as fs from 'fs-extra'; import { flatten } from '../utils'; import { DestructiveChange } from '../workers/common'; import { IntegTestSuite, LegacyIntegTestSuite } from './integ-test-suite'; +import { IntegTest } from './integration-tests'; import { AssemblyManifestReader, ManifestTrace } from './private/cloud-assembly'; -const CDK_OUTDIR_PREFIX = 'cdk-integ.out'; const DESTRUCTIVE_CHANGES = '!!DESTRUCTIVE_CHANGES:'; /** @@ -16,16 +16,9 @@ const DESTRUCTIVE_CHANGES = '!!DESTRUCTIVE_CHANGES:'; */ export interface IntegRunnerOptions { /** - * The name of the file that contains the integration test - * This should be a JavaScript file + * Information about the test to run */ - readonly fileName: string, - - /** - * The base directory where the tests are - * discovered from. - */ - readonly directory: string, + readonly test: IntegTest; /** * The AWS profile to use when invoking the CDK CLI @@ -76,13 +69,10 @@ export abstract class IntegRunner { */ public readonly testName: string; - /** - * The path to the integration test file - */ - protected readonly sourceFilePath: string; - /** * The value used in the '--app' CLI parameter + * + * Path to the integ test source file, relative to `this.directory`. */ protected readonly cdkApp: string; @@ -92,11 +82,6 @@ export abstract class IntegRunner { */ protected readonly cdkContextPath: string; - /** - * The relative path from the cwd to the snapshot directory - */ - protected readonly relativeSnapshotDir: string; - /** * The test suite from the existing snapshot */ @@ -113,6 +98,11 @@ export abstract class IntegRunner { */ protected readonly directory: string; + /** + * The test to run + */ + protected readonly test: IntegTest; + /** * Default options to pass to the CDK CLI */ @@ -124,6 +114,8 @@ export abstract class IntegRunner { /** * The directory where the CDK will be synthed to + * + * Relative to cwd. */ protected readonly cdkOutDir: string; @@ -133,22 +125,10 @@ 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); + this.test = options.test; + this.directory = this.test.directory; + this.testName = this.test.testName; + this.snapshotDir = this.test.snapshotDir; this.cdkContextPath = path.join(this.directory, 'cdk.context.json'); this.cdk = options.cdk ?? new CdkCliWrapper({ @@ -157,8 +137,8 @@ export abstract class IntegRunner { ...options.env, }, }); - this.cdkOutDir = options.integOutDir ?? `${CDK_OUTDIR_PREFIX}.${testName}`; - this.cdkApp = `node ${parsed.base}`; + this.cdkOutDir = options.integOutDir ?? this.test.temporaryOutputDir; + this.cdkApp = `node ${path.relative(this.directory, this.test.fileName)}`; this.profile = options.profile; if (this.hasSnapshot()) { this.expectedTestSuite = this.loadManifest(); @@ -194,9 +174,9 @@ export abstract class IntegRunner { // use the "expected" context. This is only run in order to read the manifest CDK_CONTEXT_JSON: JSON.stringify(this.getContext(this.expectedTestSuite?.synthContext)), }, - output: this.cdkOutDir, + output: path.relative(this.directory, this.cdkOutDir), }); - return this.loadManifest(path.join(this.directory, this.cdkOutDir)); + return this.loadManifest(this.cdkOutDir); } /** @@ -221,22 +201,22 @@ export abstract class IntegRunner { const testCases = LegacyIntegTestSuite.fromLegacy({ cdk: this.cdk, testName: this.testName, - integSourceFilePath: this.sourceFilePath, + integSourceFilePath: this.test.fileName, listOptions: { ...this.defaultArgs, all: true, app: this.cdkApp, profile: this.profile, - output: this.cdkOutDir, + output: path.relative(this.directory, this.cdkOutDir), }, }); - this.legacyContext = LegacyIntegTestSuite.getPragmaContext(this.sourceFilePath); + this.legacyContext = LegacyIntegTestSuite.getPragmaContext(this.test.fileName); return testCases; } } protected cleanup(): void { - const cdkOutPath = path.join(this.directory, this.cdkOutDir); + const cdkOutPath = this.cdkOutDir; if (fs.existsSync(cdkOutPath)) { fs.removeSync(cdkOutPath); } @@ -330,10 +310,10 @@ export abstract class IntegRunner { ...DEFAULT_SYNTH_OPTIONS.env, CDK_CONTEXT_JSON: JSON.stringify(this.getContext()), }, - output: this.relativeSnapshotDir, + output: path.relative(this.directory, this.snapshotDir), }); } else { - fs.moveSync(path.join(this.directory, this.cdkOutDir), this.snapshotDir, { overwrite: true }); + fs.moveSync(this.cdkOutDir, this.snapshotDir, { overwrite: true }); } this.cleanupSnapshot(); diff --git a/packages/@aws-cdk/integ-runner/lib/runner/snapshot-test-runner.ts b/packages/@aws-cdk/integ-runner/lib/runner/snapshot-test-runner.ts index e7c95beff0a56..956d392cbea2a 100644 --- a/packages/@aws-cdk/integ-runner/lib/runner/snapshot-test-runner.ts +++ b/packages/@aws-cdk/integ-runner/lib/runner/snapshot-test-runner.ts @@ -47,11 +47,11 @@ export class IntegSnapshotRunner extends IntegRunner { this.cdk.synthFast({ execCmd: this.cdkApp.split(' '), env, - output: this.cdkOutDir, + output: path.relative(this.directory, this.cdkOutDir), }); // read the "actual" snapshot - const actualDir = path.join(this.directory, this.cdkOutDir); + const actualDir = this.cdkOutDir; const actualStacks = this.readAssembly(actualDir); // only diff stacks that are part of the test case const actualStacksToDiff: Record = {}; diff --git a/packages/@aws-cdk/integ-runner/lib/utils.ts b/packages/@aws-cdk/integ-runner/lib/utils.ts index 4eaef31727867..74e5eb2154ae7 100644 --- a/packages/@aws-cdk/integ-runner/lib/utils.ts +++ b/packages/@aws-cdk/integ-runner/lib/utils.ts @@ -40,3 +40,61 @@ export function flatten(xs: T[][]): T[] { export function chain(commands: string[]): string { return commands.filter(c => !!c).join(' && '); } + + +/** + * A class holding a set of items which are being crossed off in time + * + * If it takes too long to cross off a new item, print the list. + */ +export class WorkList { + private readonly remaining = new Set(this.items); + private readonly timeout: number; + private timer?: NodeJS.Timeout; + + constructor(private readonly items: A[], private readonly options: WorkListOptions = {}) { + this.timeout = options.timeout ?? 60_000; + this.scheduleTimer(); + } + + public crossOff(item: A) { + this.remaining.delete(item); + this.stopTimer(); + if (this.remaining.size > 0) { + this.scheduleTimer(); + } + } + + public done() { + this.remaining.clear(); + } + + private stopTimer() { + if (this.timer) { + clearTimeout(this.timer); + this.timer = undefined; + } + } + + private scheduleTimer() { + this.timer = setTimeout(() => this.report(), this.timeout); + } + + private report() { + this.options.onTimeout?.(this.remaining); + } +} + +export interface WorkListOptions { + /** + * When to reply with remaining items + * + * @default 60000 + */ + readonly timeout?: number; + + /** + * Function to call when timeout hits + */ + readonly onTimeout?: (x: Set) => void; +} \ No newline at end of file diff --git a/packages/@aws-cdk/integ-runner/lib/workers/common.ts b/packages/@aws-cdk/integ-runner/lib/workers/common.ts index 9b10c6b195f93..0ceb3bfa5786e 100644 --- a/packages/@aws-cdk/integ-runner/lib/workers/common.ts +++ b/packages/@aws-cdk/integ-runner/lib/workers/common.ts @@ -2,7 +2,7 @@ import { format } from 'util'; import { ResourceImpact } from '@aws-cdk/cloudformation-diff'; import * as chalk from 'chalk'; import * as logger from '../logger'; -import { IntegTestConfig } from '../runner/integration-tests'; +import { IntegTestInfo } from '../runner/integration-tests'; /** * The aggregate results from running assertions on a test case @@ -28,7 +28,7 @@ export interface AssertionResult { /** * Config for an integration test */ -export interface IntegTestWorkerConfig extends IntegTestConfig { +export interface IntegTestWorkerConfig extends IntegTestInfo { /** * A list of any destructive changes * @@ -112,7 +112,7 @@ export interface IntegBatchResponse { /** * List of failed tests */ - readonly failedTests: IntegTestConfig[]; + readonly failedTests: IntegTestInfo[]; /** * List of Integration test metrics. Each entry in the @@ -178,12 +178,22 @@ export enum DiagnosticReason { */ TEST_FAILED = 'TEST_FAILED', + /** + * There was an error running the integration test + */ + TEST_ERROR = 'TEST_ERROR', + /** * The snapshot test failed because the actual * snapshot was different than the expected snapshot */ SNAPSHOT_FAILED = 'SNAPSHOT_FAILED', + /** + * The snapshot test failed because there was an error executing it + */ + SNAPSHOT_ERROR = 'SNAPSHOT_ERROR', + /** * The snapshot test succeeded */ @@ -255,24 +265,39 @@ export function formatAssertionResults(results: AssertionResults): string { export function printResults(diagnostic: Diagnostic): void { switch (diagnostic.reason) { case DiagnosticReason.SNAPSHOT_SUCCESS: - logger.success(' %s No Change! %s', diagnostic.testName, chalk.gray(`${diagnostic.duration}s`)); + logger.success(' UNCHANGED %s %s', diagnostic.testName, chalk.gray(`${diagnostic.duration}s`)); break; case DiagnosticReason.TEST_SUCCESS: - logger.success(' %s Test Succeeded! %s', diagnostic.testName, chalk.gray(`${diagnostic.duration}s`)); + logger.success(' SUCCESS %s %s', diagnostic.testName, chalk.gray(`${diagnostic.duration}s`)); break; case DiagnosticReason.NO_SNAPSHOT: - logger.error(' %s - No Snapshot! %s', diagnostic.testName, chalk.gray(`${diagnostic.duration}s`)); + logger.error(' NEW %s %s', diagnostic.testName, chalk.gray(`${diagnostic.duration}s`)); break; case DiagnosticReason.SNAPSHOT_FAILED: - logger.error(' %s - Snapshot changed! %s\n%s', diagnostic.testName, chalk.gray(`${diagnostic.duration}s`), diagnostic.message); + logger.error(' CHANGED %s %s\n %s', diagnostic.testName, chalk.gray(`${diagnostic.duration}s`), diagnostic.message); + break; + case DiagnosticReason.SNAPSHOT_ERROR: + case DiagnosticReason.TEST_ERROR: + logger.error(' ERROR %s %s\n %s', diagnostic.testName, chalk.gray(`${diagnostic.duration}s`), diagnostic.message); break; case DiagnosticReason.TEST_FAILED: - logger.error(' %s - Failed! %s\n%s', diagnostic.testName, chalk.gray(`${diagnostic.duration}s`), diagnostic.message); + logger.error(' FAILED %s %s\n %s', diagnostic.testName, chalk.gray(`${diagnostic.duration}s`), diagnostic.message); break; case DiagnosticReason.ASSERTION_FAILED: - logger.error(' %s - Assertions Failed! %s\n%s', diagnostic.testName, chalk.gray(`${diagnostic.duration}s`), diagnostic.message); + logger.error(' ASSERT %s %s\n %s', diagnostic.testName, chalk.gray(`${diagnostic.duration}s`), diagnostic.message); + break; } for (const addl of diagnostic.additionalMessages ?? []) { logger.print(` ${addl}`); } } + +export function printLaggards(testNames: Set) { + const parts = [ + ' ', + `Waiting for ${testNames.size} more`, + testNames.size < 10 ? ['(', Array.from(testNames).join(', '), ')'].join('') : '', + ]; + + logger.print(chalk.grey(parts.filter(x => x).join(' '))); +} \ No newline at end of file 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..595dc3a323172 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,6 +1,6 @@ import * as workerpool from 'workerpool'; import { IntegSnapshotRunner, IntegTestRunner } from '../../runner'; -import { IntegTestConfig } from '../../runner/integration-tests'; +import { IntegTest, IntegTestInfo } from '../../runner/integration-tests'; import { DiagnosticReason, IntegTestWorkerConfig, SnapshotVerificationOptions, Diagnostic, formatAssertionResults } from '../common'; import { IntegTestBatchRequest } from '../integ-test-worker'; @@ -13,20 +13,22 @@ import { IntegTestBatchRequest } from '../integ-test-worker'; * If the tests succeed it will then save the snapshot */ export function integTestWorker(request: IntegTestBatchRequest): IntegTestWorkerConfig[] { - const failures: IntegTestConfig[] = []; - for (const test of request.tests) { - const runner = new IntegTestRunner({ - directory: test.directory, - fileName: test.fileName, - profile: request.profile, - env: { - AWS_REGION: request.region, - }, - }, test.destructiveChanges); - + const failures: IntegTestInfo[] = []; + for (const testInfo of request.tests) { + const test = new IntegTest(testInfo); // Hydrate from data const start = Date.now(); - const tests = runner.actualTests(); + try { + const runner = new IntegTestRunner({ + test, + profile: request.profile, + env: { + AWS_REGION: request.region, + }, + }, testInfo.destructiveChanges); + + const tests = runner.actualTests(); + if (!tests || Object.keys(tests).length === 0) { throw new Error(`No tests defined for ${runner.testName}`); } @@ -39,7 +41,7 @@ export function integTestWorker(request: IntegTestBatchRequest): IntegTestWorker updateWorkflow: request.updateWorkflow, }); if (results) { - failures.push(test); + failures.push(testInfo); workerpool.workerEmit({ reason: DiagnosticReason.ASSERTION_FAILED, testName: `${runner.testName}-${testCaseName} (${request.profile}/${request.region})`, @@ -55,7 +57,7 @@ export function integTestWorker(request: IntegTestBatchRequest): IntegTestWorker }); } } catch (e) { - failures.push(test); + failures.push(testInfo); workerpool.workerEmit({ reason: DiagnosticReason.TEST_FAILED, testName: `${runner.testName}-${testCaseName} (${request.profile}/${request.region})`, @@ -65,11 +67,11 @@ export function integTestWorker(request: IntegTestBatchRequest): IntegTestWorker } } } catch (e) { - failures.push(test); + failures.push(testInfo); workerpool.workerEmit({ - reason: DiagnosticReason.TEST_FAILED, - testName: `${test.fileName} (${request.profile}/${request.region})`, - message: `Integration test failed: ${e}`, + reason: DiagnosticReason.TEST_ERROR, + testName: `${testInfo.fileName} (${request.profile}/${request.region})`, + message: `Error during integration test: ${e}`, duration: (Date.now() - start) / 1000, }); } @@ -84,19 +86,30 @@ 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, options: SnapshotVerificationOptions = {}): IntegTestWorkerConfig[] { +export function snapshotTestWorker(testInfo: IntegTestInfo, options: SnapshotVerificationOptions = {}): IntegTestWorkerConfig[] { const failedTests = new Array(); - const runner = new IntegSnapshotRunner({ fileName: test.fileName, directory: test.directory }); const start = Date.now(); + const test = new IntegTest(testInfo); // Hydrate the data record again + + const timer = setTimeout(() => { + workerpool.workerEmit({ + reason: DiagnosticReason.SNAPSHOT_ERROR, + testName: test.testName, + message: 'Test is taking a very long time', + duration: (Date.now() - start) / 1000, + }); + }, 60_000); + try { + const runner = new IntegSnapshotRunner({ test }); if (!runner.hasSnapshot()) { workerpool.workerEmit({ reason: DiagnosticReason.NO_SNAPSHOT, - testName: runner.testName, + testName: test.testName, message: 'No Snapshot', duration: (Date.now() - start) / 1000, }); - failedTests.push(test); + failedTests.push(test.info); } else { const { diagnostics, destructiveChanges } = runner.testSnapshot(options); if (diagnostics.length > 0) { @@ -105,27 +118,28 @@ export function snapshotTestWorker(test: IntegTestConfig, options: SnapshotVerif duration: (Date.now() - start) / 1000, } as Diagnostic)); failedTests.push({ - fileName: test.fileName, - directory: test.directory, + ...test.info, destructiveChanges, }); } else { workerpool.workerEmit({ reason: DiagnosticReason.SNAPSHOT_SUCCESS, - testName: runner.testName, + testName: test.testName, message: 'Success', duration: (Date.now() - start) / 1000, } as Diagnostic); } } } catch (e) { - failedTests.push(test); + failedTests.push(test.info); workerpool.workerEmit({ message: e.message, - testName: runner.testName, - reason: DiagnosticReason.SNAPSHOT_FAILED, + testName: test.testName, + reason: DiagnosticReason.SNAPSHOT_ERROR, duration: (Date.now() - start) / 1000, } as Diagnostic); + } finally { + clearTimeout(timer); } return failedTests; diff --git a/packages/@aws-cdk/integ-runner/lib/workers/integ-snapshot-worker.ts b/packages/@aws-cdk/integ-runner/lib/workers/integ-snapshot-worker.ts index 1ff88e6cba5d6..072b6c28544b4 100644 --- a/packages/@aws-cdk/integ-runner/lib/workers/integ-snapshot-worker.ts +++ b/packages/@aws-cdk/integ-runner/lib/workers/integ-snapshot-worker.ts @@ -1,8 +1,8 @@ import * as workerpool from 'workerpool'; import * as logger from '../logger'; -import { IntegTestConfig } from '../runner/integration-tests'; -import { flatten } from '../utils'; -import { printSummary, printResults, IntegTestWorkerConfig, SnapshotVerificationOptions } from './common'; +import { IntegTest } from '../runner/integration-tests'; +import { flatten, WorkList } from '../utils'; +import { printSummary, printResults, IntegTestWorkerConfig, SnapshotVerificationOptions, printLaggards } from './common'; /** * Run Snapshot tests @@ -11,19 +11,28 @@ import { printSummary, printResults, IntegTestWorkerConfig, SnapshotVerification */ export async function runSnapshotTests( pool: workerpool.WorkerPool, - tests: IntegTestConfig[], + tests: IntegTest[], options: SnapshotVerificationOptions, ): Promise { logger.highlight('\nVerifying integration test snapshots...\n'); + const todo = new WorkList(tests.map(t => t.testName), { + onTimeout: printLaggards, + }); + const failedTests: IntegTestWorkerConfig[][] = await Promise.all( - tests.map((test) => pool.exec('snapshotTestWorker', [test, options], { - on: printResults, + tests.map((test) => pool.exec('snapshotTestWorker', [test.info /* Dehydrate class -> data */, options], { + on: (x) => { + todo.crossOff(x.testName); + printResults(x); + }, })), ); + todo.done(); const testsToRun = flatten(failedTests); logger.highlight('\nSnapshot Results: \n'); printSummary(tests.length, testsToRun.length); return testsToRun; + } diff --git a/packages/@aws-cdk/integ-runner/lib/workers/integ-test-worker.ts b/packages/@aws-cdk/integ-runner/lib/workers/integ-test-worker.ts index 9c6b4fb2465c6..8ce8fb658b5d5 100644 --- a/packages/@aws-cdk/integ-runner/lib/workers/integ-test-worker.ts +++ b/packages/@aws-cdk/integ-runner/lib/workers/integ-test-worker.ts @@ -1,6 +1,6 @@ import * as workerpool from 'workerpool'; import * as logger from '../logger'; -import { IntegTestConfig } from '../runner/integration-tests'; +import { IntegTestInfo } from '../runner/integration-tests'; import { flatten } from '../utils'; import { printResults, printSummary, IntegBatchResponse, IntegTestOptions, IntegRunnerMetrics } from './common'; @@ -127,7 +127,7 @@ export async function runIntegrationTestsInParallel( if (!test) break; const testStart = Date.now(); logger.highlight(`Running test ${test.fileName} in ${worker.profile ? worker.profile + '/' : ''}${worker.region}`); - const response: IntegTestConfig[][] = await options.pool.exec('integTestWorker', [{ + const response: IntegTestInfo[][] = await options.pool.exec('integTestWorker', [{ region: worker.region, profile: worker.profile, tests: [test], diff --git a/packages/@aws-cdk/integ-runner/test/runner/integ-test-runner.test.ts b/packages/@aws-cdk/integ-runner/test/runner/integ-test-runner.test.ts index db4c4a434554d..c2b0987c3ba2b 100644 --- a/packages/@aws-cdk/integ-runner/test/runner/integ-test-runner.test.ts +++ b/packages/@aws-cdk/integ-runner/test/runner/integ-test-runner.test.ts @@ -3,7 +3,7 @@ import { Manifest } from '@aws-cdk/cloud-assembly-schema'; import { AVAILABILITY_ZONE_FALLBACK_CONTEXT_KEY } from '@aws-cdk/cx-api'; import { SynthFastOptions, DestroyOptions, ListOptions, SynthOptions, DeployOptions } from 'cdk-cli-wrapper'; import * as fs from 'fs-extra'; -import { IntegTestRunner } from '../../lib/runner'; +import { IntegTestRunner, IntegTest } from '../../lib/runner'; import { MockCdkProvider } from '../helpers'; let cdkMock: MockCdkProvider; @@ -55,8 +55,10 @@ describe('IntegTest runIntegTests', () => { // WHEN const integTest = new IntegTestRunner({ cdk: cdkMock.cdk, - fileName: 'test/test-data/integ.test-with-snapshot.js', - directory: 'test/test-data', + test: new IntegTest({ + fileName: 'test/test-data/integ.test-with-snapshot.js', + discoveryRoot: 'test/test-data', + }), }); integTest.runIntegTestCase({ testCaseName: 'integ.test-with-snapshot', @@ -107,8 +109,10 @@ describe('IntegTest runIntegTests', () => { // WHEN const integTest = new IntegTestRunner({ cdk: cdkMock.cdk, - fileName: 'test/test-data/integ.integ-test1.js', - directory: 'test/test-data', + test: new IntegTest({ + fileName: 'test/test-data/integ.integ-test1.js', + discoveryRoot: 'test/test-data', + }), }); integTest.runIntegTestCase({ testCaseName: 'integ.integ-test1', @@ -149,8 +153,10 @@ describe('IntegTest runIntegTests', () => { // WHEN const integTest = new IntegTestRunner({ cdk: cdkMock.cdk, - fileName: 'test/test-data/integ.test-with-snapshot-assets-diff.js', - directory: 'test/test-data', + test: new IntegTest({ + fileName: 'test/test-data/integ.test-with-snapshot-assets-diff.js', + discoveryRoot: 'test/test-data', + }), }); integTest.runIntegTestCase({ testCaseName: 'integ.test-with-snapshot-assets-diff', @@ -206,8 +212,10 @@ describe('IntegTest runIntegTests', () => { // WHEN const integTest = new IntegTestRunner({ cdk: cdkMock.cdk, - fileName: 'test/test-data/integ.integ-test1.js', - directory: 'test/test-data', + test: new IntegTest({ + fileName: 'test/test-data/integ.integ-test1.js', + discoveryRoot: 'test/test-data', + }), }); integTest.runIntegTestCase({ testCaseName: 'integ.integ-test1', @@ -224,8 +232,10 @@ describe('IntegTest runIntegTests', () => { // WHEN const integTest = new IntegTestRunner({ cdk: cdkMock.cdk, - fileName: 'test/test-data/integ.integ-test1.js', - directory: 'test/test-data', + test: new IntegTest({ + fileName: 'test/test-data/integ.integ-test1.js', + discoveryRoot: 'test/test-data', + }), }); integTest.runIntegTestCase({ testCaseName: 'integ.integ-test1', @@ -242,8 +252,10 @@ describe('IntegTest runIntegTests', () => { // WHEN const integTest = new IntegTestRunner({ cdk: cdkMock.cdk, - fileName: 'test/test-data/integ.integ-test1.js', - directory: 'test', + test: new IntegTest({ + fileName: 'test/test-data/integ.integ-test1.js', + discoveryRoot: 'test', + }), }); // THEN @@ -261,8 +273,10 @@ describe('IntegTest runIntegTests', () => { // WHEN new IntegTestRunner({ cdk: cdkMock.cdk, - fileName: 'test/test-data/integ.integ-test1.js', - directory: 'test/test-data', + test: new IntegTest({ + fileName: 'test/test-data/integ.integ-test1.js', + discoveryRoot: 'test/test-data', + }), }); // THEN @@ -280,8 +294,10 @@ describe('IntegTest runIntegTests', () => { // WHEN const integTest = new IntegTestRunner({ cdk: cdkMock.cdk, - fileName: 'test/test-data/integ.integ-test2.js', - directory: 'test', + test: new IntegTest({ + fileName: 'test/test-data/integ.integ-test2.js', + discoveryRoot: 'test', + }), }); // THEN @@ -307,9 +323,11 @@ describe('IntegTest runIntegTests', () => { // WHEN const integTest = new IntegTestRunner({ cdk: cdkMock.cdk, - fileName: 'test/test-data/integ.integ-test1.js', + test: new IntegTest({ + fileName: 'test/test-data/integ.integ-test1.js', + discoveryRoot: 'test/test-data', + }), profile: 'test-profile', - directory: 'test/test-data', }); integTest.runIntegTestCase({ testCaseName: 'integ.integ-test1', @@ -348,8 +366,10 @@ describe('IntegTest runIntegTests', () => { test('with hooks', () => { const integTest = new IntegTestRunner({ cdk: cdkMock.cdk, - fileName: 'test/test-data/integ.test-with-snapshot-assets.js', - directory: 'test/test-data', + test: new IntegTest({ + fileName: 'test/test-data/integ.test-with-snapshot-assets.js', + discoveryRoot: 'test/test-data', + }), }); integTest.runIntegTestCase({ testCaseName: 'integ.test-with-snapshot-assets', @@ -393,8 +413,10 @@ describe('IntegTest runIntegTests', () => { // WHEN const integTest = new IntegTestRunner({ cdk: cdkMock.cdk, - fileName: 'test/test-data/integ.test-with-snapshot.js', - directory: 'test/test-data', + test: new IntegTest({ + fileName: 'test/test-data/integ.test-with-snapshot.js', + discoveryRoot: 'test/test-data', + }), }); integTest.runIntegTestCase({ testCaseName: 'integ.test-with-snapshot', @@ -429,8 +451,10 @@ describe('IntegTest runIntegTests', () => { // WHEN const integTest = new IntegTestRunner({ cdk: cdkMock.cdk, - fileName: 'test/test-data/integ.test-with-snapshot.js', - directory: 'test/test-data', + test: new IntegTest({ + fileName: 'test/test-data/integ.test-with-snapshot.js', + discoveryRoot: 'test/test-data', + }), }); integTest.runIntegTestCase({ testCaseName: 'integ.test-with-snapshot', @@ -466,8 +490,10 @@ describe('IntegTest runIntegTests', () => { // WHEN const integTest = new IntegTestRunner({ cdk: cdkMock.cdk, - fileName: 'test/test-data/integ.test-with-snapshot.js', - directory: 'test/test-data', + test: new IntegTest({ + fileName: 'test/test-data/integ.test-with-snapshot.js', + discoveryRoot: 'test/test-data', + }), }); integTest.runIntegTestCase({ testCaseName: 'integ.test-with-snapshot', @@ -484,8 +510,10 @@ describe('IntegTest runIntegTests', () => { test('with assets manifest, assets are removed if stackUpdateWorkflow is disabled', () => { const integTest = new IntegTestRunner({ cdk: cdkMock.cdk, - fileName: 'test/test-data/integ.test-with-snapshot-assets.js', - directory: 'test/test-data', + test: new IntegTest({ + fileName: 'test/test-data/integ.test-with-snapshot-assets.js', + discoveryRoot: 'test/test-data', + }), }); integTest.runIntegTestCase({ testCaseName: 'integ.test-with-snapshot-assets', @@ -503,8 +531,10 @@ describe('IntegTest runIntegTests', () => { test('with assembly manifest, assets are removed if stackUpdateWorkflow is disabled', () => { const integTest = new IntegTestRunner({ cdk: cdkMock.cdk, - fileName: 'test/test-data/integ.test-with-snapshot-assets-diff.js', - directory: 'test/test-data', + test: new IntegTest({ + fileName: 'test/test-data/integ.test-with-snapshot-assets-diff.js', + discoveryRoot: 'test/test-data', + }), }); integTest.runIntegTestCase({ testCaseName: 'integ.test-with-snapshot-assets-diff', diff --git a/packages/@aws-cdk/integ-runner/test/runner/integration-tests.test.ts b/packages/@aws-cdk/integ-runner/test/runner/integration-tests.test.ts index 1185f848a232a..22b796b9fe52e 100644 --- a/packages/@aws-cdk/integ-runner/test/runner/integration-tests.test.ts +++ b/packages/@aws-cdk/integ-runner/test/runner/integration-tests.test.ts @@ -2,8 +2,6 @@ import * as mockfs from 'mock-fs'; import { IntegrationTests } from '../../lib/runner/integration-tests'; describe('IntegrationTests', () => { - const testsFile = '/tmp/foo/bar/does/not/exist/tests.json'; - const testsFileExclude = '/tmp/foo/bar/does/not/exist/tests-exclude.json'; const tests = new IntegrationTests('test'); let stderrMock: jest.SpyInstance; stderrMock = jest.spyOn(process.stderr, 'write').mockImplementation(() => { return true; }); @@ -14,17 +12,6 @@ describe('IntegrationTests', () => { 'integ.integ-test2.js': 'content', 'integ.integ-test3.js': 'content', }, - [testsFileExclude]: JSON.stringify({ - exclude: true, - tests: [ - 'test-data/integ.integ-test1.js', - ], - }), - [testsFile]: JSON.stringify({ - tests: [ - 'test-data/integ.integ-test1.js', - ], - }), }); }); @@ -60,46 +47,4 @@ describe('IntegrationTests', () => { 'test/test-data/integ.integ-test1.js', ); }); - - test('from file', async () => { - const integTests = await tests.fromFile(testsFile); - - const fileNames = integTests.map(test => test.fileName); - expect(integTests.length).toEqual(1); - expect(fileNames).toContain( - 'test/test-data/integ.integ-test1.js', - ); - }); - - test('from file, test not found', async () => { - mockfs({ - 'test/test-data': { - 'integ.integ-test1.js': 'content', - }, - [testsFile]: JSON.stringify({ - tests: [ - 'test-data/integ.integ-test16.js', - ], - }), - }); - const integTests = await tests.fromFile(testsFile); - - expect(integTests.length).toEqual(0); - expect(stderrMock.mock.calls[0][0]).toContain( - 'No such integ test: test-data/integ.integ-test16.js', - ); - expect(stderrMock.mock.calls[1][0]).toContain( - 'Available tests: test-data/integ.integ-test1.js test-data/integ.integ-test2.js test-data/integ.integ-test3.js', - ); - }); - - test('from file exclude', async () => { - const integTests = await tests.fromFile(testsFileExclude); - - const fileNames = integTests.map(test => test.fileName); - expect(integTests.length).toEqual(2); - expect(fileNames).not.toContain( - 'test/test-data/integ.integ-test1.js', - ); - }); }); diff --git a/packages/@aws-cdk/integ-runner/test/runner/snapshot-test-runner.test.ts b/packages/@aws-cdk/integ-runner/test/runner/snapshot-test-runner.test.ts index 28d9e80c79477..1370589d36252 100644 --- a/packages/@aws-cdk/integ-runner/test/runner/snapshot-test-runner.test.ts +++ b/packages/@aws-cdk/integ-runner/test/runner/snapshot-test-runner.test.ts @@ -2,7 +2,7 @@ import * as child_process from 'child_process'; import * as path from 'path'; import { SynthFastOptions, DestroyOptions, ListOptions, SynthOptions, DeployOptions } from 'cdk-cli-wrapper'; import * as fs from 'fs-extra'; -import { IntegSnapshotRunner } from '../../lib/runner'; +import { IntegSnapshotRunner, IntegTest } from '../../lib/runner'; import { DiagnosticReason } from '../../lib/workers/common'; import { MockCdkProvider } from '../helpers'; @@ -46,9 +46,11 @@ describe('IntegTest runSnapshotTests', () => { // WHEN const integTest = new IntegSnapshotRunner({ cdk: cdkMock.cdk, - fileName: 'test/test-data/integ.test-with-snapshot.js', - directory: 'test/test-data', - integOutDir: 'test-with-snapshot.integ.snapshot', + test: new IntegTest({ + fileName: 'test/test-data/integ.test-with-snapshot.js', + discoveryRoot: 'test/test-data', + }), + integOutDir: 'test/test-data/test-with-snapshot.integ.snapshot', }); const results = integTest.testSnapshot(); @@ -69,8 +71,10 @@ describe('IntegTest runSnapshotTests', () => { // WHEN const integTest = new IntegSnapshotRunner({ cdk: cdkMock.cdk, - fileName: 'test/test-data/integ.test-with-snapshot.js', - directory: 'test/test-data', + test: new IntegTest({ + fileName: 'test/test-data/integ.test-with-snapshot.js', + discoveryRoot: 'test/test-data', + }), }); const results = integTest.testSnapshot(); @@ -95,9 +99,11 @@ describe('IntegTest runSnapshotTests', () => { // WHEN const integTest = new IntegSnapshotRunner({ cdk: cdkMock.cdk, - fileName: 'test/test-data/integ.test-with-snapshot.js', - directory: 'test/test-data', - integOutDir: 'test-with-snapshot-diff.integ.snapshot', + test: new IntegTest({ + fileName: 'test/test-data/integ.test-with-snapshot.js', + discoveryRoot: 'test/test-data', + }), + integOutDir: 'test/test-data/test-with-snapshot-diff.integ.snapshot', }); const results = integTest.testSnapshot(); @@ -134,9 +140,11 @@ describe('IntegTest runSnapshotTests', () => { // WHEN const integTest = new IntegSnapshotRunner({ cdk: cdkMock.cdk, - fileName: path.join(__dirname, '../test-data/integ.test-with-snapshot-assets-diff.js'), - directory: 'test/test-data', - integOutDir: 'test-with-snapshot-assets.integ.snapshot', + test: new IntegTest({ + fileName: path.join(__dirname, '../test-data/integ.test-with-snapshot-assets-diff.js'), + discoveryRoot: 'test/test-data', + }), + integOutDir: 'test/test-data/test-with-snapshot-assets.integ.snapshot', }); expect(() => { integTest.testSnapshot(); @@ -158,9 +166,11 @@ describe('IntegTest runSnapshotTests', () => { // WHEN const integTest = new IntegSnapshotRunner({ cdk: cdkMock.cdk, - fileName: path.join(__dirname, '../test-data/integ.test-with-snapshot-assets.js'), - directory: 'test/test-data', - integOutDir: 'test-with-snapshot-assets-diff.integ.snapshot', + test: new IntegTest({ + fileName: path.join(__dirname, '../test-data/integ.test-with-snapshot-assets.js'), + discoveryRoot: 'test/test-data', + }), + integOutDir: 'test/test-data/test-with-snapshot-assets-diff.integ.snapshot', }); const results = integTest.testSnapshot(); diff --git a/packages/@aws-cdk/integ-runner/test/workers/integ-worker.test.ts b/packages/@aws-cdk/integ-runner/test/workers/integ-worker.test.ts index 54864d2421b4a..0c1634faf759e 100644 --- a/packages/@aws-cdk/integ-runner/test/workers/integ-worker.test.ts +++ b/packages/@aws-cdk/integ-runner/test/workers/integ-worker.test.ts @@ -65,7 +65,7 @@ describe('test runner', () => { // WHEN const test = { fileName: 'test/test-data/integ.integ-test1.js', - directory: 'test/test-data', + discoveryRoot: 'test/test-data', }; integTestWorker({ tests: [test], @@ -89,7 +89,7 @@ describe('test runner', () => { // WHEN const test = { fileName: 'test/test-data/integ.integ-test2.js', - directory: 'test/test-data', + discoveryRoot: 'test/test-data', }; spawnSyncMock.mockReset(); jest.spyOn(child_process, 'spawnSync').mockReturnValue({ @@ -108,7 +108,7 @@ describe('test runner', () => { // THEN expect(results[0]).toEqual({ fileName: expect.stringMatching(/integ.integ-test2.js$/), - directory: 'test/test-data', + discoveryRoot: 'test/test-data', }); }); @@ -116,7 +116,7 @@ describe('test runner', () => { // WHEN const test = { fileName: 'test/test-data/integ.test-with-snapshot.js', - directory: 'test/test-data', + discoveryRoot: 'test/test-data', }; const results = integTestWorker({ tests: [test], @@ -154,7 +154,7 @@ describe('test runner', () => { // WHEN const test = { fileName: 'test/test-data/integ.test-with-snapshot.js', - directory: 'test/test-data', + discoveryRoot: 'test/test-data', }; jest.spyOn(child_process, 'spawnSync').mockReturnValue({ status: 1, @@ -171,7 +171,7 @@ describe('test runner', () => { expect(results[0]).toEqual({ fileName: 'test/test-data/integ.test-with-snapshot.js', - directory: 'test/test-data', + discoveryRoot: 'test/test-data', }); }); }); @@ -181,11 +181,11 @@ describe('parallel worker', () => { const tests = [ { fileName: 'integ.test-with-snapshot.js', - directory: 'test/test-data', + discoveryRoot: 'test/test-data', }, { fileName: 'integ.another-test-with-snapshot.js', - directory: 'test/test-data', + discoveryRoot: 'test/test-data', }, ]; await runIntegrationTests({ @@ -211,7 +211,7 @@ describe('parallel worker', () => { test('run tests', async () => { const tests = [{ fileName: 'integ.test-with-snapshot.js', - directory: 'test/test-data', + discoveryRoot: 'test/test-data', }]; const results = await runIntegrationTestsInParallel({ pool, @@ -226,7 +226,7 @@ describe('parallel worker', () => { failedTests: expect.arrayContaining([ { fileName: 'integ.test-with-snapshot.js', - directory: 'test/test-data', + discoveryRoot: 'test/test-data', }, ]), metrics: expect.arrayContaining([ @@ -245,19 +245,19 @@ describe('parallel worker', () => { const tests = [ { fileName: 'integ.test-with-snapshot.js', - directory: 'test/test-data', + discoveryRoot: 'test/test-data', }, { fileName: 'integ.another-test-with-snapshot.js', - directory: 'test/test-data', + discoveryRoot: 'test/test-data', }, { fileName: 'integ.another-test-with-snapshot2.js', - directory: 'test/test-data', + discoveryRoot: 'test/test-data', }, { fileName: 'integ.another-test-with-snapshot3.js', - directory: 'test/test-data', + discoveryRoot: 'test/test-data', }, ]; const results = await runIntegrationTestsInParallel({ @@ -283,19 +283,19 @@ describe('parallel worker', () => { failedTests: expect.arrayContaining([ { fileName: 'integ.test-with-snapshot.js', - directory: 'test/test-data', + discoveryRoot: 'test/test-data', }, { fileName: 'integ.another-test-with-snapshot.js', - directory: 'test/test-data', + discoveryRoot: 'test/test-data', }, { fileName: 'integ.another-test-with-snapshot2.js', - directory: 'test/test-data', + discoveryRoot: 'test/test-data', }, { fileName: 'integ.another-test-with-snapshot3.js', - directory: 'test/test-data', + discoveryRoot: 'test/test-data', }, ]), metrics: expect.arrayContaining([ @@ -339,11 +339,11 @@ describe('parallel worker', () => { const tests = [ { fileName: 'integ.test-with-snapshot.js', - directory: 'test/test-data', + discoveryRoot: 'test/test-data', }, { fileName: 'integ.another-test-with-snapshot.js', - directory: 'test/test-data', + discoveryRoot: 'test/test-data', }, ]; const results = await runIntegrationTestsInParallel({ @@ -362,11 +362,11 @@ describe('parallel worker', () => { failedTests: expect.arrayContaining([ { fileName: 'integ.test-with-snapshot.js', - directory: 'test/test-data', + discoveryRoot: 'test/test-data', }, { fileName: 'integ.another-test-with-snapshot.js', - directory: 'test/test-data', + discoveryRoot: 'test/test-data', }, ]), metrics: expect.arrayContaining([ @@ -392,11 +392,11 @@ describe('parallel worker', () => { const tests = [ { fileName: 'integ.test-with-snapshot.js', - directory: 'test/test-data', + discoveryRoot: 'test/test-data', }, { fileName: 'integ.another-test-with-snapshot.js', - directory: 'test/test-data', + discoveryRoot: 'test/test-data', }, ]; const results = await runIntegrationTestsInParallel({ @@ -415,11 +415,11 @@ describe('parallel worker', () => { failedTests: expect.arrayContaining([ { fileName: 'integ.another-test-with-snapshot.js', - directory: 'test/test-data', + discoveryRoot: 'test/test-data', }, { fileName: 'integ.test-with-snapshot.js', - directory: 'test/test-data', + discoveryRoot: 'test/test-data', }, ]), metrics: expect.arrayContaining([ @@ -439,11 +439,11 @@ describe('parallel worker', () => { const tests = [ { fileName: 'integ.test-with-snapshot.js', - directory: 'test/test-data', + discoveryRoot: 'test/test-data', }, { fileName: 'integ.another-test-with-snapshot.js', - directory: 'test/test-data', + discoveryRoot: 'test/test-data', }, ]; const results = await runIntegrationTestsInParallel({ @@ -462,11 +462,11 @@ describe('parallel worker', () => { failedTests: expect.arrayContaining([ { fileName: 'integ.test-with-snapshot.js', - directory: 'test/test-data', + discoveryRoot: 'test/test-data', }, { fileName: 'integ.another-test-with-snapshot.js', - directory: 'test/test-data', + discoveryRoot: 'test/test-data', }, ]), metrics: expect.arrayContaining([ diff --git a/packages/@aws-cdk/integ-runner/test/workers/mock-extract_worker.ts b/packages/@aws-cdk/integ-runner/test/workers/mock-extract_worker.ts index 18f517e5f4dee..f761b5a2a4429 100644 --- a/packages/@aws-cdk/integ-runner/test/workers/mock-extract_worker.ts +++ b/packages/@aws-cdk/integ-runner/test/workers/mock-extract_worker.ts @@ -1,9 +1,9 @@ import * as workerpool from 'workerpool'; -import { IntegTestConfig } from '../../lib/runner'; +import { IntegTestInfo } from '../../lib/runner'; import { IntegTestBatchRequest } from '../../lib/workers/integ-test-worker'; -function integTestWorker(request: IntegTestBatchRequest): IntegTestConfig[] { +function integTestWorker(request: IntegTestBatchRequest): IntegTestInfo[] { return request.tests; } diff --git a/packages/@aws-cdk/integ-runner/test/workers/snapshot-worker.test.ts b/packages/@aws-cdk/integ-runner/test/workers/snapshot-worker.test.ts index 5b47873fd9987..ac9d4cda13f60 100644 --- a/packages/@aws-cdk/integ-runner/test/workers/snapshot-worker.test.ts +++ b/packages/@aws-cdk/integ-runner/test/workers/snapshot-worker.test.ts @@ -22,7 +22,7 @@ describe('Snapshot tests', () => { // WHEN const test = { fileName: path.join(directory, 'integ.integ-test1.js'), - directory: directory, + discoveryRoot: directory, }; const result = snapshotTestWorker(test); @@ -36,7 +36,7 @@ describe('Snapshot tests', () => { jest.spyOn(child_process, 'spawnSync').mockResolvedValue; const test = { fileName: path.join(directory, 'integ.test-with-snapshot.js'), - directory: directory, + discoveryRoot: directory, }; const result = snapshotTestWorker(test); @@ -49,7 +49,7 @@ describe('Snapshot tests', () => { jest.spyOn(child_process, 'spawnSync').mockRejectedValue; const test = { fileName: path.join(directory, 'integ.test-with-snapshot-assets.js'), - directory, + discoveryRoot: directory, destructiveChanges: [], }; const result = snapshotTestWorker(test);