chore(cli): add SAM CDK integration test cases

[moelasmar]

---

Add Integration test cases to cover the integration with SAM CLI tool.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

[gitpod-io]

[rix0rrr]

I appreciate the submission, but I have a couple of concerns:

• The tests seem better off as unit tests than integration tests. It's not clear to me what exactly it is they are testing, which is something you can clearly encode into a unit test (vs an integration/shapshot test). Also, if they were unit tests, you wouldn't need all the additional changes to the integ test machinery.
  • If they are going to be integration tests, they need to run the actual SAM CLI against some actual CDK apps.
• cdk-sam-integ-assert -> if you were going to do this (which I'd rather you don't), then it seems we're adding a new CLI and a new convention for what is essentially a boolean flag flip: "also check something about assets". That could have been encoded with a /// pragma, which we already have some code for, which makes this more flexible in the future as well.
• But the things you are encoding for assets are very v1-specific (or to be more precise: LegacyStackSynthesizer-specific). In v2 (or using the DefaultStackSynthesizer), asset references look completely different and all the assertions you just wrote about { Ref }s and { Fn::Join }s are going out the window. It seems unlikely these are the things you actually want to be asserting, because they are things we are asserting. But because of the nature of snapshot tests, it's hard for me to tell what it is you are trying to assert, which brings me back to point 1.

For now:

• Can you reframe the things about templates and artifacts you are trying to assert as unit tests?
• Can you describe what a real integ test involving the CLI would look like? What are we synthing, what are we deploying, and what should the end result look like?

[moelasmar]

Thanks @rix0rrr for your comment.

• to clarify what I am trying to test in this PR, I want to make sure that the synthesized template contain the expected assets metadata that SAM depends on to do its functionality, and also validate that the metadata asset paths contain the expected files.
• The reason that I did not add integration test cases with SAM (although it should be the best solution) is I do not want to add some dependency on external tool, that if there is any issue in SAM, this will block your testing and releases.

I will see how can I move this logic to be done in unit testing.

[rix0rrr]

The reason that I did not add integration test cases with SAM (although it should be the best solution) is I do not want to add some dependency on external tool, that if there is any issue in SAM, this will block your testing and releases.

I understand, and I appreciate it! Nevertheless, we should ultimately have something like this. We should make a TODO for ourselves to add these. If you do decide to add a test like this, it should probably go into the set of CLI integ tests: https://github.com/aws/aws-cdk/blob/master/packages/aws-cdk/test/integ/cli/cli.integtest.ts, which are actual integ tests, that run in a Docker image we control, so we could preinstall SAM CLI there.

I will see how can I move this logic to be done in unit testing.

Thank you!

[moelasmar]

we already add this testing in SAM repo here. We have a job that run daily against the latest CDK V1 and V2 releases to validate that the new release does not break this integration. The point here is we need to do that before the release.

[moelasmar]

If you do decide to add a test like this, it should probably go into the set of CLI integ tests: https://github.com/aws/aws-cdk/blob/master/packages/aws-cdk/test/integ/cli/cli.integtest.ts, which are actual integ tests, that run in a Docker image we control, so we could preinstall SAM CLI there.

Do you have any docs that explain how to update these integ tests, how to local validate it, and how to update the docker image?

[rix0rrr]

Do you have any docs

Of course not 😛

that explain how to update these integ tests

Not really but copy/paste should get you far

how to local validate it

To run a single integ test in the source tree:

test/integ/run-against-repo test/integ/cli/test.sh -t 'SUBSTRING OF THE TEST NAME'

https://github.com/aws/aws-cdk/blob/master/packages/aws-cdk/CONTRIBUTING.md

and how to update the docker image?

https://github.com/aws/jsii/tree/main/superchain

Pull request has been modified.

[rix0rrr]

This needs to go somewhere into some test runner script where we also install jest, not here.

[moelasmar]

yarn build fails if I removed axios from package.json as it is required to invoke the exposed api to validate the integration with sam.

Pull request has been modified.

[rix0rrr]

Must be merged only AFTER the newest jsii is released, so that jsii/superchain has the SAM CLI.

Pull request has been modified.

[rix0rrr]

When I run the new integ tests through a test pipeline, they are all failing with the same error:

================================================================

CDK synth bundled functions as expected
================================================================
...

Error: Cannot find entry file at /tmp/cdk-integ-0enfurmv5r8/src/nodejs/NodeJsFunctionConstruct/app.ts

Do they succeed if you run them locally in a superchain container? (read here how)

[moelasmar]

@rix0rrr .. Yes all test cases passed successfully locally.
I am running the following commands to execute it:

147dda696921:aws-cdk-interllij melasmar$ pwd
/Volumes/workplace/aws-cdk-interllij
147dda696921:aws-cdk-interllij melasmar$ chmod 777 /var/run/docker.sock
147dda696921:aws-cdk-interllij melasmar$ docker run --net=host \
>     --rm -it \
>     -v $HOME:$HOME \
>     -v /var/run/docker.sock:/var/run/docker.sock \
>     -v $PWD:$PWD -w $PWD \
>     -v /tmp:/tmp \
>     -e HOME=$HOME \
>     -e NPM_CONFIG_UNSAFE_PERM=true \
>     jsii/superchain:local
superchain@docker-desktop:/Volumes/workplace/aws-cdk-interllij$ cd packages/aws-cdk
superchain@docker-desktop:/Volumes/workplace/aws-cdk-interllij/packages/aws-cdk$ test/integ/run-against-repo test/integ/cli/test.sh
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
CLI Integration Tests
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
27.5.1
Using regions: us-east-1
Using framework version: * (major version 1)
 PASS  ./cli.integtest.js (3702.623 s)
  ✓ VPC Lookup (391878 ms)
  ✓ Construct with builtin Lambda function (104681 ms)
  ✓ Two ways of shoing the version (13519 ms)
  ✓ Termination protection (82796 ms)
  ✓ cdk synth (33067 ms)
  ✓ ssm parameter provider error (30355 ms)
  ✓ automatic ordering (114629 ms)
  ✓ context setting (26144 ms)
  ✓ context in stage propagates to top (17594 ms)
  ✓ deploy (66604 ms)
  ✓ deploy all (102100 ms)
  ✓ nested stack with parameters (87222 ms)
  ✓ deploy without execute a named change set (50810 ms)
  ✓ security related changes without a CLI are expected to fail (34940 ms)
  ✓ deploy wildcard with outputs (115457 ms)
  ✓ deploy with parameters (62318 ms)
  ✓ update to stack in ROLLBACK_COMPLETE state will delete stack and create a new one (103399 ms)
  ✓ stack in UPDATE_ROLLBACK_COMPLETE state can be updated (170582 ms)
  ✓ deploy with wildcard and parameters (152854 ms)
  ✓ deploy with parameters multi (61696 ms)
  ✓ deploy with notification ARN (72179 ms)
  ✓ deploy with role (97211 ms)
  ✓ cdk diff (55975 ms)
  ✓ cdk diff --fail on multiple stacks exits with error if any of the stacks contains a diff (116373 ms)
  ✓ cdk diff --fail with multiple stack exits with if any of the stacks contains a diff (98924 ms)
  ✓ cdk diff --security-only --fail exits when security changes are present (24266 ms)
  ✓ deploy stack with docker asset (74941 ms)
  ✓ deploy and test stack with lambda asset (98399 ms)
  ✓ cdk ls (23805 ms)
  ✓ synthing a stage with errors leads to failure (19813 ms)
  ✓ synthing a stage with errors can be suppressed (23341 ms)
  ✓ deploy stack without resource (122557 ms)
  ✓ IAM diff (27962 ms)
  ✓ fast deploy (204972 ms)
  ✓ failed deploy does not hang (70131 ms)
  ✓ can still load old assemblies (18617 ms)
  ✓ generating and loading assembly (136065 ms)
  ✓ templates on disk contain metadata resource, also in nested assemblies (12805 ms)
  ✓ CDK synth add the metadata properties expected by sam (400450 ms)
  ✓ CDK synth bundled functions as expected (94184 ms)
  ✓ sam can locally test the synthesized cdk application (84276 ms)

Using regions: us-east-1
Using framework version: * (major version 1)
 PASS  ./bootstrapping.integtest.js (1948.846 s)t to 600000 ms
  ✓ can bootstrap without execution (198608 ms)
  ✓ upgrade legacy bootstrap stack to new bootstrap stack while in use (297576 ms)
  ✓ can and deploy if omitting execution policies (168461 ms)
  ✓ deploy new style synthesis to new style bootstrap (181337 ms)
  ✓ deploy new style synthesis to new style bootstrap (with docker image) (161550 ms)
  ✓ deploy old style synthesis to new style bootstrap (186952 ms)
  ✓ can create a legacy bootstrap stack with --public-access-block-configuration=false (77335 ms)
  ✓ can create multiple legacy bootstrap stacks (142288 ms)
  ✓ can dump the template, modify and use it to deploy a custom bootstrap stack (146000 ms)
  ✓ switch on termination protection, switch is left alone on re-bootstrap (108776 ms)
  ✓ add tags, left alone on re-bootstrap (104034 ms)
  ✓ can deploy modern-synthesized stack even if bootstrap stack name is unknown (170042 ms)

Test Suites: 2 passed, 2 total
Tests:       53 passed, 53 total
Snapshots:   0 total
Time:        5651.938 s
Ran all test suites.

[rix0rrr]

Does that work from a fresh checkout as well? Is there a chance you have files in your working copy that aren't checked in? (Because of gitignore or other reasons?)

[moelasmar]

I tested it in Cloud9 to make sure that it does not depend on any file in my machine. I found some issue, fixed it, and it is working fine now. Could you please retry the testing flow.

[mergify]

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]

AWS CodeBuild CI Report

• CodeBuild project: AutoBuildProject89A8053A-LhjRyN9kxr8o
• Commit ID: 4c35e40
• Result: SUCCEEDED
• Build Logs (available for 30 days)

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

[mergify]

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