diff --git a/ErsatzTV.Tests/Controllers/ScriptedScheduleControllerTests.cs b/ErsatzTV.Tests/Controllers/ScriptedScheduleControllerTests.cs index 16ad649db..b8acc25bb 100644 --- a/ErsatzTV.Tests/Controllers/ScriptedScheduleControllerTests.cs +++ b/ErsatzTV.Tests/Controllers/ScriptedScheduleControllerTests.cs @@ -33,9 +33,9 @@ namespace ErsatzTV.Tests.Controllers; /// The hops this cannot host, named rather than implied: the Cli.Wrap launch of the user's /// own program (exit code, timeout, stdout capture); the Kestrel/middleware/auth transport the /// program calls back over; and MVC's binding wrapper — the input formatter's content-type -/// selection and malformed-body handling, and the [ApiController] automatic 400 a failed -/// bind produces before an action runs, since every test here hands an action an already-bound -/// object. The serializer inside that wrapper is not residue: request bodies are +/// selection and malformed-body handling, model validation (a non-nullable reference type picks +/// up an implicit required check there), and the [ApiController] automatic 400 either +/// produces before an action runs, since every test here hands an action an already-bound object. The serializer inside that wrapper is not residue: request bodies are /// deserialized with , the same configuration Startup hands /// to AddNewtonsoftJson, so the fixture is bound the way production binds it. That is not /// a formality — Newtonsoft and System.Text.Json disagree on the DTOs' required members, @@ -133,10 +133,11 @@ public class ScriptedScheduleControllerTests { // The replay above is only faithful while Bind uses MVC's own serializer, and "the two agree" is // not a safe assumption to leave unstated: Newtonsoft has no notion of the C# `required` keyword, - // so a body omitting one binds to a default and reaches the action, where System.Text.Json - // rejects the body outright. This goes through Bind itself, which is what gives it teeth — the - // fixture's own bodies parse identically under either serializer, so only a body like this one - // can witness a swap. + // so a body omitting one deserializes to a default, where System.Text.Json rejects it outright. + // The claim is about the SERIALIZER only — what MVC's validation layer then does with such a + // body (non-nullable reference types pick up an implicit required check) is the uncovered + // wrapper, not this. Binding through Bind itself is what gives this teeth: the fixture's own + // bodies parse identically under either serializer, so only a body like this one witnesses a swap. using JsonDocument document = JsonDocument.Parse(BodyMissingRequiredMember); ContentCollection bound = Bind(document.RootElement).ShouldNotBeNull(); diff --git a/ErsatzTV/Serialization/ApiJsonSettings.cs b/ErsatzTV/Serialization/ApiJsonSettings.cs index 35b3d82c0..34605582b 100644 --- a/ErsatzTV/Serialization/ApiJsonSettings.cs +++ b/ErsatzTV/Serialization/ApiJsonSettings.cs @@ -8,7 +8,7 @@ namespace ErsatzTV.Serialization; /// here, so a test can deserialize a request body exactly the way the production body binder does /// instead of mirroring 's registration — a mirror drifts silently, and the /// divergence is not cosmetic: Newtonsoft has no notion of the C# required keyword, so a body -/// omitting a required member binds to a default here where System.Text.Json would throw. +/// omitting a required member deserializes to a default here where System.Text.Json would throw. /// public static class ApiJsonSettings { diff --git a/docs/decisions/README.md b/docs/decisions/README.md index 5cd581e03..27ccffa9f 100644 --- a/docs/decisions/README.md +++ b/docs/decisions/README.md @@ -224,7 +224,7 @@ the link for rationale. Superseded/retired history lives in `archive/`. Regenera | `testing.live-e2e-prepush-timing` | Run live-E2E via `scripts/e2e-local.sh` before pushing a write-path or UI change, and exercise download endpoints with curl, never a browser tab. | 2026-07-21 | [link](records/testing/live-e2e-prepush-timing.md) | | `testing.mutation-claims-are-executed` | A `MUTATION` row in `docs/guard-inventory.md` is not a statement that someone once witnessed a red. It carries a DECLARED clause mutation in `scripts/tests/mutation_manifest.py`, and `scripts/tests/test_mutation_harness.py` applies that mutation to an isolated copy of the repository on every run and requires the row's OWN named test to go red. The manifest and the MUTATION rows are compared for SET EQUALITY in both directions, so a row cannot claim the grade without a mutation and a mutation cannot outlive the grade it justifies. EXIT STATUS IS NOT THE VERDICT: each entry also declares the DIAGNOSTIC its red must carry, matched against pytest's exception output alone, because pytest reports a crashing test exactly as it reports a detecting one and a red for an unrelated reason is evidence about nothing. WHERE THE GUARD IS ITSELF A TEST, `target` may differ from `guard` and the exact-once check applies to the declared TARGET. Two shapes are admissible and the choice is not free. Where the guard's assertion IS the check — a completeness comparison against a Markdown inventory — the mutation goes into the guarded ARTIFACT, per `testing.guard-ships-with-mutation-proof`'s checker-guard exception, because mutating such a checker's own POPULATION demonstrates a false POSITIVE while proving nothing about the detection the row claims. Where the guard is a test module wrapping a separately mutable DETECTOR or helper, the clause may be in that detector, since disarming it is a real clause disarm and the module's own assertion is what notices. THE MUTATION IS DECLARED, NEVER INFERRED: a harness that guessed which clause of a 90-line hook is the guard would manufacture the confident-but-empty coverage this exists to prevent, which is why `testing.guard-ships-with-mutation-proof` rejected a generic runner. Where a proof test already names its clause in source, the manifest reuses THAT string, so a retarget in either place is caught by the other. COARSENESS IS RECORDED, NOT HIDDEN: each entry is graded `CLAUSE` or `DETECTOR`, and a `DETECTOR` entry — one whose detector accumulates faults from independent arms, so disarming any single arm leaves its proof test green — must CARRY the finer mutation that survived, which is re-run every time and required to keep surviving. Guards that are not graded `MUTATION` each carry a STATED reason in that same manifest, keyed on the guard and compared for SET EQUALITY against the inventory's `GUARD` rows in both directions — so a new guard cannot arrive without someone writing what a proof would need, and a reason cannot outlive the row it is about. Keying the reason on the row's GRADE instead is tautological (a new guard inherits one and nobody looks at it) and a pinned COUNT moves only on net change; both were tried and are rejected. The sandbox is a real git repository built from `git ls-files` with working-tree content, never a filesystem walk. THE POPULATION IS EVERY DECLARED MUTATION-OUTCOME CLAIM, NOT ONLY THE INVENTORY'S ROWS (ersatztv#881): a sentence asserting that a specific mutation reddens, or does not redden, a named test is the same kind of claim wherever it is written — a code comment, a test docstring, a decision record — and is either a `CLAIMS` entry in `scripts/tests/mutation_manifest.py` that executes every run, or it is not written. A claim entry is keyed on the PROSE rather than on a guard: it carries the tracked `site` and the verbatim `quote`, both checked every run, so a reworded sentence reports as a RETARGET instead of drifting away from the entry that justifies it. THE QUOTE SPANS THE MUTATION AND THE OUTCOME IT ASSERTS: the outcome half is the checkable part, so a quote stopping at the mutation binds the uncontroversial words and leaves the assertion, and the test it names, outside the binding entirely. It declares `RED` or `GREEN`, and GREEN is the half no `MUTATION` row can express — a claim that a mutation is NOT noticed is read by THREE separate clauses, because each alone certifies a run that proves nothing: the run exited 0, something actually PASSED, and the named proof was shown to REACH the mutated clause. THE TWO DIRECTIONS ARE NOT SYMMETRIC, WHICH IS WHY THE THIRD CLAUSE EXISTS ON ONLY ONE OF THEM. A RED claim checks its own relevance for free — a proof that never executes the mutated file stays green and is refused in the same words as a clause that has stopped being load-bearing — while a GREEN claim gets precisely the green it asserts from a proof that reads a different file entirely, so exit status and a passing test TOGETHER still certify a run that never executed the clause. A GREEN entry therefore also declares a SECOND mutation of the SAME clause, required to REDDEN the same proof and carrying its own declared diagnostic, executed through the same red-reading path the inventory rows use; only once that passes is the green evidence about this clause at all. It is worth what the `DETECTOR` survivor was worth: re-run, and required to KEEP surviving. THE REFERENT IS DECLARED, NEVER INFERRED, exactly as the clause is: a checker hunting for sentences that merely SOUND like mutation claims would be a string predicate over prose, which this repo refuses to make load-bearing, so what is enforced is a claim that named its own referent, and the direction that cannot be mechanised — prose that declares nothing — is stated rather than papered over. TWO NEIGHBOURING PROPOSALS ARE SETTLED HERE. A QUOTATION of another file is a claim about that file: adopted, and the `quote` field IS the check, wherever the referent is declared; a general quotation-checker is rejected for the same reason inference is, since a quotation whose referent is unnamed has nothing to resolve. ANCHORING PROSE TO A STATE THE COMMIT ITSELF MOVES ('the last edit to that path', 'the only X that'): rejected as a detector and kept as a phrasing rule. The rejection is measured, and the scan is PINNED IN THE BODY rather than summarised as a figure here, for the same reason the population scan is — a count quoted without the predicate that produced it is the defect this record is about. What it shows is that the shape is indistinguishable BY PATTERN from ordinary rationale ('X is the only thing that can Y'), which this rule carves out, so a detector would fire mostly on prose it must not flag, and an advisory check that does that stops being read. Anchor to a date or a sha under `docs.no-session-narrative`'s dated-measurement clause; inside this rule's own population the anchor is the manifest entry, which is re-executed and therefore cannot go stale. | 2026-08-22 | [link](records/testing/mutation-claims-are-executed.md) | | `testing.playwright-mcp-download-and-recovery` | In Playwright-MCP E2E, fetch file-download endpoints with curl — never a browser tab or `window.open` — and if browser tools stall repeatedly, `pkill -f ms-playwright-mcp` and drive a fresh session. | 2026-07-21 | [link](records/testing/playwright-mcp-download-and-recovery.md) | -| `testing.scripted-engine-in-process-net` | Scripted playout is characterized at two in-process levels and no further. (1) `SchedulingEngineTests` drives the engine build API directly with substituted repositories — the same setup order `ScriptedPlayoutBuilder` uses (`WithPlayoutId`/`WithMode`/`WithSeed`/`BuildBetween`/`WithReferenceData`/`RestoreOrReset`, and `WithReferenceData` MUST precede the last two or `PlayoutReferenceData.PlayoutHistory` NREs). (2) `ScriptedScheduleControllerTests` replays a committed script fixture (`ErsatzTV.Tests/Controllers/Fixtures/scripted-build.json`) through the REAL `ScriptedScheduleController` + `ScriptedPlayoutBuilderService.MockSession` + `SchedulingEngine` and pins the resulting `PlayoutItem`s as a snapshot in the golden line format — so exactly ONE action-to-engine mapping exists in the repo, the production one. Fixture bodies in that replay are deserialized with `ApiJsonSettings`, the configuration `Startup` applies to `AddNewtonsoftJson`, invoked from the same function rather than mirrored: Newtonsoft and System.Text.Json disagree about the DTO `required` members, so a lookalike binder would pass a fixture the production binder would bind differently. The `Cli.Wrap` launch of the user-authored program (exit code, timeout, stdout capture), the Kestrel/middleware/`ApiAuthorizationFilter` transport it calls back over, and MVC model binding as a WRAPPER (the input formatter and the `[ApiController]` automatic 400 a failed bind produces before an action runs — the serializer inside it is covered, the wrapper around it is not) are NOT tested and are not scheduled to be; that residue is named below rather than argued away. The controller is a thin adapter, NOT a 1:1 pass-through: an unparseable playback order is a 400, an unparseable filler kind SILENTLY degrades to `FillerKind.None`, an unknown build id is a 404, and the engine no-progress `InvalidOperationException` is translated to a 400 — each pinned by a test, because a prose claim of equivalence decays. | 2026-09-05 | [link](records/testing/scripted-engine-in-process-net.md) | +| `testing.scripted-engine-in-process-net` | Scripted playout is characterized at two in-process levels and no further. (1) `SchedulingEngineTests` drives the engine build API directly with substituted repositories — the same setup order `ScriptedPlayoutBuilder` uses (`WithPlayoutId`/`WithMode`/`WithSeed`/`BuildBetween`/`WithReferenceData`/`RestoreOrReset`, and `WithReferenceData` MUST precede the last two or `PlayoutReferenceData.PlayoutHistory` NREs). (2) `ScriptedScheduleControllerTests` replays a committed script fixture (`ErsatzTV.Tests/Controllers/Fixtures/scripted-build.json`) through the REAL `ScriptedScheduleController` + `ScriptedPlayoutBuilderService.MockSession` + `SchedulingEngine` and pins the resulting `PlayoutItem`s as a snapshot in the golden line format — so exactly ONE action-to-engine mapping exists in the repo, the production one. Fixture bodies in that replay are deserialized with `ApiJsonSettings`, the configuration `Startup` applies to `AddNewtonsoftJson`, invoked from the same function rather than mirrored: Newtonsoft and System.Text.Json disagree about the DTO `required` members, so a lookalike binder would pass a fixture the production binder would bind differently. The `Cli.Wrap` launch of the user-authored program (exit code, timeout, stdout capture), the Kestrel/middleware/`ApiAuthorizationFilter` transport it calls back over, and MVC model binding as a WRAPPER (the input formatter, model validation and the `[ApiController]` automatic 400 either produces before an action runs — the serializer inside that wrapper is covered, the wrapper around it is not) are NOT tested and are not scheduled to be; that residue is named below rather than argued away. The controller is a thin adapter, NOT a 1:1 pass-through: an unparseable playback order is a 400, an unparseable filler kind SILENTLY degrades to `FillerKind.None`, an unknown build id is a 404, and the engine no-progress `InvalidOperationException` is translated to a 400 — each pinned by a test, because a prose claim of equivalence decays. | 2026-09-05 | [link](records/testing/scripted-engine-in-process-net.md) | | `testing.suite-isolated-from-production-hook-fire-log` | THE SUITE MUST NOT WRITE TO `$HOME/.cache/ersatztv/hook-fire/`, AND THAT PROPERTY IS ESTABLISHED STRUCTURALLY RATHER THAN OBSERVED. Two layers in `scripts/tests/conftest.py`, which close different routes and must both stay. FIRST, `pytest_configure` sets `ETV_HOOK_FIRE_LOG_DIR` BEFORE COLLECTION. An autouse fixture alone runs at test setup — after every module has been imported — so it cannot reach a module that snapshots `{**os.environ}` at import time (the ersatztv#785 defect, 58 leaked records per run) and cannot reach a MODULE- or SESSION-SCOPED FIXTURE at all; measured on the pre-change tree by instrumenting `subprocess.Popen` for a full run and asking which launches resolve to `$HOME/.cache/ersatztv/hook-fire`, 83 did — 37 during collection, 44 inside module- or session-scoped fixtures, and 2 from an environment built from scratch. INSTRUMENT `Popen` ALONE: `subprocess.run`, `call` and `check_output` all reach it, so a census wrapping `run` as well counts every launch twice and every figure doubles. STATE THE PREDICATE WITH THE COUNT: this one also UNDERCOUNTS the from-scratch route, because such an environment usually carries no `HOME` either and lands in the sink's OTHER default, which this predicate cannot see. The launches that carry no `ETV_HOOK_FIRE_LOG_DIR` and are built from scratch number 83 as well, and 81 of those carry no `HOME` — two different sets of the same size, which is precisely the shape a later reader reconciles wrongly. The per-test fixture stays, because a test that inspects its own log dir must not see another test's records. SECOND, a `Popen` wrapper fails EVERY launch that does not CARRY the isolated directory — the route left open when an environment is built FROM SCRATCH rather than derived from `os.environ`, where no snapshot is involved and import ordering cannot help. THE RULE IS "CARRIES AN ISOLATED DIR", NOT "IS NOT THE PRODUCTION ONE", and that is the difference between a guard and a guard-shaped hole: `etv_hook_fire_log_dir` has TWO default branches — `$HOME/.cache/...` and, when `HOME` is unset or empty, `/tmp/.cache/...` — so a check comparing against one resolved production path models one branch and waves the other through, and `/tmp/.cache/ersatztv/hook-fire` is just as shared, just as persistent and just as readable with `hook-fire-log.sh report --dir`. Measured: of the 83 from-scratch launches, 81 carry no `HOME` either. Requiring the variable covers both branches and any a future sink adds, and is independent of what `HOME` happens to be on the machine running the suite — which is also what keeps it from firing on legitimate launches where `HOME` is unset or is itself `/tmp`. The corollary is that every from-scratch environment in the suite must carry the variable, one line each. NO COUNT OF THOSE SITES IS KEPT HERE: it is a hand-written population literal that nothing derives, and the commit introducing this rule already falsified its own, by adding further construction sites in the tests that prove the rule. The guard itself is the enumerator; a missing site is a red, not a stale sentence. Its population is every launch that actually happens, derived at runtime, so a new suite is covered by existing rather than by remembering. NEVER OBSERVE THE PRODUCTION DIRECTORY TO PROVE THIS. The withdrawn guard snapshotted `st_mtime_ns` across it and required no change, which makes the oracle state every concurrent session on the machine writes: an unrelated session firing a hook inside the window failed the suite with `a test run modified the production hook-fire log`, an accusation about the suite when the writer was another process. Seen on three separate branches, green on every immediate re-run. The misattribution is the expensive part — it aims the next reader at the suite, and a guard that cries wolf gets waved through. THE RESOLVER IS A REIMPLEMENTATION AND FAILS OPEN, so it is differential-tested against `etv_hook_fire_log_dir` every run: a resolver that disagrees clears a launch the sink then points at the real log. Shell's `:-` treats an EMPTY value as absent, which `dict.get(k, default)` does not — that case is in the matrix because it is where a Python transliteration goes wrong. AND BIND A VIOLATION BEFORE ASSERTING ON IT: pytest rewrites an assert expression and prints the repr of every sub-expression, so `assert isolation_violation(os.environ) is None` dumps the environment — API keys included — into the failure output and from there into a CI log. | 2026-08-28 | [link](records/testing/suite-isolated-from-production-hook-fire-log.md) | | `testing.troubleshoot-path-cannot-test-branding` | Verify logo/watermark/bug changes through a real channel playout — a green troubleshoot run proves nothing about branding. | 2026-07-21 | [link](records/testing/troubleshoot-path-cannot-test-branding.md) | | `testing.verification-code-needs-its-own-proof` | VERIFICATION CODE IS CODE UNDER TEST. The obligation in `testing.guard-ships-with-mutation-proof` attaches to whatever emits a PASS/FAIL somebody acts on — a smoke test, a timeout wrapper, an operator-run checker, the harness that applies the mutations — and NOT only to files sitting in the guard population or named like guards; every one of the five defects measured below was in code nobody had counted as a guard. THE DISCRIMINATOR IS WHETHER A PROOF TEST CAN DRIVE THE CONSTRUCT HERMETICALLY, NOT WHETHER CI RUNS IT, and the difference is load-bearing rather than pedantic: `.claude/hooks/*.sh` and `.husky/pre-push` are operator-run and CI never fires them AS HOOKS, yet several of them carry `MUTATION` rows — among them `pretooluse-bom-guard.sh`, `pretooluse-worktree-guard.sh` and `.husky/pre-push` — because a pytest proof drives them in a sandbox. A does-CI-run-it test would sort those already-proven guards into the exempt branch. (Most of the remaining hooks are graded `NONE`; that is unproven debt, not evidence for the other discriminator.) "IT NEEDS A REAL SERVER" IS AN EXEMPTION CLAIM AND IT IS USUALLY FALSE: `scripts/mcp_smoke.py` takes its config path and server name as POSITIONAL ARGUMENTS, so a test hands it a synthetic config in a tmpdir pointing at a stub responder and drives it with no gitignored `.mcp.json` and no language server, which is how it is proven in `scripts/tests/test_mcp_smoke.py`. That coverage is a SUBSET and no count of it is kept here: the file's own docstring enumerates the cases it drives and names what it does not, so there is a list rather than a number to keep in step with it. A checker is never excused because its CALLER cannot run in CI. WHERE THE PROOF LIVES: the checker itself needs NO `docs/guard-inventory.md` row, because that population is derived from workflow and hook call sites and a transitively-reached script is rejected as a phantom; the proof goes in an ordinary `scripts/tests/test_*.py`, which joins the population automatically and carries the row, while the manifest entry names THAT test as `guard` and the checker as `target`. That shape already exists for `mutation_harness_lib.py` and needs no change to any population. ONLY WHAT NO PROOF TEST CAN DRIVE HERMETICALLY falls back to a WITNESSED NEGATIVE CONTROL — the failure path executed once, the false-green form reproduced, both recorded in the PR, and its decay stated because nothing re-runs it. `scripts/check-local-lsp.sh` is the genuine case: it asserts developer-machine installs (a global `csharp-ls`, a `host/fxr` under `DOTNET_ROOT`, a root `node_modules`) that no runner has. STOP-AND-SUBTRACT, adopted here as this record's own threshold: when a review round's finding was CREATED by the previous round's fix and that happens TWICE CONSECUTIVELY, delete the layer generating them rather than guarding it. READING THE CONSTRUCT IS NOT EXECUTING IT: ShellCheck 0.11.0 flagged NEITHER of the two shell defects below, and each is indistinguishable from correct by reading. | 2026-08-28 | [link](records/testing/verification-code-needs-its-own-proof.md) | diff --git a/docs/decisions/records/testing/scripted-engine-in-process-net.md b/docs/decisions/records/testing/scripted-engine-in-process-net.md index 0ac01f6bd..1518e218f 100644 --- a/docs/decisions/records/testing/scripted-engine-in-process-net.md +++ b/docs/decisions/records/testing/scripted-engine-in-process-net.md @@ -5,7 +5,7 @@ status: active since: '2026-09-05' supersedes: testing.scripted-playout-golden-deferred@2026-07-22 superseded-by: none -rule: 'Scripted playout is characterized at two in-process levels and no further. (1) `SchedulingEngineTests` drives the engine build API directly with substituted repositories — the same setup order `ScriptedPlayoutBuilder` uses (`WithPlayoutId`/`WithMode`/`WithSeed`/`BuildBetween`/`WithReferenceData`/`RestoreOrReset`, and `WithReferenceData` MUST precede the last two or `PlayoutReferenceData.PlayoutHistory` NREs). (2) `ScriptedScheduleControllerTests` replays a committed script fixture (`ErsatzTV.Tests/Controllers/Fixtures/scripted-build.json`) through the REAL `ScriptedScheduleController` + `ScriptedPlayoutBuilderService.MockSession` + `SchedulingEngine` and pins the resulting `PlayoutItem`s as a snapshot in the golden line format — so exactly ONE action-to-engine mapping exists in the repo, the production one. Fixture bodies in that replay are deserialized with `ApiJsonSettings`, the configuration `Startup` applies to `AddNewtonsoftJson`, invoked from the same function rather than mirrored: Newtonsoft and System.Text.Json disagree about the DTO `required` members, so a lookalike binder would pass a fixture the production binder would bind differently. The `Cli.Wrap` launch of the user-authored program (exit code, timeout, stdout capture), the Kestrel/middleware/`ApiAuthorizationFilter` transport it calls back over, and MVC model binding as a WRAPPER (the input formatter and the `[ApiController]` automatic 400 a failed bind produces before an action runs — the serializer inside it is covered, the wrapper around it is not) are NOT tested and are not scheduled to be; that residue is named below rather than argued away. The controller is a thin adapter, NOT a 1:1 pass-through: an unparseable playback order is a 400, an unparseable filler kind SILENTLY degrades to `FillerKind.None`, an unknown build id is a 404, and the engine no-progress `InvalidOperationException` is translated to a 400 — each pinned by a test, because a prose claim of equivalence decays.' +rule: 'Scripted playout is characterized at two in-process levels and no further. (1) `SchedulingEngineTests` drives the engine build API directly with substituted repositories — the same setup order `ScriptedPlayoutBuilder` uses (`WithPlayoutId`/`WithMode`/`WithSeed`/`BuildBetween`/`WithReferenceData`/`RestoreOrReset`, and `WithReferenceData` MUST precede the last two or `PlayoutReferenceData.PlayoutHistory` NREs). (2) `ScriptedScheduleControllerTests` replays a committed script fixture (`ErsatzTV.Tests/Controllers/Fixtures/scripted-build.json`) through the REAL `ScriptedScheduleController` + `ScriptedPlayoutBuilderService.MockSession` + `SchedulingEngine` and pins the resulting `PlayoutItem`s as a snapshot in the golden line format — so exactly ONE action-to-engine mapping exists in the repo, the production one. Fixture bodies in that replay are deserialized with `ApiJsonSettings`, the configuration `Startup` applies to `AddNewtonsoftJson`, invoked from the same function rather than mirrored: Newtonsoft and System.Text.Json disagree about the DTO `required` members, so a lookalike binder would pass a fixture the production binder would bind differently. The `Cli.Wrap` launch of the user-authored program (exit code, timeout, stdout capture), the Kestrel/middleware/`ApiAuthorizationFilter` transport it calls back over, and MVC model binding as a WRAPPER (the input formatter, model validation and the `[ApiController]` automatic 400 either produces before an action runs — the serializer inside that wrapper is covered, the wrapper around it is not) are NOT tested and are not scheduled to be; that residue is named below rather than argued away. The controller is a thin adapter, NOT a 1:1 pass-through: an unparseable playback order is a 400, an unparseable filler kind SILENTLY degrades to `FillerKind.None`, an unknown build id is a 404, and the engine no-progress `InvalidOperationException` is translated to a 400 — each pinned by a test, because a prose claim of equivalence decays.' signals: 'why is there no scripted golden; scripted integration harness; scripted transport vs engine; SchedulingEngine is in-process testable; Cli.Wrap external process is transport only; shell-out scoped out; MockSession seam; in-process stand-in for a scripted script; engine build API characterization; committed script fixture replay; ScriptedScheduleController is an adapter not a pass-through; scripted filler kind silently falls back to None; scripted playout no-progress halt maps to 400; scripted build 401 blind spot; EnumeratorForContent · paths: `ErsatzTV.Core.Tests/Scheduling/Engine/SchedulingEngineTests.cs`, `ErsatzTV.Tests/Controllers/ScriptedScheduleControllerTests.cs`, `ErsatzTV.Tests/Controllers/Fixtures/scripted-build.json`, `ErsatzTV.Core/Scheduling/Engine/SchedulingEngine.cs`, `ErsatzTV/Controllers/Api/ScriptedScheduleController.cs`, `ErsatzTV.Core/Scheduling/ScriptedScheduling/ScriptedPlayoutBuilder.cs`, `ErsatzTV.Core.Tests/Scheduling/Goldens/PlayoutBuildGoldenTests.cs` · issues: #563, #381, #395, #163' mechanics: docs/testing.md → "Scripted playout coverage" and "Golden-file nets" --- @@ -41,13 +41,16 @@ wrong — the four adapter mappings above are real behaviour that only a test ca **The residue, named.** Uncovered: process launch, exit code and timeout handling; Kestrel plus the forwarded-headers / compression / SPA-hosting middleware; MVC model binding as a *wrapper* — the input -formatter (content-type selection, malformed-body handling) and the `[ApiController]` automatic 400 a -failed bind produces before an action runs, since every test hands an action an already-bound object; -and `ApiAuthorizationFilter`. The *serializer* inside that wrapper is deliberately not on this list: +formatter (content-type selection, malformed-body handling), model validation (a non-nullable reference +type picks up an implicit required check there) and the `[ApiController]` automatic 400 either produces +before an action runs, since every test hands an action an already-bound object; and +`ApiAuthorizationFilter`. The *serializer* inside that wrapper is deliberately not on this list: `ScriptedScheduleControllerTests` binds with `ApiJsonSettings` itself, and `Production_Body_Binder_Ignores_Required_Members` pins the Newtonsoft semantics that choice buys — a -body omitting a `required` member reaches the action with a default rather than being rejected, which is -what a System.Text.Json stand-in would silently change. +body omitting a `required` member deserializes to a default rather than failing, which a System.Text.Json +stand-in would silently turn into a rejection. That is a claim about the SERIALIZER and is scoped to it: +what the wrapper then does with such a body — non-nullable reference types pick up an implicit required +check in MVC validation — is on the uncovered list above, not measured here. `ApiAuthorizationFilter` on that list is not hypothetical — `ScriptedScheduleController` carries no `[SkipApiAuthorization]` while diff --git a/docs/testing.md b/docs/testing.md index 26e1506b6..82ffda1bf 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -82,21 +82,24 @@ create a second action-to-engine mapping — the thing the single-mapping design program — exit code, the `PlayoutScriptedScheduleTimeoutSeconds` timeout, stdout capture; Kestrel and the `Startup` middleware, including the host/`Settings.UiPort` check that 404s a foreign request; `ApiAuthorizationFilter`, which fail-closes every mutating verb for an endpoint without -`[SkipApiAuthorization]`; and MVC model binding as a *wrapper* — the input formatter and the -`[ApiController]` automatic 400 a failed bind produces before the action runs, since the replay hands -each action an already-bound object. Hosting the real `Startup` would drag in the whole DI graph, a +`[SkipApiAuthorization]`; and MVC model binding as a *wrapper* — the input formatter, model validation +(a non-nullable reference type picks up an implicit required check there) and the `[ApiController]` +automatic 400 either produces before the action runs, since the replay hands each action an +already-bound object. Hosting the real `Startup` would drag in the whole DI graph, a hand-rolled minimal host would test a transport the product does not have, and the script is user-authored by definition — so any committed script is a stand-in either way. The decision record names the concrete blind spot this leaves. The *serializer* inside that binding wrapper is covered rather than scoped out, and the distinction is load-bearing: production binds `/api/*` bodies with Newtonsoft (`Startup` → `AddNewtonsoftJson`), which -has no notion of the C# `required` keyword, so a body omitting a required member binds to a default and -reaches the action where System.Text.Json would reject it outright. The replay therefore deserializes -fixture bodies through `ErsatzTV.Serialization.ApiJsonSettings` — the same function `Startup` calls, not -a mirror of it — and `Production_Body_Binder_Ignores_Required_Members` pins both halves of that -divergence, so a swap to a lookalike serializer reddens instead of passing on bodies the two happen to -agree about. `OpenApiSerializerContractTests` reads the same settings from the same place. +has no notion of the C# `required` keyword, so a body omitting a required member deserializes to a +default where System.Text.Json rejects it outright. That is a statement about the serializer and stops +there — what MVC validation then does with such a body belongs to the wrapper named above. The replay +deserializes fixture bodies through `ErsatzTV.Serialization.ApiJsonSettings`, the same function +`Startup` calls rather than a mirror of it, and `Production_Body_Binder_Ignores_Required_Members` pins +both halves of the divergence through the replay's own bind helper — so a swap to a lookalike serializer +reddens instead of passing on the fixture bodies the two happen to agree about. +`OpenApiSerializerContractTests` reads the same settings from the same place. ## Timezone independence