timothyandClaude Opus 5
95b2700f09
Build ErsatzTV Image / CI toolchain image resolves (pull_request) Successful in 6s
PR Gates / CI image pin matches docker/ci (pull_request) Successful in 18s
Build ErsatzTV Image / Delimiter ban (release path) (pull_request) Successful in 20s
PR Gates / decisions lifecycle (pull_request) Successful in 20s
PR Gates / Docs update reminder (pull_request) Successful in 21s
PR Gates / Fix proofs (Proves trailers) (pull_request) Successful in 11s
review-verdict/h10 Review-verdict: MERGEABLE @ 95b2700 (base: main)
Review verdict / Set review-verdict status (pull_request_target) Successful in 23s
PR Gates / Script lint and tests (ruff + pytest) (pull_request) Successful in 8m6s
Build ErsatzTV Image / Build & test (.NET) (pull_request) Successful in 9m55s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request) Successful in 6m35s
Build ErsatzTV Image / Build & push image (amd64) (pull_request) Skipped
Build ErsatzTV Image / Functional E2E (curl + UI contracts) (pull_request) Canceled after 2m56s
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (pull_request) Canceled after 0s
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (pull_request) Canceled after 0s
Both issues are #701 deferrals, and they land together because both rewrite the same decision record. #823 -- can a null reach one of the six collection-valued scalar columns? MEASURED against a real TvContext on BOTH providers (SQLite, and MySQL 8.4 on an ephemeral server), because the reasoning available beforehand pointed the wrong way. The two converters differ on their read side -- IntCollectionValueConverter maps null-or-blank to Array.Empty<int>(), while EnumCollectionJsonValueConverter would dereference the result of JsonConvert.DeserializeObject -- so the expectation was that a NULL row behaves differently per column. NEITHER RUNS: EF does not invoke a value converter for a NULL column at all. All six materialize as CLR null, the int converter's null-to-empty branch is dead on this path, and unguarded each .Contains in AlternateScheduleSelector throws NullReferenceException. A NULL reads as UNRESTRICTED -- the All*() sets -- not as empty. This is the whole semantic question and the first draft got it backwards. It is decided by the one NULL reachable WITHOUT any code writing one: Sqlite's 20240113140741_Add_PlayoutTemplate_DaysOfMonth adds the column with nullable:true and NO defaultValue, so a PlayoutTemplate row inserted before it holds NULL and by construction had no day-of-month restriction. Reading that as empty INVERTS the row's meaning and silently stops the template applying at all. All*() preserves it, and is how "no restriction recorded" is already represented (GetPlayoutAlternateSchedulesHandler, PreviewBlockPlayoutHandler). What does NOT decide it, and was wrongly cited in the first draft: the API request records normalize an omitted field with `?? []`, but that is a client omitting a field on a WRITE and says nothing about what a legacy database NULL meant. Two read sites, not one. Guarding only the selector would have left the entity->DTO mappers unguarded, and those feed the SPA: PlayoutScheduleEditors spreads the collection (`[...template.daysOfMonth]` -> TypeError on a JSON null) and playoutTemplateCalendar's appliesToDate -- an exact port of GetScheduleForDate -- calls .includes on it. Both mappers now substitute the SAME defaults, so the preview agrees with what is actually scheduled. Neither guard is assigned back onto the entity, which is the media.nullable-primitive-collection-mutation mechanism. Reachability, stated precisely rather than overclaimed. All six are nullable:true on both providers, but a nullable column does not produce a NULL row: five of the six were present at CreateTable, so a NULL there still needs code to write one, and on MySQL there is NO code-path-free NULL for any of the six. The write path ACCEPTS a null (SaveChanges succeeds, stores SQL NULL) but no caller supplies one today -- every production construction of the two commands goes through the request records. That is a property of the code, not a live caller; claiming otherwise would be the banned "it's AsNoTracking today" argument pointed the other way. #824 -- ElasticSearchIndex.UpdateSong had no regression test Issue option 1 (a non-network transport) shipped, and needed no new package: Elastic.Transport.InMemoryRequestInvoker is public in the pinned version and ElasticsearchClientSettings(NodePool, IRequestInvoker) accepts it, injected into the private _client the way #701 injects the Lucene IndexWriter. UpdateItems never runs `_client ??= CreateClient()`, so the injected instance is the one used. Two traps there are load-bearing, both measured: the canned response must carry an `X-Elastic-Product: Elasticsearch` header or the client's product check throws UnsupportedProductException INTO UpdateSong's catch, and an empty body fails to deserialize the same way. Either turns the fixture into a green measurement of the error path -- which is how it first failed here, caught by the ThrowOnWarningLogger. The document id is asserted as the LAST PATH SEGMENT, not by substring: the index name carries digits, so ShouldContain would stop discriminating for a song whose id collided with one. Six mutations executed, each disarming ITS OWN clause alone: - `??=` restored in ElasticSearchIndex only -> the Elastic fixture reddens on "metadata.Artists should be null but was []" while the LUCENE fixture stays GREEN. The #824 hole demonstrated, not described. - DaysOfWeek guard disarmed in the selector -> 4 red, 3 green (DaysOfMonth and MonthsOfYear unaffected). Each clause is independently load-bearing. - DaysOfMonth guard disarmed in Playouts.Mapper -> 1 red, 2 green. - Elastic dropped from the covered set / mapped to the SAME fixture as Lucene / mapped to a class with no [Test] -> SearchIndexMutationCoverageTests reddens on each. That coverage guard is the boundary fix the issue asked for: the covered set is compared against an ISearchIndex population DERIVED FROM THE ASSEMBLY. Its claim stops where the check does -- no static check can establish that a named fixture actually DRIVES its indexer, so it forces a human to look rather than proving coverage. ThrowOnWarningLogger moved to ErsatzTV.Tests/Support so both fixtures share it; the Lucene fixture's assertions are otherwise untouched, since it is a witnessed proof artifact. No production change in ElasticSearchIndex.cs -- #824 is coverage only. Docs: testing.md gains a "Provider-parity fixtures" section naming all THREE opt-in-MySQL fixtures and recording that CI runs none of them (#627); docs/README.md gains the matching task signal; guard-inventory.md's hand-written C# guard list goes from five files to six. Scheduling/Mapper.cs loses the UTF-8 BOM it inherited, per #311 fix-as-you-touch. Local gate (with the MySQL lane armed): ErsatzTV.Tests 2096 passed / 0 skipped, Core.Tests 693/1, Infrastructure.Tests 114, Architecture.Tests 7, Scanner.Tests 1504 -- 0 failures in each. scripts/tests 1228 passed / 2 skipped. dotnet format whitespace --verify-no-changes clean; BOM check over the touched set with the population COUNT asserted, because a bare zsh loop silently checks one concatenated filename. decisions_validate OK. Fixes #823 Fixes #824 Decisions-Edit: yes Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019zUmJZHhVP7kXg5DV237TW
158 lines
12 KiB
Markdown
158 lines
12 KiB
Markdown
# Testing map
|
|
|
|
Purpose: authoritative map of what each test project/suite covers and how to run it. Read this
|
|
before adding tests, not just `docs/contributing.md` §8 (which now just points here).
|
|
|
|
## Test projects
|
|
|
|
> **Font dependency (ersatztv#732).** `Infrastructure/Graphics/TextElementBackgroundBoxTests` is the
|
|
> only suite that rasterises text, so it needs at least one system font to lay anything out. The CI
|
|
> image installs none explicitly — fonts arrive via `playwright install --with-deps` in
|
|
> `docker/ci/Dockerfile` (351 present in the pinned image, measured 2026-08-26). This is a real
|
|
> dependency, declared here so a future slimming of that install produces a known cause rather than a
|
|
> mystery red. `Baseline_Renders_A_Non_Empty_Bitmap` exists to make that failure loud: without it a
|
|
> fontless host would render a 0x0 bitmap and every relative geometry assertion would pass vacuously.
|
|
|
|
| Project | Covers | Notes |
|
|
|---|---|---|
|
|
| `ErsatzTV.Tests` | API controllers + MediatR handlers, plus the SkiaSharp text-overlay rasteriser (`Infrastructure/Graphics/`) | In-memory SQLite fixture: a shared `SqliteConnection("Data Source=:memory:;Foreign Keys=False")` kept open + `EnsureCreatedAsync()` (**not** full migration replay) + `PRAGMA foreign_keys=OFF`, then seed; a tiny `IDbContextFactory` wraps `new TvContext(...)`. ~1,870 tests (approximate on purpose — an exact count goes stale on every PR that adds one; the previous hardcoded 828 was off by over a thousand). |
|
|
| `ErsatzTV.Core.Tests` | Domain logic, scheduling, IPTV/XMLTV generation | References `ErsatzTV.Application` directly — there is no separate `Application.Tests` project. ~650 tests + 1 skipped under `TZ=UTC` (the Block playout golden additionally skips under a non-UTC `TZ`; see Golden-file nets). |
|
|
| `ErsatzTV.Scanner.Tests` | Library scanning: scan handlers, folder scanners, NFO readers | Handler tests substitute the folder scanners + `ILibraryRepository` and assert the resulting repository writes (e.g. `ScanLocalLibraryHandlerTests` pins which `LastScan` levels a scan records — ersatztv#264). Fakes/`Testably` back the file-system-facing scanners. ~1,485 tests (approximate on purpose — an exact count goes stale on every PR that adds one). Additionally contains `Core/FFmpeg/TranscodingTests` — `[Explicit]` + `[Combinatorial]`, so it never runs in CI or a plain `dotnet test` (it needs real ffmpeg/hardware) and contributes 0 to that count; run it by name when touching the transcoding pipeline. |
|
|
| `ErsatzTV.Architecture.Tests` | Layering rules via NetArchTest.eNhancedEdition | Core↛Infra/App/EF; FFmpeg↛all; App↛concrete providers. 5 tests. See `docs/contributing.md` §1. |
|
|
| `ErsatzTV.FFmpeg.Tests` | FFmpeg command construction | Build a pipeline, assert the exact rendered arg string (`PipelineBuilderBaseTests.cs`). |
|
|
| `web/` (vitest) | React SPA unit tests | Run alongside typecheck + build (see below). Collects `src/**`, `web/scripts/**` *and* `web/vite-plugins/**`, but deliberately **excludes** `web/e2e/**` (the Playwright specs — vitest's default `**/*.spec.*` glob would otherwise run them under jsdom). Two files have git prerequisites since ersatztv#819. `web/src/api/pageSizeCallSites.guard.test.ts` needs a git **checkout** AND, through it, the **binary** — it derives its file population from `git ls-files` via `web/vite-plugins/trackedSourceFiles.ts` rather than a directory walk, and refuses rather than falling back. `web/vite-plugins/trackedSourceFiles.realgit.test.ts` needs the **binary** but no checkout: it builds its own temp repository to prove that derivation by executing it. So it is not checkout-versus-binary — supplying a `.git` alone would not let either run. Every other file runs fine with neither. `docker/Dockerfile`'s web-build stage runs the suite with exactly those two `--exclude`d, because its context carries no `.git` and `node:22-bookworm-slim` ships no git — and the two exclusions overlap rather than divide — both files need the binary. |
|
|
| `web/e2e/` (Playwright) | UI-interactive E2E flows against a **live** instance | Not a unit suite and **not** part of `npm test` — needs a running server, so it runs via `scripts/e2e-ui.sh` (boots its own fresh instance) and in CI as a step of the `functional-e2e` job. Headless Chromium, `serial`, `retries: 0`. Scope rule: assert only what the curl harness structurally cannot. See `docs/e2e-local.md` → "UI-E2E harness". |
|
|
|
|
## Golden-file nets
|
|
|
|
Three golden-file suites guard the highest-value, most-subtle output:
|
|
|
|
- **M3U**: `ErsatzTV.Core.Tests/Iptv/ChannelPlaylistGoldenTests.cs` (ersatztv#11) — env var `ETV_UPDATE_GOLDENS`
|
|
- **XMLTV**: `ChannelGuideGoldenTests` (ersatztv#28) — env var `ETV_UPDATE_GOLDENS`
|
|
- **Playout build**: `ErsatzTV.Core.Tests/Scheduling/Goldens/PlayoutBuildGoldenTests.cs` (ersatztv#163)
|
|
— env var **`ETV_UPDATE_PLAYOUT_GOLDENS`** (deliberately separate from `ETV_UPDATE_GOLDENS` so
|
|
regenerating one net can't silently rewrite the other). Snapshots the `PlayoutItem`s each builder
|
|
produces over a pinned build window. Covers the **Classic** (`PlaybackOrder.Chronological`),
|
|
**Block**, and **Sequential (YAML)** builders. The **Scripted** *end-to-end pipeline* is excluded from this
|
|
net — `ScriptedPlayoutBuilder` runs a user-authored external process that drives the engine over HTTP, which
|
|
the in-memory harness can't pin; that integration harness is tracked in ersatztv#563. The scheduling
|
|
*behavior* those scripts drive, though, lives in the in-process `SchedulingEngine` (the HTTP controller is a
|
|
1:1 pass-through) and IS directly testable — `SchedulingEngineTests` news it up with substitutes, and
|
|
`ContentEnumeratorBuilderTests` (ersatztv#395) is the direct regression net over the enumerator-construction
|
|
helper the Scripted and Sequential/YAML engines now share (decision:
|
|
`testing.scripted-playout-golden-deferred`). The **Sequential** case
|
|
(`Sequential_yaml`, ersatztv#381)
|
|
builds from a committed YAML fixture (`Goldens/Fixtures/sequential-schedule.yml`) instead of a
|
|
`ProgramSchedule`; it is TZ-independent (the `count`/`all`/`duration` handlers do UTC-only arithmetic —
|
|
it passes, not skips, under a non-UTC `TZ`) so needs no `Assume` guard. A third case,
|
|
`Classic_clock_padded` (ersatztv#77), locks clock-boundary padding: a `FillerMode.Pad` +
|
|
`PadToNearestMinute=15` PostRoll preset snaps content to `:15`, and the test both goldens the output and
|
|
asserts every content item after the first starts on a quarter-hour. Its EPG counterpart is
|
|
`ErsatzTV.Core.Tests/Channels/ChannelGuideProjectorClockPadTests.cs` (guide programmes stop on the padded
|
|
boundary). The build reads
|
|
no wall clock — time enters only via the caller-supplied `start` — so a pinned `start` is fully
|
|
deterministic. Snapshots the raw `PlayoutItem.Start`/`Finish` (UTC), **not** the `*Offset` properties
|
|
(those call `.ToLocalTime()` and would make the golden machine-TZ dependent). The **Block** case is
|
|
TZ-sensitive by construction (`BlockPlayoutBuilder` maps template times via `TimeZoneInfo.Local`), so
|
|
it is guarded with `Assume.That(TimeZoneInfo.Local.BaseUtcOffset == Zero)`: it runs under `TZ=UTC`
|
|
(CI) and reports **inconclusive** (a graceful skip, not a failure) under any other TZ. A real TZ seam
|
|
for the block builder is ersatztv#380's scope.
|
|
|
|
All three locate their golden files via `[CallerFilePath]`. A missing golden is a hard fail, not a
|
|
skip. Regenerate via `ETV_UPDATE_GOLDENS=1 dotnet test ...` (M3U/XMLTV) or
|
|
`ETV_UPDATE_PLAYOUT_GOLDENS=1 dotnet test ...` (playout build).
|
|
|
|
**Never set `ETV_UPDATE_GOLDENS` / `ETV_UPDATE_PLAYOUT_GOLDENS` in CI or from an agent.** A golden
|
|
diff during normal test runs means the code changed the output — regenerating to make the diff go
|
|
away hides the change instead of surfacing it. Only a human who has confirmed the change is
|
|
intentional should regenerate.
|
|
|
|
## Timezone independence
|
|
|
|
The suite is timezone-independent (ersatztv#24). When constructing test `PlayoutItem`s, always
|
|
set a real `Start` (e.g. `startState.CurrentTime.UtcDateTime`) — never rely on the default
|
|
`DateTime.MinValue`, which underflows `DateTimeOffset.MinValue` once a non-UTC local offset is
|
|
applied (`StartOffset` calls `ToLocalTime()`). CI runs in UTC; local runs may not.
|
|
|
|
## Running tests
|
|
|
|
Full .NET gate:
|
|
|
|
```bash
|
|
dotnet build ErsatzTV.sln
|
|
TZ=UTC dotnet test
|
|
```
|
|
|
|
CI adds `--blame-hang-timeout 2m` to catch hangs.
|
|
|
|
Fast subsets:
|
|
|
|
```bash
|
|
# single project
|
|
dotnet test ErsatzTV.Tests
|
|
|
|
# filtered
|
|
dotnet test ErsatzTV.Core.Tests --filter FullyQualifiedName~ChannelPlaylistGoldenTests
|
|
```
|
|
|
|
Web (`web/`):
|
|
|
|
```bash
|
|
npm test -- --run # vitest, single pass — use this for the SPA guards (see below)
|
|
npm test # vitest WATCH mode. `pageSizeCallSites.guard.test.ts` reads the git index ONCE
|
|
# per dev-server lifetime while the glob refreshes, so it drifts BOTH ways: a
|
|
# file created mid-session reddens it misleadingly, and a file that was already
|
|
# untracked when the watcher started stays invisible to it after `git add` — a
|
|
# green that is not authoritative. Restarting the watcher swaps the first
|
|
# problem for the second; confirm with `npm test -- --run`.
|
|
npm run typecheck # tsc -b --pretty false
|
|
npm run lint # eslint .
|
|
npm run build # tsc -b && vite build
|
|
```
|
|
|
|
UI-E2E (needs a built solution + built SPA; boots and tears down its own instance):
|
|
|
|
```bash
|
|
scripts/e2e-ui.sh # from the repo root, NOT web/
|
|
```
|
|
|
|
## Provider-parity fixtures (opt-in MySQL)
|
|
|
|
Most of `ErsatzTV.Tests` runs on the in-memory SQLite harness described above. A few fixtures in
|
|
`ErsatzTV.Tests/Integration/` instead drive a **real, migrated** database, and run the SAME body against
|
|
**both** providers because the behaviour they pin is provider-specific:
|
|
|
|
| Fixture | What is provider-specific about it |
|
|
| --- | --- |
|
|
| `LibraryFolderDedupeMigrationTests` | the #491 dedupe DML — two MySQL-only collation defects (case-insensitive grouping, then `PAD SPACE`) were unreachable from SQLite |
|
|
| `SchedulingCollectionColumnNullTests` | what a NULL column materializes as through a value converter (ersatztv#823) |
|
|
| `SearchFieldValuesProviderTests` | the search-field-values query shape, which differs per provider |
|
|
|
|
The MySQL half needs a live server, supplied as `ETV_TEST_MYSQL_CONNECTION`. **Without it these
|
|
fixtures `Assert.Ignore` — a visible skip, never a silent pass**, so an ordinary local run needs no
|
|
MySQL. Setting `ETV_REQUIRE_MYSQL_TESTS=1` turns that skip into a hard failure, for a runner that is
|
|
supposed to have one.
|
|
|
|
```bash
|
|
ETV_TEST_MYSQL_CONNECTION='Server=<host>;Port=3306;Uid=root;Pwd=<pw>;DefaultCommandTimeout=300;' \
|
|
dotnet test ErsatzTV.Tests --filter FullyQualifiedName~SchedulingCollectionColumnNullTests
|
|
```
|
|
|
|
Each test uses a database name it generates per run, so isolation does not depend on a wipe
|
|
succeeding, and drops it in teardown. **CI does not currently run any of these MySQL halves** — the
|
|
`migrations` job spins a `mysql:8.4` service but only applies migrations to a fresh EMPTY database,
|
|
so it executes no data rows; re-arming these fixtures there is tracked by ersatztv#627.
|
|
|
|
## Per-PR verification gate
|
|
|
|
Before opening a PR: build the solution, run `ErsatzTV.Tests` + `ErsatzTV.Core.Tests` (plus
|
|
`ErsatzTV.Scanner.Tests`, `ErsatzTV.Architecture.Tests` and `ErsatzTV.FFmpeg.Tests` if touched),
|
|
and run the web test/lint/typecheck/build steps above. All must be green. A golden-file diff or
|
|
an architecture-test failure is a hard stop — fix the code, don't regenerate/relax the test.
|
|
|
|
## See also
|
|
|
|
- `docs/ci-cd.md` — CI pipeline (test → migrations → build), versioning, dependency management.
|
|
- `docs/contributing.md` §1 — layering rules enforced by `ErsatzTV.Architecture.Tests`.
|
|
- `docs/contributing.md` §8 — short pointer back to this doc.
|