Skip to content

Commit

Permalink
chore: remove testLegacyBehavior and update testFutureBehavior tests (#…
Browse files Browse the repository at this point in the history 
…21949)

testLegacyBehavior only runs tests in v1 and skips them in v2, so none of these tests were running anyway. testFutureBehavior just runs the tests with the feature-flag as true, which is the default to these in v2 anyway. I'm removing these functions and their uses so that contributors aren't mislead to think they should use them.

I also fixed some spacing issues in test packages. The diff on this is kind of hard to follow but the summary of the change is this:
- all testLegacyBehavior tests have been deleted
- all testFutureBehavior tests have been updated to be standard tests
- describe blocks were removed where they wrapped around legacy or future sets of tests
- spacing fixed


----

### All Submissions:

* [ ] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md)

### Adding new Unconventional Dependencies:

* [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies)

### New Features

* [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
	* [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)?

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
@TheRealAmazonKendra
TheRealAmazonKendra authored Sep 7, 2022
1 parent 809e1b0 commit b0ba52e
Show file tree
Hide file tree
Showing 30 changed files with 425 additions and 2,621 deletions.
55 changes: 25 additions & 30 deletions packages/@aws-cdk/aws-apigateway/test/usage-plan.test.ts
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
import { Template } from '@aws-cdk/assertions';
import { testFutureBehavior } from '@aws-cdk/cdk-build-tools/lib/feature-flag';
import * as cdk from '@aws-cdk/core';
import * as cxapi from '@aws-cdk/cx-api';
import * as apigateway from '../lib';

const RESOURCE_TYPE = 'AWS::ApiGateway::UsagePlan';
Expand Down Expand Up @@ -298,35 +296,32 @@ describe('usage plan', () => {
expect(logicalIds).toEqual(['mylogicalid']);
});

describe('future flag: @aws-cdk/aws-apigateway:usagePlanKeyOrderInsensitiveId', () => {
const flags = { [cxapi.APIGATEWAY_USAGEPLANKEY_ORDERINSENSITIVE_ID]: true };

testFutureBehavior('UsagePlanKeys have unique logical ids', flags, cdk.App, (app) => {
// GIVEN
const stack = new cdk.Stack(app, 'my-stack');
const usagePlan = new apigateway.UsagePlan(stack, 'my-usage-plan');
const apiKey1 = new apigateway.ApiKey(stack, 'my-api-key-1', {
apiKeyName: 'my-api-key-1',
});
const apiKey2 = new apigateway.ApiKey(stack, 'my-api-key-2', {
apiKeyName: 'my-api-key-2',
});

// WHEN
usagePlan.addApiKey(apiKey1);
usagePlan.addApiKey(apiKey2);

// THEN
const template = app.synth().getStackByName(stack.stackName).template;
const logicalIds = Object.entries(template.Resources)
.filter(([_, v]) => (v as any).Type === 'AWS::ApiGateway::UsagePlanKey')
.map(([k, _]) => k);

expect(logicalIds).toEqual([
'myusageplanUsagePlanKeyResourcemystackmyapikey1EE9AA1B359121274',
'myusageplanUsagePlanKeyResourcemystackmyapikey2B4E8EB1456DC88E9',
]);
test('UsagePlanKeys have unique logical ids', () => {
// GIVEN
const app = new cdk.App();
const stack = new cdk.Stack(app, 'my-stack');
const usagePlan = new apigateway.UsagePlan(stack, 'my-usage-plan');
const apiKey1 = new apigateway.ApiKey(stack, 'my-api-key-1', {
apiKeyName: 'my-api-key-1',
});
const apiKey2 = new apigateway.ApiKey(stack, 'my-api-key-2', {
apiKeyName: 'my-api-key-2',
});

// WHEN
usagePlan.addApiKey(apiKey1);
usagePlan.addApiKey(apiKey2);

// THEN
const template = app.synth().getStackByName(stack.stackName).template;
const logicalIds = Object.entries(template.Resources)
.filter(([_, v]) => (v as any).Type === 'AWS::ApiGateway::UsagePlanKey')
.map(([k, _]) => k);

expect(logicalIds).toEqual([
'myusageplanUsagePlanKeyResourcemystackmyapikey1EE9AA1B359121274',
'myusageplanUsagePlanKeyResourcemystackmyapikey2B4E8EB1456DC88E9',
]);
});
});
});
81 changes: 18 additions & 63 deletions packages/@aws-cdk/aws-cloudfront/test/distribution.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,7 @@ import { Match, Template } from '@aws-cdk/assertions';
import * as acm from '@aws-cdk/aws-certificatemanager';
import * as lambda from '@aws-cdk/aws-lambda';
import * as s3 from '@aws-cdk/aws-s3';
import { testFutureBehavior, testLegacyBehavior } from '@aws-cdk/cdk-build-tools/lib/feature-flag';
import { App, Duration, Stack } from '@aws-cdk/core';
import { CLOUDFRONT_DEFAULT_SECURITY_POLICY_TLS_V1_2_2021 } from '@aws-cdk/cx-api';
import {
CfnDistribution,
Distribution,
Expand Down Expand Up @@ -289,7 +287,6 @@ ellipsis so a user would know there was more to ...`,
});

describe('multiple behaviors', () => {

test('a second behavior can\'t be specified with the catch-all path pattern', () => {
const origin = defaultOrigin();

Expand Down Expand Up @@ -443,7 +440,6 @@ describe('multiple behaviors', () => {
});

describe('certificates', () => {

test('should fail if using an imported certificate from outside of us-east-1', () => {
const origin = defaultOrigin();
const certificate = acm.Certificate.fromCertificateArn(stack, 'Cert', 'arn:aws:acm:eu-west-1:123456789012:certificate/12345678-1234-1234-1234-123456789012');
Expand Down Expand Up @@ -475,61 +471,25 @@ describe('certificates', () => {
}).toThrow(/Must specify at least one domain name/);
});

describe('adding a certificate and domain renders the correct ViewerCertificate and Aliases property', () => {
testFutureBehavior(
'when @aws-cdk/aws-cloudfront:defaultSecurityPolicyTLSv1.2_2021 is enabled, use the TLSv1.2_2021 security policy by default',
{ [CLOUDFRONT_DEFAULT_SECURITY_POLICY_TLS_V1_2_2021]: true },
App,
(customApp) => {
const customStack = new Stack(customApp);

const certificate = acm.Certificate.fromCertificateArn(customStack, 'Cert', 'arn:aws:acm:us-east-1:123456789012:certificate/12345678-1234-1234-1234-123456789012');

new Distribution(customStack, 'Dist', {
defaultBehavior: { origin: defaultOrigin() },
domainNames: ['example.com', 'www.example.com'],
certificate,
});

Template.fromStack(customStack).hasResourceProperties('AWS::CloudFront::Distribution', {
DistributionConfig: {
Aliases: ['example.com', 'www.example.com'],
ViewerCertificate: {
AcmCertificateArn: 'arn:aws:acm:us-east-1:123456789012:certificate/12345678-1234-1234-1234-123456789012',
SslSupportMethod: 'sni-only',
MinimumProtocolVersion: 'TLSv1.2_2021',
},
},
});
},
);

testLegacyBehavior(
'when @aws-cdk/aws-cloudfront:defaultSecurityPolicyTLSv1.2_2021 is disabled, use the TLSv1.2_2019 security policy by default',
App,
(customApp) => {
const customStack = new Stack(customApp);

const certificate = acm.Certificate.fromCertificateArn(customStack, 'Cert', 'arn:aws:acm:us-east-1:123456789012:certificate/12345678-1234-1234-1234-123456789012');

new Distribution(customStack, 'Dist', {
defaultBehavior: { origin: defaultOrigin() },
domainNames: ['example.com', 'www.example.com'],
certificate,
});

Template.fromStack(customStack).hasResourceProperties('AWS::CloudFront::Distribution', {
DistributionConfig: {
Aliases: ['example.com', 'www.example.com'],
ViewerCertificate: {
AcmCertificateArn: 'arn:aws:acm:us-east-1:123456789012:certificate/12345678-1234-1234-1234-123456789012',
SslSupportMethod: 'sni-only',
MinimumProtocolVersion: 'TLSv1.2_2019',
},
},
});
test('use the TLSv1.2_2021 security policy by default', () => {
const certificate = acm.Certificate.fromCertificateArn(stack, 'Cert', 'arn:aws:acm:us-east-1:123456789012:certificate/12345678-1234-1234-1234-123456789012');

new Distribution(stack, 'Dist', {
defaultBehavior: { origin: defaultOrigin() },
domainNames: ['example.com', 'www.example.com'],
certificate,
});

Template.fromStack(stack).hasResourceProperties('AWS::CloudFront::Distribution', {
DistributionConfig: {
Aliases: ['example.com', 'www.example.com'],
ViewerCertificate: {
AcmCertificateArn: 'arn:aws:acm:us-east-1:123456789012:certificate/12345678-1234-1234-1234-123456789012',
SslSupportMethod: 'sni-only',
MinimumProtocolVersion: 'TLSv1.2_2021',
},
},
);
});
});

test('adding a certificate with non default security policy protocol', () => {
Expand All @@ -552,11 +512,9 @@ describe('certificates', () => {
},
});
});

});

describe('custom error responses', () => {

test('should fail if only the error code is provided', () => {
const origin = defaultOrigin();

Expand Down Expand Up @@ -611,7 +569,6 @@ describe('custom error responses', () => {
},
});
});

});

describe('logging', () => {
Expand Down Expand Up @@ -915,7 +872,6 @@ describe('with Lambda@Edge functions', () => {
});

describe('with CloudFront functions', () => {

test('can add a CloudFront function to the default behavior', () => {
new Distribution(stack, 'MyDist', {
defaultBehavior: {
Expand Down Expand Up @@ -949,7 +905,6 @@ describe('with CloudFront functions', () => {
},
});
});

});

test('price class is included if provided', () => {
Expand Down
12 changes: 4 additions & 8 deletions packages/@aws-cdk/aws-codepipeline-actions/test/lambda/lambda-invoke-action.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,11 @@ import * as codepipeline from '@aws-cdk/aws-codepipeline';
import * as lambda from '@aws-cdk/aws-lambda';
import * as s3 from '@aws-cdk/aws-s3';
import * as sns from '@aws-cdk/aws-sns';
import { testFutureBehavior } from '@aws-cdk/cdk-build-tools/lib/feature-flag';
import { App, Aws, Lazy, SecretValue, Stack, Token } from '@aws-cdk/core';
import * as cxapi from '@aws-cdk/cx-api';
import * as cpactions from '../../lib';

/* eslint-disable quote-props */

const s3GrantWriteCtx = { [cxapi.S3_GRANT_WRITE_WITHOUT_ACL]: true };

describe('', () => {
describe('Lambda invoke Action', () => {
test('properly serializes the object passed in userParameters', () => {
Expand Down Expand Up @@ -160,11 +156,11 @@ describe('', () => {
}));
});

testFutureBehavior("assigns the Action's Role with write permissions to the Bucket if it has only outputs", s3GrantWriteCtx, App, (app) => {
test("assigns the Action's Role with write permissions to the Bucket if it has only outputs", () => {
const stack = stackIncludingLambdaInvokeCodePipeline({
lambdaOutput: new codepipeline.Artifact(),
// no input to the Lambda Action - we want write permissions only in this case
}, app);
});

Template.fromStack(stack).hasResourceProperties('AWS::IAM::Policy', {
PolicyName: 'PipelineInvokeLambdaCodePipelineActionRoleDefaultPolicy103F34DA',
Expand Down Expand Up @@ -205,11 +201,11 @@ describe('', () => {
});
});

testFutureBehavior("assigns the Action's Role with read-write permissions to the Bucket if it has both inputs and outputs", s3GrantWriteCtx, App, (app) => {
test("assigns the Action's Role with read-write permissions to the Bucket if it has both inputs and outputs", () => {
const stack = stackIncludingLambdaInvokeCodePipeline({
lambdaInput: new codepipeline.Artifact(),
lambdaOutput: new codepipeline.Artifact(),
}, app);
});

Template.fromStack(stack).hasResourceProperties('AWS::IAM::Policy', {
PolicyName: 'PipelineInvokeLambdaCodePipelineActionRoleDefaultPolicy103F34DA',
Expand Down
Loading

0 comments on commit b0ba52e

Please sign in to comment.