fix(integ-runner): update workflow doesn't support resource replaceme…

…nt (#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*
aws · Mar 27, 2023 · 07d3aa7 · 07d3aa7
1 parent 20aeb0f
commit 07d3aa7
Show file tree

Hide file tree

Showing 4 changed files with 75 additions and 11 deletions.
diff --git a/packages/@aws-cdk/integ-runner/lib/runner/integ-test-runner.ts b/packages/@aws-cdk/integ-runner/lib/runner/integ-test-runner.ts
@@ -285,23 +285,41 @@ export class IntegTestRunner extends IntegRunner {
           lookups: this.expectedTestSuite?.enableLookups,
         });
       }
-      // now deploy the "actual" test. If there are any assertions
-      // deploy the assertion stack as well
+      // now deploy the "actual" test.
       this.cdk.deploy({
         ...deployArgs,
         lookups: this.actualTestSuite.enableLookups,
         stacks: [
           ...actualTestCase.stacks,
-          ...actualTestCase.assertionStack ? [actualTestCase.assertionStack] : [],
         ],
-        rollback: false,
         output: path.relative(this.directory, this.cdkOutDir),
         ...actualTestCase?.cdkCommandOptions?.deploy?.args,
-        ...actualTestCase.assertionStack ? { outputsFile: path.relative(this.directory, path.join(this.cdkOutDir, 'assertion-results.json')) } : undefined,
         context: this.getContext(actualTestCase?.cdkCommandOptions?.deploy?.args?.context),
         app: this.cdkApp,
       });
 
+      // If there are any assertions
+      // deploy the assertion stack as well
+      // This is separate from the above deployment because we want to
+      // set `rollback: false`. This allows the assertion stack to deploy all the
+      // assertions instead of failing at the first failed assertion
+      // combining it with the above deployment would prevent any replacement updates
+      if (actualTestCase.assertionStack) {
+        this.cdk.deploy({
+          ...deployArgs,
+          lookups: this.actualTestSuite.enableLookups,
+          stacks: [
+            actualTestCase.assertionStack,
+          ],
+          rollback: false,
+          output: path.relative(this.directory, this.cdkOutDir),
+          ...actualTestCase?.cdkCommandOptions?.deploy?.args,
+          outputsFile: path.relative(this.directory, path.join(this.cdkOutDir, 'assertion-results.json')),
+          context: this.getContext(actualTestCase?.cdkCommandOptions?.deploy?.args?.context),
+          app: this.cdkApp,
+        });
+      }
+
       if (actualTestCase.hooks?.postDeploy) {
         actualTestCase.hooks.postDeploy.forEach(cmd => {
           exec(chunks(cmd), {

diff --git a/packages/@aws-cdk/integ-runner/test/runner/integ-test-runner.test.ts b/packages/@aws-cdk/integ-runner/test/runner/integ-test-runner.test.ts
@@ -51,7 +51,7 @@ describe('IntegTest runIntegTests', () => {
     });
 
     // THEN
-    expect(cdkMock.mocks.deploy).toHaveBeenCalledTimes(2);
+    expect(cdkMock.mocks.deploy).toHaveBeenCalledTimes(3);
     expect(cdkMock.mocks.destroy).toHaveBeenCalledTimes(1);
     expect(cdkMock.mocks.synthFast).toHaveBeenCalledTimes(1);
     expect(cdkMock.mocks.deploy).toHaveBeenCalledWith({
@@ -83,7 +83,6 @@ describe('IntegTest runIntegTests', () => {
       }),
       versionReporting: false,
       lookups: false,
-      rollback: false,
       stacks: ['test-stack', 'new-test-stack'],
     });
     expect(cdkMock.mocks.destroy).toHaveBeenCalledWith({
@@ -130,7 +129,6 @@ describe('IntegTest runIntegTests', () => {
       context: expect.not.objectContaining({
         [AVAILABILITY_ZONE_FALLBACK_CONTEXT_KEY]: ['test-region-1a', 'test-region-1b', 'test-region-1c'],
       }),
-      rollback: false,
       lookups: false,
       stacks: ['stack1'],
       output: 'cdk-integ.out.xxxxx.integ-test1.js.snapshot',
@@ -176,7 +174,6 @@ describe('IntegTest runIntegTests', () => {
       }),
       versionReporting: false,
       lookups: true,
-      rollback: false,
       stacks: ['test-stack'],
       output: 'cdk-integ.out.xxxxx.test-with-snapshot-assets-diff.js.snapshot',
       profile: undefined,
@@ -206,6 +203,52 @@ describe('IntegTest runIntegTests', () => {
     });
   });
 
+  test('with an assertion stack', () => {
+    // WHEN
+    const integTest = new IntegTestRunner({
+      cdk: cdkMock.cdk,
+      test: new IntegTest({
+        fileName: 'test/test-data/xxxxx.test-with-snapshot.js',
+        discoveryRoot: 'test/test-data',
+      }),
+    });
+    integTest.runIntegTestCase({
+      testCaseName: 'xxxxx.test-with-snapshot',
+    });
+
+    // THEN
+    expect(cdkMock.mocks.deploy).toHaveBeenCalledTimes(3);
+    expect(cdkMock.mocks.destroy).toHaveBeenCalledTimes(1);
+    expect(cdkMock.mocks.synthFast).toHaveBeenCalledTimes(1);
+    expect(cdkMock.mocks.deploy).toHaveBeenNthCalledWith(1, expect.objectContaining({
+      app: 'xxxxx.test-with-snapshot.js.snapshot',
+      context: expect.any(Object),
+      stacks: ['test-stack'],
+    }));
+    expect(cdkMock.mocks.deploy).toHaveBeenNthCalledWith(2, expect.not.objectContaining({
+      rollback: false,
+    }));
+    expect(cdkMock.mocks.deploy).toHaveBeenNthCalledWith(3, expect.objectContaining({
+      app: 'node xxxxx.test-with-snapshot.js',
+      stacks: ['Bundling/DefaultTest/DeployAssert'],
+      rollback: false,
+    }));
+    expect(cdkMock.mocks.destroy).toHaveBeenCalledWith({
+      app: 'node xxxxx.test-with-snapshot.js',
+      pathMetadata: false,
+      assetMetadata: false,
+      context: expect.not.objectContaining({
+        'vpc-provider:account=12345678:filter.isDefault=true:region=test-region:returnAsymmetricSubnets=true': expect.objectContaining({
+          vpcId: 'vpc-60900905',
+        }),
+      }),
+      versionReporting: false,
+      force: true,
+      all: true,
+      output: 'cdk-integ.out.xxxxx.test-with-snapshot.js.snapshot',
+    });
+  });
+
   test('no clean', () => {
     // WHEN
     const integTest = new IntegTestRunner({
@@ -298,7 +341,6 @@ describe('IntegTest runIntegTests', () => {
         }),
       }),
       profile: 'test-profile',
-      rollback: false,
       lookups: false,
       stacks: ['stack1'],
       output: 'cdk-integ.out.xxxxx.integ-test1.js.snapshot',
@@ -564,7 +606,7 @@ describe('IntegTest runIntegTests', () => {
     });
 
     // THEN
-    expect(cdkMock.mocks.deploy).toHaveBeenCalledTimes(2);
+    expect(cdkMock.mocks.deploy).toHaveBeenCalledTimes(3);
     expect(cdkMock.mocks.destroy).toHaveBeenCalledTimes(1);
     expect(cdkMock.mocks.synthFast).toHaveBeenCalledTimes(1);
     expect(cdkMock.mocks.deploy).toHaveBeenCalledWith(expect.objectContaining({

diff --git a/...integ-runner/test/test-data/cdk-integ.out.xxxxx.test-with-snapshot.js.snapshot/integ.json b/...integ-runner/test/test-data/cdk-integ.out.xxxxx.test-with-snapshot.js.snapshot/integ.json
@@ -3,6 +3,8 @@
   "testCases": {
     "xxxxx.test-with-snapshot": {
       "stacks": ["test-stack", "new-test-stack"],
+      "assertionStack": "Bundling/DefaultTest/DeployAssert",
+      "assertionStackName": "BundlingDefaultTestDeployAssertAACA0CAF",
       "diffAssets": true,
       "allowDestroy": [
         "AWS::IAM::Role"

diff --git a/...ages/@aws-cdk/integ-runner/test/test-data/xxxxx.test-with-snapshot.js.snapshot/integ.json b/...ages/@aws-cdk/integ-runner/test/test-data/xxxxx.test-with-snapshot.js.snapshot/integ.json
@@ -3,6 +3,8 @@
   "testCases": {
     "xxxxx.test-with-snapshot": {
       "stacks": ["test-stack"],
+      "assertionStack": "Bundling/DefaultTest/DeployAssert",
+      "assertionStackName": "BundlingDefaultTestDeployAssertAACA0CAF",
       "diffAssets": true,
       "allowDestroy": [
         "AWS::IAM::Role"