Skip to content

Commit c946615

Browse files
authored
fix(integ-runner): don't allow new legacy tests (aws#20614)
All new integration tests that are created should use the new `IntegTest` construct in the [@aws-cdk/integ-tests](https://github.com/aws/aws-cdk/tree/main/packages/%40aws-cdk/integ-tests) module. We will eventually be migrating all of our tests to the new construct and removing the "legacy" mode, so this PR will fail any new test that is added that doesn't use the `IntegTest` construct. I've also renamed some of the `test-data` files so that they will not be picked up by the `integ-runner` if you execute it on the entire repo. ---- ### All Submissions: * [ ] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) ### Adding new Unconventional Dependencies: * [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/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/main/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*
1 parent f98029e commit c946615

File tree

20 files changed

+249
-184
lines changed

20 files changed

+249
-184
lines changed

CONTRIBUTING.md

Lines changed: 8 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -256,7 +256,7 @@ Work your magic. Here are some guidelines:
256256

257257
Integration tests perform a few functions in the CDK code base -
258258
1. Acts as a regression detector. It does this by running `cdk synth` on the integration test and comparing it against
259-
the `*.expected.json` file. This highlights how a change affects the synthesized stacks.
259+
the `*.integ.snapshot` directory. This highlights how a change affects the synthesized stacks.
260260
2. Allows for a way to verify if the stacks are still valid CloudFormation templates, as part of an intrusive change.
261261
This is done by running `yarn integ` which will run `cdk deploy` across all of the integration tests in that package. If you are developing a new integration test or for some other reason want to work on a single integration test over and over again without running through all the integration tests you can do so using `yarn integ integ.test-name.js`
262262
Remember to set up AWS credentials before doing this.
@@ -275,9 +275,10 @@ new features unless there is a good reason why one is not needed.
275275
4. Adding a new supported version (e.g. a new [AuroraMysqlEngineVersion](https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_rds.AuroraMysqlEngineVersion.html))
276276
5. Adding any functionality via a [Custom Resource](https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.custom_resources-readme.html)
277277

278-
To the extent possible, include a section (like below) in the integration test file that specifies how the successfully
279-
deployed stack can be verified for correctness. Correctness here implies that the resources have been set up correctly.
280-
The steps here are usually AWS CLI commands but they need not be.
278+
All integration tests going forward should use the [IntegTest](https://github.com/aws/aws-cdk/tree/main/packages/%40aws-cdk/integ-tests)
279+
construct. Over time we will be updating all of our existing tests to use this construct. It
280+
allows for more control over configuring each tests as well as the ability to perform
281+
assertions against the deployed infrastructure.
281282

282283
```ts
283284
/*
@@ -288,8 +289,8 @@ The steps here are usually AWS CLI commands but they need not be.
288289
```
289290

290291
Examples:
291-
* [integ.destinations.ts](https://github.com/aws/aws-cdk/blob/main/packages/%40aws-cdk/aws-lambda-destinations/test/integ.destinations.ts#L7)
292-
* [integ.token-authorizer.lit.ts](https://github.com/aws/aws-cdk/blob/main/packages/%40aws-cdk/aws-apigateway/test/authorizers/integ.token-authorizer.lit.ts#L7-L12)
292+
* [integ.destinations.ts](https://github.com/aws/aws-cdk/blob/master/packages/%40aws-cdk/aws-lambda-destinations/test/integ.destinations.ts#L7)
293+
* [integ.put-events.ts](https://github.com/aws/aws-cdk/blob/main/packages/%40aws-cdk/aws-stepfunctions-tasks/test/eventbridge/integ.put-events.ts)
293294

294295
**What do do if you cannot run integration tests**
295296

@@ -827,16 +828,7 @@ $ <path to the AWS CDK repo>/link-all.sh
827828

828829
### Running integration tests in parallel
829830

830-
Integration tests may take a long time to complete. We can speed this up by running them in parallel
831-
in different regions.
832-
833-
```shell
834-
# Install GNU parallel (may require uninstall 'moreutils' if you have it)
835-
$ apt-get install parallel
836-
$ brew install parallel
837-
838-
$ scripts/run-integ-parallel @aws-cdk/aws-ec2 @aws-cdk/aws-autoscaling ...
839-
```
831+
See the [Integration testing guide](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md#running-large-numbers-of-tests)
840832

841833
### Visualizing dependencies in a CloudFormation Template
842834

INTEGRATION_TESTS.md

Lines changed: 51 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,7 @@ _integ.lambda.ts_
8585
import * as iam from '@aws-cdk/aws-iam';
8686
import * as cdk from '@aws-cdk/core';
8787
import * as lambda from '../lib';
88+
import * as integ from '@aws-cdk/integ-tests';
8889

8990
const app = new cdk.App();
9091

@@ -96,6 +97,10 @@ const fn = new lambda.Function(stack, 'MyLambda', {
9697
runtime: lambda.Runtime.NODEJS_14_X,
9798
});
9899

100+
new integ.IntegTest(app, 'LambdaTest', {
101+
testCases: [stack],
102+
});
103+
99104
app.synth();
100105
```
101106

@@ -223,7 +228,52 @@ but it will not validate that the Lambda function can be invoked. Because of thi
223228
to deploy the Lambda Function _and_ then rerun the assertions to ensure that the function can still be invoked.
224229

225230
### Assertions
226-
...Coming soon...
231+
232+
Sometimes it is necessary to perform some form of _assertion_ against the deployed infrastructure to validate that the
233+
test succeeds. A good example of this is the `@aws-cdk/aws-stepfunctions-tasks` module which creates integrations between
234+
AWS StepFunctions and other AWS services.
235+
236+
If we look at the [integ.put-events.ts](https://github.com/aws/aws-cdk/blob/main/packages/%40aws-cdk/aws-stepfunctions-tasks/test/eventbridge/integ.put-events.ts)
237+
integration test we can see that we are creating an `@aws-cdk/aws-events.EventBus` along with a `@aws-cdk/aws-stepfunctions.StateMachine`
238+
which will send an event to the `EventBus`. In a typical integration test we would just deploy the test and the fact that the
239+
infrastructure deployed successfully would be enough of a validation that the test succeeded. In this case though, we ideally
240+
want to validate that the _integration_ connecting `StepFunctions` to the `EventBus` has been setup correctly, and the only
241+
way to do that is to actually trigger the `StateMachine` and validate that it was successful.
242+
243+
```ts
244+
declare const app: App;
245+
declare const sm: sfn.StateMachine;
246+
declare const stack: Stack;
247+
248+
const testCase = new IntegTest(app, 'PutEvents', {
249+
testCases: [stack],
250+
});
251+
252+
// Start an execution
253+
const start = testCase.assertions.awsApiCall('StepFunctions', 'startExecution', {
254+
stateMachineArn: sm.stateMachineArn,
255+
});
256+
257+
// describe the results of the execution
258+
const describe = testCase.assertions.awsApiCall('StepFunctions', 'describeExecution', {
259+
executionArn: start.getAttString('executionArn'),
260+
});
261+
262+
// assert the results
263+
describe.expect(ExpectedResult.objectLike({
264+
status: 'SUCCEEDED',
265+
}));
266+
```
267+
268+
Not every test requires an assertion. We typically do not need to assert CloudFormation behavior. For example, if we create an S3 Bucket
269+
with Encryption, we do not need to assert that Encryption is set on the bucket. We can trust that the CloudFormation behavior works.
270+
Some things you should look for in deciding if the test needs an assertion:
271+
272+
- Integrations between services (i.e. integration libraries like `@aws-cdk/aws-lambda-destinations`, `@aws-cdk/aws-stepfunctions-tasks`, etc)
273+
- Anything that bundles or deploys custom code (i.e. does a Lambda function bundled with `@aws-cdk/aws-lambda-nodejs` still invoke or did we break bundling behavior)
274+
- IAM/Networking connections.
275+
- This one is a bit of a judgement call. Most things do not need assertions, but sometimes we handle complicated configurations involving IAM permissions or
276+
Networking access.
227277

228278
## Running Integration Tests
229279

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,16 @@ export class IntegTestRunner extends IntegRunner {
5858
constructor(options: IntegRunnerOptions, destructiveChanges?: DestructiveChange[]) {
5959
super(options);
6060
this._destructiveChanges = destructiveChanges;
61+
62+
// We don't want new tests written in the legacy mode.
63+
// If there is no existing snapshot _and_ this is a legacy
64+
// test then point the user to the new `IntegTest` construct
65+
if (!this.hasSnapshot() && this.isLegacyTest) {
66+
throw new Error(`${this.testName} is a new test. Please use the IntegTest construct ` +
67+
'to configure the test\n' +
68+
'https://github.com/aws/aws-cdk/tree/main/packages/%40aws-cdk/integ-tests',
69+
);
70+
}
6171
}
6272

6373
/**

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,7 @@ export abstract class IntegRunner {
126126

127127
protected _destructiveChanges?: DestructiveChange[];
128128
private legacyContext?: Record<string, any>;
129+
protected isLegacyTest?: boolean;
129130

130131
constructor(options: IntegRunnerOptions) {
131132
this.test = options.test;
@@ -214,6 +215,7 @@ export abstract class IntegRunner {
214215
},
215216
});
216217
this.legacyContext = LegacyIntegTestSuite.getPragmaContext(this.test.fileName);
218+
this.isLegacyTest = true;
217219
return testCases;
218220
}
219221
}

0 commit comments

Comments
 (0)