integ-runner always writes enableLookups: true to the golden snapshot manifest, ignoring the test's real value
- Dominant language
- TypeScript
- Stars
- 105
- Forks
- 122
- Avg merge
- 1d 17h
- Merged PRs (30d)
- 71
Description
### Describe the bug
`CdkTestApp.cleanupGoldenSnapshot` unconditionally overwrites the test suite's `enableLookups` with a hardcoded `true` before saving the golden `integ.json` manifest:
https://github.com/aws/aws-cdk-cli/blob/main/packages/@aws-cdk/integ-runner/lib/runner/cdk-test-app.ts#L594-L605
```ts
const actualTestSuite = this.testSuite;
actualTestSuite.enableLookups = true; // <-- always true, regardless of the test's real setting
if (actualTestSuite instanceof LegacyIntegTestSuite) {
actualTestSuite.saveManifest(this.outputDirectory, this.legacyContext);
} else if (actualTestSuite instanceof IntegTestSuite) {
actualTestSuite.saveManifest(this.outputDirectory);
}
```
At this point `actualTestSuite.enableLookups` already correctly reflects the test's real setting — for a legacy test it comes from parsing the `pragma:enable-lookups` pragma in the test source (`LegacyIntegTestSuite.fromLegacy`); for a modern test it comes from the test's own synthesized `integ.json` (`IntegTestSuite.fromPath` → `IntegManifestReader`). The `= true` assignment clobbers that correct value right before persisting it.
### Impact
- Every golden snapshot's `integ.json` gets `"enableLookups": true` written into it, regardless of whether the test actually enables lookups.
- On the next run, `SnapshotTestRunner`/`synthForSnapshotComparison` reads this value back as its initial guess (`expectedSuite.enableLookups ?? true`) for whether to seed synth with lookup context. For any test whose real `enableLookups` is `false`, this wrong guess forces `synthForSnapshotComparison` to detect a mismatch and re-synth a second time on every single run — wasted work on every CI run for every such test.
- The persisted manifest permanently misrepresents the test's actual configuration, which is also used by the update workflow.
### Reproduction Steps / confirmation
`packages/@aws-cdk/integ-runner/test/test-data/xxxxx.test-with-snapshot.js` has no `pragma:enable-lookups`. Running it through `IntegTestRunner.runIntegTestCase` (which internally calls `cleanupGoldenSnapshot`) and inspecting the `Manifest.saveIntegManifest` call shows `enableLookups: true` written for this test, even though it should be `false`.
### Expected Behavior
The golden manifest should record the test's actual `enableLookups` value.
### Possible Solution
Remove the hardcoded `actualTestSuite.enableLookups = true;` line — the value is already correctly populated by the time `cleanupGoldenSnapshot` runs.
### Additional information / context
Found while auditing the integ-runner's snapshot-management logic. I have a fix + regression test ready and will open a PR referencing this issue.
### CDK CLI Version
latest (main)
### Framework Version
N/A
### Node.js Version
N/A
### OS
N/A
### Language
TypeScript
### Language Version
N/A
### Other information
N/A
Contributor guide
Research direction
Start in packages/@aws-cdk/integ-runner/lib/runner/cdk-test-app.ts around cleanupGoldenSnapshot and inspect the existing test data at packages/@aws-cdk/integ-runner/test/test-data/xxxxx.test-with-snapshot.js. Run the reproduction through IntegTestRunner.runIntegTestCase and inspect the Manifest.saveIntegManifest call. Done means the golden integ.json preserves the test's actual enableLookups value and a regression test covers the false case.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- typescript
- Domain
- cli, testing
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 35/100