Skip to content

Commit 07d3aa7

Browse files
authored
fix(integ-runner): update workflow doesn't support resource replacement (#24720)
For the update workflow we deploy the stack update with the `--no-rollback` flag so that the assertion stack will deploy all the way through instead of failing on the first assertion failure. This causes issues if you are deploying an update that includes resource replacement since resource replacement is not allowed when using `--no-rollback` To fix this, this PR splits the stack update deployment into two separate deployments. The first deploys the test case stack and the second deploys the assertion stack with `--no-rollback`. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent 20aeb0f commit 07d3aa7

File tree

4 files changed

+75
-11
lines changed

4 files changed

+75
-11
lines changed

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

+23-5
Original file line numberDiff line numberDiff line change
@@ -285,23 +285,41 @@ export class IntegTestRunner extends IntegRunner {
285285
lookups: this.expectedTestSuite?.enableLookups,
286286
});
287287
}
288-
// now deploy the "actual" test. If there are any assertions
289-
// deploy the assertion stack as well
288+
// now deploy the "actual" test.
290289
this.cdk.deploy({
291290
...deployArgs,
292291
lookups: this.actualTestSuite.enableLookups,
293292
stacks: [
294293
...actualTestCase.stacks,
295-
...actualTestCase.assertionStack ? [actualTestCase.assertionStack] : [],
296294
],
297-
rollback: false,
298295
output: path.relative(this.directory, this.cdkOutDir),
299296
...actualTestCase?.cdkCommandOptions?.deploy?.args,
300-
...actualTestCase.assertionStack ? { outputsFile: path.relative(this.directory, path.join(this.cdkOutDir, 'assertion-results.json')) } : undefined,
301297
context: this.getContext(actualTestCase?.cdkCommandOptions?.deploy?.args?.context),
302298
app: this.cdkApp,
303299
});
304300

301+
// If there are any assertions
302+
// deploy the assertion stack as well
303+
// This is separate from the above deployment because we want to
304+
// set `rollback: false`. This allows the assertion stack to deploy all the
305+
// assertions instead of failing at the first failed assertion
306+
// combining it with the above deployment would prevent any replacement updates
307+
if (actualTestCase.assertionStack) {
308+
this.cdk.deploy({
309+
...deployArgs,
310+
lookups: this.actualTestSuite.enableLookups,
311+
stacks: [
312+
actualTestCase.assertionStack,
313+
],
314+
rollback: false,
315+
output: path.relative(this.directory, this.cdkOutDir),
316+
...actualTestCase?.cdkCommandOptions?.deploy?.args,
317+
outputsFile: path.relative(this.directory, path.join(this.cdkOutDir, 'assertion-results.json')),
318+
context: this.getContext(actualTestCase?.cdkCommandOptions?.deploy?.args?.context),
319+
app: this.cdkApp,
320+
});
321+
}
322+
305323
if (actualTestCase.hooks?.postDeploy) {
306324
actualTestCase.hooks.postDeploy.forEach(cmd => {
307325
exec(chunks(cmd), {

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

+48-6
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ describe('IntegTest runIntegTests', () => {
5151
});
5252

5353
// THEN
54-
expect(cdkMock.mocks.deploy).toHaveBeenCalledTimes(2);
54+
expect(cdkMock.mocks.deploy).toHaveBeenCalledTimes(3);
5555
expect(cdkMock.mocks.destroy).toHaveBeenCalledTimes(1);
5656
expect(cdkMock.mocks.synthFast).toHaveBeenCalledTimes(1);
5757
expect(cdkMock.mocks.deploy).toHaveBeenCalledWith({
@@ -83,7 +83,6 @@ describe('IntegTest runIntegTests', () => {
8383
}),
8484
versionReporting: false,
8585
lookups: false,
86-
rollback: false,
8786
stacks: ['test-stack', 'new-test-stack'],
8887
});
8988
expect(cdkMock.mocks.destroy).toHaveBeenCalledWith({
@@ -130,7 +129,6 @@ describe('IntegTest runIntegTests', () => {
130129
context: expect.not.objectContaining({
131130
[AVAILABILITY_ZONE_FALLBACK_CONTEXT_KEY]: ['test-region-1a', 'test-region-1b', 'test-region-1c'],
132131
}),
133-
rollback: false,
134132
lookups: false,
135133
stacks: ['stack1'],
136134
output: 'cdk-integ.out.xxxxx.integ-test1.js.snapshot',
@@ -176,7 +174,6 @@ describe('IntegTest runIntegTests', () => {
176174
}),
177175
versionReporting: false,
178176
lookups: true,
179-
rollback: false,
180177
stacks: ['test-stack'],
181178
output: 'cdk-integ.out.xxxxx.test-with-snapshot-assets-diff.js.snapshot',
182179
profile: undefined,
@@ -206,6 +203,52 @@ describe('IntegTest runIntegTests', () => {
206203
});
207204
});
208205

206+
test('with an assertion stack', () => {
207+
// WHEN
208+
const integTest = new IntegTestRunner({
209+
cdk: cdkMock.cdk,
210+
test: new IntegTest({
211+
fileName: 'test/test-data/xxxxx.test-with-snapshot.js',
212+
discoveryRoot: 'test/test-data',
213+
}),
214+
});
215+
integTest.runIntegTestCase({
216+
testCaseName: 'xxxxx.test-with-snapshot',
217+
});
218+
219+
// THEN
220+
expect(cdkMock.mocks.deploy).toHaveBeenCalledTimes(3);
221+
expect(cdkMock.mocks.destroy).toHaveBeenCalledTimes(1);
222+
expect(cdkMock.mocks.synthFast).toHaveBeenCalledTimes(1);
223+
expect(cdkMock.mocks.deploy).toHaveBeenNthCalledWith(1, expect.objectContaining({
224+
app: 'xxxxx.test-with-snapshot.js.snapshot',
225+
context: expect.any(Object),
226+
stacks: ['test-stack'],
227+
}));
228+
expect(cdkMock.mocks.deploy).toHaveBeenNthCalledWith(2, expect.not.objectContaining({
229+
rollback: false,
230+
}));
231+
expect(cdkMock.mocks.deploy).toHaveBeenNthCalledWith(3, expect.objectContaining({
232+
app: 'node xxxxx.test-with-snapshot.js',
233+
stacks: ['Bundling/DefaultTest/DeployAssert'],
234+
rollback: false,
235+
}));
236+
expect(cdkMock.mocks.destroy).toHaveBeenCalledWith({
237+
app: 'node xxxxx.test-with-snapshot.js',
238+
pathMetadata: false,
239+
assetMetadata: false,
240+
context: expect.not.objectContaining({
241+
'vpc-provider:account=12345678:filter.isDefault=true:region=test-region:returnAsymmetricSubnets=true': expect.objectContaining({
242+
vpcId: 'vpc-60900905',
243+
}),
244+
}),
245+
versionReporting: false,
246+
force: true,
247+
all: true,
248+
output: 'cdk-integ.out.xxxxx.test-with-snapshot.js.snapshot',
249+
});
250+
});
251+
209252
test('no clean', () => {
210253
// WHEN
211254
const integTest = new IntegTestRunner({
@@ -298,7 +341,6 @@ describe('IntegTest runIntegTests', () => {
298341
}),
299342
}),
300343
profile: 'test-profile',
301-
rollback: false,
302344
lookups: false,
303345
stacks: ['stack1'],
304346
output: 'cdk-integ.out.xxxxx.integ-test1.js.snapshot',
@@ -564,7 +606,7 @@ describe('IntegTest runIntegTests', () => {
564606
});
565607

566608
// THEN
567-
expect(cdkMock.mocks.deploy).toHaveBeenCalledTimes(2);
609+
expect(cdkMock.mocks.deploy).toHaveBeenCalledTimes(3);
568610
expect(cdkMock.mocks.destroy).toHaveBeenCalledTimes(1);
569611
expect(cdkMock.mocks.synthFast).toHaveBeenCalledTimes(1);
570612
expect(cdkMock.mocks.deploy).toHaveBeenCalledWith(expect.objectContaining({

packages/@aws-cdk/integ-runner/test/test-data/cdk-integ.out.xxxxx.test-with-snapshot.js.snapshot/integ.json

+2
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33
"testCases": {
44
"xxxxx.test-with-snapshot": {
55
"stacks": ["test-stack", "new-test-stack"],
6+
"assertionStack": "Bundling/DefaultTest/DeployAssert",
7+
"assertionStackName": "BundlingDefaultTestDeployAssertAACA0CAF",
68
"diffAssets": true,
79
"allowDestroy": [
810
"AWS::IAM::Role"

packages/@aws-cdk/integ-runner/test/test-data/xxxxx.test-with-snapshot.js.snapshot/integ.json

+2
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33
"testCases": {
44
"xxxxx.test-with-snapshot": {
55
"stacks": ["test-stack"],
6+
"assertionStack": "Bundling/DefaultTest/DeployAssert",
7+
"assertionStackName": "BundlingDefaultTestDeployAssertAACA0CAF",
68
"diffAssets": true,
79
"allowDestroy": [
810
"AWS::IAM::Role"

0 commit comments

Comments
 (0)