From 9da385311db08d5ca9851df8e45233f803083268 Mon Sep 17 00:00:00 2001 From: Edwin van den Houdt Date: Tue, 18 Aug 2026 20:28:03 +0200 Subject: [PATCH] docs: close WP-71, record verification result Co-Authored-By: Claude Sonnet 5 --- docs/project/backlog/README.md | 1 + .../backlog/WP-71-test-framework-coherence.md | 134 ++++++++++++++++++ 2 files changed, 135 insertions(+) create mode 100644 docs/project/backlog/WP-71-test-framework-coherence.md diff --git a/docs/project/backlog/README.md b/docs/project/backlog/README.md index 200ac83..e5b4fd7 100644 --- a/docs/project/backlog/README.md +++ b/docs/project/backlog/README.md @@ -121,6 +121,7 @@ for its existing violations, so every WP ends green. | [WP-68](WP-68-ddd-aggregate-hardening.md) | Aggregate invariants + status modelling (architecture review) | 12 · DDD hardening | done | | [WP-69](WP-69-intake-scholing-threshold-enforcement.md) | Enforce the scholing threshold server-side | 12 · DDD hardening | todo | | [WP-70](WP-70-test-data-builders.md) | Test-data builders: illegal fixtures unrepresentable (ADR-0006) | 12 · DDD hardening | done | +| [WP-71](WP-71-test-framework-coherence.md) | Test framework coherence: BDD/DDD alignment, close the escape hatches | 12 · DDD hardening | done | Sequencing dependencies (stated in the WPs too): 01 before 10–15 (axe covers story churn); 03/04 before 05–09 (boundaries stop new violations during refactors); 06 before 07 (typed diff --git a/docs/project/backlog/WP-71-test-framework-coherence.md b/docs/project/backlog/WP-71-test-framework-coherence.md new file mode 100644 index 0000000..96b30f6 --- /dev/null +++ b/docs/project/backlog/WP-71-test-framework-coherence.md @@ -0,0 +1,134 @@ +# WP-71 — Test framework coherence: BDD/DDD alignment + closing the illegal-state escape hatches + +Status: done (b937e55..3652ff8) +Phase: 12 — DDD hardening + +## Why + +WP-70 shipped test-data builders and ADR-0006, then a three-angle audit (BDD conventions, +DDD alignment, type-safety of test code) asked whether the framework as a whole is "the best +way to set up testing." It was not — and the gap was not where WP-70 looked. + +**WP-70 built the door but left the walls open.** `unwrapOk` had zero call sites; `given()` +was adopted in 4 specs. Meanwhile 76 `as any` casts survived in the three biggest wizard +specs, and the reason was systemic: `eslint.config.mjs` blanket-exempted every `*.spec.ts` +from the `any` ban, and no gate anywhere ran `tsc --noEmit` over spec files, so a wrong cast +could never fail the build. One assertion +(`org-template.machine.spec.ts`, `s.tag === 'loaded' && s.dirty`) passed vacuously whenever +the tag was wrong. + +Alongside that: the documented "never assert on `$localize` copy" ban was broken in 5 files; +`bdd.mdx` mis-cited its own exemplar as "one transition per test"; `layers.mdx` still taught +the pre-WP-67 six-context structure with no `apps/`+`libs/` split; backend tests were +organised by technical concern (`RuleTests.cs` held 5 aggregates as nested classes) rather +than by aggregate; and duplicated FE/BE rules (the scholing threshold, the phone-format regex) +had no test spanning the seam, so they could silently diverge. + +## Read first + +- `docs/reference/architecture/0006-test-data-builders.md` (WP-70's ADR). +- `libs/shared/docs/bdd.mdx`, `libs/shared/docs/layers.mdx` (both rewritten by this WP). +- `backend/tests/BigRegister.Tests/Acceptance/BesluitLifecycleTests.cs` — the canonical G/W/T + shape both docs now point at. + +## Decisions (pre-made, don't relitigate) + +1. **No Gherkin/Cucumber.** Feature files bind steps by runtime string matching, which + directly undoes the compile-time guarantees WP-70 added, and need two frameworks + (.NET + TS) for an audience of developers, not scenario-co-authoring stakeholders. Instead: + generate a business-readable behaviour spec FROM the test names (`gen-behaviour-spec.mjs`, + modeled on the existing `gen-snippets.mjs`), gated for drift in CI. Test names stay the + single source of truth. +2. **Playwright stays** — no change to the e2e framework. +3. **Given/When/Then becomes the default structure for ALL tests** (user override of the + audit's initial recommendation). `bdd.mdx`'s old "no G/W/T ceremony" clause is removed and + inverted; ADR-0006 already matched. Present-tense declarative naming is unchanged. The + _retrofit_ in this WP covers the acceptance specs, the canonical exemplars, and every file + the other tracks already opened — not a mechanical sweep of all ~600 tests (tracked as a + follow-up). +4. **Hardening = one helper + four gates.** `expectTag(state, tag)` replaces every unsafe + narrowing cast; the ESLint spec exemption is removed; `npm run typecheck` is added; a + dependency-cruiser rule keeps test helpers out of production. +5. **FE/BE seam: document + one worked pattern**, not full seam coverage. `check-seam.sh` + catches the scholing-threshold literal drift; the other three divergences (phone regex, + disjoint eligibility fixtures, toelichting rule) are documented, not fixed. Scholing + _enforcement_ stays WP-69's job. + +## Files + +| Track | Representative paths | +| ------------------------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| A · hardening | `libs/shared/src/testing/expect-tag.ts` (new); the three wizard machine specs + `*has-progress.spec.ts` + `aanvraag-view.spec.ts`; `eslint.config.mjs`, `.dependency-cruiser.base.js`, `package.json`, CI; `Builders/AanvraagBuilder.cs`, endpoint test files | +| B · BDD | `libs/shared/docs/bdd.mdx`, ADR-0006; the 5 copy-assertion files; the multi-behaviour title splits | +| C · DDD | `libs/shared/docs/layers.mdx`; `RuleTests.cs` → `Domain/*RuleTests.cs`; new specs for `isStatusConsistent`, `OrgTemplateRules.RejectDraft`, both apps' `session.ts` | +| D+E · living docs + seam | `scripts/gen-behaviour-spec.mjs`, `libs/shared/docs/behaviour-spec.mdx` (generated), `scripts/check-seam.sh` | + +## Steps + +Executed as four tracks: A/C/D+E ran file-disjoint in parallel first; B ran after, since its +doc rewrite needed to reflect what A/C actually converted. Each track ended its own layer +green; a combined gate followed; then per-track commits. + +## Acceptance criteria + +- [x] `expectTag` replaces all 76 `as any` + 12 `as Extract<>` state-narrowing casts across + the three biggest wizard specs and the `*has-progress`/`besluit`/`change-request` specs. + Zero tests legitimately started failing — every wrong-variant read the casts were hiding + turned out to already be correct. +- [x] The vacuous assertion in `org-template.machine.spec.ts` (and, on inspection, every + sibling instance of the same pattern in that file) is fixed. +- [x] Four new gates proven to actually fail before being trusted: `npm run lint` fails on a + planted `any`; a deliberately-wrong `expectTag` call fails with a named + "expected X, got Y" error, not `undefined`; `npm run dep:check` fails on a planted + production import of `libs/shared/src/testing`; `npm run typecheck` fails on a planted + type error in a spec. +- [x] `check-seam.sh` proven to fail when the two scholing-threshold literals are set to + different values, with both file paths and values named in the error. +- [x] Backend test count: 220 (WP-70 baseline) → 230 (+9 `OrgTemplateRuleTests`, +1 from the + `Unknown_id_404s_and_zorgverlener_is_forbidden` split). File count in + `RuleTests.cs`'s place: 0 (deleted) → 7 files under `Domain/`, same total test count + moved (plus the new file). +- [x] Frontend test count grew only from legitimate title splits (no assertion dropped) and + the new session/isStatusConsistent specs — before/after counts reported per file by the + owning track. +- [x] `layers.mdx` reflects the actual WP-67 monorepo structure (`apps/`+`libs/`, + dependency-cruiser as the real enforcement mechanism, not ESLint). +- [x] `bdd.mdx`'s "one transition per test" citation of `registratie-wizard.machine.spec.ts` + is true again (the cited test was split). +- [x] Every one of the 5 documented `$localize`-copy-assertion violations is fixed or + explicitly justified as the doc's own escape hatch (a `reden` free-text passthrough with + no backing tag — `aanvraag-view.spec.ts`/`beoordeling-view.spec.ts` — left alone with an + inline comment explaining why, rather than forcing a fake enum into production code). +- [x] `npm run ci` green (lint, typecheck, dep:check, format, check:tokens, check:seam, all + four test projects, both localized builds, audit, backend format+test, snippet drift, + behaviour-spec drift, api-client drift). `npm run build-storybook` green (the new/edited + MDX pages build without error). + +## Verification + +```bash +npm run typecheck && npm run lint && npm run dep:check && npm run check:seam +npm run ci # green (2026-08-18) +npm run build-storybook # green +cd backend && dotnet test BigRegister.slnx --filter "Category!=Integration" # 230/230 +``` + +## Out of scope + +- Universal Given/When/Then sweep across all ~600 tests (staged instead — retrofit covers the + files this WP already touched; the rest adopt it as they're next edited). +- Scholing threshold _enforcement_ — WP-69 owns it. +- Reconciling the phone-format regex divergence in production code (documented, not fixed; + verified not a live bug — the FE normalises `+31`→`0` before the wire). +- Making `Aanvraag` genuinely immutable (EF refactor, inherited from WP-70). +- E2E test isolation via a dev-only seed endpoint (inherited from WP-70). +- `RegistrationStatus`'s flat-record gap on the backend (inherited from WP-70). + +## Risks + +- `expectTag`'s runtime throw only fires when a spec actually calls it with the wrong tag — + it does not retroactively audit every state a reducer can reach. A future variant added to + a union still needs its own test coverage; the helper only makes existing coverage honest. +- `check-seam.sh` covers exactly one FE/BE literal pair (the scholing threshold). The other + three documented divergences (phone regex, eligibility fixtures, toelichting) have no + automated guard — a future edit to either side can still silently diverge undetected.