Files
ehoandClaude Sonnet 5 3652ff8d3f docs(test): make Given/When/Then the default BDD structure (WP-71)
bdd.mdx previously banned "Given/When/Then ceremony" outright, which
directly contradicted WP-70's own acceptance tests (Acceptance/
BesluitLifecycleTests.cs already used // Given/When/Then comments) and
the backend's organically-evolved PascalCase_snake_sentence convention,
which the doc gave zero guidance for. Reverses that rule: every test is
now structured Given -> When -> Then, with a genuinely empty phase
omitted rather than faked; present-tense declarative naming and the
one-behaviour-per-test rule are unchanged. ADR-0006 gets a cross-reference
so both documents agree everywhere, not just in acceptance tests.

Also closes out the doc's other named-but-unenforced rules found by the
audit: fixes the 5 files asserting rendered $localize copy instead of
the underlying tag/message-id (the compliant pattern already existed in
werkvoorraad-item-view.spec.ts), splits the multi-behaviour titles the
doc itself calls a smell (";", "and", "/"), and fixes bdd.mdx's own false
citation of registratie-wizard.machine.spec.ts as "one transition per
test" by actually splitting that test into one-transition-per-test.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-08-18 20:25:30 +02:00

13 KiB
Raw Permalink Blame History

ADR-0006 — Test data through the production door (builders, replay, and where each applies)

Status: Accepted · Date: 2026-08-18

Context

Decision #3 in CLAUDE.md is "make illegal states unrepresentable," and the production code mostly honours it: AanvraagStatus (backend) is a sealed class with a private constructor reachable only through five static factories; the frontend's wizards are tagged-union state machines driven by a pure reduce; form inputs are branded value objects reachable only through a parse* that returns Result.

The test suites are the one place this invariant is not enforced — they build fixtures by hand instead of through those same doors:

  • Backend. Aanvraag (Data/ApplicationStore.cs) is a mutable EF-backed bag: Submitted, Referentie, BesluitStatus, SubmittedAt are independent public setters. Its own StatusAt dereferences Referentie! three times — "Submitted ⇒ Referentie != null" is convention, not type. Two test files (RuleTests.cs, OpenZaakZaakSourceTests.cs) kept eight such fixtures internally consistent by hand, each re-deciding for itself which fields a given scenario needs.
  • Frontend. No shared fixture helper existed anywhere in apps/ or libs/. Every spec redefined its own throwaway literal function (editing1/editing2/editing3, editingWith, a local ok()), each hardcoding fields like errors: {} — asserting against a shape the real reducer may never actually produce, because the literal skips the reducer entirely.
  • E2E. The one seeded citizen's BSN and a diploma id were duplicated as bare string literals across every spec, coupled to SeedData.cs's exact ordering by comment only, with no compiler check if the seed ever changed shape.

A hand-rolled literal is not "faster test setup" — it is a second, unchecked implementation of the domain's construction rules, sitting right next to the real one.

Decision

Build test data through the same door production code uses. A test-data helper's job is to supply defaults, never to bypass invariants.

Concretely: reject any test helper shaped as a field-by-field builder (.withX().withY()... over an otherwise-open constructor) — that is an object literal with extra syntax, and it re-opens every illegal state the production type closed. Each layer instead gets the narrowest helper that cannot construct an illegal instance, because it has no path to one.

1. Backend aggregates with a lifecycle → a type-state builder

Where a production type enforces its invariants (or should), the test builder mirrors that enforcement as separate types per stage, so an illegal call is a compile error, not a runtime surprise:

Given.Concept()                  // ConceptAanvraag — only .Submitted() or .Build() exist
     .Submitted()                // SubmittedAanvraag — only .Decided() or .Build() exist
     .Decided(Besluit.Afwijzen, "reden");   // DecidedAanvraag

Given.Concept().Decided(...) does not compile — Decided is simply not a member of ConceptAanvraag. Where the production rule is more subtle than "which methods exist" (e.g. "Afwijzen requires a toelichting"), the builder calls the real production rule (BeoordelingRules.RequiresToelichting) rather than re-stating it — this is what keeps the builder from drifting out of sync with the domain as the domain changes.

Use this shape whenever a production aggregate has an ordered lifecycle and either (a) already guards it with factories (mirror them 1:1), or (b) doesn't yet guard it (as with Aanvraag itself, see Consequences) — the test-only builder is not a substitute for fixing the production type, but it stops the test suite from being the place the ungated shape leaks out into assertions.

2. Frontend state machines → replay real messages through the real reducer

No object is built directly. A fixture is the result of running real Msgs through the real reduce:

export const given =
  <S, M>(reduce: (s: S, m: M) => S, initial: S) =>
  (...msgs: M[]): S =>
    msgs.reduce(reduce, initial);

export const givenIntake = given(reduce, initial); // per-context wrapper, pure TS

There is no way to hand-write a Submitting state whose draft contradicts its step, or to assert errors: {} into existence — the only states reachable are the ones the reducer can actually produce, because production is the only code path that produces them.

3. Value objects → unwrapOk, never a cast

A test that needs a valid branded value calls the real parse* and unwraps it:

export const unwrapOk = <E, T>(r: Result<E, T>): T => {
  if (!r.ok) throw new Error('unwrapOk: parser rejected the input');
  return r.value;
};
const postcode = unwrapOk(parsePostcode('1234 AB'));

This closes the 'garbage' as Postcode route — a spec can only ever hold a value the real parser accepted.

4. RemoteData → named constructors, not ad-hoc literals

loading() / success(v) / failure(e) in libs/shared/src/testing/remote-data.ts replace the per-spec local ok()/loading/failure literals. RemoteData has no invariant to protect (it's a plain closed union with no smart constructor in production either), so this one is about removing duplication, not closing an illegal-state gap — named constructors belong here because they are shorter and consistent, not because the literal was unsafe.

5. E2E — shared actors/seed-refs, not a DSL

E2E fixtures are named, not built: Actors.zorgverlener, SeedRefs.diplomaZonderPolicyVragen in e2e/support/actors.ts, with loginAs(page, actor) replacing the duplicated login sequence. No page-object layer, no Given/When/Then runner — Playwright specs stay flat page.getByRole sequences (matching WP-19's "smoke, not full coverage" scope), the only change is that the values they use have one source instead of N copies. See "Where this does not reach" below for why the deeper e2e problem is out of scope here.

Decision table — what to reach for, by test type

Test type Where it lives Fixture idiom Do not
Domain aggregate with a guarded lifecycle (backend) *.Tests/Builders/ Type-state builder mirroring the production factories; delegate any non-trivial rule to the real rule class A field-by-field .WithX() builder, or an object initializer with all fields public
Pure reducer / state machine (frontend) domain/*.testing.ts given(reduce, initial)(...msgs) — replay real messages A literal returning { tag: 'Editing', ... } by hand
Value object / parser co-located with the parser's spec unwrapOk(parseX(raw)) 'x' as BrandedType
Plain closed union with no invariant (e.g. RemoteData) libs/shared/src/testing/ Named one-line constructors (loading(), success(v)) Redefining the same literal per spec file
Trust-boundary parse* (adapter) co-located, per test-strategy skill Hand-written DTO literals are correct here — the point of the test is "what if the untrusted shape is wrong," so the fixture must be a raw, possibly-malformed literal, not a validated domain value Routing malformed-input tests through a builder that can't express malformed shapes
UI component Storybook story + axe Args as input()s on the component; no fixture builder needed A component test with a hand-built store/model
Acceptance / behaviour test (either side) Acceptance/*Tests.cs (backend), *.acceptance.spec.ts (frontend) The same builder/replay idiom as above, composed into one Given→When→Then read A separate BDD/Gherkin runner — the language's own test framework plus the builder is enough
E2E e2e/support/ Named actor/seed-ref constants + a thin loginAs-style setup helper A page-object framework or DSL — out of proportion to a 3-spec smoke suite

The common thread: the fixture idiom is only ever a thinner or safer path to the same construction the domain already performs — never a parallel, unchecked one. The trust- boundary row is the deliberate exception, not a contradiction: there the entire point of the test is to exercise what happens when the input isn't valid, so the fixture must be able to represent the invalid shape a builder would refuse to construct.

A note on Given/When/Then and bdd.mdx

This ADR's "Given.Concept()...Build()" builder chain and the acceptance-test row above ("composed into one Given→When→Then read") already used Given/When/Then before it was the repo-wide default. libs/shared/docs/bdd.mdx has since made G/W/T structure — a // Given / // When / // Then comment (or, in TypeScript, the equivalent unlabelled ordering) inside every test body, acceptance or not — the documented convention for all tests, not only acceptance ones (reversing its own earlier "no G/W/T ceremony" rule). The two documents now agree everywhere: this ADR's Given builder is the fixture idiom; bdd.mdx rule 1 is the structural convention every test using that fixture (and every other test besides) follows.

Consequences

  • + An illegal backend fixture (e.g. a decided-but-not-submitted Aanvraag) is now a compile error in the builder path, not a silent bad test.
  • + Frontend specs can no longer assert against a state the reducer cannot actually reach; a hardcoded errors: {} fixture literal can't drift from what validation actually produces.
  • + One seeded identity/diploma reference in e2e instead of N copies — a reseed shows up as one changed constant, not a hunt through three spec files.
  • Aanvraag itself is not made illegal-states-unrepresentable by this ADR — it remains a mutable EF-backed class (WP-68 kept it that way deliberately; ApplicationStore is its only production writer). The builder is a test-only enforcement layer sitting in front of a production type that still allows the bad shape directly. Closing that gap for real means an EF-mapping change, tracked as a follow-up, not done here.
  • A type-state builder is more ceremony than a constructor call for a one-off fixture. Reach for it only where a lifecycle actually has ordered stages worth protecting — a flat value type doesn't need one (see the RemoteData row above).

Where this does not reach (deliberately out of scope)

  • E2E test isolation. The three Playwright specs share one mutable backend and admit it in their own comments ("restart the backend between CI runs"). The real fix is a dev-only seed endpoint each test can call to build its own isolated citizen/aanvraag — a new production-adjacent surface that needs its own security review, not a fixture-idiom change. Tracked as a follow-up; not fixed here.
  • RegistrationStatus (Domain/Registrations/) has the same class of gap as Aanvraag — a flat record with four always-present nullable fields, whose own doc-comment says only one tag ever uses the deadline field — but is out of this ADR's scope (a separate WP).
  • apps/behandelportal e2e coverage is currently zero; adding it is a coverage gap, not a fixture-idiom question, and is a separate follow-up.