fix(integ-runner): integ-runner produces snapshot that doesn't validate (#666)

The integ-runner uses `synthFast` to produce the assembly that will be
used to compare the snapshot to, to determine if there is "a change".

However, when a snapshot has changed and a test is run, there is a fork:

- In dry-run mode: `synthFast` will be used to produce the updated
snapshot.
- In real-mode, the result of `this.deploy()` will be the updated
snapshot (but also: the next time it will be compared to the result of
`synthFast`).

We are running into a situation where the results of `synthFast` and
`this.deploy` are different, and snapshot validation fails.

In this change, we always generate the snapshot using `synthFast`, and
ignore the template produced by `this.deploy()`. This makes sure that
the templates will at least compile equal.

I'm not sure why this ever worked this way, because the template that is
deployed can be specialized to an account and region, but the snapshot
can never be, so we need to investigate a little more.


---
By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache-2.0 license

Author: rix0rrr

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

@@ -7,7 +7,7 @@ import * as chokidar from 'chokidar';
 import * as fs from 'fs-extra';
 import * as workerpool from 'workerpool';
 import type { IntegRunnerOptions } from './runner-base';
-import { IntegRunner, DEFAULT_SYNTH_OPTIONS } from './runner-base';
+import { IntegRunner } from './runner-base';
 import * as logger from '../logger';
 import { chunks, exec, promiseWithResolvers } from '../utils';
 import type { DestructiveChange, AssertionResults, AssertionResult } from '../workers/common';
@@ -237,17 +237,11 @@ export class IntegTestRunner extends IntegRunner {
           updateWorkflowEnabled,
           options.testCaseName,
         );
-      } else {
-        await this.cdk.synthFast({
-          execCmd: this.cdkApp.split(' '),
-          context: this.getContext(actualTestSuite.enableLookups ? DEFAULT_SYNTH_OPTIONS.context : {}),
-          env: DEFAULT_SYNTH_OPTIONS.env,
-          output: path.relative(this.directory, this.cdkOutDir),
-        });
       }
+
       // only create the snapshot if there are no failed assertion results
       // (i.e. no failures)
-      if (!assertionResults || !Object.values(assertionResults).some(result => result.status === 'fail')) {
+      if (!Object.values(assertionResults ?? {}).some(result => result.status === 'fail')) {
         await this.createSnapshot();
       }
     } catch (e) {

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

@@ -328,16 +328,12 @@ export abstract class IntegRunner {
 
     // if lookups are enabled then we need to synth again
     // using dummy context and save that as the snapshot
-    if ((await this.actualTestSuite()).enableLookups) {
-      await this.cdk.synthFast({
-        execCmd: this.cdkApp.split(' '),
-        context: this.getContext(DEFAULT_SYNTH_OPTIONS.context),
-        env: DEFAULT_SYNTH_OPTIONS.env,
-        output: path.relative(this.directory, this.snapshotDir),
-      });
-    } else {
-      fs.moveSync(this.cdkOutDir, this.snapshotDir, { overwrite: true });
-    }
+    await this.cdk.synthFast({
+      execCmd: this.cdkApp.split(' '),
+      context: this.getContext(DEFAULT_SYNTH_OPTIONS.context),
+      env: DEFAULT_SYNTH_OPTIONS.env,
+      output: path.relative(this.directory, this.snapshotDir),
+    });
 
     await this.cleanupSnapshot();
   }

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

@@ -59,7 +59,7 @@ describe('IntegTest runIntegTests', () => {
     // THEN
     expect(cdkMock.mocks.deploy).toHaveBeenCalledTimes(3);
     expect(cdkMock.mocks.destroy).toHaveBeenCalledTimes(1);
-    expect(cdkMock.mocks.synthFast).toHaveBeenCalledTimes(1);
+    expect(cdkMock.mocks.synthFast).toHaveBeenCalledTimes(2);
     expect(cdkMock.mocks.deploy).toHaveBeenCalledWith({
       app: 'xxxxx.test-with-snapshot.js.snapshot',
       requireApproval: 'never',
@@ -124,7 +124,7 @@ describe('IntegTest runIntegTests', () => {
     // THEN
     expect(cdkMock.mocks.deploy).toHaveBeenCalledTimes(1);
     expect(cdkMock.mocks.destroy).toHaveBeenCalledTimes(1);
-    expect(cdkMock.mocks.synthFast).toHaveBeenCalledTimes(1);
+    expect(cdkMock.mocks.synthFast).toHaveBeenCalledTimes(2);
     expect(cdkMock.mocks.deploy).toHaveBeenCalledWith({
       app: 'node xxxxx.integ-test1.js',
       requireApproval: 'never',
@@ -229,7 +229,7 @@ describe('IntegTest runIntegTests', () => {
     // THEN
     expect(cdkMock.mocks.deploy).toHaveBeenCalledTimes(3);
     expect(cdkMock.mocks.destroy).toHaveBeenCalledTimes(1);
-    expect(cdkMock.mocks.synthFast).toHaveBeenCalledTimes(1);
+    expect(cdkMock.mocks.synthFast).toHaveBeenCalledTimes(2);
     expect(cdkMock.mocks.deploy).toHaveBeenNthCalledWith(1, expect.objectContaining({
       app: 'xxxxx.test-with-snapshot.js.snapshot',
       context: expect.any(Object),
@@ -276,7 +276,7 @@ describe('IntegTest runIntegTests', () => {
     // THEN
     expect(cdkMock.mocks.deploy).toHaveBeenCalledTimes(1);
     expect(cdkMock.mocks.destroy).toHaveBeenCalledTimes(0);
-    expect(cdkMock.mocks.synthFast).toHaveBeenCalledTimes(1);
+    expect(cdkMock.mocks.synthFast).toHaveBeenCalledTimes(2);
   });
 
   test('dryrun', async () => {
@@ -340,7 +340,7 @@ describe('IntegTest runIntegTests', () => {
     // THEN
     expect(cdkMock.mocks.deploy).toHaveBeenCalledTimes(1);
     expect(cdkMock.mocks.destroy).toHaveBeenCalledTimes(1);
-    expect(cdkMock.mocks.synthFast).toHaveBeenCalledTimes(1);
+    expect(cdkMock.mocks.synthFast).toHaveBeenCalledTimes(2);
     expect(cdkMock.mocks.deploy).toHaveBeenCalledWith({
       app: 'node xxxxx.integ-test1.js',
       requireApproval: 'never',
@@ -623,7 +623,7 @@ describe('IntegTest runIntegTests', () => {
     // THEN
     expect(cdkMock.mocks.deploy).toHaveBeenCalledTimes(3);
     expect(cdkMock.mocks.destroy).toHaveBeenCalledTimes(1);
-    expect(cdkMock.mocks.synthFast).toHaveBeenCalledTimes(1);
+    expect(cdkMock.mocks.synthFast).toHaveBeenCalledTimes(2);
     expect(cdkMock.mocks.deploy).toHaveBeenCalledWith(expect.objectContaining({
       app: 'node --no-warnings xxxxx.test-with-snapshot.js',
     }));