Skip to content

Commit

Permalink
chore(s3): migrate tests to assertions (#18079)
Browse files Browse the repository at this point in the history
----

*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 Dec 18, 2021
1 parent 6a6bf65 commit bdd91cb
Show file tree
Hide file tree
Showing 9 changed files with 123 additions and 283 deletions.
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-s3/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@
},
"license": "Apache-2.0",
"devDependencies": {
"@aws-cdk/assert-internal": "0.0.0",
"@aws-cdk/assertions": "0.0.0",
"@aws-cdk/cdk-build-tools": "0.0.0",
"@aws-cdk/cdk-integ-tools": "0.0.0",
"@aws-cdk/cfn2ts": "0.0.0",
Expand Down
16 changes: 6 additions & 10 deletions packages/@aws-cdk/aws-s3/test/aspect.test.ts
Original file line number Diff line number Diff line change
@@ -1,29 +1,27 @@
import '@aws-cdk/assert-internal/jest';
import { SynthUtils } from '@aws-cdk/assert-internal';
import * as cdk from '@aws-cdk/core';
import { IConstruct } from 'constructs';
import * as s3 from '../lib';

describe('aspect', () => {
test('bucket must have versioning: failure', () => {
// GIVEN
const stack = new cdk.Stack();
const app = new cdk.App();
const stack = new cdk.Stack(app);
new s3.Bucket(stack, 'MyBucket');

// WHEN
cdk.Aspects.of(stack).add(new BucketVersioningChecker());

// THEN
const assembly = SynthUtils.synthesize(stack);
const assembly = app.synth().getStackArtifact(stack.artifactId);
const errorMessage = assembly.messages.find(m => m.entry.data === 'Bucket versioning is not enabled');
expect(errorMessage).toBeDefined();


});

test('bucket must have versioning: success', () => {
// GIVEN
const stack = new cdk.Stack();
const app = new cdk.App();
const stack = new cdk.Stack(app);
new s3.Bucket(stack, 'MyBucket', {
versioned: true,
});
Expand All @@ -32,10 +30,8 @@ describe('aspect', () => {
cdk.Aspects.of(stack).add(new BucketVersioningChecker());

// THEN
const assembly = SynthUtils.synthesize(stack);
const assembly = app.synth().getStackArtifact(stack.artifactId);
expect(assembly.messages.length).toEqual(0);


});
});

Expand Down
18 changes: 4 additions & 14 deletions packages/@aws-cdk/aws-s3/test/bucket-policy.test.ts
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 { AnyPrincipal, PolicyStatement } from '@aws-cdk/aws-iam';
import { RemovalPolicy, Stack, App } from '@aws-cdk/core';
import * as s3 from '../lib';
Expand All @@ -20,7 +20,7 @@ describe('bucket policy', () => {
principals: [new AnyPrincipal()],
}));

expect(stack).toHaveResource('AWS::S3::BucketPolicy', {
Template.fromStack(stack).hasResourceProperties('AWS::S3::BucketPolicy', {
Bucket: {
'Ref': 'MyBucketF68F3FF0',
},
Expand All @@ -36,8 +36,6 @@ describe('bucket policy', () => {
],
},
});


});

test('when specifying a removalPolicy at creation', () => {
Expand All @@ -54,7 +52,7 @@ describe('bucket policy', () => {
principals: [new AnyPrincipal()],
}));

expect(stack).toMatchTemplate({
Template.fromStack(stack).templateMatches({
'Resources': {
'MyBucketF68F3FF0': {
'Type': 'AWS::S3::Bucket',
Expand Down Expand Up @@ -84,8 +82,6 @@ describe('bucket policy', () => {
},
},
});


});

test('when specifying a removalPolicy after creation', () => {
Expand All @@ -99,7 +95,7 @@ describe('bucket policy', () => {
}));
myBucket.policy?.applyRemovalPolicy(RemovalPolicy.RETAIN);

expect(stack).toMatchTemplate({
Template.fromStack(stack).templateMatches({
'Resources': {
'MyBucketF68F3FF0': {
'Type': 'AWS::S3::Bucket',
Expand Down Expand Up @@ -129,8 +125,6 @@ describe('bucket policy', () => {
},
},
});


});

test('fails if bucket policy has no actions', () => {
Expand All @@ -143,8 +137,6 @@ describe('bucket policy', () => {
}));

expect(() => app.synth()).toThrow(/A PolicyStatement must specify at least one \'action\' or \'notAction\'/);


});

test('fails if bucket policy has no IAM principals', () => {
Expand All @@ -157,7 +149,5 @@ describe('bucket policy', () => {
}));

expect(() => app.synth()).toThrow(/A PolicyStatement used in a resource-based policy must specify at least one IAM principal/);


});
});
Loading

0 comments on commit bdd91cb

Please sign in to comment.