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.

Sign up for GitHub

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

Jump to bottom

chore(ecs-service-extensions): migrate tests to assertions #18649

Merged
merged 2 commits into from
Jan 31, 2022
Merged

chore(ecs-service-extensions): migrate tests to assertions #18649

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
986fb4c
chore(ecs-service-containers): migrate tests to assertions
kaizencc Jan 25, 2022
63c5138
Merge branch 'master' into conroy/ecs-service-containers
mergify[bot] Jan 31, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/@aws-cdk-containers/ecs-service-extensions/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@
"@aws-cdk/cfn2ts": "0.0.0",
"jest": "^27.4.7",
"@aws-cdk/pkglint": "0.0.0",
"@aws-cdk/assert-internal": "0.0.0"
"@aws-cdk/assertions": "0.0.0"
},
"dependencies": {
"@aws-cdk/aws-applicationautoscaling": "0.0.0",
Expand Down
34 changes: 9 additions & 25 deletions packages/@aws-cdk-containers/ecs-service-extensions/test/appmesh.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import '@aws-cdk/assert-internal/jest';
import { Match, Template } from '@aws-cdk/assertions';
import * as appmesh from '@aws-cdk/aws-appmesh';
import * as ecs from '@aws-cdk/aws-ecs';
import * as cdk from '@aws-cdk/core';
Expand Down Expand Up @@ -34,7 +34,7 @@ describe('appmesh', () => {

// THEN
// Ensure that task has an App Mesh sidecar
expect(stack).toHaveResource('AWS::ECS::TaskDefinition', {
Template.fromStack(stack).hasResourceProperties('AWS::ECS::TaskDefinition', {
ContainerDefinitions: [
{
Cpu: 256,
Expand Down Expand Up @@ -210,7 +210,7 @@ describe('appmesh', () => {
});

// Ensure that the service has the right settings
expect(stack).toHaveResource('AWS::ECS::Service', {
Template.fromStack(stack).hasResourceProperties('AWS::ECS::Service', {
Cluster: {
Ref: 'productionenvironmentclusterC6599D2D',
},
Expand Down Expand Up @@ -256,8 +256,6 @@ describe('appmesh', () => {
Ref: 'myservicetaskdefinitionF3E2D86F',
},
});


});

test('should have the right maximumPercentage at desired count == 1', () => {
Expand Down Expand Up @@ -288,15 +286,13 @@ describe('appmesh', () => {
desiredCount: 1,
});

expect(stack).toHaveResourceLike('AWS::ECS::Service', {
Template.fromStack(stack).hasResourceProperties('AWS::ECS::Service', {
DeploymentConfiguration: {
MaximumPercent: 200,
MinimumHealthyPercent: 100,
},
DesiredCount: 1,
});


});

test('should have the right maximumPercentage at desired count == 2', () => {
Expand Down Expand Up @@ -327,15 +323,13 @@ describe('appmesh', () => {
desiredCount: 2,
});

expect(stack).toHaveResourceLike('AWS::ECS::Service', {
Template.fromStack(stack).hasResourceProperties('AWS::ECS::Service', {
DeploymentConfiguration: {
MaximumPercent: 150,
MinimumHealthyPercent: 100,
},
DesiredCount: 2,
});


});

test('should have the right maximumPercentage at desired count == 3', () => {
Expand Down Expand Up @@ -366,15 +360,13 @@ describe('appmesh', () => {
desiredCount: 3,
});

expect(stack).toHaveResourceLike('AWS::ECS::Service', {
Template.fromStack(stack).hasResourceProperties('AWS::ECS::Service', {
DeploymentConfiguration: {
MaximumPercent: 150,
MinimumHealthyPercent: 100,
},
DesiredCount: 3,
});


});

test('should have the right maximumPercentage at desired count == 4', () => {
Expand Down Expand Up @@ -405,15 +397,13 @@ describe('appmesh', () => {
desiredCount: 4,
});

expect(stack).toHaveResourceLike('AWS::ECS::Service', {
Template.fromStack(stack).hasResourceProperties('AWS::ECS::Service', {
DeploymentConfiguration: {
MaximumPercent: 125,
MinimumHealthyPercent: 100,
},
DesiredCount: 4,
});


});

test('should have the right maximumPercentage at desired count > 4', () => {
Expand Down Expand Up @@ -444,15 +434,13 @@ describe('appmesh', () => {
desiredCount: 8,
});

expect(stack).toHaveResourceLike('AWS::ECS::Service', {
Template.fromStack(stack).hasResourceProperties('AWS::ECS::Service', {
DeploymentConfiguration: {
MaximumPercent: 125,
MinimumHealthyPercent: 100,
},
DesiredCount: 8,
});


});

test('should be able to create multiple App Mesh enabled services and connect', () => {
Expand Down Expand Up @@ -516,9 +504,7 @@ describe('appmesh', () => {
greeterService.connectTo(greetingService);

// THEN
expect(stack).toHaveResource('AWS::ECS::TaskDefinition');


Template.fromStack(stack).hasResource('AWS::ECS::TaskDefinition', Match.anyValue());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could also be resourceCountIs( , 1). Which one is better?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the slight difference here is that resourceCountIs(,1) ensures that there is only 1 resource AWS::ECS::TaskDefinition. I'm 99% sure that when I was migrating, this test had multiple task definitions, which is why I migrated it this way.

Maybe its worth thinking about adding comparisons to resourceCountIs? Something like resourecCountIs(, Match.greaterThan(1)).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah okay, thanks for the explanation. Make sense. I think it's worth opening a feature request to Assertions for this one.

});

test('should detect when attempting to connect services from two different envs', () => {
Expand Down Expand Up @@ -572,7 +558,5 @@ describe('appmesh', () => {
expect(() => {
developmentNameService.connectTo(productionNameService);
}).toThrow(/Unable to connect service 'name-development' in environment 'development' to service 'name-production' in environment 'production' because services can not be connected across environment boundaries/);


});
});
18 changes: 4 additions & 14 deletions packages/@aws-cdk-containers/ecs-service-extensions/test/assign-public-ip.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import '@aws-cdk/assert-internal/jest';
import { Template } from '@aws-cdk/assertions';
import * as autoscaling from '@aws-cdk/aws-autoscaling';
import * as ec2 from '@aws-cdk/aws-ec2';
import * as ecs from '@aws-cdk/aws-ecs';
Expand Down Expand Up @@ -29,15 +29,13 @@ describe('assign public ip', () => {
});

// THEN
expect(stack).toHaveResourceLike('AWS::ECS::Service', {
Template.fromStack(stack).hasResourceProperties('AWS::ECS::Service', {
NetworkConfiguration: {
AwsvpcConfiguration: {
AssignPublicIp: 'ENABLED',
},
},
});


});

test('errors when adding a public ip to ec2-backed service', () => {
Expand Down Expand Up @@ -76,8 +74,6 @@ describe('assign public ip', () => {
serviceDescription,
});
}).toThrow(/Fargate/i);


});

test('should not add a task record manager by default', () => {
Expand All @@ -103,8 +99,6 @@ describe('assign public ip', () => {

// THEN
expect(service.ecsService.node.tryFindChild('TaskRecordManager')).toBeUndefined();


});

test('should add a task record manager when dns is requested', () => {
Expand Down Expand Up @@ -138,8 +132,6 @@ describe('assign public ip', () => {

// THEN
expect(service.ecsService.node.tryFindChild('TaskRecordManager')).toBeDefined();


});

test('task record manager listens for ecs events', () => {
Expand Down Expand Up @@ -172,7 +164,7 @@ describe('assign public ip', () => {
});

// THEN
expect(stack).toHaveResourceLike('AWS::Events::Rule', {
Template.fromStack(stack).hasResourceProperties('AWS::Events::Rule', {
EventPattern: {
'source': ['aws.ecs'],
'detail-type': [
Expand All @@ -185,7 +177,7 @@ describe('assign public ip', () => {
},
});

expect(stack).toHaveResourceLike('AWS::Events::Rule', {
Template.fromStack(stack).hasResourceProperties('AWS::Events::Rule', {
EventPattern: {
'source': ['aws.ecs'],
'detail-type': [
Expand All @@ -197,7 +189,5 @@ describe('assign public ip', () => {
},
},
});


});
});
7 changes: 2 additions & 5 deletions packages/@aws-cdk-containers/ecs-service-extensions/test/cloudwatch-agent.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import '@aws-cdk/assert-internal/jest';
import { Template } from '@aws-cdk/assertions';
import * as ecs from '@aws-cdk/aws-ecs';
import * as cdk from '@aws-cdk/core';
import { CloudwatchAgentExtension, Container, Environment, Service, ServiceDescription } from '../lib';
Expand Down Expand Up @@ -27,7 +27,7 @@ describe('cloudwatch agent', () => {
});

// THEN
expect(stack).toHaveResource('AWS::ECS::TaskDefinition', {
Template.fromStack(stack).hasResourceProperties('AWS::ECS::TaskDefinition', {
ContainerDefinitions: [
{
Cpu: 256,
Expand Down Expand Up @@ -102,8 +102,5 @@ describe('cloudwatch agent', () => {
],
},
});


});

});
25 changes: 9 additions & 16 deletions packages/@aws-cdk-containers/ecs-service-extensions/test/container.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import '@aws-cdk/assert-internal/jest';
import { Template } from '@aws-cdk/assertions';
import * as autoscaling from '@aws-cdk/aws-autoscaling';
import * as ec2 from '@aws-cdk/aws-ec2';
import * as ecs from '@aws-cdk/aws-ecs';
Expand Down Expand Up @@ -46,9 +46,9 @@ describe('container', () => {
});

// THEN
expect(stack).toCountResources('AWS::ECS::Service', 1);
Template.fromStack(stack).resourceCountIs('AWS::ECS::Service', 1);

expect(stack).toHaveResource('AWS::ECS::TaskDefinition', {
Template.fromStack(stack).hasResourceProperties('AWS::ECS::TaskDefinition', {
ContainerDefinitions: [
{
Cpu: 256,
Expand Down Expand Up @@ -86,8 +86,6 @@ describe('container', () => {
],
},
});


});

test('should be able to enable default logging behavior - with enable default log driver feature flag', () => {
Expand Down Expand Up @@ -129,12 +127,12 @@ describe('container', () => {
});

// THEN
expect(stack).toCountResources('AWS::ECS::Service', 1);
Template.fromStack(stack).resourceCountIs('AWS::ECS::Service', 1);

// Ensure that the log group was created
expect(stack).toHaveResource('AWS::Logs::LogGroup');
Template.fromStack(stack).resourceCountIs('AWS::Logs::LogGroup', 1);

expect(stack).toHaveResource('AWS::ECS::TaskDefinition', {
Template.fromStack(stack).hasResourceProperties('AWS::ECS::TaskDefinition', {
ContainerDefinitions: [
{
Cpu: 256,
Expand Down Expand Up @@ -184,8 +182,6 @@ describe('container', () => {
],
},
});


});

test('should be able to add user-provided log group in the log driver options', () => {
Expand Down Expand Up @@ -228,12 +224,12 @@ describe('container', () => {
});

// THEN
expect(stack).toCountResources('AWS::ECS::Service', 1);
Template.fromStack(stack).resourceCountIs('AWS::ECS::Service', 1);

// Ensure that the log group was created
expect(stack).toHaveResource('AWS::Logs::LogGroup');
Template.fromStack(stack).resourceCountIs('AWS::Logs::LogGroup', 1);

expect(stack).toHaveResource('AWS::ECS::TaskDefinition', {
Template.fromStack(stack).hasResourceProperties('AWS::ECS::TaskDefinition', {
ContainerDefinitions: [
{
Cpu: 256,
Expand Down Expand Up @@ -283,8 +279,6 @@ describe('container', () => {
],
},
});


});

test('should error when log group is provided in the container extension and another observability extension is added', () => {
Expand Down Expand Up @@ -312,5 +306,4 @@ describe('container', () => {
});
}).toThrow(/Log configuration already specified. You cannot provide a log group for the application container of service 'my-service' while also adding log configuration separately using service extensions./);
});

});
Loading