From 01099564b0f1006ec35d6e624c6dd217bdbb24af Mon Sep 17 00:00:00 2001 From: Mike Cowgill Date: Tue, 15 Jan 2019 21:16:06 -0800 Subject: [PATCH] refactoring to add in a TestStack for easy unit testing when aspects are involved --- .../test/test.pipeline-deploy-stack-action.ts | 4 +-- packages/@aws-cdk/assert/lib/expect.ts | 9 ++++++ packages/@aws-cdk/assert/lib/index.ts | 1 + packages/@aws-cdk/assert/lib/test-stack.ts | 7 +++++ .../aws-autoscaling/lib/auto-scaling-group.ts | 4 ++- .../test/test.scheduled-action.ts | 11 ++------ .../aws-cloudtrail/test/test.cloudtrail.ts | 4 +-- packages/@aws-cdk/aws-ec2/test/test.vpc.ts | 16 ++--------- .../test/ec2/integ.lb-awsvpc-nw.expected.json | 6 ++++ .../test/ec2/integ.lb-bridge-nw.expected.json | 6 ++++ .../@aws-cdk/aws-ecs/test/test.ecs-cluster.ts | 4 +-- packages/@aws-cdk/aws-kms/test/test.key.ts | 28 +++++++------------ 12 files changed, 53 insertions(+), 47 deletions(-) create mode 100644 packages/@aws-cdk/assert/lib/test-stack.ts diff --git a/packages/@aws-cdk/app-delivery/test/test.pipeline-deploy-stack-action.ts b/packages/@aws-cdk/app-delivery/test/test.pipeline-deploy-stack-action.ts index 25f41261ec849..14ba06ce442ee 100644 --- a/packages/@aws-cdk/app-delivery/test/test.pipeline-deploy-stack-action.ts +++ b/packages/@aws-cdk/app-delivery/test/test.pipeline-deploy-stack-action.ts @@ -9,7 +9,7 @@ import cxapi = require('@aws-cdk/cx-api'); import fc = require('fast-check'); import nodeunit = require('nodeunit'); -import { countResources, expect, haveResource, isSuperObject } from '@aws-cdk/assert'; +import { countResources, expect, haveResource, isSuperObject, TestStack } from '@aws-cdk/assert'; import { PipelineDeployStackAction } from '../lib/pipeline-deploy-stack-action'; interface SelfUpdatingPipeline { @@ -299,7 +299,7 @@ class FakeAction extends api.Action { } function getTestStack(): cdk.Stack { - return new cdk.Stack(undefined, 'TestStack', { env: { account: '123456789012', region: 'us-east-1' } }); + return new TestStack(undefined, 'TestStack', { env: { account: '123456789012', region: 'us-east-1' } }); } function createSelfUpdatingStack(pipelineStack: cdk.Stack): SelfUpdatingPipeline { diff --git a/packages/@aws-cdk/assert/lib/expect.ts b/packages/@aws-cdk/assert/lib/expect.ts index a983da0d96cd3..caf6e60123ee4 100644 --- a/packages/@aws-cdk/assert/lib/expect.ts +++ b/packages/@aws-cdk/assert/lib/expect.ts @@ -1,6 +1,7 @@ import cdk = require('@aws-cdk/cdk'); import api = require('@aws-cdk/cx-api'); import { StackInspector } from './inspector'; +import { TestStack } from './test-stack'; export function expect(stack: api.SynthesizedStack | cdk.Stack, skipValidation = false): StackInspector { // Can't use 'instanceof' here, that breaks if we have multiple copies @@ -18,6 +19,10 @@ export function expect(stack: api.SynthesizedStack | cdk.Stack, skipValidation = } } + if (isTestStack(stack)) { + stack.testInvokeAspects(); + } + sstack = { name: stack.name, template: stack.toCloudFormation(), @@ -50,3 +55,7 @@ function collectStackMetadata(root: cdk.ConstructNode): api.StackMetadata { } return result; } + +function isTestStack(t: any): t is TestStack { + return t.testInvokeAspects !== undefined; +} diff --git a/packages/@aws-cdk/assert/lib/index.ts b/packages/@aws-cdk/assert/lib/index.ts index e811cd2e0bb0d..426ce5c9bcf19 100644 --- a/packages/@aws-cdk/assert/lib/index.ts +++ b/packages/@aws-cdk/assert/lib/index.ts @@ -1,6 +1,7 @@ export * from './assertion'; export * from './expect'; export * from './inspector'; +export * from './test-stack'; export * from './assertions/exist'; export * from './assertions/have-resource'; diff --git a/packages/@aws-cdk/assert/lib/test-stack.ts b/packages/@aws-cdk/assert/lib/test-stack.ts new file mode 100644 index 0000000000000..3c58062176ab9 --- /dev/null +++ b/packages/@aws-cdk/assert/lib/test-stack.ts @@ -0,0 +1,7 @@ +import cdk = require('@aws-cdk/cdk'); + +export class TestStack extends cdk.Stack { + public testInvokeAspects() { + this.invokeAspects(); + } +} diff --git a/packages/@aws-cdk/aws-autoscaling/lib/auto-scaling-group.ts b/packages/@aws-cdk/aws-autoscaling/lib/auto-scaling-group.ts index 0b091505e01f4..3c040d65e633a 100644 --- a/packages/@aws-cdk/aws-autoscaling/lib/auto-scaling-group.ts +++ b/packages/@aws-cdk/aws-autoscaling/lib/auto-scaling-group.ts @@ -199,7 +199,9 @@ export class AutoScalingGroup extends cdk.Construct implements IAutoScalingGroup }); this.connections = new ec2.Connections({ securityGroups: [this.securityGroup] }); this.securityGroups.push(this.securityGroup); - this.apply(new cdk.Tag(NAME_TAG, this.node.path, { applyToLaunchInstances: true })); + this.apply(new cdk.Tag(NAME_TAG, this.node.path, { + applyToLaunchInstances: true, + })); this.role = new iam.Role(this, 'InstanceRole', { assumedBy: new iam.ServicePrincipal('ec2.amazonaws.com') diff --git a/packages/@aws-cdk/aws-autoscaling/test/test.scheduled-action.ts b/packages/@aws-cdk/aws-autoscaling/test/test.scheduled-action.ts index 770c57cc29c6a..afe70277416ac 100644 --- a/packages/@aws-cdk/aws-autoscaling/test/test.scheduled-action.ts +++ b/packages/@aws-cdk/aws-autoscaling/test/test.scheduled-action.ts @@ -1,4 +1,4 @@ -import { expect, haveResource, MatchStyle } from '@aws-cdk/assert'; +import { expect, haveResource, MatchStyle, TestStack } from '@aws-cdk/assert'; import ec2 = require('@aws-cdk/aws-ec2'); import cdk = require('@aws-cdk/cdk'); import { Test } from 'nodeunit'; @@ -7,7 +7,7 @@ import autoscaling = require('../lib'); export = { 'can schedule an action'(test: Test) { // GIVEN - const stack = new cdk.Stack(); + const stack = new TestStack(); const asg = makeAutoScalingGroup(stack); // WHEN @@ -27,7 +27,7 @@ export = { 'correctly formats date objects'(test: Test) { // GIVEN - const stack = new cdk.Stack(); + const stack = new TestStack(); const asg = makeAutoScalingGroup(stack); // WHEN @@ -114,11 +114,6 @@ function makeAutoScalingGroup(scope: cdk.Construct) { }); } -class TestStack extends cdk.Stack { - public testInvokeAspects(): void { - this.invokeAspects(); - } -} function getTestStack(): TestStack { return new TestStack(undefined, 'TestStack', { env: { account: '1234', region: 'us-east-1' } }); } diff --git a/packages/@aws-cdk/aws-cloudtrail/test/test.cloudtrail.ts b/packages/@aws-cdk/aws-cloudtrail/test/test.cloudtrail.ts index ae6066ec43ff4..7a6212234e2aa 100644 --- a/packages/@aws-cdk/aws-cloudtrail/test/test.cloudtrail.ts +++ b/packages/@aws-cdk/aws-cloudtrail/test/test.cloudtrail.ts @@ -1,4 +1,4 @@ -import { expect, haveResource, not } from '@aws-cdk/assert'; +import { expect, haveResource, not, TestStack } from '@aws-cdk/assert'; import { Stack } from '@aws-cdk/cdk'; import { Test } from 'nodeunit'; import { CloudTrail, LogRetention, ReadWriteType } from '../lib'; @@ -147,5 +147,5 @@ export = { }; function getTestStack(): Stack { - return new Stack(undefined, 'TestStack', { env: { account: '123456789012', region: 'us-east-1' } }); + return new TestStack(undefined, 'TestStack', { env: { account: '123456789012', region: 'us-east-1' } }); } diff --git a/packages/@aws-cdk/aws-ec2/test/test.vpc.ts b/packages/@aws-cdk/aws-ec2/test/test.vpc.ts index 4c9767d332a87..5af8408d46fce 100644 --- a/packages/@aws-cdk/aws-ec2/test/test.vpc.ts +++ b/packages/@aws-cdk/aws-ec2/test/test.vpc.ts @@ -1,5 +1,5 @@ -import { countResources, expect, haveResource, haveResourceLike, isSuperObject } from '@aws-cdk/assert'; -import { AvailabilityZoneProvider, Construct, Stack, Tag } from '@aws-cdk/cdk'; +import { countResources, expect, haveResource, haveResourceLike, isSuperObject, TestStack } from '@aws-cdk/assert'; +import { AvailabilityZoneProvider, Construct, Tag } from '@aws-cdk/cdk'; import { Test } from 'nodeunit'; import { CfnVPC, DefaultInstanceTenancy, IVpcNetwork, SubnetType, VpcNetwork } from '../lib'; @@ -28,7 +28,6 @@ export = { 'the Name tag is defaulted to path'(test: Test) { const stack = getTestStack(); new VpcNetwork(stack, 'TheVPC'); - stack.testInvokeAspects(); expect(stack).to( haveResource('AWS::EC2::VPC', hasTags( [ {Key: 'Name', Value: 'TheVPC'} ])) @@ -294,7 +293,6 @@ export = { subnetName: 'egress' }, }); - stack.testInvokeAspects(); expect(stack).to(countResources("AWS::EC2::NatGateway", 3)); for (let i = 1; i < 4; i++) { expect(stack).to(haveResource('AWS::EC2::Subnet', hasTags([{ @@ -353,7 +351,6 @@ export = { // overwrite to set propagate vpc.apply(new Tag('BusinessUnit', 'Marketing', {include: [CfnVPC.resourceTypeName]})); vpc.apply(new Tag('VpcType', 'Good')); - stack.testInvokeAspects(); expect(stack).to(haveResource("AWS::EC2::VPC", hasTags(toCfnTags(allTags)))); const taggables = ['Subnet', 'InternetGateway', 'NatGateway', 'RouteTable']; const propTags = toCfnTags(tags); @@ -367,7 +364,6 @@ export = { 'Subnet Name will propagate to route tables and NATGW'(test: Test) { const stack = getTestStack(); const vpc = new VpcNetwork(stack, 'TheVPC'); - stack.testInvokeAspects(); for (const subnet of vpc.publicSubnets) { const tag = {Key: 'Name', Value: subnet.node.path}; expect(stack).to(haveResource('AWS::EC2::NatGateway', hasTags([tag]))); @@ -384,10 +380,8 @@ export = { const vpc = new VpcNetwork(stack, 'TheVPC'); const tag = {Key: 'Late', Value: 'Adder'}; - stack.testInvokeAspects(); expect(stack).notTo(haveResource('AWS::EC2::VPC', hasTags([tag]))); vpc.apply(new Tag(tag.Key, tag.Value)); - stack.testInvokeAspects(); expect(stack).to(haveResource('AWS::EC2::VPC', hasTags([tag]))); test.done(); }, @@ -536,12 +530,6 @@ export = { }, }; -class TestStack extends Stack { - public testInvokeAspects(): void { - this.invokeAspects(); - } -} - function getTestStack(): TestStack { return new TestStack(undefined, 'TestStack', { env: { account: '123456789012', region: 'us-east-1' } }); } diff --git a/packages/@aws-cdk/aws-ecs/test/ec2/integ.lb-awsvpc-nw.expected.json b/packages/@aws-cdk/aws-ecs/test/ec2/integ.lb-awsvpc-nw.expected.json index fc5ddba7d30ee..cff577e35c37d 100644 --- a/packages/@aws-cdk/aws-ecs/test/ec2/integ.lb-awsvpc-nw.expected.json +++ b/packages/@aws-cdk/aws-ecs/test/ec2/integ.lb-awsvpc-nw.expected.json @@ -608,6 +608,12 @@ } } }, + "Tags": [ + { + "Key": "Name", + "Value": "aws-ecs-integ/EcsCluster/DefaultAutoScalingGroup" + } + ], "Timeout": 310 }, "DependsOn": [ diff --git a/packages/@aws-cdk/aws-ecs/test/ec2/integ.lb-bridge-nw.expected.json b/packages/@aws-cdk/aws-ecs/test/ec2/integ.lb-bridge-nw.expected.json index 4e80a7b3cdb0f..31e990b4fb5d9 100644 --- a/packages/@aws-cdk/aws-ecs/test/ec2/integ.lb-bridge-nw.expected.json +++ b/packages/@aws-cdk/aws-ecs/test/ec2/integ.lb-bridge-nw.expected.json @@ -629,6 +629,12 @@ } } }, + "Tags": [ + { + "Key": "Name", + "Value": "aws-ecs-integ-ecs/EcsCluster/DefaultAutoScalingGroup" + } + ], "Timeout": 310 }, "DependsOn": [ diff --git a/packages/@aws-cdk/aws-ecs/test/test.ecs-cluster.ts b/packages/@aws-cdk/aws-ecs/test/test.ecs-cluster.ts index 8748b7cadbc87..ca3181e8665fa 100644 --- a/packages/@aws-cdk/aws-ecs/test/test.ecs-cluster.ts +++ b/packages/@aws-cdk/aws-ecs/test/test.ecs-cluster.ts @@ -1,4 +1,4 @@ -import { expect, haveResource } from '@aws-cdk/assert'; +import { expect, haveResource, TestStack } from '@aws-cdk/assert'; import ec2 = require('@aws-cdk/aws-ec2'); import { InstanceType } from '@aws-cdk/aws-ec2'; import cdk = require('@aws-cdk/cdk'); @@ -9,7 +9,7 @@ export = { "When creating an ECS Cluster": { "with only required properties set, it correctly sets default properties"(test: Test) { // GIVEN - const stack = new cdk.Stack(); + const stack = new TestStack(); const vpc = new ec2.VpcNetwork(stack, 'MyVpc', {}); const cluster = new ecs.Cluster(stack, 'EcsCluster', { vpc, diff --git a/packages/@aws-cdk/aws-kms/test/test.key.ts b/packages/@aws-cdk/aws-kms/test/test.key.ts index 7ba07a37e171e..98859138bf705 100644 --- a/packages/@aws-cdk/aws-kms/test/test.key.ts +++ b/packages/@aws-cdk/aws-kms/test/test.key.ts @@ -1,13 +1,13 @@ -import { exactlyMatchTemplate, expect } from '@aws-cdk/assert'; +import { exactlyMatchTemplate, expect, TestStack } from '@aws-cdk/assert'; import { PolicyDocument, PolicyStatement } from '@aws-cdk/aws-iam'; -import { App, Stack, Tag } from '@aws-cdk/cdk'; +import { App, Tag } from '@aws-cdk/cdk'; import { Test } from 'nodeunit'; import { EncryptionKey } from '../lib'; export = { 'default key'(test: Test) { const app = new App(); - const stack = new Stack(app, 'TestStack'); + const stack = new TestStack(app, 'TestStack'); new EncryptionKey(stack, 'MyKey'); @@ -67,7 +67,7 @@ export = { 'default with some permission'(test: Test) { const app = new App(); - const stack = new Stack(app, 'Test'); + const stack = new TestStack(app, 'Test'); const key = new EncryptionKey(stack, 'MyKey'); const p = new PolicyStatement().addAllResources().addAction('kms:encrypt'); @@ -138,9 +138,7 @@ export = { }, 'key with some options'(test: Test) { - // const app = getTestApp(); const app = new App(); - // const stack = new Stack(app, 'Test'); const stack = new TestStack(app, 'Test'); const key = new EncryptionKey(stack, 'MyKey', { @@ -154,7 +152,7 @@ export = { key.apply(new Tag('tag1', 'value1')); key.apply(new Tag('tag2', 'value2')); key.apply(new Tag('tag3', '')); - // app.testInvokeAspects(); + stack.testInvokeAspects(); expect(app.synthesizeStack(stack.name)).to(exactlyMatchTemplate({ Resources: { @@ -237,7 +235,7 @@ export = { 'addAlias creates an alias'(test: Test) { const app = new App(); - const stack = new Stack(app, 'Test'); + const stack = new TestStack(app, 'Test'); const key = new EncryptionKey(stack, 'MyKey', { enableKeyRotation: true, @@ -317,7 +315,7 @@ export = { }, 'import/export can be used to bring in an existing key'(test: Test) { - const stack1 = new Stack(); + const stack1 = new TestStack(); const policy = new PolicyDocument(); policy.addStatement(new PolicyStatement().addAllResources()); const myKey = new EncryptionKey(stack1, 'MyKey', { policy }); @@ -356,7 +354,7 @@ export = { } }); - const stack2 = new Stack(); + const stack2 = new TestStack(); const myKeyImported = EncryptionKey.import(stack2, 'MyKeyImported', exportedKeyRef); // addAlias can be called on imported keys. @@ -381,7 +379,7 @@ export = { 'addToResourcePolicy allowNoOp and there is no policy': { 'succeed if set to true (default)'(test: Test) { - const stack = new Stack(); + const stack = new TestStack(); const key = EncryptionKey.import(stack, 'Imported', { keyArn: 'foo/bar' }); @@ -392,7 +390,7 @@ export = { 'fails if set to false'(test: Test) { - const stack = new Stack(); + const stack = new TestStack(); const key = EncryptionKey.import(stack, 'Imported', { keyArn: 'foo/bar' }); @@ -405,9 +403,3 @@ export = { } } }; - -class TestStack extends Stack { - public testInvokeAspects(): void { - this.invokeAspects(); - } -}