docs: close WP-71, record verification result
CI / changes (push) Successful in 8s
CI / lint (push) Successful in 1m11s
CI / frontend (push) Successful in 2m30s
CI / backend (push) Successful in 1m54s
CI / e2e (push) Successful in 3m9s
CI / semgrep (push) Successful in 1m7s
CI / api-client-drift (push) Successful in 1m53s
CI / storybook-a11y (push) Successful in 10m55s
CI / changes (push) Successful in 8s
CI / lint (push) Successful in 1m11s
CI / frontend (push) Successful in 2m30s
CI / backend (push) Successful in 1m54s
CI / e2e (push) Successful in 3m9s
CI / semgrep (push) Successful in 1m7s
CI / api-client-drift (push) Successful in 1m53s
CI / storybook-a11y (push) Successful in 10m55s
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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.
|
||||
Reference in New Issue
Block a user