Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

expect (from @aws-cdk/assert) fails when passed a CloudFormationStackArtifact #6083

Closed
cmackie opened this issue Feb 4, 2020 · 1 comment
Assignees
Labels
@aws-cdk/assert Related to the @aws-cdk/assert package bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. p2

Comments

@cmackie
Copy link

cmackie commented Feb 4, 2020

The override of expect defined in @aws-cdk/assert fails when passed a CloudFormationStackArtifact object retrieved with the SynthUtils api. This only happens in projects which do not have a flat dependency tree, instead producing the full dependency tree (which should be supported) in node_modules. The root of the problem is that @aws-cdk/core has @aws-cdk/cx-api as a direct dependency even though it exposes classes within @aws-cdk/cx-api (the CloudFormationStackArtifact retrieved though SynthUtils is ultimately created in ConstructNode. This means that when the instanceof operator is used on a CloudFormationStackArtifact anywhere in @aws-cdk/assert, the left hand side was created by the @aws-cdk/cx-api module required by @aws-cdk/core, whereas the right hand side class comes from the @aws-cdk/cx-api module required by @aws-cdk/assert. This causes instanceof to return false.

In addition to the issue with expect (which I think is most serious), this problem causes several other methods (like SynthUtils.toCloudFormation within @aws-cdk/assert to misbehave subtly).

Reproduction Steps

While I've only been able to naturally reproduce this in my own project (which I cannot share here), this is fairly easy to reproduce with the simple project created by cdk init. Just follow these steps:

  1. mkdir dep-test && cd dep-test
  2. cdk init --typescript
  3. Replace dep-test/test/dep-test.test.ts with the attached file.
    dep-test.test.txt

At this point, running npm run test succeeds because npm install creates a flat node_modules. To reproduce the error, we need to simulate the results of using an older version of npm, or any other build system which creates the full dependency tree.

  1. mkdir -p node_modules/@aws-cdk/assert/node_modules/@aws-cdk && cp -r node_modules/@aws-cdk/cx-api "$_"
  2. mkdir -p node_modules/@aws-cdk/core/node_modules/@aws-cdk && cp -r node_modules/@aws-cdk/cx-api "$_"

Now running npm run test fails, for the reason I described earlier in this report.

Error Log

TypeError: Cannot read property 'root' of undefined

       8 |     const stack = new HelloCdk.HelloCdkStack(app, 'MyTestStack');
       9 |     // THEN
    > 10 |     expectCDK(SynthUtils.synthesize(stack));
         |     ^
      11 | });

      at Function._synthesizeWithNested (node_modules/@aws-cdk/assert/lib/synth-utils.ts:54:29)
      at Object.expect (node_modules/@aws-cdk/assert/lib/expect.ts:8:90)
      at Object.<anonymous> (test/hello-cdk.test.ts:10:5)

Environment

  • CLI Version : 1.22.0
  • Framework Version: 1.22.0
  • OS : macOS 10.14.6
  • Language : TypeScript

Other


This is 🐛 Bug Report

@cmackie cmackie added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Feb 4, 2020
@SomayaB SomayaB added the @aws-cdk/assert Related to the @aws-cdk/assert package label Feb 4, 2020
@eladb eladb added the p2 label Feb 5, 2020
@eladb
Copy link
Contributor

eladb commented Feb 5, 2020

We are aware that there are issues related to how CDK is released ("hyper-modular"). The current best practice is to always use a single version line and have npm flatten the dependency graph.

You can stay tuned here for updates: aws/aws-cdk-rfcs#6

@eladb eladb closed this as completed Feb 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/assert Related to the @aws-cdk/assert package bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. p2
Projects
None yet
Development

No branches or pull requests

3 participants