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

chore: remove testLegacyBehavior and update testFutureBehavior tests #21949

Merged
merged 4 commits into from
Sep 7, 2022
Merged

chore: remove testLegacyBehavior and update testFutureBehavior tests #21949

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d644f7c
delete testLegacyBehavior
TheRealAmazonKendra Sep 7, 2022
1c6f1e3
remove testFutureBehavior
TheRealAmazonKendra Sep 7, 2022
33edad9
remove reference from index
TheRealAmazonKendra Sep 7, 2022
fb52b2d
Merge branch 'main' into TheRealAmazonKendra/update-feature-flag-tests
mergify[bot] Sep 7, 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
55 changes: 25 additions & 30 deletions packages/@aws-cdk/aws-apigateway/test/usage-plan.test.ts
View file
Open in desktop
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
Open in desktop
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', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't we still need to test the legacy behavior though? For some of the feature flags it is possible to still set the flag to false.

Not sure if it would be better to keep testLegacyBehavior or just set the feature flag through node.setContext()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are places that the feature flags are set as false to do that very thing. testLegacyBehavior, however, just skips the tests in v2, it doesn't set the flags to false.

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
Open in desktop
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