From d0ace8f2db53d56cdb670979c7c173ee17b6bcd8 Mon Sep 17 00:00:00 2001 From: Cory Hall <43035978+corymhall@users.noreply.github.com> Date: Thu, 14 Apr 2022 14:03:02 -0400 Subject: [PATCH] feat(integ-runner): test update path when running tests (#19915) This PR adds an "update path" workflow to the integration test workflow. When a change is being made to an existing integration test we want to be able to ensure that the change not only works for _new_ stacks, but also with _existing_ stacks. In order to accomplish this, the runner will first deploy the existing snapshot (i.e. `cdk deploy --app /path/to/snapshot`) and then perform a stack update by deploying the integration test. The update path can be disabled by passing the `--disable-update-path` command line option. This PR adds a couple of pieces of functionality in order to enable testing the "update path". 1. The runner will attempt to checkout the snapshot from the origin before running the test. This is to try and make sure that you are actually testing an update of the existing snapshot and not an intermediate snapshot (e.g. if you have been iterating locally). 2. When the runner performs the snapshot diff, it will check for any destructive changes and return that information as a warning to the user. 3. If any destructive changes are found, the runner will record that information as trace metadata in the `manifest.json` file. This will enable us to create automation that checks for this as part of the PR workflow Added logic to include the `allowDestroy` option in items 2 & 3 above. If a resource type is included in the `allowDestroy` list, then it will not be reported. ---- ### All Submissions: * [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md) ### Adding new Unconventional Dependencies: * [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md/#adding-new-unconventional-dependencies) ### New Features * [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/master/INTEGRATION_TESTS.md)? * [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)? *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/README.md | 33 +++ packages/@aws-cdk/integ-runner/lib/cli.ts | 52 +++- .../lib/runner/private/cloud-assembly.ts | 57 ++++- .../integ-runner/lib/runner/private/utils.ts | 28 ++ .../integ-runner/lib/runner/runners.ts | 235 +++++++++++++++-- .../integ-runner/lib/workers/common.ts | 43 +++- .../lib/workers/extract/extract_worker.ts | 22 +- .../lib/workers/integ-snapshot-worker.ts | 31 +-- .../lib/workers/integ-test-worker.ts | 1 + .../@aws-cdk/integ-runner/test/helpers.ts | 24 ++ .../runner/private/cloud-assembly.test.ts | 238 +++++++++++++++++ .../runner/private/integ-manifest.test.ts | 69 +++++ .../integ-runner/test/runner/runners.test.ts | 239 +++++++++--------- .../test-stack.template.json | 23 +- .../integ.json | 12 + .../test-stack.template.json | 23 +- .../test/workers/integ-worker.test.ts | 118 +++++---- .../test/workers/snapshot-worker.test.ts | 29 ++- 18 files changed, 1019 insertions(+), 258 deletions(-) create mode 100644 packages/@aws-cdk/integ-runner/lib/runner/private/utils.ts create mode 100644 packages/@aws-cdk/integ-runner/test/helpers.ts create mode 100644 packages/@aws-cdk/integ-runner/test/runner/private/cloud-assembly.test.ts create mode 100644 packages/@aws-cdk/integ-runner/test/runner/private/integ-manifest.test.ts create mode 100644 packages/@aws-cdk/integ-runner/test/test-data/test-with-snapshot.integ.snapshot/integ.json diff --git a/packages/@aws-cdk/integ-runner/README.md b/packages/@aws-cdk/integ-runner/README.md index 86a344cb59a69..a0c68a97f99d3 100644 --- a/packages/@aws-cdk/integ-runner/README.md +++ b/packages/@aws-cdk/integ-runner/README.md @@ -60,6 +60,8 @@ to be a self contained CDK app. The runner will execute the following for each f If this is set to `true` then the list of tests provided will be excluded - `--from-file` Read the list of tests from this file +- `--disable-update-workflow` (default=`false`) + If this is set to `true` then the [update workflow](#update-workflow) will be disabled Example: @@ -150,6 +152,37 @@ Test Results: Tests: 1 passed, 1 total ``` +#### Update Workflow + +By default, integration tests are run with the "update workflow" enabled. This can be disabled by using the `--disable-update-workflow` command line option. + +If an existing snapshot is being updated, the integration test runner will first deploy the existing snapshot and then perform a stack update +with the new changes. This is to test for cases where an update would cause a breaking change only on a stack update. + +The `integ-runner` will also attempt to warn you if you are making any destructive changes with a message like: + +```bash +!!! This test contains destructive changes !!! + Stack: aws-cdk-lambda-1 - Resource: MyLambdaServiceRole4539ECB6 - Impact: WILL_DESTROY + Stack: aws-cdk-lambda-1 - Resource: AliasAliasPermissionAF30F9E8 - Impact: WILL_REPLACE + Stack: aws-cdk-lambda-1 - Resource: AliasFunctionUrlDC6EC566 - Impact: WILL_REPLACE + Stack: aws-cdk-lambda-1 - Resource: Aliasinvokefunctionurl4CA9917B - Impact: WILL_REPLACE +!!! If these destructive changes are necessary, please indicate this on the PR !!! +``` + +If the destructive changes are expected (and required) then please indicate this on your PR. + +##### New tests + +If you are adding a new test which creates a new snapshot then you should run that specific test with `--disable-update-workflow`. +For example, if you are working on a new test `integ.new-test.js` then you would run: + +```bash +yarn integ --update-on-failed --disable-update-workflow integ.new-test.js +``` + +This is because for a new test we do not need to test the update workflow (there is nothing to update). + ### integ.json schema See [@aws-cdk/cloud-assembly-schema/lib/integ-tests/schema.ts](../cloud-assembly-schema/lib/integ-tests/schema.ts) diff --git a/packages/@aws-cdk/integ-runner/lib/cli.ts b/packages/@aws-cdk/integ-runner/lib/cli.ts index 6a1bd80545e8d..14800d394b477 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 * as path from 'path'; +import * as chalk from 'chalk'; import * as workerpool from 'workerpool'; import * as logger from './logger'; import { IntegrationTests, IntegTestConfig } from './runner/integ-tests'; -import { runSnapshotTests, runIntegrationTests, IntegRunnerMetrics } from './workers'; +import { runSnapshotTests, runIntegrationTests, IntegRunnerMetrics, IntegTestWorkerConfig, DestructiveChange } from './workers'; // https://github.com/yargs/yargs/issues/1929 // https://github.com/evanw/esbuild/issues/1492 @@ -27,6 +28,7 @@ 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('disable-update-workflow', { type: 'boolean', default: 'false', desc: 'If this is "true" then the stack update workflow will be disabled' }) .argv; const pool = workerpool.pool(path.join(__dirname, '../lib/workers/extract/index.js'), { @@ -34,7 +36,8 @@ async function main() { }); // list of integration tests that will be executed - const testsToRun: IntegTestConfig[] = []; + const testsToRun: IntegTestWorkerConfig[] = []; + const destructiveChanges: DestructiveChange[] = []; const testsFromArgs: IntegTestConfig[] = []; const parallelRegions = arrayFromYargs(argv['parallel-regions']); const testRegions: string[] = parallelRegions ?? ['us-east-1', 'us-east-2', 'us-west-2']; @@ -43,7 +46,7 @@ async function main() { const fromFile: string | undefined = argv['from-file']; const exclude: boolean = argv.exclude; - let failedSnapshots: IntegTestConfig[] = []; + let failedSnapshots: IntegTestWorkerConfig[] = []; if (argv['max-workers'] < testRegions.length * (profiles ?? [1]).length) { logger.warning('You are attempting to run %s tests in parallel, but only have %s workers. Not all of your profiles+regions will be utilized', argv.profiles*argv['parallel-regions'], argv['max-workers']); } @@ -65,14 +68,19 @@ async function main() { testsFromArgs.push(...(await new IntegrationTests(argv.directory).fromCliArgs(argv._.map((x: any) => x.toString()), exclude))); } - // If `--force` is not used then first validate the snapshots and gather - // the failed snapshot tests. If `--force` is used then we will skip snapshot - // tests and run integration tests for all tests + // 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); + for (const failure of failedSnapshots) { + destructiveChanges.push(...failure.destructiveChanges ?? []); + } if (!argv.force) { - failedSnapshots = await runSnapshotTests(pool, testsFromArgs); testsToRun.push(...failedSnapshots); } else { - testsToRun.push(...testsFromArgs); + // 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)); } // run integration tests if `--update-on-failed` OR `--force` is used @@ -85,6 +93,7 @@ async function main() { clean: argv.clean, dryRun: argv['dry-run'], verbose: argv.verbose, + updateWorkflow: !argv['disable-update-workflow'], }); if (!success) { throw new Error('Some integration tests failed!'); @@ -101,6 +110,10 @@ async function main() { void pool.terminate(); } + if (destructiveChanges.length > 0) { + printDestructiveChanges(destructiveChanges); + throw new Error('Some changes were destructive!'); + } if (failedSnapshots.length > 0) { let message = ''; if (!runUpdateOnFailed) { @@ -108,6 +121,17 @@ async function main() { } throw new Error(`Some snapshot tests failed!\n${message}`); } + +} + +function printDestructiveChanges(changes: DestructiveChange[]): void { + if (changes.length > 0) { + logger.warning('!!! This test contains %s !!!', chalk.bold('destructive changes')); + changes.forEach(change => { + logger.warning(' Stack: %s - Resource: %s - Impact: %s', change.stackName, change.logicalId, change.impact); + }); + logger.warning('!!! If these destructive changes are necessary, please indicate this on the PR !!!'); + } } function printMetrics(metrics: IntegRunnerMetrics[]): void { @@ -134,6 +158,18 @@ function arrayFromYargs(xs: string[]): string[] | undefined { return xs.filter(x => x !== ''); } +/** + * Merge the tests we received from command line arguments with + * 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[] { + const failedTestNames = new Set(failedSnapshotTests.map(test => test.fileName)); + const final: IntegTestWorkerConfig[] = failedSnapshotTests; + final.push(...testFromArgs.filter(test => !failedTestNames.has(test.fileName))); + return final; +} + export function cli() { main().then().catch(err => { logger.error(err); diff --git a/packages/@aws-cdk/integ-runner/lib/runner/private/cloud-assembly.ts b/packages/@aws-cdk/integ-runner/lib/runner/private/cloud-assembly.ts index 13689e7c3ad36..b4e404619b6f1 100644 --- a/packages/@aws-cdk/integ-runner/lib/runner/private/cloud-assembly.ts +++ b/packages/@aws-cdk/integ-runner/lib/runner/private/cloud-assembly.ts @@ -2,6 +2,19 @@ import * as path from 'path'; import { AssemblyManifest, Manifest, ArtifactType, AwsCloudFormationStackProperties, ArtifactManifest, MetadataEntry } from '@aws-cdk/cloud-assembly-schema'; import * as fs from 'fs-extra'; +/** + * Trace information for stack + * map of resource logicalId to trace message + */ +export type StackTrace = Map; + +/** + * Trace information for a assembly + * + * map of stackId to StackTrace + */ +export type ManifestTrace = Map; + /** * Reads a Cloud Assembly manifest */ @@ -64,6 +77,17 @@ export class AssemblyManifestReader { return stacks; } + /** + * Write trace data to the assembly manifest metadata + */ + public recordTrace(trace: ManifestTrace): void { + const newManifest = { + ...this.manifest, + artifacts: this.renderArtifacts(trace), + }; + Manifest.saveAssemblyManifest(newManifest, this.manifestFileName); + } + /** * Clean the manifest of any unneccesary data. Currently that includes * the metadata trace information since this includes trace information like @@ -77,10 +101,22 @@ export class AssemblyManifestReader { Manifest.saveAssemblyManifest(newManifest, this.manifestFileName); } - private renderArtifactMetadata(metadata: { [id: string]: MetadataEntry[] } | undefined): { [id: string]: MetadataEntry[] } { + private renderArtifactMetadata(artifact: ArtifactManifest, trace?: StackTrace): { [id: string]: MetadataEntry[] } | undefined { const newMetadata: { [id: string]: MetadataEntry[] } = {}; - for (const [metadataId, metadataEntry] of Object.entries(metadata ?? {})) { + if (!artifact.metadata) return artifact.metadata; + for (const [metadataId, metadataEntry] of Object.entries(artifact.metadata ?? {})) { newMetadata[metadataId] = metadataEntry.map((meta: MetadataEntry) => { + if (meta.type === 'aws:cdk:logicalId' && trace && meta.data) { + const traceData = trace.get(meta.data.toString()); + if (traceData) { + trace.delete(meta.data.toString()); + return { + type: meta.type, + data: meta.data, + trace: [traceData], + }; + } + } // return metadata without the trace data return { type: meta.type, @@ -88,15 +124,28 @@ export class AssemblyManifestReader { }; }); } + if (trace && trace.size > 0) { + for (const [id, data] of trace.entries()) { + newMetadata[id] = [{ + type: 'aws:cdk:logicalId', + data: id, + trace: [data], + }]; + } + } return newMetadata; } - private renderArtifacts(): { [id: string]: ArtifactManifest } | undefined { + private renderArtifacts(trace?: ManifestTrace): { [id: string]: ArtifactManifest } | undefined { const newArtifacts: { [id: string]: ArtifactManifest } = {}; for (const [artifactId, artifact] of Object.entries(this.manifest.artifacts ?? {})) { + let stackTrace: StackTrace | undefined = undefined; + if (artifact.type === ArtifactType.AWS_CLOUDFORMATION_STACK && trace) { + stackTrace = trace.get(artifactId); + } newArtifacts[artifactId] = { ...artifact, - metadata: this.renderArtifactMetadata(artifact.metadata), + metadata: this.renderArtifactMetadata(artifact, stackTrace), }; } return newArtifacts; diff --git a/packages/@aws-cdk/integ-runner/lib/runner/private/utils.ts b/packages/@aws-cdk/integ-runner/lib/runner/private/utils.ts new file mode 100644 index 0000000000000..b05ab7f3c5f8a --- /dev/null +++ b/packages/@aws-cdk/integ-runner/lib/runner/private/utils.ts @@ -0,0 +1,28 @@ +// Helper functions for CDK Exec +import { spawnSync } from 'child_process'; + +/** + * Our own execute function which doesn't use shells and strings. + */ +export function exec(commandLine: string[], options: { cwd?: string, verbose?: boolean, env?: any } = { }): any { + const proc = spawnSync(commandLine[0], commandLine.slice(1), { + stdio: ['ignore', 'pipe', options.verbose ? 'inherit' : 'pipe'], // inherit STDERR in verbose mode + env: { + ...process.env, + ...options.env, + }, + cwd: options.cwd, + }); + + if (proc.error) { throw proc.error; } + if (proc.status !== 0) { + if (process.stderr) { // will be 'null' in verbose mode + process.stderr.write(proc.stderr); + } + throw new Error(`Command exited with ${proc.status ? `status ${proc.status}` : `signal ${proc.signal}`}`); + } + + const output = proc.stdout.toString('utf-8').trim(); + + return output; +} diff --git a/packages/@aws-cdk/integ-runner/lib/runner/runners.ts b/packages/@aws-cdk/integ-runner/lib/runner/runners.ts index df22eb5afe0e3..8e545a38dc142 100644 --- a/packages/@aws-cdk/integ-runner/lib/runner/runners.ts +++ b/packages/@aws-cdk/integ-runner/lib/runner/runners.ts @@ -2,14 +2,16 @@ import * as path from 'path'; import { Writable, WritableOptions } from 'stream'; import { StringDecoder, NodeStringDecoder } from 'string_decoder'; import { TestCase, RequireApproval, DefaultCdkOptions } from '@aws-cdk/cloud-assembly-schema'; -import { diffTemplate, formatDifferences } from '@aws-cdk/cloudformation-diff'; +import { diffTemplate, formatDifferences, ResourceDifference, ResourceImpact } from '@aws-cdk/cloudformation-diff'; import { AVAILABILITY_ZONE_FALLBACK_CONTEXT_KEY, FUTURE_FLAGS, TARGET_PARTITIONS } from '@aws-cdk/cx-api'; import { CdkCliWrapper, ICdk } from 'cdk-cli-wrapper'; import * as fs from 'fs-extra'; -import { Diagnostic, DiagnosticReason } from '../workers/common'; +import * as logger from '../logger'; +import { Diagnostic, DiagnosticReason, DestructiveChange } from '../workers/common'; import { canonicalizeTemplate } from './private/canonicalize-assets'; -import { AssemblyManifestReader } from './private/cloud-assembly'; +import { AssemblyManifestReader, ManifestTrace } from './private/cloud-assembly'; import { IntegManifestReader } from './private/integ-manifest'; +import { exec } from './private/utils'; const CDK_OUTDIR_PREFIX = 'cdk-integ.out'; const CDK_INTEG_STACK_PRAGMA = '/// !cdk-integ'; @@ -18,6 +20,7 @@ const SET_CONTEXT_PRAGMA_PREFIX = 'pragma:set-context:'; const VERIFY_ASSET_HASHES = 'pragma:include-assets-hashes'; const ENABLE_LOOKUPS_PRAGMA = 'pragma:enable-lookups'; const DISABLE_UPDATE_WORKFLOW = 'pragma:disable-update-workflow'; +const DESTRUCTIVE_CHANGES = '!!DESTRUCTIVE_CHANGES:'; /** * Options for creating an integration test runner @@ -129,6 +132,8 @@ export abstract class IntegRunner { protected readonly profile?: string; + protected _destructiveChanges?: DestructiveChange[]; + constructor(options: IntegRunnerOptions) { const parsed = path.parse(options.fileName); this.directory = parsed.dir; @@ -211,11 +216,26 @@ export abstract class IntegRunner { if (fs.existsSync(cdkOutPath)) { fs.removeSync(cdkOutPath); } - if (fs.existsSync(this.snapshotDir)) { - this.removeAssetsCacheFromSnapshot(); - const assembly = AssemblyManifestReader.fromPath(this.snapshotDir); - assembly.cleanManifest(); - } + } + + /** + * If there are any destructive changes to a stack then this will record + * those in the manifest.json file + */ + private renderTraceData(): ManifestTrace { + const traceData: ManifestTrace = new Map(); + const destructiveChanges = this._destructiveChanges ?? []; + destructiveChanges.forEach(change => { + const trace = traceData.get(change.stackName); + if (trace) { + trace.set(change.logicalId, `${DESTRUCTIVE_CHANGES} ${change.impact}`); + } else { + traceData.set(change.stackName, new Map([ + [change.logicalId, `${DESTRUCTIVE_CHANGES} ${change.impact}`], + ])); + } + }); + return traceData; } /** @@ -272,10 +292,14 @@ export abstract class IntegRunner { } else { fs.moveSync(path.join(this.directory, this.cdkOutDir), this.snapshotDir, { overwrite: true }); } - if (this.disableUpdateWorkflow) { - this.removeAssetsFromSnapshot(); + if (fs.existsSync(this.snapshotDir)) { + if (this.disableUpdateWorkflow) { + this.removeAssetsFromSnapshot(); + } + const assembly = AssemblyManifestReader.fromPath(this.snapshotDir); + assembly.cleanManifest(); + assembly.recordTrace(this.renderTraceData()); } - this.cleanup(); } /** @@ -423,6 +447,17 @@ export interface RunOptions { * @default false */ readonly dryRun?: boolean; + + /** + * If this is set to false then the stack update workflow will + * not be run + * + * The update workflow exists to check for cases where a change would cause + * a failure to an existing stack, but not for a newly created stack. + * + * @default true + */ + readonly updateWorkflow?: boolean; } /** @@ -430,21 +465,101 @@ export interface RunOptions { * integration tests */ export class IntegTestRunner extends IntegRunner { - constructor(options: IntegRunnerOptions) { + constructor(options: IntegRunnerOptions, destructiveChanges?: DestructiveChange[]) { super(options); + this._destructiveChanges = destructiveChanges; + } + + /** + * When running integration tests with the update path workflow + * it is important that the snapshot that is deployed is the current snapshot + * from the upstream branch. In order to guarantee that, first checkout the latest + * (to the user) snapshot from upstream + * + * It is not straightforward to figure out what branch the current + * working branch was created from. This is a best effort attempt to do so. + * This assumes that there is an 'origin'. `git remote show origin` returns a list of + * all branches and we then search for one that starts with `HEAD branch: ` + */ + private checkoutSnapshot(): void { + const cwd = path.dirname(this.snapshotDir); + // 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 + try { + const origin: string = exec(['git', 'remote', 'show', 'origin'], { + cwd, + }); + const originLines = origin.split('\n'); + for (const line of originLines) { + if (line.trim().startsWith('HEAD branch: ')) { + baseBranch = line.trim().split('HEAD branch: ')[1]; + } + } + } catch (e) { + logger.warning('%s\n%s', + 'Could not determine git origin branch.', + `You need to manually checkout the snapshot directory ${this.snapshotDir}` + + 'from the merge-base (https://git-scm.com/docs/git-merge-base)', + ); + logger.warning('error: %s', e); + } + + // if we found the base branch then get the merge-base (most recent common commit) + // and checkout the snapshot using that commit + if (baseBranch) { + try { + const base = exec(['git', 'merge-base', 'HEAD', baseBranch], { + cwd, + }); + exec(['git', 'checkout', base, '--', this.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}`, + ); + logger.warning('error: %s', e); + } + } } /** * Orchestrates running integration tests. Currently this includes * - * 1. Deploying the integration test stacks - * 2. Saving the snapshot - * 3. Destroying the integration test stacks + * 1. (if update workflow is enabled) Deploying the snapshot test stacks + * 2. Deploying the integration test stacks + * 2. Saving the snapshot (if successful) + * 3. Destroying the integration test stacks (if clean=false) + * + * The update workflow exists to check for cases where a change would cause + * a failure to an existing stack, but not for a newly created stack. */ public runIntegTestCase(options: RunOptions): void { const clean = options.clean ?? true; + const updateWorkflowEnabled = (options.updateWorkflow ?? true) && (options.testCase.stackUpdateWorkflow ?? true); try { if (!options.dryRun) { + // if the update workflow is not disabled, first + // perform a deployment with the exising snapshot + // then perform a deployment (which will be a stack update) + // with the current integration test + // We also only want to run the update workflow if there is an existing + // snapshot (otherwise there is nothing to update) + if (!this.disableUpdateWorkflow && updateWorkflowEnabled && this.hasSnapshot()) { + // make sure the snapshot is the latest from 'origin' + this.checkoutSnapshot(); + this.cdk.deploy({ + ...this.defaultArgs, + stacks: options.testCase.stacks, + requireApproval: RequireApproval.NEVER, + output: this.cdkOutDir, + app: this.relativeSnapshotDir, + lookups: this.enableLookups, + ...options.testCase.cdkCommandOptions?.deploy, + }); + } this.cdk.deploy({ ...this.defaultArgs, profile: this.profile, @@ -453,11 +568,14 @@ export class IntegTestRunner extends IntegRunner { output: this.cdkOutDir, app: this.cdkApp, lookups: this.enableLookups, + ...options.testCase.cdkCommandOptions?.deploy, }); } else { const env: Record = { ...DEFAULT_SYNTH_OPTIONS.env, }; + // if lookups are enabled then we need to synth + // with the "dummy" context if (this.enableLookups) { env.CDK_CONTEXT_JSON = JSON.stringify(this.getContext()); } @@ -480,6 +598,7 @@ export class IntegTestRunner extends IntegRunner { force: true, app: this.cdkApp, output: this.cdkOutDir, + ...options.testCase.cdkCommandOptions?.destroy, }); } } @@ -520,8 +639,10 @@ export class IntegSnapshotRunner extends IntegRunner { /** * Synth the integration tests and compare the templates * to the existing snapshot. + * + * @returns any diagnostics and any destructive changes */ - public testSnapshot(): Diagnostic[] { + public testSnapshot(): { diagnostics: Diagnostic[], destructiveChanges: DestructiveChange[] } { try { // read the existing snapshot const expectedStacks = this.readAssembly(this.snapshotDir); @@ -529,7 +650,7 @@ export class IntegSnapshotRunner extends IntegRunner { const env: Record = { ...DEFAULT_SYNTH_OPTIONS.env, }; - // if lookups are enabled then write the dummy context file + // if lookups are enabled then use "dummy" context if (this.enableLookups) { env.CDK_CONTEXT_JSON = JSON.stringify(this.getContext()); } @@ -551,9 +672,39 @@ export class IntegSnapshotRunner extends IntegRunner { } } - private diffAssembly(existing: Record, actual: Record): Diagnostic[] { + /** + * For a given stack return all resource types that are allowed to be destroyed + * as part of a stack update + * + * @param stackId the stack id + * @returns a list of resource types or undefined if none are found + */ + private getAllowedDestroyTypesForStack(stackId: string): string[] | undefined { + for (const testCase of Object.values(this.tests ?? {})) { + if (testCase.stacks.includes(stackId)) { + return testCase.allowDestroy; + } + } + return undefined; + } + + /** + * Find any differences between the existing and expected snapshots + * + * @param existing - the existing (expected) snapshot + * @param actual - the new (actual) snapshot + * @returns any diagnostics and any destructive changes + */ + private diffAssembly( + existing: Record, + actual: Record, + ): { diagnostics: Diagnostic[], destructiveChanges: DestructiveChange[] } { const verifyHashes = this.pragmas().includes(VERIFY_ASSET_HASHES); const failures: Diagnostic[] = []; + const destructiveChanges: DestructiveChange[] = []; + + // check if there is a CFN template in the current snapshot + // that does not exist in the "actual" snapshot for (const templateId of Object.keys(existing)) { if (!actual.hasOwnProperty(templateId)) { failures.push({ @@ -565,6 +716,8 @@ export class IntegSnapshotRunner extends IntegRunner { } for (const templateId of Object.keys(actual)) { + // check if there is a CFN template in the "actual" snapshot + // that does not exist in the current snapshot if (!existing.hasOwnProperty(templateId)) { failures.push({ testName: this.testName, @@ -574,15 +727,50 @@ export class IntegSnapshotRunner extends IntegRunner { } else { let actualTemplate = actual[templateId]; let expectedTemplate = existing[templateId]; + const allowedDestroyTypes = this.getAllowedDestroyTypesForStack(templateId) ?? []; + // if we are not verifying asset hashes then remove the specific + // asset hashes from the templates so they are not part of the diff + // comparison if (!verifyHashes) { actualTemplate = canonicalizeTemplate(actualTemplate); expectedTemplate = canonicalizeTemplate(expectedTemplate); } - const diff = diffTemplate(expectedTemplate, actualTemplate); - if (!diff.isEmpty) { + const templateDiff = diffTemplate(expectedTemplate, actualTemplate); + if (!templateDiff.isEmpty) { + // go through all the resource differences and check for any + // "destructive" changes + templateDiff.resources.forEachDifference((logicalId: string, change: ResourceDifference) => { + // if the change is a removal it will not show up as a 'changeImpact' + // so need to check for it separately, unless it is a resourceType that + // has been "allowed" to be destroyed + const resourceType = change.oldValue?.Type ?? change.newValue?.Type; + if (resourceType && allowedDestroyTypes.includes(resourceType)) { + return; + } + if (change.isRemoval) { + destructiveChanges.push({ + impact: ResourceImpact.WILL_DESTROY, + logicalId, + stackName: templateId, + }); + } else { + switch (change.changeImpact) { + case ResourceImpact.MAY_REPLACE: + case ResourceImpact.WILL_ORPHAN: + case ResourceImpact.WILL_DESTROY: + case ResourceImpact.WILL_REPLACE: + destructiveChanges.push({ + impact: change.changeImpact, + logicalId, + stackName: templateId, + }); + break; + } + } + }); const writable = new StringWritable({}); - formatDifferences(writable, diff); + formatDifferences(writable, templateDiff); failures.push({ reason: DiagnosticReason.SNAPSHOT_FAILED, message: writable.data, @@ -592,7 +780,10 @@ export class IntegSnapshotRunner extends IntegRunner { } } - return failures; + return { + diagnostics: failures, + destructiveChanges, + }; } private readAssembly(dir: string): Record { diff --git a/packages/@aws-cdk/integ-runner/lib/workers/common.ts b/packages/@aws-cdk/integ-runner/lib/workers/common.ts index 0a550660f9372..298e36cfb8de0 100644 --- a/packages/@aws-cdk/integ-runner/lib/workers/common.ts +++ b/packages/@aws-cdk/integ-runner/lib/workers/common.ts @@ -1,7 +1,41 @@ +import { ResourceImpact } from '@aws-cdk/cloudformation-diff'; import * as chalk from 'chalk'; import * as logger from '../logger'; import { IntegTestConfig } from '../runner/integ-tests'; +/** + * Config for an integration test + */ +export interface IntegTestWorkerConfig extends IntegTestConfig { + /** + * A list of any destructive changes + * + * @default [] + */ + readonly destructiveChanges?: DestructiveChange[]; +} + +/** + * Information on any destructive changes + */ +export interface DestructiveChange { + /** + * The logicalId of the resource with a destructive change + */ + readonly logicalId: string; + + /** + * The name of the stack that contains the destructive change + */ + readonly stackName: string; + + /** + * The impact of the destructive change + */ + readonly impact: ResourceImpact; +} + + /** * Represents integration tests metrics for a given worker */ @@ -57,7 +91,7 @@ export interface IntegTestOptions { * A list of integration tests to run * in this batch */ - readonly tests: IntegTestConfig[]; + readonly tests: IntegTestWorkerConfig[]; /** * Whether or not to destroy the stacks at the @@ -82,6 +116,13 @@ export interface IntegTestOptions { * @default false */ readonly verbose?: boolean; + + /** + * If this is set to true then the stack update workflow will be disabled + * + * @default true + */ + readonly updateWorkflow?: boolean; } /** 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 4f6c233cae142..9b0c0854997a9 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,7 +1,7 @@ import * as workerpool from 'workerpool'; import { IntegTestConfig } from '../../runner/integ-tests'; import { IntegSnapshotRunner, IntegTestRunner } from '../../runner/runners'; -import { DiagnosticReason } from '../common'; +import { DiagnosticReason, IntegTestWorkerConfig } from '../common'; import { IntegTestBatchRequest } from '../integ-test-worker'; /** @@ -12,7 +12,7 @@ import { IntegTestBatchRequest } from '../integ-test-worker'; * * If the tests succeed it will then save the snapshot */ -export function integTestWorker(request: IntegTestBatchRequest): IntegTestConfig[] { +export function integTestWorker(request: IntegTestBatchRequest): IntegTestWorkerConfig[] { const failures: IntegTestConfig[] = []; for (const test of request.tests) { const runner = new IntegTestRunner({ @@ -21,7 +21,7 @@ export function integTestWorker(request: IntegTestBatchRequest): IntegTestConfig env: { AWS_REGION: request.region, }, - }); + }, test.destructiveChanges); const start = Date.now(); try { if (!runner.hasSnapshot()) { @@ -37,6 +37,7 @@ export function integTestWorker(request: IntegTestBatchRequest): IntegTestConfig testCase: testCase, clean: request.clean, dryRun: request.dryRun, + updateWorkflow: request.updateWorkflow, }); workerpool.workerEmit({ reason: DiagnosticReason.TEST_SUCCESS, @@ -74,8 +75,8 @@ export function integTestWorker(request: IntegTestBatchRequest): IntegTestConfig * if there is an existing snapshot, and if there is will * check if there are any changes */ -export function snapshotTestWorker(test: IntegTestConfig): IntegTestConfig[] { - const failedTests = new Array(); +export function snapshotTestWorker(test: IntegTestConfig): IntegTestWorkerConfig[] { + const failedTests = new Array(); const runner = new IntegSnapshotRunner({ fileName: test.fileName }); const start = Date.now(); try { @@ -88,13 +89,16 @@ export function snapshotTestWorker(test: IntegTestConfig): IntegTestConfig[] { }); failedTests.push(test); } else { - const snapshotDiagnostics = runner.testSnapshot(); - if (snapshotDiagnostics.length > 0) { - snapshotDiagnostics.forEach(diagnostic => workerpool.workerEmit({ + const { diagnostics, destructiveChanges } = runner.testSnapshot(); + if (diagnostics.length > 0) { + diagnostics.forEach(diagnostic => workerpool.workerEmit({ ...diagnostic, duration: (Date.now() - start) / 1000, })); - failedTests.push(test); + failedTests.push({ + fileName: test.fileName, + destructiveChanges, + }); } else { workerpool.workerEmit({ reason: DiagnosticReason.SNAPSHOT_SUCCESS, 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 fc1c7a897693e..b1afb261809e6 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,42 +1,17 @@ import * as workerpool from 'workerpool'; import * as logger from '../logger'; import { IntegTestConfig } from '../runner/integ-tests'; -import { printSummary, Diagnostic, printResults, flatten } from './common'; - -/** - * Options for running snapshot tests - */ -export interface SnapshotBatchRequest { - /** - * List of tests to run - */ - readonly tests: IntegTestConfig[]; -} - -/** - * Snapshot test results - */ -export interface SnapshotBatchResponse { - /** - * Test diagnostics - */ - diagnostics: Diagnostic[]; - - /** - * List of failed tests - */ - failedTests: IntegTestConfig[]; -} +import { printSummary, printResults, flatten, IntegTestWorkerConfig } 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 { +export async function runSnapshotTests(pool: workerpool.WorkerPool, tests: IntegTestConfig[]): Promise { logger.highlight('\nVerifying integration test snapshots...\n'); - const failedTests: IntegTestConfig[][] = await Promise.all( + const failedTests: IntegTestWorkerConfig[][] = await Promise.all( tests.map((test) => pool.exec('snapshotTestWorker', [test], { on: printResults, })), 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 1b45d5d480f33..bececbcfbe881 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 @@ -133,6 +133,7 @@ export async function runIntegrationTestsInParallel( clean: options.clean, dryRun: options.dryRun, verbose: options.verbose, + updateWorkflow: options.updateWorkflow, }], { on: printResults, }); diff --git a/packages/@aws-cdk/integ-runner/test/helpers.ts b/packages/@aws-cdk/integ-runner/test/helpers.ts new file mode 100644 index 0000000000000..05090927c4658 --- /dev/null +++ b/packages/@aws-cdk/integ-runner/test/helpers.ts @@ -0,0 +1,24 @@ +import { ICdk, CdkCliWrapper, CdkCliWrapperOptions, SynthFastOptions, DestroyOptions, ListOptions, SynthOptions, DeployOptions } from 'cdk-cli-wrapper'; + +export class MockCdkProvider { + public readonly cdk: ICdk; + constructor(options: CdkCliWrapperOptions) { + this.cdk = new CdkCliWrapper(options); + } + + public mockDeploy(mock?: (options: DeployOptions) => void) { + this.cdk.deploy = mock ?? jest.fn().mockImplementation(); + } + public mockSynth(mock?: (options: SynthOptions) => void) { + this.cdk.synth = mock ?? jest.fn().mockImplementation(); + } + public mockSynthFast(mock?: (options: SynthFastOptions) => void) { + this.cdk.synthFast = mock ?? jest.fn().mockImplementation(); + } + public mockDestroy(mock?: (options: DestroyOptions) => void) { + this.cdk.destroy = mock ?? jest.fn().mockImplementation(); + } + public mockList(mock?: (options: ListOptions) => string) { + this.cdk.list = mock ?? jest.fn().mockImplementation(); + } +} diff --git a/packages/@aws-cdk/integ-runner/test/runner/private/cloud-assembly.test.ts b/packages/@aws-cdk/integ-runner/test/runner/private/cloud-assembly.test.ts new file mode 100644 index 0000000000000..7ee3ffbc60370 --- /dev/null +++ b/packages/@aws-cdk/integ-runner/test/runner/private/cloud-assembly.test.ts @@ -0,0 +1,238 @@ +import * as path from 'path'; +import { Manifest } from '@aws-cdk/cloud-assembly-schema'; +import * as mockfs from 'mock-fs'; +import { AssemblyManifestReader } from '../../../lib/runner/private/cloud-assembly'; + +describe('cloud assembly manifest reader', () => { + const manifestFile = '/tmp/foo/bar/does/not/exist/manifest.json'; + const manifestStack = '/tmp/foo/bar/does/not/exist/test-stack.template.json'; + beforeEach(() => { + mockfs({ + [manifestStack]: JSON.stringify({ + data: 'data', + }), + [manifestFile]: JSON.stringify({ + version: '17.0.0', + artifacts: { + 'Tree': { + type: 'cdk:tree', + properties: { + file: 'tree.json', + }, + }, + 'test-stack': { + type: 'aws:cloudformation:stack', + environment: 'aws://unknown-account/unknown-region', + properties: { + templateFile: 'test-stack.template.json', + validateOnSynth: false, + }, + metadata: { + '/test-stack/MyFunction1/ServiceRole/Resource': [ + { + type: 'aws:cdk:logicalId', + data: 'MyFunction1ServiceRole9852B06B', + trace: [ + 'some trace', + 'some more trace', + ], + }, + ], + '/test-stack/MyFunction1/Resource': [ + { + type: 'aws:cdk:logicalId', + data: 'MyFunction12A744C2E', + trace: [ + 'some trace', + 'some more trace', + ], + }, + ], + }, + displayName: 'test-stack', + }, + }, + }), + }); + }); + + afterEach(() => { + mockfs.restore(); + }); + + test('can read manifest from file', () => { + expect(() => { + AssemblyManifestReader.fromFile(manifestFile); + }).not.toThrow(); + }); + + test('throws if manifest not found', () => { + expect(() => { + AssemblyManifestReader.fromFile('some-other-file'); + }).toThrow(/Cannot read integ manifest 'some-other-file':/); + }); + + test('can read manifest from path', () => { + expect(() => { + AssemblyManifestReader.fromPath(path.dirname(manifestFile)); + }).not.toThrow(); + }); + + test('fromPath sets directory correctly', () => { + const manifest = AssemblyManifestReader.fromPath(path.dirname(manifestFile)); + expect(manifest.directory).toEqual('/tmp/foo/bar/does/not/exist'); + }); + + test('can get stacks from manifest', () => { + const manifest = AssemblyManifestReader.fromFile(manifestFile); + + expect(manifest.stacks).toEqual({ + 'test-stack': { data: 'data' }, + }); + }); + + test('can clean stack trace', () => { + // WHEN + const manifest = AssemblyManifestReader.fromFile(manifestFile); + manifest.cleanManifest(); + + // THEN + const newManifest = Manifest.loadAssetManifest(manifestFile); + expect(newManifest).toEqual({ + version: '17.0.0', + artifacts: { + 'Tree': { + type: 'cdk:tree', + properties: { + file: 'tree.json', + }, + }, + 'test-stack': { + type: 'aws:cloudformation:stack', + environment: 'aws://unknown-account/unknown-region', + properties: { + templateFile: 'test-stack.template.json', + validateOnSynth: false, + }, + metadata: { + '/test-stack/MyFunction1/ServiceRole/Resource': [ + { + type: 'aws:cdk:logicalId', + data: 'MyFunction1ServiceRole9852B06B', + }, + ], + '/test-stack/MyFunction1/Resource': [ + { + type: 'aws:cdk:logicalId', + data: 'MyFunction12A744C2E', + }, + ], + }, + displayName: 'test-stack', + }, + }, + }); + }); + + test('can add stack trace', () => { + // WHEN + const manifest = AssemblyManifestReader.fromFile(manifestFile); + manifest.recordTrace(new Map([ + ['test-stack', new Map([ + ['MyFunction12A744C2E', 'some trace'], + ])], + ])); + + // THEN + const newManifest = Manifest.loadAssetManifest(manifestFile); + expect(newManifest).toEqual({ + version: '17.0.0', + artifacts: { + 'Tree': { + type: 'cdk:tree', + properties: { + file: 'tree.json', + }, + }, + 'test-stack': { + type: 'aws:cloudformation:stack', + environment: 'aws://unknown-account/unknown-region', + properties: { + templateFile: 'test-stack.template.json', + validateOnSynth: false, + }, + metadata: { + '/test-stack/MyFunction1/ServiceRole/Resource': [ + { + type: 'aws:cdk:logicalId', + data: 'MyFunction1ServiceRole9852B06B', + }, + ], + '/test-stack/MyFunction1/Resource': [ + { + type: 'aws:cdk:logicalId', + data: 'MyFunction12A744C2E', + trace: ['some trace'], + }, + ], + }, + displayName: 'test-stack', + }, + }, + }); + }); + + test('can add stack trace for old resource', () => { + // WHEN + const manifest = AssemblyManifestReader.fromFile(manifestFile); + manifest.recordTrace(new Map([ + ['test-stack', new Map([ + ['MyFunction', 'some trace'], + ])], + ])); + + // THEN + const newManifest = Manifest.loadAssetManifest(manifestFile); + expect(newManifest).toEqual({ + version: '17.0.0', + artifacts: { + 'Tree': { + type: 'cdk:tree', + properties: { + file: 'tree.json', + }, + }, + 'test-stack': { + type: 'aws:cloudformation:stack', + environment: 'aws://unknown-account/unknown-region', + properties: { + templateFile: 'test-stack.template.json', + validateOnSynth: false, + }, + metadata: { + '/test-stack/MyFunction1/ServiceRole/Resource': [ + { + type: 'aws:cdk:logicalId', + data: 'MyFunction1ServiceRole9852B06B', + }, + ], + '/test-stack/MyFunction1/Resource': [ + { + type: 'aws:cdk:logicalId', + data: 'MyFunction12A744C2E', + }, + ], + 'MyFunction': [ + { + type: 'aws:cdk:logicalId', + data: 'MyFunction', + trace: ['some trace'], + }, + ], + }, + displayName: 'test-stack', + }, + }, + }); + }); +}); diff --git a/packages/@aws-cdk/integ-runner/test/runner/private/integ-manifest.test.ts b/packages/@aws-cdk/integ-runner/test/runner/private/integ-manifest.test.ts new file mode 100644 index 0000000000000..8dffa0bb608e1 --- /dev/null +++ b/packages/@aws-cdk/integ-runner/test/runner/private/integ-manifest.test.ts @@ -0,0 +1,69 @@ +import * as path from 'path'; +import * as mockfs from 'mock-fs'; +import { IntegManifestReader } from '../../../lib/runner/private/integ-manifest'; + +describe('Integ manifest reader', () => { + const manifestFile = '/tmp/foo/bar/does/not/exist/integ.json'; + beforeEach(() => { + mockfs({ + [manifestFile]: JSON.stringify({ + version: 'v1.0.0', + testCases: { + test1: { + stacks: ['MyStack'], + diffAssets: false, + }, + test2: { + stacks: ['MyOtherStack'], + diffAssets: true, + }, + }, + }), + }); + }); + + afterEach(() => { + mockfs.restore(); + }); + + test('can read manifest from file', () => { + expect(() => { + IntegManifestReader.fromFile(manifestFile); + }).not.toThrow(); + }); + + test('throws if manifest not found', () => { + expect(() => { + IntegManifestReader.fromFile('some-other-file'); + }).toThrow(/Cannot read integ manifest 'some-other-file':/); + }); + + test('can read manifest from path', () => { + expect(() => { + IntegManifestReader.fromPath(path.dirname(manifestFile)); + }).not.toThrow(); + }); + + test('fromPath sets directory correctly', () => { + const manifest = IntegManifestReader.fromPath(path.dirname(manifestFile)); + expect(manifest.directory).toEqual('/tmp/foo/bar/does/not/exist'); + }); + + test('can get stacks from manifest', () => { + const manifest = IntegManifestReader.fromFile(manifestFile); + + expect(manifest.tests).toEqual({ + testCases: { + test1: { + stacks: ['MyStack'], + diffAssets: false, + }, + test2: { + stacks: ['MyOtherStack'], + diffAssets: true, + }, + }, + enableLookups: false, + }); + }); +}); diff --git a/packages/@aws-cdk/integ-runner/test/runner/runners.test.ts b/packages/@aws-cdk/integ-runner/test/runner/runners.test.ts index 94092fcd38516..a957894489515 100644 --- a/packages/@aws-cdk/integ-runner/test/runner/runners.test.ts +++ b/packages/@aws-cdk/integ-runner/test/runner/runners.test.ts @@ -1,34 +1,58 @@ +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 { IntegTestRunner, IntegSnapshotRunner } from '../../lib/runner/runners'; import { DiagnosticReason } from '../../lib/workers/common'; +import { MockCdkProvider } from '../helpers'; -describe('IntegTest runSnapshotTests', () => { - let synthMock: jest.SpyInstance; - beforeEach(() => { - jest.spyOn(process.stderr, 'write').mockImplementation(() => { return true; }); - jest.spyOn(process.stdout, 'write').mockImplementation(() => { return true; }); - jest.spyOn(fs, 'moveSync').mockImplementation(() => { return true; }); - jest.spyOn(fs, 'removeSync').mockImplementation(() => { return true; }); - jest.spyOn(fs, 'writeFileSync').mockImplementation(() => { return true; }); - }); - afterEach(() => { - jest.clearAllMocks(); - jest.resetAllMocks(); - jest.restoreAllMocks(); +let cdkMock: MockCdkProvider; +let synthMock: (options: SynthOptions) => void; +let synthFastMock: (options: SynthFastOptions) => void; +let deployMock: (options: DeployOptions) => void; +let listMock: (options: ListOptions) => string; +let destroyMock: (options: DestroyOptions) => void; +beforeEach(() => { + cdkMock = new MockCdkProvider({ directory: 'test/test-data' }); + listMock = jest.fn().mockImplementation(() => { + return 'stackabc'; }); + synthMock = jest.fn().mockImplementation(); + deployMock = jest.fn().mockImplementation(); + destroyMock = jest.fn().mockImplementation(); + synthFastMock = jest.fn().mockImplementation(); + cdkMock.mockSynth(synthMock); + cdkMock.mockList(listMock); + cdkMock.mockDeploy(deployMock); + cdkMock.mockSynthFast(synthFastMock); + cdkMock.mockDestroy(destroyMock); + jest.spyOn(child_process, 'spawnSync').mockImplementation(); + jest.spyOn(process.stderr, 'write').mockImplementation(() => { return true; }); + jest.spyOn(process.stdout, 'write').mockImplementation(() => { return true; }); + jest.spyOn(fs, 'moveSync').mockImplementation(() => { return true; }); + jest.spyOn(fs, 'removeSync').mockImplementation(() => { return true; }); + jest.spyOn(fs, 'writeFileSync').mockImplementation(() => { return true; }); +}); + +afterEach(() => { + jest.clearAllMocks(); + jest.resetAllMocks(); + jest.restoreAllMocks(); +}); + +describe('IntegTest runSnapshotTests', () => { test('with defaults no diff', () => { // WHEN const integTest = new IntegSnapshotRunner({ + cdk: cdkMock.cdk, fileName: 'test/test-data/integ.test-with-snapshot.js', integOutDir: 'test-with-snapshot.integ.snapshot', }); - synthMock = jest.spyOn(integTest.cdk, 'synthFast').mockImplementation(); integTest.testSnapshot(); // THEN - expect(synthMock).toHaveBeenCalledTimes(1); - expect(synthMock.mock.calls[0][0]).toEqual({ + expect(synthFastMock).toHaveBeenCalledTimes(1); + expect(synthFastMock).toHaveBeenCalledWith({ env: expect.objectContaining({ CDK_INTEG_ACCOUNT: '12345678', CDK_INTEG_REGION: 'test-region', @@ -41,15 +65,15 @@ describe('IntegTest runSnapshotTests', () => { test('with defaults and diff', () => { // WHEN const integTest = new IntegSnapshotRunner({ + cdk: cdkMock.cdk, fileName: path.join(__dirname, '../test-data/integ.test-with-snapshot.js'), integOutDir: 'test-with-snapshot-diff.integ.snapshot', }); - synthMock = jest.spyOn(integTest.cdk, 'synthFast').mockImplementation(); - const diagnostics = integTest.testSnapshot(); + const results = integTest.testSnapshot(); // THEN - expect(synthMock).toHaveBeenCalledTimes(1); - expect(synthMock.mock.calls[0][0]).toEqual({ + expect(synthFastMock).toHaveBeenCalledTimes(1); + expect(synthFastMock).toHaveBeenCalledWith({ execCmd: ['node', 'integ.test-with-snapshot.js'], env: expect.objectContaining({ CDK_INTEG_ACCOUNT: '12345678', @@ -57,27 +81,37 @@ describe('IntegTest runSnapshotTests', () => { }), output: 'test-with-snapshot-diff.integ.snapshot', }); - expect(diagnostics).toEqual(expect.arrayContaining([expect.objectContaining({ + expect(results.diagnostics).toEqual(expect.arrayContaining([expect.objectContaining({ reason: DiagnosticReason.SNAPSHOT_FAILED, testName: integTest.testName, message: expect.stringContaining('foobar'), })])); + expect(results.destructiveChanges).not.toEqual([{ + impact: 'WILL_DESTROY', + logicalId: 'MyFunction1ServiceRole9852B06B', + stackName: 'test-stack', + }]); + expect(results.destructiveChanges).toEqual([{ + impact: 'WILL_DESTROY', + logicalId: 'MyLambdaFuncServiceRoleDefaultPolicyBEB0E748', + stackName: 'test-stack', + }]); }); test('dont diff asset hashes', () => { // WHEN const integTest = new IntegSnapshotRunner({ + cdk: cdkMock.cdk, fileName: path.join(__dirname, '../test-data/integ.test-with-snapshot-assets-diff.js'), integOutDir: 'test-with-snapshot-assets.integ.snapshot', }); - synthMock = jest.spyOn(integTest.cdk, 'synthFast').mockImplementation(); expect(() => { integTest.testSnapshot(); }).not.toThrow(); // THEN - expect(synthMock).toHaveBeenCalledTimes(1); - expect(synthMock.mock.calls[0][0]).toEqual({ + expect(synthFastMock).toHaveBeenCalledTimes(1); + expect(synthFastMock).toHaveBeenCalledWith({ execCmd: ['node', 'integ.test-with-snapshot-assets-diff.js'], env: expect.objectContaining({ CDK_INTEG_ACCOUNT: '12345678', @@ -90,15 +124,15 @@ describe('IntegTest runSnapshotTests', () => { test('diff asset hashes', () => { // WHEN const integTest = new IntegSnapshotRunner({ + cdk: cdkMock.cdk, fileName: path.join(__dirname, '../test-data/integ.test-with-snapshot-assets.js'), integOutDir: 'test-with-snapshot-assets-diff.integ.snapshot', }); - synthMock = jest.spyOn(integTest.cdk, 'synthFast').mockImplementation(); - const diagnostics = integTest.testSnapshot(); + const results = integTest.testSnapshot(); // THEN - expect(synthMock).toHaveBeenCalledTimes(1); - expect(synthMock.mock.calls[0][0]).toEqual({ + expect(synthFastMock).toHaveBeenCalledTimes(1); + expect(synthFastMock).toHaveBeenCalledWith({ execCmd: ['node', 'integ.test-with-snapshot-assets.js'], env: expect.objectContaining({ CDK_INTEG_ACCOUNT: '12345678', @@ -106,7 +140,7 @@ describe('IntegTest runSnapshotTests', () => { }), output: 'test-with-snapshot-assets-diff.integ.snapshot', }); - expect(diagnostics).toEqual(expect.arrayContaining([expect.objectContaining({ + expect(results.diagnostics).toEqual(expect.arrayContaining([expect.objectContaining({ reason: DiagnosticReason.SNAPSHOT_FAILED, testName: integTest.testName, message: expect.stringContaining('Parameters'), @@ -115,30 +149,9 @@ describe('IntegTest runSnapshotTests', () => { }); describe('IntegTest runIntegTests', () => { - let integTest: IntegTestRunner; - let deployMock: jest.SpyInstance; - let destroyMock: jest.SpyInstance; - let synthMock: jest.SpyInstance; - let listMock: jest.SpyInstance; - // let stderrMock: jest.SpyInstance; - beforeEach(() => { - integTest = new IntegTestRunner({ fileName: 'test/test-data/integ.integ-test1.js' }); - deployMock = jest.spyOn(integTest.cdk, 'deploy').mockImplementation(); - destroyMock = jest.spyOn(integTest.cdk, 'destroy').mockImplementation(); - synthMock = jest.spyOn(integTest.cdk, 'synthFast').mockImplementation(); - listMock = jest.spyOn(integTest.cdk, 'list').mockImplementation(); - jest.spyOn(process.stderr, 'write').mockImplementation(() => { return true; }); - jest.spyOn(fs, 'moveSync').mockImplementation(() => { return true; }); - jest.spyOn(fs, 'removeSync').mockImplementation(() => { return true; }); - jest.spyOn(fs, 'writeFileSync').mockImplementation(() => { return true; }); - }); - afterEach(() => { - jest.clearAllMocks(); - jest.resetAllMocks(); - jest.restoreAllMocks(); - }); test('with defaults', () => { // WHEN + const integTest = new IntegTestRunner({ cdk: cdkMock.cdk, fileName: 'test/test-data/integ.test-with-snapshot.js' }); integTest.runIntegTestCase({ testCase: { stacks: ['stack1'], @@ -146,32 +159,43 @@ describe('IntegTest runIntegTests', () => { }); // THEN - expect(deployMock).toHaveBeenCalledTimes(1); + expect(deployMock).toHaveBeenCalledTimes(2); expect(destroyMock).toHaveBeenCalledTimes(1); - expect(synthMock).toHaveBeenCalledTimes(0); - expect(deployMock.mock.calls[0][0]).toEqual({ - app: 'node integ.integ-test1.js', + expect(synthFastMock).toHaveBeenCalledTimes(0); + expect(deployMock).toHaveBeenCalledWith({ + app: 'test-with-snapshot.integ.snapshot', requireApproval: 'never', pathMetadata: false, assetMetadata: false, versionReporting: false, lookups: false, stacks: ['stack1'], - output: 'cdk-integ.out.integ-test1', + output: 'cdk-integ.out.test-with-snapshot', }); - expect(destroyMock.mock.calls[0][0]).toEqual({ - app: 'node integ.integ-test1.js', + expect(deployMock).toHaveBeenCalledWith({ + app: 'node integ.test-with-snapshot.js', + requireApproval: 'never', + pathMetadata: false, + assetMetadata: false, + versionReporting: false, + lookups: false, + stacks: ['stack1'], + output: 'cdk-integ.out.test-with-snapshot', + }); + expect(destroyMock).toHaveBeenCalledWith({ + app: 'node integ.test-with-snapshot.js', pathMetadata: false, assetMetadata: false, versionReporting: false, force: true, stacks: ['stack1'], - output: 'cdk-integ.out.integ-test1', + output: 'cdk-integ.out.test-with-snapshot', }); }); test('with profile', () => { // WHEN + const integTest = new IntegTestRunner({ cdk: cdkMock.cdk, fileName: 'test/test-data/integ.integ-test1.js' }); integTest.runIntegTestCase({ testCase: { stacks: ['stack1'], @@ -181,8 +205,8 @@ describe('IntegTest runIntegTests', () => { // THEN expect(deployMock).toHaveBeenCalledTimes(1); expect(destroyMock).toHaveBeenCalledTimes(1); - expect(synthMock).toHaveBeenCalledTimes(0); - expect(deployMock.mock.calls[0][0]).toEqual({ + expect(synthFastMock).toHaveBeenCalledTimes(0); + expect(deployMock).toHaveBeenCalledWith({ app: 'node integ.integ-test1.js', requireApproval: 'never', pathMetadata: false, @@ -192,7 +216,7 @@ describe('IntegTest runIntegTests', () => { stacks: ['stack1'], output: 'cdk-integ.out.integ-test1', }); - expect(destroyMock.mock.calls[0][0]).toEqual({ + expect(destroyMock).toHaveBeenCalledWith({ app: 'node integ.integ-test1.js', pathMetadata: false, assetMetadata: false, @@ -205,11 +229,7 @@ describe('IntegTest runIntegTests', () => { test('with lookups', () => { // WHEN - integTest = new IntegTestRunner({ fileName: path.join(__dirname, '../test-data/integ.test-with-snapshot-assets-diff.js') }); - deployMock = jest.spyOn(integTest.cdk, 'deploy').mockImplementation(); - destroyMock = jest.spyOn(integTest.cdk, 'destroy').mockImplementation(); - synthMock = jest.spyOn(integTest.cdk, 'synthFast').mockImplementation(); - listMock = jest.spyOn(integTest.cdk, 'list').mockImplementation(); + const integTest = new IntegTestRunner({ cdk: cdkMock.cdk, fileName: 'test/test-data/integ.test-with-snapshot-assets-diff.js' }); integTest.runIntegTestCase({ testCase: { stacks: ['test-stack'], @@ -217,10 +237,20 @@ describe('IntegTest runIntegTests', () => { }); // THEN - expect(deployMock).toHaveBeenCalledTimes(1); + expect(deployMock).toHaveBeenCalledTimes(2); expect(destroyMock).toHaveBeenCalledTimes(1); - expect(synthMock).toHaveBeenCalledTimes(1); - expect(deployMock.mock.calls[0][0]).toEqual({ + expect(synthFastMock).toHaveBeenCalledTimes(1); + expect(deployMock).toHaveBeenCalledWith({ + app: 'test-with-snapshot-assets-diff.integ.snapshot', + requireApproval: 'never', + pathMetadata: false, + assetMetadata: false, + versionReporting: false, + lookups: true, + stacks: ['test-stack'], + output: 'cdk-integ.out.test-with-snapshot-assets-diff', + }); + expect(deployMock).toHaveBeenCalledWith({ app: 'node integ.test-with-snapshot-assets-diff.js', requireApproval: 'never', pathMetadata: false, @@ -230,7 +260,7 @@ describe('IntegTest runIntegTests', () => { stacks: ['test-stack'], output: 'cdk-integ.out.test-with-snapshot-assets-diff', }); - expect(synthMock.mock.calls[0][0]).toEqual({ + expect(synthFastMock).toHaveBeenCalledWith({ execCmd: ['node', 'integ.test-with-snapshot-assets-diff.js'], env: expect.objectContaining({ CDK_INTEG_ACCOUNT: '12345678', @@ -239,7 +269,7 @@ describe('IntegTest runIntegTests', () => { }), output: 'test-with-snapshot-assets-diff.integ.snapshot', }); - expect(destroyMock.mock.calls[0][0]).toEqual({ + expect(destroyMock).toHaveBeenCalledWith({ app: 'node integ.test-with-snapshot-assets-diff.js', pathMetadata: false, assetMetadata: false, @@ -252,6 +282,7 @@ describe('IntegTest runIntegTests', () => { test('no clean', () => { // WHEN + const integTest = new IntegTestRunner({ cdk: cdkMock.cdk, fileName: 'test/test-data/integ.integ-test1.js' }); integTest.runIntegTestCase({ clean: false, testCase: { @@ -262,11 +293,12 @@ describe('IntegTest runIntegTests', () => { // THEN expect(deployMock).toHaveBeenCalledTimes(1); expect(destroyMock).toHaveBeenCalledTimes(0); - expect(synthMock).toHaveBeenCalledTimes(0); + expect(synthFastMock).toHaveBeenCalledTimes(0); }); test('dryrun', () => { // WHEN + const integTest = new IntegTestRunner({ cdk: cdkMock.cdk, fileName: 'test/test-data/integ.integ-test1.js' }); integTest.runIntegTestCase({ dryRun: true, testCase: { @@ -277,11 +309,12 @@ describe('IntegTest runIntegTests', () => { // THEN expect(deployMock).toHaveBeenCalledTimes(0); expect(destroyMock).toHaveBeenCalledTimes(0); - expect(synthMock).toHaveBeenCalledTimes(1); + expect(synthFastMock).toHaveBeenCalledTimes(1); }); test('determine test stack via pragma', () => { // WHEN + const integTest = new IntegTestRunner({ cdk: cdkMock.cdk, fileName: 'test/test-data/integ.integ-test1.js' }); integTest.generateSnapshot(); // THEN @@ -295,11 +328,12 @@ describe('IntegTest runIntegTests', () => { test('generate snapshot', () => { // WHEN + const integTest = new IntegTestRunner({ cdk: cdkMock.cdk, fileName: 'test/test-data/integ.integ-test1.js' }); integTest.generateSnapshot(); // THEN - expect(synthMock).toHaveBeenCalledTimes(1); - expect(synthMock.mock.calls[0][0]).toEqual({ + expect(synthFastMock).toHaveBeenCalledTimes(1); + expect(synthFastMock).toHaveBeenCalledWith({ execCmd: ['node', 'integ.integ-test1.js'], output: 'cdk-integ.out.integ-test1', env: expect.objectContaining({ @@ -311,28 +345,9 @@ describe('IntegTest runIntegTests', () => { }); describe('IntegTest no pragma', () => { - let integTest: IntegTestRunner; - let synthMock: jest.SpyInstance; - beforeEach(() => { - integTest = new IntegTestRunner({ fileName: 'test/test-data/integ.integ-test2.js' }); - jest.spyOn(integTest.cdk, 'deploy').mockImplementation(); - jest.spyOn(integTest.cdk, 'destroy').mockImplementation(); - synthMock = jest.spyOn(integTest.cdk, 'synthFast').mockImplementation(); - jest.spyOn(integTest.cdk, 'list').mockImplementation(() => { - return 'stackabc'; - }); - jest.spyOn(process.stderr, 'write').mockImplementation(() => { return true; }); - jest.spyOn(fs, 'moveSync').mockImplementation(() => { return true; }); - jest.spyOn(fs, 'removeSync').mockImplementation(() => { return true; }); - jest.spyOn(fs, 'writeFileSync').mockImplementation(() => { return true; }); - }); - afterEach(() => { - jest.clearAllMocks(); - jest.resetAllMocks(); - jest.restoreAllMocks(); - }); test('get stacks from list', async () => { // WHEN + const integTest = new IntegTestRunner({ cdk: cdkMock.cdk, fileName: 'test/test-data/integ.integ-test2.js' }); integTest.generateSnapshot(); // THEN @@ -341,8 +356,8 @@ describe('IntegTest no pragma', () => { stacks: ['stackabc'], }, })); - expect(synthMock).toHaveBeenCalledTimes(1); - expect(synthMock.mock.calls[0][0]).toEqual({ + expect(synthFastMock).toHaveBeenCalledTimes(1); + expect(synthFastMock).toHaveBeenCalledWith({ execCmd: ['node', 'integ.integ-test2.js'], env: expect.objectContaining({ CDK_INTEG_ACCOUNT: '12345678', @@ -354,29 +369,9 @@ describe('IntegTest no pragma', () => { }); describe('IntegTest runIntegTests with profile', () => { - let integTest: IntegTestRunner; - let deployMock: jest.SpyInstance; - let destroyMock: jest.SpyInstance; - let synthMock: jest.SpyInstance; - // let stderrMock: jest.SpyInstance; - beforeEach(() => { - integTest = new IntegTestRunner({ fileName: 'test/test-data/integ.integ-test1.js', profile: 'test-profile' }); - deployMock = jest.spyOn(integTest.cdk, 'deploy').mockImplementation(); - destroyMock = jest.spyOn(integTest.cdk, 'destroy').mockImplementation(); - synthMock = jest.spyOn(integTest.cdk, 'synthFast').mockImplementation(); - jest.spyOn(integTest.cdk, 'list').mockImplementation(); - jest.spyOn(process.stderr, 'write').mockImplementation(() => { return true; }); - jest.spyOn(fs, 'moveSync').mockImplementation(() => { return true; }); - jest.spyOn(fs, 'removeSync').mockImplementation(() => { return true; }); - jest.spyOn(fs, 'writeFileSync').mockImplementation(() => { return true; }); - }); - afterEach(() => { - jest.clearAllMocks(); - jest.resetAllMocks(); - jest.restoreAllMocks(); - }); test('with defaults', () => { // WHEN + const integTest = new IntegTestRunner({ cdk: cdkMock.cdk, fileName: 'test/test-data/integ.integ-test1.js', profile: 'test-profile' }); integTest.runIntegTestCase({ testCase: { stacks: ['stack1'], @@ -386,8 +381,8 @@ describe('IntegTest runIntegTests with profile', () => { // THEN expect(deployMock).toHaveBeenCalledTimes(1); expect(destroyMock).toHaveBeenCalledTimes(1); - expect(synthMock).toHaveBeenCalledTimes(0); - expect(deployMock.mock.calls[0][0]).toEqual({ + expect(synthFastMock).toHaveBeenCalledTimes(0); + expect(deployMock).toHaveBeenCalledWith({ app: 'node integ.integ-test1.js', requireApproval: 'never', pathMetadata: false, @@ -398,7 +393,7 @@ describe('IntegTest runIntegTests with profile', () => { stacks: ['stack1'], output: 'cdk-integ.out.integ-test1', }); - expect(destroyMock.mock.calls[0][0]).toEqual({ + expect(destroyMock).toHaveBeenCalledWith({ app: 'node integ.integ-test1.js', pathMetadata: false, assetMetadata: false, diff --git a/packages/@aws-cdk/integ-runner/test/test-data/test-with-snapshot-diff.integ.snapshot/test-stack.template.json b/packages/@aws-cdk/integ-runner/test/test-data/test-with-snapshot-diff.integ.snapshot/test-stack.template.json index 3d62830b46139..0fe046e1b3176 100644 --- a/packages/@aws-cdk/integ-runner/test/test-data/test-with-snapshot-diff.integ.snapshot/test-stack.template.json +++ b/packages/@aws-cdk/integ-runner/test/test-data/test-with-snapshot-diff.integ.snapshot/test-stack.template.json @@ -1,6 +1,6 @@ { "Resources": { - "MyFunction1ServiceRole9852B06B": { + "MyFunction2ServiceRole": { "Type": "AWS::IAM::Role", "Properties": { "AssumeRolePolicyDocument": { @@ -31,6 +31,27 @@ ] } }, + "MyLambdaFuncServiceRoleDefaultPolicy": { + "Type": "AWS::IAM::Policy", + "Properties": { + "PolicyDocument": { + "Statement": [ + { + "Action": "*", + "Effect": "Allow", + "Resource": "*" + } + ], + "Version": "2012-10-17" + }, + "PolicyName": "MyLambdaFuncServiceRoleDefaultPolicyBEB0E748", + "Roles": [ + { + "Ref": "MyLambdaFuncServiceRole" + } + ] + } + }, "MyFunction12A744C2E": { "Type": "AWS::Lambda::Function", "Properties": { diff --git a/packages/@aws-cdk/integ-runner/test/test-data/test-with-snapshot.integ.snapshot/integ.json b/packages/@aws-cdk/integ-runner/test/test-data/test-with-snapshot.integ.snapshot/integ.json new file mode 100644 index 0000000000000..7aa90b7f5ec23 --- /dev/null +++ b/packages/@aws-cdk/integ-runner/test/test-data/test-with-snapshot.integ.snapshot/integ.json @@ -0,0 +1,12 @@ +{ + "version": "v1.0.0", + "testCases": { + "test1": { + "stacks": ["test-stack"], + "diffAssets": true, + "allowDestroy": [ + "AWS::IAM::Role" + ] + } + } +} diff --git a/packages/@aws-cdk/integ-runner/test/test-data/test-with-snapshot.integ.snapshot/test-stack.template.json b/packages/@aws-cdk/integ-runner/test/test-data/test-with-snapshot.integ.snapshot/test-stack.template.json index 40f4c8238c04f..efed158f80851 100644 --- a/packages/@aws-cdk/integ-runner/test/test-data/test-with-snapshot.integ.snapshot/test-stack.template.json +++ b/packages/@aws-cdk/integ-runner/test/test-data/test-with-snapshot.integ.snapshot/test-stack.template.json @@ -31,6 +31,27 @@ ] } }, + "MyLambdaFuncServiceRoleDefaultPolicyBEB0E748": { + "Type": "AWS::IAM::Policy", + "Properties": { + "PolicyDocument": { + "Statement": [ + { + "Action": "*", + "Effect": "Allow", + "Resource": "*" + } + ], + "Version": "2012-10-17" + }, + "PolicyName": "MyLambdaFuncServiceRoleDefaultPolicyBEB0E748", + "Roles": [ + { + "Ref": "MyFunction1ServiceRole9852B06B" + } + ] + } + }, "MyFunction12A744C2E": { "Type": "AWS::Lambda::Function", "Properties": { @@ -51,4 +72,4 @@ ] } } -} \ No newline at end of file +} 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 73e4d31d25c6e..fe9def07bc544 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 @@ -4,28 +4,58 @@ import * as fs from 'fs-extra'; import * as workerpool from 'workerpool'; import { integTestWorker } from '../../lib/workers/extract'; import { runIntegrationTestsInParallel, runIntegrationTests } from '../../lib/workers/integ-test-worker'; - -describe('test runner', () => { - beforeEach(() => { - jest.spyOn(fs, 'moveSync').mockImplementation(() => { return true; }); - jest.spyOn(fs, 'emptyDirSync').mockImplementation(() => { return true; }); - jest.spyOn(fs, 'unlinkSync').mockImplementation(() => { return true; }); - jest.spyOn(fs, 'moveSync').mockImplementation(() => { return true; }); - jest.spyOn(fs, 'removeSync').mockImplementation(() => { return true; }); - jest.spyOn(fs, 'writeFileSync').mockImplementation(() => { return true; }); - }); - afterEach(() => { - jest.clearAllMocks(); - jest.resetAllMocks(); - jest.restoreAllMocks(); +let stderrMock: jest.SpyInstance; +let pool: workerpool.WorkerPool; +let spawnSyncMock: jest.SpyInstance; +beforeAll(() => { + pool = workerpool.pool(path.join(__dirname, './mock-extract_worker.js')); +}); +beforeEach(() => { + jest.spyOn(fs, 'moveSync').mockImplementation(() => { return true; }); + jest.spyOn(fs, 'emptyDirSync').mockImplementation(() => { return true; }); + jest.spyOn(fs, 'unlinkSync').mockImplementation(() => { return true; }); + jest.spyOn(fs, 'removeSync').mockImplementation(() => { return true; }); + jest.spyOn(fs, 'writeFileSync').mockImplementation(() => { return true; }); + spawnSyncMock = jest.spyOn(child_process, 'spawnSync').mockReturnValueOnce({ + status: 0, + stderr: Buffer.from('HEAD branch: master\nother'), + stdout: Buffer.from('HEAD branch: master\nother'), + pid: 123, + output: ['stdout', 'stderr'], + signal: null, + }).mockReturnValueOnce({ + status: 0, + stderr: Buffer.from('abc'), + stdout: Buffer.from('abc'), + pid: 123, + output: ['stdout', 'stderr'], + signal: null, + }).mockReturnValue({ + status: 0, + stderr: Buffer.from('stack1'), + stdout: Buffer.from('stack1'), + pid: 123, + output: ['stdout', 'stderr'], + signal: null, }); + stderrMock = jest.spyOn(process.stderr, 'write').mockImplementation(() => { return true; }); + jest.spyOn(process.stdout, 'write').mockImplementation(() => { return true; }); +}); +afterEach(() => { + jest.clearAllMocks(); + jest.resetAllMocks(); + jest.restoreAllMocks(); +}); +afterAll(async () => { + await pool.terminate(); +}); +describe('test runner', () => { test('no snapshot', () => { // WHEN const test = { fileName: 'test/test-data/integ.integ-test1.js', }; - const spawnSyncMock = jest.spyOn(child_process, 'spawnSync').mockImplementation(); integTestWorker({ tests: [test], region: 'us-east-1', @@ -48,14 +78,7 @@ describe('test runner', () => { const test = { fileName: 'test/test-data/integ.integ-test2.js', }; - jest.spyOn(child_process, 'spawnSync').mockReturnValue({ - status: 0, - stderr: Buffer.from(''), - stdout: Buffer.from(''), - pid: 123, - output: ['stdout', 'stderr'], - signal: null, - }); + jest.spyOn(child_process, 'spawnSync').mockImplementation(); const results = integTestWorker({ tests: [test], region: 'us-east-1', @@ -69,19 +92,35 @@ describe('test runner', () => { const test = { fileName: 'test/test-data/integ.test-with-snapshot.js', }; - jest.spyOn(child_process, 'spawnSync').mockReturnValue({ - status: 0, - stderr: Buffer.from('stack1'), - stdout: Buffer.from('stack1'), - pid: 123, - output: ['stdout', 'stderr'], - signal: null, - }); const results = integTestWorker({ tests: [test], region: 'us-east-1', }); + expect(spawnSyncMock).toHaveBeenCalledWith( + expect.stringMatching(/git/), + ['remote', 'show', 'origin'], + expect.objectContaining({ + cwd: 'test/test-data', + }), + ); + + expect(spawnSyncMock).toHaveBeenCalledWith( + expect.stringMatching(/git/), + ['merge-base', 'HEAD', 'master'], + expect.objectContaining({ + cwd: 'test/test-data', + }), + ); + + expect(spawnSyncMock).toHaveBeenCalledWith( + expect.stringMatching(/git/), + ['checkout', 'abc', '--', 'test-with-snapshot.integ.snapshot'], + expect.objectContaining({ + cwd: 'test/test-data', + }), + ); + expect(results.length).toEqual(0); }); @@ -108,23 +147,6 @@ describe('test runner', () => { }); describe('parallel worker', () => { - let pool: workerpool.WorkerPool; - let stderrMock: jest.SpyInstance; - beforeEach(() => { - pool = workerpool.pool(path.join(__dirname, './mock-extract_worker.js')); - jest.spyOn(child_process, 'spawnSync').mockImplementation(); - stderrMock = jest.spyOn(process.stderr, 'write').mockImplementation(() => { return true; }); - jest.spyOn(process.stdout, 'write').mockImplementation(() => { return true; }); - jest.spyOn(fs, 'moveSync').mockImplementation(() => { return true; }); - jest.spyOn(fs, 'removeSync').mockImplementation(() => { return true; }); - jest.spyOn(fs, 'writeFileSync').mockImplementation(() => { return true; }); - }); - afterEach(async () => { - await pool.terminate(); - jest.clearAllMocks(); - jest.resetAllMocks(); - jest.restoreAllMocks(); - }); test('run all integration tests', async () => { const 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 d39b7e820047a..fa9a7d80015c6 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 @@ -3,20 +3,21 @@ import * as path from 'path'; import * as fs from 'fs-extra'; import { snapshotTestWorker } from '../../lib/workers/extract'; +beforeEach(() => { + jest.spyOn(process.stderr, 'write').mockImplementation(() => { return true; }); + jest.spyOn(process.stdout, 'write').mockImplementation(() => { return true; }); + jest.spyOn(fs, 'moveSync').mockImplementation(() => { return true; }); + jest.spyOn(fs, 'removeSync').mockImplementation(() => { return true; }); + jest.spyOn(fs, 'writeFileSync').mockImplementation(() => { return true; }); +}); +afterEach(() => { + jest.clearAllMocks(); + jest.resetAllMocks(); + jest.restoreAllMocks(); +}); + const directory = path.join(__dirname, '../test-data'); describe('Snapshot tests', () => { - beforeEach(() => { - jest.spyOn(process.stderr, 'write').mockImplementation(() => { return true; }); - jest.spyOn(process.stdout, 'write').mockImplementation(() => { return true; }); - jest.spyOn(fs, 'moveSync').mockImplementation(() => { return true; }); - jest.spyOn(fs, 'removeSync').mockImplementation(() => { return true; }); - jest.spyOn(fs, 'writeFileSync').mockImplementation(() => { return true; }); - }); - afterEach(() => { - jest.clearAllMocks(); - jest.resetAllMocks(); - jest.restoreAllMocks(); - }); test('no snapshot', () => { // WHEN const test = { @@ -40,7 +41,7 @@ describe('Snapshot tests', () => { const result = snapshotTestWorker(test); // THEN - expect(result.length).toEqual(0); + expect(result.length).toEqual(1); }); test('failed snapshot', () => { @@ -48,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: directory, + destructiveChanges: [], }; const result = snapshotTestWorker(test);