-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
(assert): Snapshot testing not working with 1.101.0 - directory references causes tests to diff #14468
Comments
Hi @markusl - I'm not sure I understand the test case vs the snapshot shown here. I believe the test case is trying to match the snapshot of the CloudFormation template, but the diff is not that of a CloudFormation template. Besides that, looking at the diff, the issue seems to be that the location of the |
@nija-at consider the following regression test case: test('TestStack', () => {
const app = new cdk.App({
context: {
'@aws-cdk/core:newStyleStackSynthesis': 'true',
},
outdir: 'cdk.out',
stackTraces: false,
});
// WHEN
const stack = new cdk.Stack(app, 'TestStack');
// THEN
expect(SynthUtils.toCloudFormation(stack)).toMatchSnapshot();
}); With CDK up to 1.100.0 it generates the following snapshot and the regression test can be used in a CI system to check that nothing has changed:
However, after updating to 1.101.0, the snapshot changes to a much larger one. This would not be a problem but it contains a part which changes every time when you run it in a CI system such as AWS CodeBuild:
|
That diff makes a lot more sense now. It seems something changed that's impacting this conditional - aws-cdk/packages/@aws-cdk/assert-internal/lib/synth-utils.ts Lines 21 to 25 in 527f662
It was previously returning the template but now returns the entire synth output. My initial suspect would be this - b1ecd3d I'll need to dive into this more to confirm and identify the root cause. |
@markusl the reason is probably that there are two different instances of |
+1 to @rix0rrr. You've likely upgraded some of your CDK packages to 1.101.0 but not all. This will cause the |
@nija-at We did upgrade all of the main application CDK dependencies but we also have some third-party CDK constructs installed but we don't have control over them. Is there any other way to resolve this? I think it's unlikely that a project with three or more external dependencies would ever be perfectly synchronized regarding CDK library dependencies |
Can you confirm that this is the issue? Did you find multiple copies of |
@rix0rrr yes, that is correct:
|
The `assert` library sometimes fails to get the template out of a `CloudFormationStackArtifact` if there are multiple copies of the CDK in the dependency tree. Fix by replacing an `instanceof` with a duck-type check. Fixes #14468.
…14544) The `assert` library sometimes fails to get the template out of a `CloudFormationStackArtifact` if there are multiple copies of the CDK in the dependency tree. Fix by replacing an `instanceof` with a duck-type check. Fixes #14468. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
|
…ws#14544) The `assert` library sometimes fails to get the template out of a `CloudFormationStackArtifact` if there are multiple copies of the CDK in the dependency tree. Fix by replacing an `instanceof` with a duck-type check. Fixes aws#14468. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
I am afraid the same problem happens here: aws-cdk/packages/aws-cdk/lib/api/cloudformation-deployments.ts Lines 300 to 302 in b02311c
|
…ws#14544) The `assert` library sometimes fails to get the template out of a `CloudFormationStackArtifact` if there are multiple copies of the CDK in the dependency tree. Fix by replacing an `instanceof` with a duck-type check. Fixes aws#14468. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Follow up on a similar issue (the one I mention just above): #17974 |
Fixes #17974 Follow-up of #14468 This follows the implementation of aws/constructs#955 to be more robust. ---- ### All Submissions: * [x] 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 `cdk-integ` to deploy the infrastructure and generate the snapshot (i.e. `cdk-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*
Fixes aws#17974 Follow-up of aws#14468 This follows the implementation of aws/constructs#955 to be more robust. ---- ### All Submissions: * [x] 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 `cdk-integ` to deploy the infrastructure and generate the snapshot (i.e. `cdk-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*
We utilize snapshots for a large amount of regression testing like this:
Reproduction Steps
See previous code.
What did you expect to happen?
Should so that regression testing using snapshots is possible.
What actually happened?
It seems that 1.101.0 has changed somehow and generates a new directory reference with every build which causes diffs:
- Snapshot - 2 + Received + 2 @@ -249,11 +249,11 @@ "libraries": Object {}, }, "version": "9.0.0", }, "bootstrapStackVersionSsmParameter": "/cdk-bootstrap/hnb659fds/version", - "file": "/Users/user/repo/cdk.out/Stack.assets.json", + "file": "/codebuild/output/src976733808/src/git-codecommit.eu-central-1.amazonaws.com/v1/repos/repo/cdk.out/Stack.assets.json", "id": "Stack.assets", "manifest": Object { "properties": Object { "bootstrapStackVersionSsmParameter": "/cdk-bootstrap/hnb659fds/version", "file": "Stack.assets.json", @@ -270,11 +270,11 @@ AssetManifestArtifact { "_dependencyIDs": Array [], "_deps": Array [], "assembly": [Circular], "bootstrapStackVersionSsmParameter": "/cdk-bootstrap/hnb659fds/version", - "file": "/Users/user/repo/cdk.out/Stack.assets.json", + "file": "/codebuild/output/src976733808/src/git-codecommit.eu-central-1.amazonaws.com/v1/repos/repo/cdk.out/Stack.assets.json", "id": "Stack.assets", "manifest": Object { "properties": Object { "bootstrapStackVersionSsmParameter": "/cdk-bootstrap/hnb659fds/version", "file": "Stack.assets.json", 63 | --
Environment
Other
This is 🐛 Bug Report
The text was updated successfully, but these errors were encountered: