feat(integ-tests): enhancements to integ-tests #20180

corymhall · 2022-05-02T20:32:20Z

This PR contains various enhancements including

integ-tests
- removed dependency on other CDK libraries (other than core)
- API ergonomics improvements
  - renamed queryAws to awsApiCall
  - added some additional methods
- Now using Match from @aws-cdk/assertions for the assertions provider
- DeployAssert now creates its own stack
  - This stack is written to a new IntegManifest property so that it can be treated differently
    (i.e. don't diff this stack)
- Additional assertion types (OBJECT_LIKE)
- Refactored assertion results
  - removed separate results handler in favor of just writing results to a stack output
- utility for invoking lambda functions (separate from awsApiCall)
- IntegTest now creates a test case by default.
- Added IntegTestCaseStack class
integ-runner
- Updated to handle the results of assertions
  - When running with update workflow, the assertion stack is only deployed during the
    "update" deployment
  - The stack outputs containing the assertion results are are written to a
    file that the runner can read.

I've also converted/added assertions to a couple of existing integration tests

aws-lambda/test/integ.bundling.ts
aws-lambda-destinations/test/integ.destinations.ts
aws-stepfunctions-tasks/test/eventbridge/integ.put-events.ts

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 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

This PR contains various enhancements including - API ergonomics improvements - renamed `queryAws` to `awsApiCall` - Now using `Match` from @aws-cdk/assertions for the assertions provider - `DeployAssert` now creates its own stack - Additional assertion types (OBJECT_LIKE) - utility for invoking lambda functions (separate from `awsApiCall`)

gitpod-io · 2022-05-02T20:32:27Z

rix0rrr · 2022-05-04T08:58:36Z

packages/@aws-cdk/aws-lambda-destinations/test/integ.destinations.ts

+    },
+    responsePayload: 'success',
+  }),
+  assertionType: AssertionType.OBJECT_LIKE,


Can we not do ExpectedResult.objectLike() instead of this?

Combining what you said in the previous comment maybe I'll add another method so you could do

message.assertObjectLikeAtPath('Messages.0.Body', { ..., });

Or we could reduce the methods down and put the type of assertion on ExpectedResult? Something like

message.assert(ExpectedResult.objectLike()); message.assert(ExpectedResult.objectEquals()); message.assertAtPath('Messages.0.Body', ExpectedResult.stringEquals());

I think ideally you would not need to use the EqualsAssertion class directly.

rix0rrr · 2022-05-04T09:00:08Z

packages/@aws-cdk/aws-lambda-destinations/test/integ.destinations.ts

+  WaitTimeSeconds: 20,
+});
+
+new EqualsAssertion(integ.assert, 'ReceiveMessage', {


Helper function on message for this?

message.assertEquals(ExpectedResult.fromObject({ // ... }));

?

Or if we need to select a subobject:

message.atPath('Message.0.Body')

rix0rrr · 2022-05-04T09:09:04Z

packages/@aws-cdk/aws-stepfunctions-tasks/test/eventbridge/integ.put-events.ts

+});
+
+// assert the results
+describe.assertObjectLike({


Oh shit that exists 😅

rix0rrr · 2022-05-04T09:11:38Z

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

@@ -238,26 +245,79 @@ export class IntegTestRunner extends IntegRunner {
          lookups: this.expectedTestSuite?.enableLookups,
        });
      }
+      // now deploy the "actual" test. If there are any assertions
+      // deploy the assertion stack as well
      this.cdk.deploy({


Is this not async?

It is not, it uses spawnSync.

rix0rrr · 2022-05-04T09:15:31Z

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

+   * Process the outputsFile which contains the assertions results as stack
+   * outputs
+   */
+  private processAssertionResults(file: string, assertionStackId: string): AssertionResults | undefined {


Is there a need for this to return undefined? Can we not just return an empty object?

Motivation: I like avoiding ifs and special cases as much as I can.

I need to check if the results have any assertion failures so I can fail the test and report on the failures. I would either need to check for undefined or an empty object and undefined seemed better. I can be persuaded otherwise if you think it is not though.

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

rix0rrr · 2022-05-04T09:49:43Z

...mbda-destinations/test/destinations.integ.snapshot/aws-cdk-lambda-destinations.template.json

+   "Export": {
+    "Name": "aws-cdk-lambda-destinations:ExportsOutputRefSnsSqsC4810B27404A5AFF"
+   }


This is going to prevent replacements. Feels a little scary, is that definitely what we want?

I think so? I think in a lot of cases we would want to prevent replacements as that could be a breaking change. If it was really necessary then you could run without the update workflow. I'm not sure what the alternative would be that wouldn't have it's own tradeoffs.

rix0rrr · 2022-05-04T09:55:17Z

packages/@aws-cdk/integ-tests/lib/assertions/providers/lambda-handler/assertion.ts

    let result: AssertionResult;
+    let matcher;
+    console.log(`Testing equality between ${JSON.stringify(request.actual)} and ${JSON.stringify(request.expected)}`);
+
    switch (request.assertionType) {


Instead of having a single assertionType, could we do something arbitrarily recursive? We could encode it the same way as CloudFormation intrinsics: Something like:

{ '$ObjectLike': { Message: [ Body: { 'Match::Like': { Elements: { '$ArrayWith': [{ Asdf: 3 }] }, } } ] }}

Then in the handler we'd recursively turn that structure into matchers and arguments to matchers before executing.

On the construction side, we'd have helpers to construct this JSON so consumers don't have to see the ugliness:

export class Match { public static arrayWith(xs: string[]): any { return { '$ArrayWith': xs }; } }

hmm that is an interesting idea

Added. Let me know what you think about the API.

rix0rrr · 2022-05-04T09:56:46Z

packages/@aws-cdk/integ-tests/lib/assertions/providers/lambda-handler/assertion.ts

+          status: 'fail',
+          message: [
+            ...matchResult.toHumanStrings(),
+            JSON.stringify(matchResult.target, undefined, 2),
+          ].join('\n'),


If we choose to make the matcher succeed even if the assertion fails, do we also have a resource to make the deployment fail based on the outputs of all registered assertions? That will be necessary if we want to run assertions without the integ runner (like in CDK pipelines).

We could just have a prop that the user could pass that would cause the custom resource to fail if the assertion fails.

Added a prop, let me know what you think.

rix0rrr · 2022-05-10T12:13:12Z

packages/@aws-cdk/integ-tests/lib/assertions/providers/lambda-handler/assertion.ts

+          case '$ArrayWith':
+            return Match.arrayWith(v[nested]);
+          case '$ObjectLike':
+            return Match.objectLike(v[nested]);
+          case '$StringLike':
+            return Match.stringLikeRegexp(v[nested]);


Why is this case statement duplicated in the constructor? We shouldn't need both.

Yeah that is true. Updated to not duplicate this in the constructor.

rix0rrr · 2022-05-10T12:15:19Z

packages/@aws-cdk/integ-tests/lib/assertions/providers/lambda-handler/assertion.ts

+   */
+  public getMatcher(): Matcher {
+    try {
+      const final = JSON.parse(JSON.stringify(this.parsedObj), function(_k, v) {


That's an interesting way of doing it. I would have expected an explicit recursion but I suppose this will work

rix0rrr · 2022-05-10T12:20:34Z

packages/@aws-cdk/integ-tests/lib/assertions/deploy-assert.ts

+    scope = new Stack(scope, 'DeployAssert');
+    super(scope, 'Default');


This is weird to me. Why is the Stack not inside the DeployAssert ?

We can hold on to the stack in a private readonly scope so that we can still create other resources inside the right Stack. But the swapparoo here is a little confusing.

It's so that we don't have to create DeployAssert in the scope of a stack. I was originally going to make DeployAssert extend Stack but I thought it would be nice if DeployAssert could be treated like a stack, but the user wouldn't be able to access any of the stack functionality (the methods they would see would just be the DeployAssert methods and not any Stack` methods).

So you could create any of the assertion resources by passing in DeployAssert as the scope

const deployAssert = new DeployAssert(app); new AwsApiCall(deployAssert, 'AwsApiCall', {...});

We could change it to what you suggested and make it a public readonly stack which the user would have to know to use

const deployAssert = new DeployAssert(app); new AwsApiCall(deployAssert.stack, 'AwsApiCall', {...});

rix0rrr · 2022-05-10T12:21:19Z

packages/@aws-cdk/integ-tests/lib/assertions/assertions.ts

   */
-  readonly expected: any;
+  readonly reportFailure?: boolean;


failDeployment ?

rix0rrr · 2022-05-10T12:22:18Z

packages/@aws-cdk/integ-tests/lib/assertions/providers/lambda-handler/assertion.ts

+        }),
+      };
+      if (request.reportFailure) {
+        throw new Error(result.data);


Is this throw going to be shown with the error message in the stack deployment? Do we run this code through the custom resource provider?

Yes this throw will be shown in the error message in the stack deployment. The throw is caught in the provider and returned in the response to CloudFormation.

rix0rrr

Approved modulo a comment explaining the stack swapparoo

mergify · 2022-05-16T15:59:05Z

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).

aws-cdk-automation · 2022-05-16T16:48:14Z

AWS CodeBuild CI Report

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

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

mergify · 2022-05-16T16:48:28Z

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).

This PR contains various enhancements including - `integ-tests` - removed dependency on other CDK libraries (other than core) - API ergonomics improvements - renamed `queryAws` to `awsApiCall` - added some additional methods - Now using `Match` from @aws-cdk/assertions for the assertions provider - `DeployAssert` now creates its own stack - This stack is written to a new IntegManifest property so that it can be treated differently (i.e. don't diff this stack) - Additional assertion types (OBJECT_LIKE) - Refactored assertion results - removed separate results handler in favor of just writing results to a stack output - utility for invoking lambda functions (separate from `awsApiCall`) - `IntegTest` now creates a test case by default. - Added `IntegTestCaseStack` class - `integ-runner` - Updated to handle the results of assertions - When running with update workflow, the assertion stack is only deployed during the "update" deployment - The stack outputs containing the assertion results are are written to a file that the runner can read. I've also converted/added assertions to a couple of existing integration tests - `aws-lambda/test/integ.bundling.ts` - `aws-lambda-destinations/test/integ.destinations.ts` - `aws-stepfunctions-tasks/test/eventbridge/integ.put-events.ts` ---- ### 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 `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*

corymhall requested a review from rix0rrr May 2, 2022 20:32

aws-cdk-automation requested a review from a team May 2, 2022 20:32

github-actions bot added the p2 label May 2, 2022

mergify bot added the contribution/core This is a PR that came from AWS. label May 2, 2022

updating schema version

e128318

corymhall marked this pull request as ready for review May 3, 2022 11:48

Merge branch 'master' into corymhall/integ-tests/assertion-results

cfb1517

corymhall added the pr-linter/exempt-readme The PR linter will not require README changes label May 3, 2022

exporting Match from @aws-cdk/assertions

aa53dfd

corymhall requested a review from TheRealAmazonKendra May 3, 2022 15:17

rix0rrr reviewed May 4, 2022

View reviewed changes

corymhall added 4 commits May 4, 2022 12:28

renaming SdkQuery to AwsApiCall

8b730f2

adding matcher utility

285434a

updates

74a9e39

add option to report assertion failures to CloudFormation

875a839

corymhall requested a review from rix0rrr May 4, 2022 19:50

adding missing docstring

a2e6f8d

rix0rrr reviewed May 10, 2022

View reviewed changes

updates based on pr comments

86da9c5

corymhall requested a review from rix0rrr May 16, 2022 14:09

rix0rrr added the pr/do-not-merge This PR should not be merged at this time. label May 16, 2022

rix0rrr approved these changes May 16, 2022

View reviewed changes

adding comment about scope swapparoo

619b2f9

corymhall removed the pr/do-not-merge This PR should not be merged at this time. label May 16, 2022

Merge branch 'master' into corymhall/integ-tests/assertion-results

1295a69

mergify bot merged commit 3ff3fb7 into aws:master May 16, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(integ-tests): enhancements to integ-tests #20180

feat(integ-tests): enhancements to integ-tests #20180

corymhall commented May 2, 2022 •

edited

Loading

gitpod-io bot commented May 2, 2022

rix0rrr May 4, 2022

corymhall May 4, 2022

rix0rrr May 4, 2022

rix0rrr May 4, 2022

rix0rrr May 4, 2022

corymhall May 4, 2022

rix0rrr May 4, 2022

corymhall May 4, 2022

rix0rrr May 4, 2022

corymhall May 4, 2022

rix0rrr May 4, 2022

corymhall May 4, 2022

corymhall May 4, 2022

rix0rrr May 4, 2022

corymhall May 4, 2022

corymhall May 4, 2022

rix0rrr May 10, 2022

corymhall May 16, 2022

rix0rrr May 10, 2022

rix0rrr May 10, 2022

corymhall May 16, 2022

rix0rrr May 10, 2022

corymhall May 16, 2022

rix0rrr May 10, 2022

corymhall May 16, 2022

rix0rrr left a comment

mergify bot commented May 16, 2022

aws-cdk-automation commented May 16, 2022

mergify bot commented May 16, 2022

		scope = new Stack(scope, 'DeployAssert');
		super(scope, 'Default');

feat(integ-tests): enhancements to integ-tests #20180

feat(integ-tests): enhancements to integ-tests #20180

Conversation

corymhall commented May 2, 2022 • edited Loading

All Submissions:

Adding new Unconventional Dependencies:

New Features

gitpod-io bot commented May 2, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rix0rrr left a comment

Choose a reason for hiding this comment

mergify bot commented May 16, 2022

aws-cdk-automation commented May 16, 2022

AWS CodeBuild CI Report

mergify bot commented May 16, 2022

corymhall commented May 2, 2022 •

edited

Loading