Skip to content

Commit 3f6ff1f

Browse files
docs(test): reframe bundling as overhead-plus-smoke-test, not "pure overhead" (#366)
PR #371 review finding 3: "pure overhead" undersold the tradeoff. esbuild bundling also validates that every Lambda `entry` resolves and the handler compiles, so disabling it in unit tests means a broken entry path or handler TS error stops surfacing at `Template.fromStack()` and only fails at real synth/deploy or in the `agent/` build. The tradeoff is still acceptable — real synth/deploy bundles, and handler code has its own compile/test under `agent/` — but the wording now names what unit tests cede. Updated in both places the phrase lived: the disable-bundling.ts header and AGENTS.md. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 77b0c7a commit 3f6ff1f

2 files changed

Lines changed: 5 additions & 3 deletions

File tree

AGENTS.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +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. This does not stop tests from synthesizing — `Template.fromStack()` still runs a full synth; it only skips the esbuild step. **Do not re-enable bundling** in a test unless you are specifically asserting on the bundled-asset output (hash / S3 key) — and if you must, opt out narrowly via that test's `App` `postCliContext` (e.g. `new App({ postCliContext: { 'aws:cdk:bundling-stacks': ['**'] } })`), not globally. Note that constructor `context` does **not** work for this key: CDK overwrites it with `CDK_CONTEXT_JSON`, so only `postCliContext` (applied last) overrides the global disable. Minimize full-stack synths regardless: synthesize each distinct stack config once in `beforeAll` and assert against the cached `Template`. See #366.
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 for those assertions it is overhead — plus a smoke-test of Lambda `entry` resolution and handler compilation that unit tests intentionally cede to the `agent/` build (a broken entry path or handler TS error stops surfacing at `Template.fromStack()` and instead fails at real synth/deploy or in the `agent/` build). 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. This does not stop tests from synthesizing — `Template.fromStack()` still runs a full synth; it only skips the esbuild step. **Do not re-enable bundling** in a test unless you are specifically asserting on the bundled-asset output (hash / S3 key) — and if you must, opt out narrowly via that test's `App` `postCliContext` (e.g. `new App({ postCliContext: { 'aws:cdk:bundling-stacks': ['**'] } })`), not globally. Note that constructor `context` does **not** work for this key: CDK overwrites it with `CDK_CONTEXT_JSON`, so only `postCliContext` (applied last) overrides the global disable. Minimize full-stack synths regardless: synthesize each distinct stack config once in `beforeAll` and assert against the cached `Template`. See #366.
4949
- **`MISE_EXPERIMENTAL=1`** — required for namespaced tasks like **`mise //cdk:build`** (see [CONTRIBUTING.md](./CONTRIBUTING.md)).
5050
- **`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.
5151
- **`prek install`** fails if Git **`core.hooksPath`** is set — another hook manager owns hooks; see [CONTRIBUTING.md](./CONTRIBUTING.md).

cdk/test/setup/disable-bundling.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,10 @@
2222
// `Template.fromStack()` triggers a full CDK synth, which bundles every
2323
// NodejsFunction via esbuild — ~28s for the AgentStack (413 resources). Unit
2424
// 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.
25+
// so for those assertions bundling is overhead — plus a smoke-test of Lambda
26+
// `entry` resolution and handler compilation that unit tests intentionally cede
27+
// to the `agent/` build (real synth/deploy still bundles). Skipping it cuts a
28+
// single synth ~15× (~28.7s -> ~1.9s). See #366.
2729
//
2830
// `aws:cdk:bundling-stacks: []` tells the CLI/synth to bundle no stacks. CDK
2931
// reads CDK_CONTEXT_JSON when an `App` is constructed, so a bare `new App()`

0 commit comments

Comments
 (0)