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

(assertions): Can't do snapshot testing of Stacks using the same App #18847

Open
ncaq opened this issue Feb 7, 2022 · 6 comments
Open

(assertions): Can't do snapshot testing of Stacks using the same App #18847

ncaq opened this issue Feb 7, 2022 · 6 comments
Labels
@aws-cdk/assertions Related to the @aws-cdk/assertv2 package bug This issue is a bug. effort/medium Medium work item – several days of effort p3

Comments

@ncaq
Copy link
Contributor

ncaq commented Feb 7, 2022

What is the problem?

I migrate AWS CDK v1 to v2.
And I use assertion from assert because assert is deprecated.

My project have snapshot test.
When I rewrote this into assertions, I received an error that was difficult to resolve.
I thought that the complicated configuration I had done so far was bad, but it was reproduced when I created a new AWS CDK project and wrote test code.

Reproduction Steps

mkdir foo
cd foo
cdk init app --language typescript
emacsclient test/snapshot.test.ts

paste.

import { App, Stack } from "aws-cdk-lib";
import { Template } from "aws-cdk-lib/assertions";

test("snapshot", () => {
  const app = new App();

  const fooStack = new Stack(app, "FooStack");
  const fooTemplate = Template.fromStack(fooStack);
  expect(fooTemplate.toJSON()).toMatchSnapshot("FooStack");

  const barStack = new Stack(app, "BarStack");
  const barTemplate = Template.fromStack(barStack);
  expect(barTemplate.toJSON()).toMatchSnapshot("BarStack");
});
npm run test

What did you expect to happen?

This is a snapshot test, so we expect it to succeed unconditionally the first time.

What actually happened?

> foo@0.1.0 test
> jest

 PASS  test/foo.test.ts
 FAIL  test/snapshot.test.ts (7.841 s)
  ● snapshot

    Unable to find artifact with id "BarStack"

      10 |
      11 |   const barStack = new Stack(app, "BarStack");
    > 12 |   const barTemplate = Template.fromStack(barStack);
         |                                ^
      13 |   expect(barTemplate.toJSON()).toMatchSnapshot("BarStack");
      14 | });
      15 |

      at CloudAssembly.getStackArtifact (node_modules/aws-cdk-lib/cx-api/lib/cloud-assembly.ts:79:31)
      at toTemplate (node_modules/aws-cdk-lib/assertions/lib/template.ts:150:19)
      at Function.fromStack (node_modules/aws-cdk-lib/assertions/lib/template.ts:28:17)
      at Object.<anonymous> (test/snapshot.test.ts:12:32)

 › 1 snapshot written.
Snapshot Summary
 › 1 snapshot written from 1 test suite.

Test Suites: 1 failed, 1 passed, 2 total
Tests:       1 failed, 1 passed, 2 total
Snapshots:   1 written, 1 total
Time:        8.175 s
Ran all test suites.

CDK CLI Version

2.10.0 (build e5b301f)

Framework Version

No response

Node.js Version

v16.13.1

OS

Linux strawberry 5.15.19-gentoo #1 SMP Sat Feb 5 10:10:29 JST 2022 x86_64 AMD Ryzen Threadripper 1950X 16-Core Processor AuthenticAMD GNU/Linux

Language

Typescript

Language Version

3.9.7

Other information

relation?

@ncaq ncaq added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Feb 7, 2022
@github-actions github-actions bot added the @aws-cdk/assertions Related to the @aws-cdk/assertv2 package label Feb 7, 2022
@ncaq
Copy link
Contributor Author

ncaq commented Feb 7, 2022

The reason why I use the same App all the time is that in the actual production code, I create a stack of ECR repositories and reference the fields to the stack of ECS repositories.
When I did this with a new App, I got a different error.

@NGL321 NGL321 added p2 and removed needs-triage This issue or PR still needs to be triaged. labels Feb 14, 2022
@amirmishani
Copy link

@ncaq did you ever figure this out? I've run into the same issue.

@ncaq
Copy link
Contributor Author

ncaq commented May 6, 2022

@amirmishani
It has not been resolved.
I am using the previous deprecated package for now.

@anvuori
Copy link

anvuori commented May 10, 2022

Try creating both stacks first, and only after that create the Template objects and assertions. Somehow fixed things for me, still no idea why 🤷

@jamestelfer
Copy link
Contributor

jamestelfer commented Nov 3, 2022

When Template.toStack() is called, it calls synth() on the underlying App instance.

synth() caches the result of the synthesis (note the if (!this.assembly on L182. (Note that App extends Stage.)

public synth(options: StageSynthesisOptions = { }): cxapi.CloudAssembly {
if (!this.assembly || options.force) {
this.assembly = synthesize(this, {
skipValidation: options.skipValidation,
validateOnSynthesis: options.validateOnSynthesis,
});
}
return this.assembly;
}

So when you reuse the App instance and call Template.toStack() multiple times, you'll only get the first result, which will not have any stacks that you've subsequently added. This behaviour could possibly change, but it's likely to regress test performance negatively when fromStack() is called repeatedly.

The answer to this is:

  1. Don't share App instances across tests. This probably applies in the current case: there are two Stacks, use separate tests with different App instances.
  2. OR if they're in the same test, arrange your tests in the arrange/act/assert style, setting up the app and both stack instances first, then running assertions later.

@jamestelfer
Copy link
Contributor

jamestelfer commented Nov 3, 2022

So in the current example, I suggest doing something like (comments optional):

import { App, Stack } from "aws-cdk-lib";
import { Template } from "aws-cdk-lib/assertions";

test("snapshot foo", () => {
  // arrange
  const app = new App();

  // act
  const fooStack = new Stack(app, "FooStack");
  const fooTemplate = Template.fromStack(fooStack);

  // assert
  expect(fooTemplate.toJSON()).toMatchSnapshot("FooStack");
});

test("snapshot bar", () => {
  const app = new App();

  const barStack = new Stack(app, "BarStack");
  const barTemplate = Template.fromStack(barStack);

  expect(barTemplate.toJSON()).toMatchSnapshot("BarStack");
});

@khushail khushail added the effort/medium Medium work item – several days of effort label Jun 3, 2024
@pahud pahud added p3 and removed p2 labels Jun 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/assertions Related to the @aws-cdk/assertv2 package bug This issue is a bug. effort/medium Medium work item – several days of effort p3
Projects
None yet
Development

No branches or pull requests

8 participants