Skip to content

Commit 656f93c

Browse files
committed
docs(designs): revert PR 2 location to tests/integration/golden_templates
The previous commit moved the subprocess runner under tests/regression/, but tests/regression/ is reserved for sam-vs-aws-cli parity tests (deploy_regression compares sam deploy output to aws cloudformation deploy output; package_regression does the same for package). Goldens assert the opposite: sam CLI output matches its own pinned past output, not aws-cli's current output. Putting them under tests/regression would conflate two different test semantics and invite future contributors to misuse the suite. Move back to tests/integration/golden_templates/ alongside the existing tests/integration/buildcmd/, tests/integration/package/, and tests/integration/deploy/ — all subprocess-driven SAM CLI behavior tests, which is exactly what the integration tier is. Adds back the requirement for a new \`golden-templates\` entry in integration-tests.yml's matrix (one extra line); explicitly documents why tests/regression/ would be the wrong home so the choice stands up to future review.
1 parent d4a649a commit 656f93c

1 file changed

Lines changed: 37 additions & 27 deletions

File tree

designs/golden_templates_and_semver_check.md

Lines changed: 37 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -29,13 +29,13 @@ What will be changed?
2929
---------------------
3030

3131
Add a new `tests/golden/` corpus and harness (per-PR unit tier), a daily
32-
subprocess-driven tier under `tests/regression/golden_templates/`, and a
32+
subprocess-driven tier under `tests/integration/golden_templates/`, and a
3333
tiny standalone GitHub workflow that enforces the semver rule. The corpus
3434
covers both LE and non-LE templates across four axes: SAM resources,
3535
packageable CFN resources, LE intrinsics, and cross-cutting features.
3636

3737
The two tiers share `tests/golden/templates/` as their single corpus
38-
source — the regression tier reads case directories straight from the
38+
source — the integration tier reads case directories straight from the
3939
unit tier so authoring a new case lands in both places at once.
4040

4141
Success criteria for the change
@@ -57,7 +57,7 @@ Success criteria for the change
5757
Out-of-Scope
5858
------------
5959

60-
- No deploy step. The regression tier stops at packaged-template output
60+
- No deploy step. The integration tier stops at packaged-template output
6161
— no CloudFormation, no real Lambdas.
6262
- No CFN compatibility check (e.g. `cfn-lint`, `ValidateTemplate`) at this
6363
stage. The corpus pins what *SAM-CLI* produces; CFN-side parity is a
@@ -143,19 +143,23 @@ tests/golden/ # NEW
143143
├── language_extensions/<case>/... # LE axis
144144
└── cross_cutting/<case>/... # nested stacks, AWS::Include, etc.
145145
146-
tests/regression/golden_templates/ # NEW
146+
tests/integration/golden_templates/ # NEW
147147
├── test_golden_subprocess.py # imports tests/golden/templates as data
148148
└── structural_compare.py # looser comparator for real-CLI output
149149
```
150150

151151
The `tests/golden/templates/` directory is the single source of truth for
152-
the corpus. Both the unit tier (`tests/golden/`) and the regression tier
153-
(`tests/regression/golden_templates/`) read from it; nothing is duplicated.
154-
155-
The regression tier sits alongside the existing `tests/regression/deploy/`
156-
and `tests/regression/package/` suites — they're structurally similar:
157-
real `sam` subprocess invocation, real AWS credentials via the
158-
integration-tests workflow's OIDC role, daily-only execution.
152+
the corpus. Both the unit tier (`tests/golden/`) and the integration tier
153+
(`tests/integration/golden_templates/`) read from it; nothing is duplicated.
154+
155+
The integration tier sits alongside the existing
156+
`tests/integration/buildcmd/`, `tests/integration/package/`, and
157+
`tests/integration/deploy/` suites. ``tests/regression/`` is reserved for
158+
``sam`` vs ``aws cloudformation`` parity tests — a different semantic
159+
that compares SAM CLI output against the AWS CLI's current output —
160+
which is not what the golden corpus asserts. Goldens compare SAM CLI
161+
against its own pinned past behavior, so they belong under
162+
``tests/integration/``.
159163

160164
### Per-case directory layout
161165

@@ -206,7 +210,7 @@ no `boto3` stubbing.
206210
- Single trailing newline.
207211

208212
The harness invokes the same high-level functions the CLI invokes; it does
209-
not re-implement the pipeline. The regression tier verifies that contract
213+
not re-implement the pipeline. The integration tier verifies that contract
210214
holds.
211215

212216
### Pytest collection
@@ -320,15 +324,21 @@ estimated <30s for ~50 cases. The existing Windows `TEMP=D:\Temp` shim
320324
already handles temp-dir contention. No new workflow file (the semver gate
321325
above is its own workflow, but the corpus tests ride on `build.yml`).
322326

323-
### Regression tier
327+
### Integration tier
328+
329+
Daily run on the existing `integration-tests.yml` workflow's matrix
330+
(one new entry: `golden-templates`). The suite sits alongside the
331+
existing `tests/integration/buildcmd/`, `tests/integration/package/`,
332+
and `tests/integration/deploy/` directories — all subprocess-driven
333+
``sam`` invocations against real AWS resources.
324334

325-
Daily run on the existing `integration-tests.yml` workflow. The
326-
workflow's `other-and-e2e` matrix entry already collects
327-
`tests/regression/`, so adding `tests/regression/golden_templates/` is
328-
picked up automatically — **no new matrix entry, no workflow edit**. The
329-
suite sits alongside the existing `tests/regression/deploy/` and
330-
`tests/regression/package/` directories, which have the same shape (real
331-
`sam` subprocess + real AWS credentials, daily-only).
335+
We deliberately do **not** put this under ``tests/regression/``. The
336+
existing ``tests/regression/`` suites are ``sam`` vs
337+
``aws cloudformation`` parity tests — they assert SAM CLI's output
338+
matches AWS CLI's *current* output. Goldens assert the opposite: SAM
339+
CLI's output matches its own *pinned past* output. Different semantics;
340+
sharing a directory would invite future contributors to confuse the
341+
two.
332342

333343
Per case:
334344

@@ -347,7 +357,7 @@ Per case:
347357
dict-vs-string differences (the #9029 bug class).
348358

349359
The comparator lives in
350-
`tests/regression/golden_templates/structural_compare.py` and is reused
360+
`tests/integration/golden_templates/structural_compare.py` and is reused
351361
for both build and package outputs.
352362

353363
#### Why structural, not byte-exact
@@ -359,7 +369,7 @@ output) or constant churn from environmental noise. Structural comparison
359369
answers "did the user-visible output shape change?" — which is what catches
360370
the four bugs we just fixed.
361371

362-
If the unit tier passes but the regression tier fails for a case, the
372+
If the unit tier passes but the integration tier fails for a case, the
363373
in-process harness has drifted from the CLI surface — investigation, not
364374
a re-pin.
365375

@@ -369,9 +379,9 @@ Each PR is independently reviewable; LOC estimates are upper-bound net diff.
369379

370380
- **PR 0** (this design doc).
371381
- **PR 1** — Harness skeleton, three sentinel cases, semver gate workflow.
372-
- **PR 2** — Regression-tier subprocess runner under
373-
`tests/regression/golden_templates/`. Picked up by the existing
374-
`other-and-e2e` matrix entry — no workflow edit required.
382+
- **PR 2** — Integration-tier subprocess runner under
383+
`tests/integration/golden_templates/`. Adds a new `golden-templates`
384+
entry to the existing `integration-tests.yml` matrix.
375385
- **PR 3** — SAM resources axis: Function variants (~10 cases).
376386
- **PR 4** — SAM resources axis: Api / HttpApi / StateMachine / others
377387
(~10 cases).
@@ -404,7 +414,7 @@ Security Considerations
404414
-----------------------
405415

406416
The harness intentionally invokes only in-process SAM-CLI code; no
407-
network calls, no credentials, no AWS API access. The regression tier
417+
network calls, no credentials, no AWS API access. The integration tier
408418
uses the existing integration-tests workflow's IAM role and S3 bucket;
409419
no new permissions required.
410420

@@ -423,7 +433,7 @@ Subprocess-only harness (no in-process)
423433
Every golden runs the real `sam` CLI as a subprocess. Rejected: process
424434
startup × N templates is too slow for a per-PR gate; flakier on Windows;
425435
deterministic package output without `moto` is hard. Subprocess invocation
426-
is kept, but only at the daily regression tier where speed and
436+
is kept, but only at the daily integration tier where speed and
427437
determinism matter less.
428438

429439
Snapshot plugin (`syrupy`, `pytest-snapshot`)

0 commit comments

Comments
 (0)