Skip to content

Commit 6cf7a98

Browse files
perf(test): disable Lambda bundling in CDK unit-test synths (#366)
Template.fromStack() triggers a full CDK synth that bundles every NodejsFunction via esbuild — ~28s for the 413-resource AgentStack. Unit tests assert on CloudFormation structure, not bundled Lambda code, so the bundling is pure overhead. A jest setupFiles hook sets aws:cdk:bundling-stacks: [] via CDK_CONTEXT_JSON, which a bare new App() picks up with no call-site changes. Measured: single AgentStack synth 28.7s -> 1.9s (~15x); github-tags.test.ts ~135s -> 9s; full //cdk:test 155s -> 25s (8-core local). All 2177 tests pass; coverage thresholds unchanged (lines 92.36 >= 91). Durable regression guards added: AGENTS.md "Common mistakes" entry and a .abca/commands/review_pr.md checklist item — do not re-enable bundling in tests unless asserting on a bundled asset, and minimize full-stack synths (synth once in beforeAll). Both link docs/design/CI_BUILD_PERFORMANCE.md (PR #364). Refs #366. Part of #363. Likely supersedes the sharding approach (#365) — to be re-evaluated once this lands. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 86fda64 commit 6cf7a98

4 files changed

Lines changed: 55 additions & 0 deletions

File tree

.abca/commands/review_pr.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,12 @@ Then apply principal-architect judgment over the diff:
9797
IDs, cdk-nag clean. Watch for cost and operational footguns.
9898
- **Tests** — Are unit tests added/updated under the matching `*/test/` tree? Do they cover the
9999
new behavior and failure paths, not just the happy path?
100+
- **Test performance (CDK synth)** — New/changed CDK tests must not re-enable Lambda bundling at
101+
synth or synthesize the same stack repeatedly. `cdk/` disables bundling globally via
102+
`test/setup/disable-bundling.ts` (~15× faster synth); flag any test that turns
103+
`aws:cdk:bundling-stacks` back on without asserting on a bundled asset, or that calls
104+
`new App()` + `Template.fromStack()` per-test instead of once in `beforeAll`. See
105+
[CI build performance](../../docs/design/CI_BUILD_PERFORMANCE.md).
100106
- **Routing** — Changes should land in the right package per the AGENTS.md routing table
101107
(agent runtime in `agent/`, API/Lambdas in `cdk/`, CLI in `cli/`).
102108

AGENTS.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ Handler entry tests: `cdk/test/handlers/orchestrate-task.test.ts`, `create-task.
4545
- Adding or editing files in **`docs/design/`** or **`docs/guides/`** without running **`cd docs && node scripts/sync-starlight.mjs`** — CI will reject ("Fail build on mutation") because the Starlight mirror files in `docs/src/content/docs/` are stale. Always commit the regenerated mirrors alongside source changes.
4646
- Changing **`cdk/.../types.ts`** without updating **`cli/src/types.ts`** — CLI and API drift.
4747
- Running raw **`jest`/`tsc`/`cdk`** from muscle memory — prefer **`mise //cdk:test`**, **`mise //cdk:compile`**, **`mise //cdk:synth`** (see [Commands you can use](#commands-you-can-use)).
48+
- **Bundling Lambda assets in CDK unit tests**`Template.fromStack()` triggers a full synth that bundles every `NodejsFunction` via esbuild (~28s for `AgentStack`). Unit tests assert on CloudFormation structure, not bundled code, so this is pure overhead. The `cdk/` Jest config disables it globally via `test/setup/disable-bundling.ts` (sets `aws:cdk:bundling-stacks: []` in `CDK_CONTEXT_JSON`), which a bare `new App()` picks up automatically. **Do not re-enable bundling** in a test unless you are specifically asserting on a bundled asset (hash / S3 key) — and if you must, opt out narrowly in that test's `App` context, not globally. Minimize full-stack synths regardless: synthesize each distinct stack config once in `beforeAll` and assert against the cached `Template`. See [CI build performance](./docs/design/CI_BUILD_PERFORMANCE.md) and #366.
4849
- **`MISE_EXPERIMENTAL=1`** — required for namespaced tasks like **`mise //cdk:build`** (see [CONTRIBUTING.md](./CONTRIBUTING.md)).
4950
- **`mise run build`** builds **`//agent:quality`** alongside **`//cdk:build`** (the deployed image bundles **`agent/`**, so agent quality is part of the build) — these run as parallel `depends`, not in a fixed order; agent changes belong in the **`agent/`** tree.
5051
- **`prek install`** fails if Git **`core.hooksPath`** is set — another hook manager owns hooks; see [CONTRIBUTING.md](./CONTRIBUTING.md).

cdk/package.json

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,9 @@
106106
"watchPathIgnorePatterns": [
107107
"/node_modules/"
108108
],
109+
"setupFiles": [
110+
"<rootDir>/test/setup/disable-bundling.ts"
111+
],
109112
"setupFilesAfterEnv": [
110113
"aws-cdk-lib/testhelpers/jest-autoclean"
111114
],

cdk/test/setup/disable-bundling.ts

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
/**
2+
* MIT No Attribution
3+
*
4+
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
5+
*
6+
* Permission is hereby granted, free of charge, to any person obtaining a copy of
7+
* the Software without restriction, including without limitation the rights to
8+
* use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of
9+
* the Software, and to permit persons to whom the Software is furnished to do so.
10+
*
11+
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
12+
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
13+
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
14+
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
15+
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
16+
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
17+
* SOFTWARE.
18+
*/
19+
20+
// Disable Lambda asset bundling during unit-test synthesis.
21+
//
22+
// `Template.fromStack()` triggers a full CDK synth, which bundles every
23+
// NodejsFunction via esbuild — ~28s for the AgentStack (413 resources). Unit
24+
// tests assert on CloudFormation structure/properties, not bundled Lambda code,
25+
// so bundling is pure overhead here: skipping it cuts a single synth ~15×
26+
// (~28.7s -> ~1.9s). See #366 and docs/design/CI_BUILD_PERFORMANCE.md.
27+
//
28+
// `aws:cdk:bundling-stacks: []` tells the CLI/synth to bundle no stacks. CDK
29+
// reads CDK_CONTEXT_JSON when an `App` is constructed, so a bare `new App()`
30+
// (the overwhelming-majority pattern in our tests) picks this up with no
31+
// call-site changes. Tests that pass their own `{ context }` still merge on top.
32+
//
33+
// If a test ever needs real bundled assets (e.g. asserting on an asset hash /
34+
// S3 key), it must opt out by constructing its `App` with
35+
// `aws:cdk:bundling-stacks: ['**']` (or the specific stack id) in its context.
36+
const BUNDLING_DISABLED_CONTEXT = { 'aws:cdk:bundling-stacks': [] as string[] };
37+
38+
const existing = process.env.CDK_CONTEXT_JSON;
39+
if (existing) {
40+
// Preserve any context already provided; our key wins only if unset.
41+
const merged = { ...BUNDLING_DISABLED_CONTEXT, ...JSON.parse(existing) };
42+
process.env.CDK_CONTEXT_JSON = JSON.stringify(merged);
43+
} else {
44+
process.env.CDK_CONTEXT_JSON = JSON.stringify(BUNDLING_DISABLED_CONTEXT);
45+
}

0 commit comments

Comments
 (0)