chore: migrate njlynch's modules to assertions#18204
Conversation
kaizencc
left a comment
There was a problem hiding this comment.
Can't approve since I started this PR, but this is my stamp of approval :).
Just the one comment/question about Match.not...
| }); | ||
|
|
||
| expect(stack).not.toHaveResourceLike('AWS::ElasticLoadBalancingV2::TargetGroup', { | ||
| const matches = Template.fromStack(stack).findResources('AWS::ElasticLoadBalancingV2::TargetGroup', { |
There was a problem hiding this comment.
The original intention here is to do:
Template.fromStack(stack).hasResourceProperties('AWS::ElasticLoadBalancingV2::TargetGroup', Match.not({
...
});@rix0rrr seems to hate this but I think it's reasonable; Match.not simply inverts the result so it succeeds with any matching failure. Curious if this is something you looked at and actively avoided.
There was a problem hiding this comment.
I looked at it, it just felt... wrong? And it's not entirely equivalent; for example, the above fails if there are no TargetGroups. Maybe I should be using it, as I guess it's the more idiomatic approach for assertions, but something about the way the API currently expresses itself makes me wrinkle my nose a bit. I'd like to think of a better approach, but also didn't want to block this migration on it.
There was a problem hiding this comment.
Thanks for the response. The equivalence issue is true, but to play devils advocate, I don't think the Assertions API was developed with the idea of exact 1-1 matching with assert-internal. But I hear what both you and Rico are saying.
|
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). |
| ])); | ||
|
|
||
|
|
||
| expect(vpc.node.metadataEntry).toContainEqual({ |
There was a problem hiding this comment.
@kaizen3031593 this looks like the solution to what we were talking about yesterday.
There was a problem hiding this comment.
Good call. Although I am about to write a PR for messages support that would look something like this:
AssertAnnotations.fromStack(stack).hasWarning('*', {
entry: {
type: 'aws:cdk:warning',
data: "fromVpcAttributes...",
},
});|
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 CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
|
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). |
This PR is complete when these 16 modules are accounted for: - [x] route53 - [x] route53-targets - [x] route53-patterns - [x] elasticloadbalancingv2 - [x] elasticloadbalancingv2-targets - [x] elasticloadbalancingv2-actions - [x] elasticloadbalancing - [x] cloudfront - [x] cloudfront-origins - [x] secretsmanager - [x] ec2 - [x] certificates-manager - [x] sns-subscriptions - [x] sqs - [x] ssm - [x] kms ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
This PR is complete when these 16 modules are accounted for:
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license