Skip to content

Commit

Permalink
chore: fix tests that use node.metadataentry (#19545)
Browse files Browse the repository at this point in the history
Does this for everything except the tests in core that are specifically testing `Aspects`. There's less of a chance that those tests are disturbed by adding annotations elsewhere in the CDK.

Closes #19544 

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
kaizencc authored Mar 25, 2022
1 parent d406ead commit d81cce7
Show file tree
Hide file tree
Showing 14 changed files with 47 additions and 166 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -95,13 +95,7 @@ describe('scalable target', () => {
});

// THEN
expect(target.node.metadataEntry).toEqual([{
type: 'aws:cdk:warning',
data: 'cron: If you don\'t pass \'minute\', by default the event runs every minute. Pass \'minute: \'*\'\' if that\'s what you intend, or \'minute: 0\' to run once per hour instead.',
trace: undefined,
}]);
Annotations.fromStack(stack).hasWarning('/Default/Target', "cron: If you don't pass 'minute', by default the event runs every minute. Pass 'minute: '*'' if that's what you intend, or 'minute: 0' to run once per hour instead.");

Annotations.fromStack(stack).hasWarning('/Default/Target', Match.stringLikeRegexp("cron: If you don't pass 'minute', by default the event runs every minute.*"));
});

test('scheduled scaling shows no warning when minute is * in cron', () => {
Expand All @@ -121,9 +115,7 @@ describe('scalable target', () => {
});

// THEN
expect(target.node.metadataEntry).toEqual([]);
const annotations = Annotations.fromStack(stack).findWarning('*', Match.anyValue());
expect(annotations.length).toBe(0);
Annotations.fromStack(stack).hasNoWarning('/Default/Target', Match.stringLikeRegexp("cron: If you don't pass 'minute', by default the event runs every minute.*"));
});

test('step scaling on MathExpression', () => {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Match, Template } from '@aws-cdk/assertions';
import { Annotations, Match, Template } from '@aws-cdk/assertions';
import * as ec2 from '@aws-cdk/aws-ec2';
import * as cdk from '@aws-cdk/core';
import {
Expand Down Expand Up @@ -39,11 +39,7 @@ describe('AutoScalingGroupRequireImdsv2Aspect', () => {
},
}));

expect(asg.node.metadataEntry).toContainEqual({
data: expect.stringContaining('CfnLaunchConfiguration.MetadataOptions field is a CDK token.'),
type: 'aws:cdk:warning',
trace: undefined,
});
Annotations.fromStack(stack).hasWarning('/Stack/AutoScalingGroup', Match.stringLikeRegexp('.*CfnLaunchConfiguration.MetadataOptions field is a CDK token.'));
});

test('requires IMDSv2', () => {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
import { Match, Template } from '@aws-cdk/assertions';
import { Annotations, Match, Template } from '@aws-cdk/assertions';
import * as cloudwatch from '@aws-cdk/aws-cloudwatch';
import * as ec2 from '@aws-cdk/aws-ec2';
import * as iam from '@aws-cdk/aws-iam';
import * as sns from '@aws-cdk/aws-sns';
import { testDeprecated } from '@aws-cdk/cdk-build-tools';
import * as cxschema from '@aws-cdk/cloud-assembly-schema';
import * as cdk from '@aws-cdk/core';
import * as autoscaling from '../lib';

Expand Down Expand Up @@ -875,7 +874,7 @@ describe('auto scaling group', () => {
const stack = new cdk.Stack();
const vpc = mockVpc(stack);

const asg = new autoscaling.AutoScalingGroup(stack, 'MyStack', {
new autoscaling.AutoScalingGroup(stack, 'MyStack', {
instanceType: ec2.InstanceType.of(ec2.InstanceClass.M4, ec2.InstanceSize.MICRO),
machineImage: new ec2.AmazonLinuxImage(),
vpc,
Expand All @@ -890,16 +889,15 @@ describe('auto scaling group', () => {
});

// THEN
expect(asg.node.metadataEntry[0].type).toEqual(cxschema.ArtifactMetadataEntryType.WARN);
expect(asg.node.metadataEntry[0].data).toEqual('iops will be ignored without volumeType: EbsDeviceVolumeType.IO1');
Annotations.fromStack(stack).hasWarning('/Default/MyStack', 'iops will be ignored without volumeType: EbsDeviceVolumeType.IO1');
});

test('warning if iops and volumeType !== IO1', () => {
// GIVEN
const stack = new cdk.Stack();
const vpc = mockVpc(stack);

const asg = new autoscaling.AutoScalingGroup(stack, 'MyStack', {
new autoscaling.AutoScalingGroup(stack, 'MyStack', {
instanceType: ec2.InstanceType.of(ec2.InstanceClass.M4, ec2.InstanceSize.MICRO),
machineImage: new ec2.AmazonLinuxImage(),
vpc,
Expand All @@ -915,8 +913,7 @@ describe('auto scaling group', () => {
});

// THEN
expect(asg.node.metadataEntry[0].type).toEqual(cxschema.ArtifactMetadataEntryType.WARN);
expect(asg.node.metadataEntry[0].data).toEqual('iops will be ignored without volumeType: EbsDeviceVolumeType.IO1');
Annotations.fromStack(stack).hasWarning('/Default/MyStack', 'iops will be ignored without volumeType: EbsDeviceVolumeType.IO1');
});

test('step scaling on metric', () => {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Template } from '@aws-cdk/assertions';
import { Annotations, Template, Match } from '@aws-cdk/assertions';
import { testFutureBehavior, testLegacyBehavior } from '@aws-cdk/cdk-build-tools';
import * as cdk from '@aws-cdk/core';
import * as cxapi from '@aws-cdk/cx-api';
Expand Down Expand Up @@ -41,11 +41,7 @@ describe('RequireImdsv2Aspect', () => {

// THEN
expect(visitMock).toHaveBeenCalled();
expect(construct.node.metadataEntry).not.toContainEqual({
data: expect.stringContaining(errmsg),
type: 'aws:cdk:warning',
trace: undefined,
});
Annotations.fromStack(stack).hasNoWarning('/Stack/Construct', errmsg);
});

describe('InstanceRequireImdsv2Aspect', () => {
Expand Down Expand Up @@ -100,11 +96,7 @@ describe('RequireImdsv2Aspect', () => {
// Aspect normally creates a LaunchTemplate for the Instance to toggle IMDSv1,
// so we can assert that one was not created
Template.fromStack(stack).resourceCountIs('AWS::EC2::LaunchTemplate', 0);
expect(instance.node.metadataEntry).toContainEqual({
data: expect.stringContaining('Cannot toggle IMDSv1 because this Instance is associated with an existing Launch Template.'),
type: 'aws:cdk:warning',
trace: undefined,
});
Annotations.fromStack(stack).hasWarning('/Stack/Instance', Match.stringLikeRegexp('.*Cannot toggle IMDSv1 because this Instance is associated with an existing Launch Template.'));
});

test('suppresses Launch Template warnings', () => {
Expand All @@ -126,11 +118,7 @@ describe('RequireImdsv2Aspect', () => {
aspect.visit(instance);

// THEN
expect(instance.node.metadataEntry).not.toContainEqual({
data: expect.stringContaining('Cannot toggle IMDSv1 because this Instance is associated with an existing Launch Template.'),
type: 'aws:cdk:warning',
trace: undefined,
});
Annotations.fromStack(stack).hasNoWarning('/Stack/Instance', 'Cannot toggle IMDSv1 because this Instance is associated with an existing Launch Template.');
});

testFutureBehavior('launch template name is unique with feature flag', { [cxapi.EC2_UNIQUE_IMDSV2_LAUNCH_TEMPLATE_NAME]: true }, cdk.App, (app2) => {
Expand Down Expand Up @@ -197,11 +185,7 @@ describe('RequireImdsv2Aspect', () => {
aspect.visit(launchTemplate);

// THEN
expect(launchTemplate.node.metadataEntry).toContainEqual({
data: expect.stringContaining('LaunchTemplateData is a CDK token.'),
type: 'aws:cdk:warning',
trace: undefined,
});
Annotations.fromStack(stack).hasWarning('/Stack/LaunchTemplate', Match.stringLikeRegexp('.*LaunchTemplateData is a CDK token.'));
});

test('warns when MetadataOptions is a CDK token', () => {
Expand All @@ -217,11 +201,7 @@ describe('RequireImdsv2Aspect', () => {
aspect.visit(launchTemplate);

// THEN
expect(launchTemplate.node.metadataEntry).toContainEqual({
data: expect.stringContaining('LaunchTemplateData.MetadataOptions is a CDK token.'),
type: 'aws:cdk:warning',
trace: undefined,
});
Annotations.fromStack(stack).hasWarning('/Stack/LaunchTemplate', Match.stringLikeRegexp('.*LaunchTemplateData.MetadataOptions is a CDK token.'));
});

test('requires IMDSv2', () => {
Expand Down
13 changes: 5 additions & 8 deletions packages/@aws-cdk/aws-ec2/test/instance.test.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
import * as path from 'path';
import { Match, Template } from '@aws-cdk/assertions';
import { Annotations, Match, Template } from '@aws-cdk/assertions';
import { Key } from '@aws-cdk/aws-kms';
import { Asset } from '@aws-cdk/aws-s3-assets';
import { StringParameter } from '@aws-cdk/aws-ssm';
import * as cxschema from '@aws-cdk/cloud-assembly-schema';
import { Stack } from '@aws-cdk/core';
import {
AmazonLinuxImage, BlockDeviceVolume, CloudFormationInit,
Expand Down Expand Up @@ -347,7 +346,7 @@ describe('instance', () => {
});

test('warning if iops without volumeType', () => {
const instance = new Instance(stack, 'Instance', {
new Instance(stack, 'Instance', {
vpc,
machineImage: new AmazonLinuxImage(),
instanceType: InstanceType.of(InstanceClass.T3, InstanceSize.LARGE),
Expand All @@ -362,12 +361,11 @@ describe('instance', () => {
});

// THEN
expect(instance.node.metadataEntry[0].type).toEqual(cxschema.ArtifactMetadataEntryType.WARN);
expect(instance.node.metadataEntry[0].data).toEqual('iops will be ignored without volumeType: IO1, IO2, or GP3');
Annotations.fromStack(stack).hasWarning('/Default/Instance', 'iops will be ignored without volumeType: IO1, IO2, or GP3');
});

test('warning if iops and invalid volumeType', () => {
const instance = new Instance(stack, 'Instance', {
new Instance(stack, 'Instance', {
vpc,
machineImage: new AmazonLinuxImage(),
instanceType: InstanceType.of(InstanceClass.T3, InstanceSize.LARGE),
Expand All @@ -383,8 +381,7 @@ describe('instance', () => {
});

// THEN
expect(instance.node.metadataEntry[0].type).toEqual(cxschema.ArtifactMetadataEntryType.WARN);
expect(instance.node.metadataEntry[0].data).toEqual('iops will be ignored without volumeType: IO1, IO2, or GP3');
Annotations.fromStack(stack).hasWarning('/Default/Instance', 'iops will be ignored without volumeType: IO1, IO2, or GP3');
});
});

Expand Down
7 changes: 4 additions & 3 deletions packages/@aws-cdk/aws-ec2/test/launch-template.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Template } from '@aws-cdk/assertions';
import { Annotations, Template, Match } from '@aws-cdk/assertions';
import {
CfnInstanceProfile,
Role,
Expand Down Expand Up @@ -585,14 +585,15 @@ describe('LaunchTemplate marketOptions', () => {
[7, 1],
])('for range duration errors: %p', (duration: number, expectedErrors: number) => {
// WHEN
const template = new LaunchTemplate(stack, 'Template', {
new LaunchTemplate(stack, 'Template', {
spotOptions: {
blockDuration: Duration.hours(duration),
},
});

// THEN
expect(template.node.metadataEntry).toHaveLength(expectedErrors);
const errors = Annotations.fromStack(stack).findError('/Default/Template', Match.anyValue());
expect(errors).toHaveLength(expectedErrors);
});

test('for bad duration', () => {
Expand Down
8 changes: 2 additions & 6 deletions packages/@aws-cdk/aws-ec2/test/vpc.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Match, Template } from '@aws-cdk/assertions';
import { Annotations, Match, Template } from '@aws-cdk/assertions';
import { testDeprecated } from '@aws-cdk/cdk-build-tools';
import { CfnOutput, CfnResource, Fn, Lazy, Stack, Tags } from '@aws-cdk/core';
import {
Expand Down Expand Up @@ -1557,11 +1557,7 @@ describe('vpc', () => {
subnetIds: { 'Fn::Split': [',', { 'Fn::ImportValue': 'myPublicSubnetIds' }] },
});

expect(vpc.node.metadataEntry).toContainEqual({
data: expect.stringContaining("fromVpcAttributes: 'availabilityZones' is a list token: the imported VPC will not work with constructs that require a list of subnets at synthesis time. Use 'Vpc.fromLookup()' or 'Fn.importListValue' instead."),
type: 'aws:cdk:warning',
trace: undefined,
});
Annotations.fromStack(stack).hasWarning('/TestStack/VPC', "fromVpcAttributes: 'availabilityZones' is a list token: the imported VPC will not work with constructs that require a list of subnets at synthesis time. Use 'Vpc.fromLookup()' or 'Fn.importListValue' instead.");
});

test('fromVpcAttributes using fixed-length list tokens', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -364,11 +364,6 @@ test('Scheduled Ec2 Task shows warning when minute is not defined in cron', () =
});

// THEN
expect(stack.node.metadataEntry).toEqual([{
type: 'aws:cdk:warning',
data: 'cron: If you don\'t pass \'minute\', by default the event runs every minute. Pass \'minute: \'*\'\' if that\'s what you intend, or \'minute: 0\' to run once per hour instead.',
trace: undefined,
}]);
Annotations.fromStack(stack).hasWarning('/Default', "cron: If you don't pass 'minute', by default the event runs every minute. Pass 'minute: '*'' if that's what you intend, or 'minute: 0' to run once per hour instead.");
});

Expand All @@ -388,7 +383,5 @@ test('Scheduled Ec2 Task shows no warning when minute is * in cron', () => {
});

// THEN
expect(stack.node.metadataEntry).toEqual([]);
const annotations = Annotations.fromStack(stack).findWarning('*', Match.anyValue());
expect(annotations.length).toBe(0);
Annotations.fromStack(stack).hasNoWarning('/Default', Match.anyValue());
});
Original file line number Diff line number Diff line change
Expand Up @@ -427,11 +427,6 @@ test('Scheduled Fargate Task shows warning when minute is not defined in cron',
});

// THEN
expect(stack.node.metadataEntry).toEqual([{
type: 'aws:cdk:warning',
data: 'cron: If you don\'t pass \'minute\', by default the event runs every minute. Pass \'minute: \'*\'\' if that\'s what you intend, or \'minute: 0\' to run once per hour instead.',
trace: undefined,
}]);
Annotations.fromStack(stack).hasWarning('/Default', "cron: If you don't pass 'minute', by default the event runs every minute. Pass 'minute: '*'' if that's what you intend, or 'minute: 0' to run once per hour instead.");
});

Expand All @@ -451,7 +446,5 @@ test('Scheduled Fargate Task shows no warning when minute is * in cron', () => {
});

// THEN
expect(stack.node.metadataEntry).toEqual([]);
const annotations = Annotations.fromStack(stack).findWarning('*', Match.anyValue());
expect(annotations.length).toBe(0);
Annotations.fromStack(stack).hasNoWarning('/Default', Match.anyValue());
});
6 changes: 3 additions & 3 deletions packages/@aws-cdk/aws-ecs/test/ec2/ec2-service.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Match, Template } from '@aws-cdk/assertions';
import { Annotations, Match, Template } from '@aws-cdk/assertions';
import * as autoscaling from '@aws-cdk/aws-autoscaling';
import * as ec2 from '@aws-cdk/aws-ec2';
import * as elb from '@aws-cdk/aws-elasticloadbalancing';
Expand Down Expand Up @@ -1252,7 +1252,7 @@ describe('ec2 service', () => {
memoryLimitMiB: 512,
});

const service = new ecs.Ec2Service(stack, 'Ec2Service', {
new ecs.Ec2Service(stack, 'Ec2Service', {
cluster,
taskDefinition,
deploymentController: {
Expand All @@ -1261,7 +1261,7 @@ describe('ec2 service', () => {
});

// THEN
expect(service.node.metadataEntry[0].data).toEqual('taskDefinition and launchType are blanked out when using external deployment controller.');
Annotations.fromStack(stack).hasWarning('/Default/Ec2Service', 'taskDefinition and launchType are blanked out when using external deployment controller.');
Template.fromStack(stack).hasResourceProperties('AWS::ECS::Service', {
Cluster: {
Ref: 'EcsCluster97242B84',
Expand Down
Loading

0 comments on commit d81cce7

Please sign in to comment.