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

aws-cdk-lib/assertions: Unable to find artifact with id when creating two instances of the same Stack in Jest test #24689

Closed
kadishmal opened this issue Mar 19, 2023 · 9 comments · Fixed by #31865
Assignees
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 p1

Comments

@kadishmal
Copy link

kadishmal commented Mar 19, 2023

Describe the bug

I'm migrating from CDK v1 to CDK v2. Have fixed all the syntax issues and now fixing unit tests.

In V1 tests I have the following code which I migrated to v2:

describe('Website Stack', () => {
  const app = new App();
  const envOregon = { region: 'us-west-2' };

  const stages = [
    {
      stage: STAGE.PROD,
    },
    {
      stage: STAGE.BETA,
    },
  ];

  stages.forEach(({ stage }) => {
    const stack = new WebsiteStack(app, `${stage}WebsiteStack`, {
      env: envOregon,
      subdomain: stage,
    });

    // Here it fails with `Unable to find artifact with id`
    const template = Template.fromStack(stack);

It fails to find the the second instance on the last line with Template.fromStack(). It always can't find the second stack even when I change the order of the stack definition.

Expected Behavior

Previously in V1 the test used to work by finding the second stack as well.

Current Behavior

Internally I see that Template.fromStack() calls the following code:

return new Template(toTemplate(stack), templateParsingOptions);

... which in turn calls:

return stack.nestedStackParent
    ? JSON.parse(fs.readFileSync(path.join(assembly.directory, stack.templateFile)).toString('utf-8'))
    : assembly.getStackArtifact(stack.artifactId).template;

In the above code fs.readFileSync(path.join(assembly.directory, stack.templateFile)) fails because for some reason the test does not compile/synthesize the second instance. In the assembly.directory I only see the first instance, whichever is the first.

$ ls -lat "/private/var/folders/l7/p9ksylw17x34lc5_xstwndmr0000gp/T/cdk.out4nhobd"
cdk.out
manifest.json
tree.json
WebsiteStack.assets.json
WebsiteStack.template.json

The directory has no template for the BetaWebsiteStack.

Reproduction Steps

  • Followed the migration guide to move to CDK v2.
  • All syntax errors are fixed.
  • All deprecated constructs are migrated.
  • Run npm test in CDK v2 project.

Possible Solution

I managed to fix the test by creating a separate instance of the App for each stack as follows:

describe('Website Stack', () => {
  const envOregon = { region: 'us-west-2' };

  const stages = [
    {
      stage: STAGE.PROD,
    },
    {
      stage: STAGE.BETA,
    },
  ];

  stages.forEach(({ stage }) => {
    const app = new App();

    const stack = new WebsiteStack(app, `${stage}WebsiteStack`, {
      env: envOregon,
      subdomain: stage,
    });

    // Here it fails with `Unable to find artifact with id`
    const template = Template.fromStack(stack);

Additional Information/Context

No response

CDK CLI Version

2.69.0 (build 60a5b2a)

Framework Version

2.69.0

Node.js Version

v16.15.0

OS

macOS 13.2.1

Language

Typescript

Language Version

5.0.2

Other information

No response

@kadishmal kadishmal added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Mar 19, 2023
@github-actions github-actions bot added the aws-cdk-lib Related to the aws-cdk-lib package label Mar 19, 2023
@pahud
Copy link
Contributor

pahud commented Mar 20, 2023

Thank you for your report as well as the workaround. I am making it p1 for more visibility.

@pahud pahud added p1 effort/medium Medium work item – several days of effort @aws-cdk/assertions Related to the @aws-cdk/assertv2 package and removed needs-triage This issue or PR still needs to be triaged. labels Mar 20, 2023
@Styerp
Copy link
Contributor

Styerp commented Apr 10, 2023

I can't speak to the difference between v1 and v2, but I can say why you see this right now.

This happens because the assertions library synthesizes the template for an App the first time it's called. The CloudAssembly library caches the synthesized template and returns it for all subsequent calls to app.synth(). When you make a new app for each stage, you get around this limitation, since the stack you're looking for gets synthesized inside the loop.

@damnhandy
Copy link

We had not seen this until later CDK releases, around 2.69.0, when the older @aws-cdk/assert package was removed. This work around works for us as well.

@anentropic
Copy link

painful wait for docker builds for each App instantiated (see #27991) are a reason I want to use single app for all tests

there is a better/alternative workaround than app-per-stack which is just to instantiate all the stacks before ever calling Template.from_stack (at least in Python that seems to work) ... i.e. OP could have two forEach loops, one to instantiate stacks and a second afterwards to get the template from each stack

this pattern makes e.g. my pytest fixtures a bit crufty (I wanted fixtures which return a single stack in shared app, but this issue means the template-returning fixtures have to request all stack fixtures to ensure they have been registered before any synth occurs)

@golesuman
Copy link

golesuman commented Jun 6, 2024

I have similar issue and we are loading the stacks as fixtures using the session scope and also using the templates as fixtures but with module scope but sometimes it still throws the error. But the pytest document says that session fixtures are excuted first but not sure why the issue still persists though.

Edit: I was able to fix it in pytest by using autouse=True for fixtures that is used for generating templates.

@moelasmar moelasmar removed the aws-cdk-lib Related to the aws-cdk-lib package label Aug 28, 2024
@rix0rrr
Copy link
Contributor

rix0rrr commented Oct 18, 2024

We probably need a way to mark a construct tree as immutable, so that you are not tempted to add anything to it after it has been synthesized. That would prevent these confusing behaviors.

I do think we have the concept of "locking" a construct, so this might be something worthwhile to investigate.

@rix0rrr
Copy link
Contributor

rix0rrr commented Oct 22, 2024

On second thought: there are probably 100s of people that have written CDK apps with mutations at the end that have no effect, but also don't harm and they're not noticing something is wrong. These people will complain if their programs start erroring because of additional validation.

Probably safer to just make app.synth() not hold on to the cached version and synth again every time synth is called, except the one invocation that is scheduled on `process.on('exit')'.

Should probably inspect the code of CDK Pipelines as well, and see it's not doing unnecessary invocations to synth

Copy link

Comments on closed issues and PRs are hard for our team to see.
If you need help, please open a new issue that references this one.

1 similar comment
Copy link

Comments on closed issues and PRs are hard for our team to see.
If you need help, please open a new issue that references this one.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 29, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 29, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
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 p1
Projects
None yet
9 participants