diff --git a/ErsatzTV.Tests/Controllers/OpenApiSerializerContractTests.cs b/ErsatzTV.Tests/Controllers/OpenApiSerializerContractTests.cs index ded33791f..ed0cd68da 100644 --- a/ErsatzTV.Tests/Controllers/OpenApiSerializerContractTests.cs +++ b/ErsatzTV.Tests/Controllers/OpenApiSerializerContractTests.cs @@ -25,7 +25,11 @@ namespace ErsatzTV.Tests.Controllers; public class OpenApiSerializerContractTests { // The configuration Startup.ConfigureServices -> AddNewtonsoftJson applies, from the same function - // rather than a mirror of it: a mirror drifts silently and this test would keep passing. + // rather than a hand-copy of it. This fixture is the WRITE-side witness for the naming strategy: with + // a plain JsonSerializerSettings here, all four cases go red on PascalCase keys. It says nothing about + // NullValueHandling (every DTO member below is populated, so nothing is dropped either way) or the + // StringEnumConverter (it compares key NAMES, not values). docs/testing.md -> "Scripted playout + // coverage" tabulates which suite witnesses which half. private static readonly JsonSerializerSettings RuntimeSettings = ApiJsonSettings.Create(); private static IEnumerable Cases() diff --git a/ErsatzTV.Tests/Controllers/ScriptedScheduleControllerTests.cs b/ErsatzTV.Tests/Controllers/ScriptedScheduleControllerTests.cs index e9f47b2e0..2459e5982 100644 --- a/ErsatzTV.Tests/Controllers/ScriptedScheduleControllerTests.cs +++ b/ErsatzTV.Tests/Controllers/ScriptedScheduleControllerTests.cs @@ -38,11 +38,15 @@ namespace ErsatzTV.Tests.Controllers; /// 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, -/// which pins. The reasoning for -/// the scope-out is in docs/testing.md → "Scripted playout coverage" and the decision record -/// testing.scripted-engine-in-process-net. +/// AddNewtonsoftJson, so the fixture is bound the way production binds it. Two tests hold +/// the two ways that binder can be replaced, since the fixture's own bodies parse identically +/// under all of them: against a +/// System.Text.Json swap, and +/// against a +/// plain Newtonsoft settings object. What no test here observes is Startup's own +/// registration — removes the duplicate rather than detecting +/// drift in one. The reasoning for the scope-out is in docs/testing.md → "Scripted playout +/// coverage" and the decision record testing.scripted-engine-in-process-net. /// /// /// Deviation from the three golden-file nets (docs/contributing.md §10): the expected output is a @@ -60,12 +64,18 @@ public class ScriptedScheduleControllerTests private static readonly Guid BuildId = Guid.Parse("00000000-0000-0000-0000-000000000563"); private static readonly DateTimeOffset Start = new(2026, 1, 15, 6, 0, 0, TimeSpan.Zero); - // What Startup hands to AddNewtonsoftJson, from the same function rather than a mirror of it. + // What Startup hands to AddNewtonsoftJson, from the same function rather than a hand-copy of it. + // Replacing this with a plain `new()` reddens Production_Body_Binder_Keeps_Declared_Defaults_... private static readonly JsonSerializerSettings BodyBinderSettings = ApiJsonSettings.Create(); - // A body that omits the `required` member "collection"; the two serializers disagree about it. + // A body that omits the `required` member "collection"; Newtonsoft and System.Text.Json disagree. private const string BodyMissingRequiredMember = """{"key":"content","order":"chronological"}"""; + // A body sending "order" as an explicit null. NullValueHandling.Ignore keeps the DTO's declared + // default; Newtonsoft's own default (Include) overwrites it with null. + private const string BodyWithExplicitNullOrder = + """{"key":"content","collection":"Test Collection","order":null}"""; + // Every action the fixture is expected to exercise. A fixture edit that drops one must fail loudly // rather than quietly shrinking what the snapshot covers. private static readonly string[] ExpectedActions = @@ -138,7 +148,9 @@ public class ScriptedScheduleControllerTests // 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. + // bodies parse identically under either serializer, so only a body like this one witnesses the + // swap. It witnesses THAT swap only — a Newtonsoft settings object that has merely lost the + // production configuration still binds this body correctly, which is the sibling test's job. using JsonDocument document = JsonDocument.Parse(BodyMissingRequiredMember); ContentCollection bound = Bind(document.RootElement).ShouldNotBeNull(); @@ -152,6 +164,28 @@ public class ScriptedScheduleControllerTests new JsonSerializerOptions(JsonSerializerDefaults.Web))); } + [Test] + public async Task Production_Body_Binder_Keeps_Declared_Defaults_Over_An_Explicit_Null() + { + // The other half of the binder-fidelity claim, and the half that a NEWTONSOFT lookalike witnesses: + // production sets NullValueHandling.Ignore, so an explicit `"order": null` leaves ContentCollection + // at its declared "shuffle" rather than overwriting it. Newtonsoft's own default is Include, which + // writes the null through — and AddCollection's Enum.TryParse then rejects it as a 400. So this is + // a behaviour difference a script would see, not a settings-shape assertion: it reddens if the bind + // settings are replaced by a plain JsonSerializerSettings that merely looks like the production one. + (ScriptedScheduleController controller, SchedulingEngine engine) = NewSession(); + + using JsonDocument document = JsonDocument.Parse(BodyWithExplicitNullOrder); + ContentCollection bound = Bind(document.RootElement).ShouldNotBeNull(); + bound.Order.ShouldBe("shuffle"); + + IActionResult result = await controller.AddCollection(BuildId, bound, CancellationToken.None); + result.ShouldBeOfType(); + + // and the content is usable, i.e. the surviving default really was a valid playback order + engine.AddCount(ContentKey, 1, Option.None, null, false).ShouldBeTrue(); + } + [Test] public async Task Unknown_Build_Id_Returns_404() { @@ -337,7 +371,8 @@ public class ScriptedScheduleControllerTests // Deserializes a fixture body with the SAME configuration the production body binder uses // (Startup -> AddNewtonsoftJson -> ApiJsonSettings), so the fixture's field names and casing are - // asserted against the serializer MVC actually runs rather than a lookalike that agrees by luck. + // asserted against the serializer MVC actually runs. The two Production_Body_Binder_* tests, not + // this helper, are what make a replacement of that configuration visible. private static T Bind(JsonElement body) { body.ValueKind.ShouldBe(JsonValueKind.Object); diff --git a/ErsatzTV/Serialization/ApiJsonSettings.cs b/ErsatzTV/Serialization/ApiJsonSettings.cs index 34605582b..65baa5a9a 100644 --- a/ErsatzTV/Serialization/ApiJsonSettings.cs +++ b/ErsatzTV/Serialization/ApiJsonSettings.cs @@ -6,9 +6,15 @@ namespace ErsatzTV.Serialization; /// /// The Newtonsoft settings the MVC JSON formatters read and write `/api/*` bodies with. Defined once, /// 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 deserializes to a default here where System.Text.Json would throw. +/// instead of mirroring 's registration. This REMOVES the duplicate rather than +/// detecting drift in one: no test observes Startup.ConfigureServices, so re-inlining its +/// AddNewtonsoftJson lambda as a hand-copy of would redden nothing. What +/// the tests do catch is a caller that binds with something else — a body omitting a C# +/// required member deserializes to a default here where System.Text.Json throws, and +/// .Ignore keeps a DTO's declared default over an explicit +/// null where plain Newtonsoft writes the null through. Both are pinned by +/// ScriptedScheduleControllerTests; the naming strategy is pinned on the write side by +/// OpenApiSerializerContractTests. See docs/testing.md → "Scripted playout coverage". /// public static class ApiJsonSettings { diff --git a/docs/decisions/README.md b/docs/decisions/README.md index 27ccffa9f..9e8a45aa5 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, 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.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 — and two tests WITNESS that choice instead of asserting it: `Production_Body_Binder_Ignores_Required_Members` reddens under a System.Text.Json binder, `Production_Body_Binder_Keeps_Declared_Defaults_Over_An_Explicit_Null` reddens under a bare `JsonSerializerSettings`. What no test observes is the `Startup` registration itself, so the extraction REMOVES the duplicate rather than detecting its drift. 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 1518e218f..2bd97ec21 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, 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.' +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 — and two tests WITNESS that choice instead of asserting it: `Production_Body_Binder_Ignores_Required_Members` reddens under a System.Text.Json binder, `Production_Body_Binder_Keeps_Declared_Defaults_Over_An_Explicit_Null` reddens under a bare `JsonSerializerSettings`. What no test observes is the `Startup` registration itself, so the extraction REMOVES the duplicate rather than detecting its drift. 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" --- @@ -45,19 +45,34 @@ formatter (content-type selection, malformed-body handling), model validation (a 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 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. +`ScriptedScheduleControllerTests` binds with `ApiJsonSettings` itself, and two tests hold the two ways +that binder can be replaced (mutants run 2026-09-05, 9 tests in the fixture): + +| Replace the replay binder with | Result | +| --- | --- | +| `System.Text.Json` with web defaults | 2 red — Newtonsoft has no notion of the C# `required` keyword, so a body omitting one deserializes to a default rather than being rejected | +| a bare `new JsonSerializerSettings()` — Newtonsoft, but without the production configuration | 1 red — `NullValueHandling.Ignore` keeps `ContentCollection.Order` at its declared `"shuffle"` over an explicit `"order": null`, where Newtonsoft's own `Include` default writes the null through and `AddCollection`'s `Enum.TryParse` then returns a 400 | + +Both claims are about the SERIALIZER and are 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. + +**What the extraction does and does not buy.** `ApiJsonSettings` exists so the binder is defined once; +it is not a drift *detector*, and the difference matters because the branch cannot claim one. Nothing in +the suite observes `Startup.ConfigureServices` — re-inlining the `AddNewtonsoftJson` lambda as a hand-copy +of `Apply` reddens no test, because a byte-equal mirror is behaviourally indistinguishable by +construction. What the two suites *do* catch is a mirror that has lost something: the table above for the +read path, and `OpenApiSerializerContractTests` for the write path, where dropping `CustomContractResolver` +turns all 4 cases red on PascalCase keys (measured 2026-09-05). Drift confined to `ReferenceLoopHandling` +or the `StringEnumConverter` is witnessed by neither, since neither suite serializes a cycle and +`OpenApiSerializerContractTests` compares key names rather than values. `ApiAuthorizationFilter` on that list is not hypothetical — `ScriptedScheduleController` carries no `[SkipApiAuthorization]` while `ScriptedPlayoutBuilder` passes the script only host, build id and mode, with no `X-Api-Key`, and `entrypoint.py` builds its client with no credential. Whether a scripted build therefore 401s on its first -POST in this fork is unverified against a live instance and is tracked separately; it is recorded here as -the concrete shape of what an engine-level net cannot see, so the scope-out is not read as a claim of +POST in this fork is unverified against a live instance and is tracked in ersatztv#913; it is recorded here +as the concrete shape of what an engine-level net cannot see, so the scope-out is not read as a claim of equivalence. **Determinism constraints on the fixtures.** Both use Chronological order and only instant-preserving diff --git a/docs/testing.md b/docs/testing.md index 82ffda1bf..41003c4c5 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -88,18 +88,29 @@ automatic 400 either produces before the action runs, since the replay hands eac 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. +names the concrete blind spot this leaves, and it is tracked as ersatztv#913. -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 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. +The *serializer* inside that binding wrapper is covered rather than scoped out. The replay deserializes +fixture bodies through `ErsatzTV.Serialization.ApiJsonSettings`, the same function `Startup` hands to +`AddNewtonsoftJson`, and two tests witness that choice through the replay's own bind helper — the +fixture's own bodies parse the same way under any of these serializers, so each needs a body built to +separate them (mutants run 2026-09-05): + +| Replace the replay binder with | Result | +| --- | --- | +| `System.Text.Json` with web defaults | 2 of 9 red. Newtonsoft has no notion of the C# `required` keyword, so a body omitting one deserializes to a default where System.Text.Json rejects it outright. | +| a bare `new JsonSerializerSettings()` — still Newtonsoft, but without the production configuration | 1 of 9 red. `NullValueHandling.Ignore` keeps `ContentCollection.Order` at its declared `"shuffle"` over an explicit `"order": null`; Newtonsoft's own `Include` default writes the null through, and `AddCollection`'s `Enum.TryParse` then returns a 400. | + +Both are statements about the serializer and stop there — what MVC validation does with such a body +belongs to the wrapper named above. + +`ApiJsonSettings` exists so that binder is *defined* once; it is not a drift detector, and the difference +is worth stating because it bounds what the suite can promise. Nothing here observes +`Startup.ConfigureServices`, so re-inlining its `AddNewtonsoftJson` lambda as a hand-copy reddens no test +— a byte-equal mirror is behaviourally indistinguishable by construction. A mirror that has *lost* +something is caught: the table above on the read path, and `OpenApiSerializerContractTests` on the write +path, where dropping `CustomContractResolver` turns all 4 cases red on PascalCase keys. Drift confined to +`ReferenceLoopHandling` or the `StringEnumConverter` is witnessed by neither. ## Timezone independence