Skip to content

Commit d0e0ab9

Browse files
corymhallcorymhall
authored
fix(integ-tests): can't enable lookups when creating an IntegTest (#22075)
We were not exposing the `enableLookups` property when creating an `IntegTest`. This also updates `integ-runner` to ensure that we are correctly utilizing `enableLooups`. There was an undiscovered (because you couldn't set `enableLookups=true` :) ) bug which set the dummy context on _every_ command (i.e. deploy, destroy) when it should have only been set when synthing the snapshot. ---- ### 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*
1 parent 5fc7ca2 commit d0e0ab9

File tree

7 files changed

+109
-20
lines changed

7 files changed

+109
-20
lines changed

‎packages/@aws-cdk/integ-runner/lib/runner/integ-test-runner.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,9 @@ export class IntegTestRunner extends IntegRunner {
176176
} else {
177177
const env: Record<string, any> = {
178178
...DEFAULT_SYNTH_OPTIONS.env,
179-
CDK_CONTEXT_JSON: JSON.stringify(this.getContext()),
179+
CDK_CONTEXT_JSON: JSON.stringify(this.getContext({
180+
...this.actualTestSuite.enableLookups ? DEFAULT_SYNTH_OPTIONS.context : {},
181+
})),
180182
};
181183
this.cdk.synthFast({
182184
execCmd: this.cdkApp.split(' '),

‎packages/@aws-cdk/integ-runner/lib/runner/runner-base.ts

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -321,7 +321,7 @@ export abstract class IntegRunner {
321321
execCmd: this.cdkApp.split(' '),
322322
env: {
323323
...DEFAULT_SYNTH_OPTIONS.env,
324-
CDK_CONTEXT_JSON: JSON.stringify(this.getContext()),
324+
CDK_CONTEXT_JSON: JSON.stringify(this.getContext(DEFAULT_SYNTH_OPTIONS.context)),
325325
},
326326
output: path.relative(this.directory, this.snapshotDir),
327327
});
@@ -361,11 +361,7 @@ export abstract class IntegRunner {
361361
.filter(([k, _]) => !FUTURE_FLAGS_EXPIRED.includes(k))
362362
.forEach(([k, v]) => futureFlags[k] = v);
363363

364-
const enableLookups = (this.actualTestSuite ?? this.expectedTestSuite)?.enableLookups;
365364
return {
366-
// if lookups are enabled then we need to synth
367-
// with the "dummy" context
368-
...enableLookups ? DEFAULT_SYNTH_OPTIONS.context : {},
369365
...futureFlags,
370366
...this.legacyContext,
371367
...additionalContext,

‎packages/@aws-cdk/integ-runner/lib/runner/snapshot-test-runner.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,9 @@ export class IntegSnapshotRunner extends IntegRunner {
4141
// to produce the "correct" snapshot
4242
const env = {
4343
...DEFAULT_SYNTH_OPTIONS.env,
44-
CDK_CONTEXT_JSON: JSON.stringify(this.getContext()),
44+
CDK_CONTEXT_JSON: JSON.stringify(this.getContext({
45+
...this.actualTestSuite.enableLookups ? DEFAULT_SYNTH_OPTIONS.context : {},
46+
})),
4547
};
4648
this.cdk.synthFast({
4749
execCmd: this.cdkApp.split(' '),
@@ -85,6 +87,7 @@ export class IntegSnapshotRunner extends IntegRunner {
8587
additionalMessages.push(
8688
'Repro:',
8789
` ${[...envCmd, 'cdk synth', `-a '${this.cdkApp}'`, `-o '${this.cdkOutDir}'`, ...Object.entries(this.getContext()).flatMap(([k, v]) => typeof v !== 'object' ? [`-c '${k}=${v}'`] : [])].join(' ')}`,
90+
8891
);
8992
}
9093

‎packages/@aws-cdk/integ-runner/test/runner/integ-test-runner.test.ts

Lines changed: 28 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,11 @@ describe('IntegTest runIntegTests', () => {
7171
requireApproval: 'never',
7272
pathMetadata: false,
7373
assetMetadata: false,
74-
context: expect.any(Object),
74+
context: expect.not.objectContaining({
75+
'vpc-provider:account=12345678:filter.isDefault=true:region=test-region:returnAsymmetricSubnets=true': expect.objectContaining({
76+
vpcId: 'vpc-60900905',
77+
}),
78+
}),
7579
profile: undefined,
7680
versionReporting: false,
7781
lookups: false,
@@ -84,7 +88,11 @@ describe('IntegTest runIntegTests', () => {
8488
assetMetadata: false,
8589
output: 'cdk-integ.out.test-with-snapshot',
8690
profile: undefined,
87-
context: expect.any(Object),
91+
context: expect.not.objectContaining({
92+
'vpc-provider:account=12345678:filter.isDefault=true:region=test-region:returnAsymmetricSubnets=true': expect.objectContaining({
93+
vpcId: 'vpc-60900905',
94+
}),
95+
}),
8896
versionReporting: false,
8997
lookups: false,
9098
rollback: false,
@@ -94,7 +102,11 @@ describe('IntegTest runIntegTests', () => {
94102
app: 'node xxxxx.test-with-snapshot.js',
95103
pathMetadata: false,
96104
assetMetadata: false,
97-
context: expect.any(Object),
105+
context: expect.not.objectContaining({
106+
'vpc-provider:account=12345678:filter.isDefault=true:region=test-region:returnAsymmetricSubnets=true': expect.objectContaining({
107+
vpcId: 'vpc-60900905',
108+
}),
109+
}),
98110
versionReporting: false,
99111
profile: undefined,
100112
force: true,
@@ -169,7 +181,7 @@ describe('IntegTest runIntegTests', () => {
169181
requireApproval: 'never',
170182
pathMetadata: false,
171183
assetMetadata: false,
172-
context: expect.objectContaining({
184+
context: expect.not.objectContaining({
173185
'vpc-provider:account=12345678:filter.isDefault=true:region=test-region:returnAsymmetricSubnets=true': expect.objectContaining({
174186
vpcId: 'vpc-60900905',
175187
}),
@@ -186,15 +198,15 @@ describe('IntegTest runIntegTests', () => {
186198
env: expect.objectContaining({
187199
CDK_INTEG_ACCOUNT: '12345678',
188200
CDK_INTEG_REGION: 'test-region',
189-
CDK_CONTEXT_JSON: expect.anything(),
201+
CDK_CONTEXT_JSON: expect.stringContaining('"vpcId":"vpc-60900905"'),
190202
}),
191203
output: 'test-with-snapshot-assets-diff.integ.snapshot',
192204
});
193205
expect(destroyMock).toHaveBeenCalledWith({
194206
app: 'node xxxxx.test-with-snapshot-assets-diff.js',
195207
pathMetadata: false,
196208
assetMetadata: false,
197-
context: expect.objectContaining({
209+
context: expect.not.objectContaining({
198210
'vpc-provider:account=12345678:filter.isDefault=true:region=test-region:returnAsymmetricSubnets=true': expect.objectContaining({
199211
vpcId: 'vpc-60900905',
200212
}),
@@ -292,7 +304,11 @@ describe('IntegTest runIntegTests', () => {
292304
pathMetadata: false,
293305
assetMetadata: false,
294306
versionReporting: false,
295-
context: expect.any(Object),
307+
context: expect.not.objectContaining({
308+
'vpc-provider:account=12345678:filter.isDefault=true:region=test-region:returnAsymmetricSubnets=true': expect.objectContaining({
309+
vpcId: 'vpc-60900905',
310+
}),
311+
}),
296312
profile: 'test-profile',
297313
rollback: false,
298314
lookups: false,
@@ -304,7 +320,11 @@ describe('IntegTest runIntegTests', () => {
304320
pathMetadata: false,
305321
assetMetadata: false,
306322
versionReporting: false,
307-
context: expect.any(Object),
323+
context: expect.not.objectContaining({
324+
'vpc-provider:account=12345678:filter.isDefault=true:region=test-region:returnAsymmetricSubnets=true': expect.objectContaining({
325+
vpcId: 'vpc-60900905',
326+
}),
327+
}),
308328
profile: 'test-profile',
309329
force: true,
310330
all: true,

‎packages/@aws-cdk/integ-tests/lib/manifest-synthesizer.ts

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,15 @@ const emptyManifest: IntegManifest = {
99
};
1010

1111
export class IntegManifestSynthesizer {
12-
constructor(private readonly testCases: IntegTestCase[]) {}
12+
constructor(private readonly testCases: IntegTestCase[], private readonly enableLookups?: boolean) {}
1313

1414
synthesize(session: ISynthesisSession) {
15-
const manifest = this.testCases
16-
.map(tc => tc.manifest)
17-
.reduce(mergeManifests, emptyManifest);
15+
const manifest: IntegManifest = {
16+
enableLookups: this.enableLookups,
17+
...this.testCases
18+
.map(tc => tc.manifest)
19+
.reduce(mergeManifests, emptyManifest),
20+
};
1821

1922
const snapshotDir = session.assembly.outdir;
2023

‎packages/@aws-cdk/integ-tests/lib/test-case.ts

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,16 @@ export interface IntegTestProps extends TestOptions {
115115
* List of test cases that make up this test
116116
*/
117117
readonly testCases: Stack[];
118+
119+
/**
120+
* Enable lookups for this test. If lookups are enabled
121+
* then `stackUpdateWorkflow` must be set to false.
122+
* Lookups should only be enabled when you are explicitely testing
123+
* lookups.
124+
*
125+
* @default false
126+
*/
127+
readonly enableLookups?: boolean;
118128
}
119129

120130
/**
@@ -127,9 +137,11 @@ export class IntegTest extends Construct {
127137
*/
128138
public readonly assertions: IDeployAssert;
129139
private readonly testCases: IntegTestCase[];
140+
private readonly enableLookups?: boolean;
130141
constructor(scope: Construct, id: string, props: IntegTestProps) {
131142
super(scope, id);
132143

144+
this.enableLookups = props.enableLookups;
133145
const defaultTestCase = new IntegTestCase(this, 'DefaultTest', {
134146
stacks: props.testCases.filter(stack => !IntegTestCaseStack.isIntegTestCaseStack(stack)),
135147
hooks: props.hooks,
@@ -152,7 +164,7 @@ export class IntegTest extends Construct {
152164
validate: () => {
153165
attachCustomSynthesis(this, {
154166
onSynthesize: (session: ISynthesisSession) => {
155-
const synthesizer = new IntegManifestSynthesizer(this.testCases);
167+
const synthesizer = new IntegManifestSynthesizer(this.testCases, this.enableLookups);
156168
synthesizer.synthesize(session);
157169
},
158170
});

‎packages/@aws-cdk/integ-tests/test/manifest-synthesizer.test.ts

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,59 @@ describe(IntegManifestSynthesizer, () => {
8282
});
8383
});
8484

85+
test('with options', () => {
86+
// GIVEN
87+
const app = new App();
88+
const stack = new Stack(app, 'stack');
89+
90+
// WHEN
91+
new IntegTest(app, 'Integ', {
92+
testCases: [stack],
93+
enableLookups: true,
94+
hooks: {
95+
preDeploy: ['echo "preDeploy"'],
96+
},
97+
diffAssets: true,
98+
allowDestroy: ['AWS::IAM::Role'],
99+
stackUpdateWorkflow: false,
100+
cdkCommandOptions: {
101+
deploy: {
102+
args: {
103+
profile: 'profile',
104+
},
105+
},
106+
},
107+
});
108+
const integAssembly = app.synth();
109+
const integManifest = Manifest.loadIntegManifest(path.join(integAssembly.directory, 'integ.json'));
110+
111+
// THEN
112+
expect(integManifest).toEqual({
113+
version: Manifest.version(),
114+
enableLookups: true,
115+
testCases: {
116+
['Integ/DefaultTest']: {
117+
assertionStack: 'Integ/DefaultTest/DeployAssert',
118+
assertionStackName: 'IntegDefaultTestDeployAssert4E6713E1',
119+
stacks: ['stack'],
120+
hooks: {
121+
preDeploy: ['echo "preDeploy"'],
122+
},
123+
diffAssets: true,
124+
allowDestroy: ['AWS::IAM::Role'],
125+
stackUpdateWorkflow: false,
126+
cdkCommandOptions: {
127+
deploy: {
128+
args: {
129+
profile: 'profile',
130+
},
131+
},
132+
},
133+
},
134+
},
135+
});
136+
});
137+
85138
test('with IntegTestCaseStack', () => {
86139
// GIVEN
87140
const app = new App();

0 commit comments

Comments
 (0)