feat: add new integration test runner#19529

Merged

mergify[bot] merged 22 commits intoaws:masterfrom

corymhall:corymhall/integ-test-runner

Mar 30, 2022

Contributor

corymhall commented Mar 23, 2022 •

edited

Loading

Creating this PR so that we can iterate on the design of the integration
test runner.

We will create two packages (names up for debate)

@aws-cdk/integ-runner (this PR)
CLI tool that will execute integration tests
@aws-cdk/integ-tests (future PR)
Library that is used to create integration test cases. This library will
synthesize an assembly file that will be consumed by the runner.

This PR does not yet implement running new style integration tests (generated from the @aws-cdk/integ-tests library).
The initial implementation of the runner will only work on the legacy (i.e. existing) integration tests.

All Submissions:

Have you followed the guidelines in our Contributing guide?

Adding new Unconventional Dependencies:

This PR adds new unconventional dependencies following the process described here

New Features

Have you added the new feature to an integration test?
- Did you use cdk-integ to deploy the infrastructure and generate the snapshot (i.e. cdk-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

corymhall commented

View reviewed changes

packages/@aws-cdk/integ-runner/README.md


		This will search for integration tests recursively from the current directory and then execute them in parallel across `us-east-1`, `us-east-2`, & `us-west-2`.

		### integ.json schema

Contributor Author

corymhall Mar 23, 2022

This would be added as a new schema in @aws-cdk/cloud-assembly-schema

packages/@aws-cdk/integ-runner/README.md

+              You would then use the `IntegTest` construct to create your test cases
+              ```ts
+              new IntegTeset(app, 'ArmTest', {

Contributor Author

corymhall Mar 23, 2022

This could be a Construct that uses attachCustomSynthesis to synthesize the manifest.

jogold reviewed

View reviewed changes

packages/@aws-cdk/integ-runner/README.md Outdated Show resolved Hide resolved

gitpod-io bot commented Mar 23, 2022

mergify bot added the contribution/core label

corymhall and others added 2 commits

March 23, 2022 19:27


          feat: integ-runner [WIP]

9e9026c

Creating this PR so that we can iterate on the design of the integration
test runner.

We will create two packages (names up for debate)

1. `@aws-cdk/integ-runner` (this package)
CLI tool that will execute integration tests

2. `@aws-cdk/integ-tests`
Library that is used to create integration test cases. This library will
synthesize an assembly file that will be consumed by the runner.


          Update packages/@aws-cdk/integ-runner/README.md

39074db

Co-authored-by: Jonathan Goldwasser <jogold@users.noreply.github.com>

otaviomacedo reviewed

View reviewed changes

packages/@aws-cdk/integ-runner/README.md

+              interface MyIntegTestProps extends StackOptions {
+                functionProps?: FunctionOptions;
+              }
+              class MyIntegTest extends Stack {

Contributor

otaviomacedo Mar 24, 2022

Is this the stack under test?

Contributor Author

corymhall Mar 24, 2022

Potentially. But maybe only if we add it to a "test case"? That would allow us to do things like only verify certain stacks within a snapshot (not sure if there is an actual need for this)

packages/@aws-cdk/integ-runner/README.md

+. Check if a snapshot file exists (i.e. `/integ.*.expected.snapshot$/`)
+. If the snapshot does not exist
+a. Synth the integ app which will produce the `integ.json` file

Contributor

otaviomacedo Mar 24, 2022

If I understand it correctly, for each integ.*.ts, it will generate an integ.json file plus as many snapshots as there are tests (instances of IntegTest) in the integ.*.ts file. Is this correct?

Contributor Author

corymhall Mar 24, 2022

I was thinking that there would be a 1:1 between integ.*.ts and snapshots. Maybe we change the name of the construct to something like IntegTestCase?

An integ.*.ts file contains an "Integration Test" which corresponds to a single CDK application (i.e. one snapshot). Within that application we can create any number of Stacks which we then use in "test cases".

We could also model it differently where you create a single IntegTest and then register any test cases to it integTest.addTestCase()

Contributor

otaviomacedo Mar 28, 2022

Got it.

packages/@aws-cdk/integ-runner/README.md Outdated

+                 *
+                 * @default true
+                 */
+                readonly update?: boolean;

Contributor

otaviomacedo Mar 24, 2022

This update property has a different meaning than the --update command line arg, right? I wonder if this may cause some confusion.

Contributor Author

corymhall Mar 24, 2022

Good point, what about stackUpdateWorkflow?

corymhall added 4 commits

March 24, 2022 12:06


          update readme

4e44bf3


          some initial work

8fbae30


          updates

38618d1


          update readme

bd37997

github-actions bot added the p2 label

corymhall added 8 commits

March 28, 2022 14:12


          updating deps

7a6833e


          bumping schema version


          fixing build

7fa3bde


          some fixes

c2ef840


          updates

93becac


          fixing test output

52621bb


          checking in tsconfig.json

c506ba8


          updates

501c1cf

Contributor Author

corymhall commented Mar 28, 2022

Current experience.

corymhall changed the title ~~feat: integ-runner [WIP]~~ feat: add new integration test runner

corymhall marked this pull request as ready for review

March 28, 2022 19:47

corymhall requested a review from otaviomacedo

March 28, 2022 19:47

corymhall commented

View reviewed changes

packages/@aws-cdk/integ-runner/lib/runner/private/logger.ts

		@@ -0,0 +1,20 @@
		import { Writable } from 'stream';

Contributor Author

corymhall Mar 28, 2022

I took this file from packages/aws-cdk. It might be better if we had a shared library we could put stuff like this.

Contributor

otaviomacedo Mar 28, 2022

Is there anything in core that we could reuse?


          fixing enum

c8aa819

corymhall commented

View reviewed changes

...s-cdk/integ-runner/test/test-data/cdk-integ.out.test-with-snapshot-assets-diff/manifest.json

+                          "type": "aws:cdk:logicalId",
+                          "data": "MyFunction1ServiceRole9852B06B",
+                          "trace": [
+                            "new Role (/home/hallcor/work/aws-cdk/integ-test-runner/packages/@aws-cdk/aws-iam/lib/role.js:50:22)",

Contributor Author

corymhall Mar 28, 2022

We are not diffing this trace, but should we find a way to remove this?

otaviomacedo reviewed

View reviewed changes

packages/@aws-cdk/integ-runner/bin/integ-runner.ts Outdated

+                  const diagnostics = new Array<Diagnostic>();
+                  if (!argv.force) {
+                    const requests = batchTests(testsFromArgs);

Contributor

otaviomacedo Mar 28, 2022

(minor) there are two responsibilities in this method: manage the execution of snapshot tests (lines 97 to 116) and manage the execution of integration tests (119 to 148). I suggest you extract them to two different methods.

packages/@aws-cdk/integ-runner/bin/integ-runner.ts Show resolved Hide resolved

corymhall added 2 commits

March 29, 2022 14:05


          updates based on review comments

f53f219


          refactoring some stuff

799f198

- Moving commands back to `cdk-cli-wrapper`
- Adding parameter for enabling lookups

corymhall added the pr/do-not-merge label

otaviomacedo approved these changes

View reviewed changes


          added queuing to process the tests in parallel more efficiently.

d895fa7

otaviomacedo reviewed

View reviewed changes

packages/@aws-cdk/integ-runner/lib/workers/integ-test-worker.ts Outdated Show resolved Hide resolved

packages/@aws-cdk/integ-runner/lib/workers/integ-test-worker.ts Outdated Show resolved Hide resolved

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

+                async function runTest(): Promise<void> {
+                  do {
+                    const region = queue.nextAvailableRegion;

Contributor

otaviomacedo Mar 30, 2022

What is the advantage of doing this versus fixing a region per execution?

Contributor Author

corymhall Mar 30, 2022

That is a good point, i'll do that instead.

Contributor Author

corymhall Mar 30, 2022

Done! Don't need that queue anymore! Much simpler.


          refactoring queue

corymhall removed the pr/do-not-merge label

Contributor

mergify bot commented Mar 30, 2022

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

mergify bot and others added 2 commits

March 30, 2022 16:53


          Merge branch 'master' into corymhall/integ-test-runner

ed84d27


          trigger build

e55da8f

Contributor

mergify bot commented Mar 30, 2022

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).


          Merge branch 'master' into corymhall/integ-test-runner

5a62055

Collaborator

aws-cdk-automation commented Mar 30, 2022

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject89A8053A-LhjRyN9kxr8o
Commit ID: 5a62055
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

mergify bot merged commit e7f43d1 into aws:master

Contributor

mergify bot commented Mar 30, 2022

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

madeline-k added a commit that referenced this pull request


          Revert "feat: add new integration test runner (#19529)"

f3655ca

This reverts commit e7f43d1.

mergify bot pushed a commit that referenced this pull request


          chore: Revert "feat: add new integration test runner (#19529)" (#19651)

99924af

This reverts commit e7f43d1.


----

### All Submissions:

* [ ] 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 `cdk-integ` to deploy the infrastructure and generate the snapshot (i.e. `cdk-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*

StevePotter pushed a commit to StevePotter/aws-cdk that referenced this pull request


          feat: add new integration test runner (aws#19529)

0fbddb8

Creating this PR so that we can iterate on the design of the integration
test runner.

We will create two packages (names up for debate)

1. `@aws-cdk/integ-runner` (this PR)
CLI tool that will execute integration tests

2. `@aws-cdk/integ-tests` (future PR)
Library that is used to create integration test cases. This library will
synthesize an assembly file that will be consumed by the runner.

This PR does not yet implement running new style integration tests (generated from the `@aws-cdk/integ-tests` library).
The initial implementation of the runner will only work on the legacy (i.e. existing) integration tests. 


----

### All Submissions:

* [ ] 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 `cdk-integ` to deploy the infrastructure and generate the snapshot (i.e. `cdk-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*

StevePotter pushed a commit to StevePotter/aws-cdk that referenced this pull request


          chore: Revert "feat: add new integration test runner (aws#19529)" (aw…

244d604

…s#19651)

This reverts commit e7f43d1.


----

### All Submissions:

* [ ] 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 `cdk-integ` to deploy the infrastructure and generate the snapshot (i.e. `cdk-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*

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

contribution/core p2