JR-023: correct the missing-dependency logs, and test them
Both failure paths still told admins the overlay was "falling back to patching index.html on disk". JR-021 deleted that fallback, so the message was false -- and it was false in the worst place, since an admin reads it precisely when debugging a missing overlay and would go hunting for a patch that no longer exists. They now name the install URL and say the overlay is disabled while every other feature is unaffected. The not-found case is a Warning, not Information: a headline feature being off should not sit among startup chatter. UT-012..015 cover the branch. The File Transformation assembly is genuinely absent from the test host, so TryRegister exercises its real not-found path rather than a seam invented for the test. UT-015 pins that a null payload returns empty rather than throwing -- this callback runs inside another plugin's request path on every page served, so throwing would break the web client itself, not just JRay's overlay. Making those runnable needed the test project to reference Jellyfin.Controller and Jellyfin.Model without the plugin's ExcludeAssets=runtime. My earlier claim that DisableTransitiveFrameworkReferences alone sufficed was too narrow: it drops the demand for the web *framework*, but ILogger and MediaBrowser.Common live in the excluded assets, so anything past dependency-free logic failed at run time with FileNotFoundException. Both settings are needed, and the register now says so. Second mutation check: downgrading the warning to Information fails UT-013 and nothing else. Source restored and re-verified. JR-023 reaches Done for the detection half. The config-page banner stays T4 -- verifiable only against a live server. TRACES: UT-012, UT-013, UT-014, UT-015 | JR-023 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Claude Opus 5
+32
-16
@@ -31,11 +31,17 @@ Tag code with `// TRACES: JR-012 | SR-002`.
|
||||
| UT-009 | **Prioritised series inside an ignored genre — series wins** | JR-016 | **Passing** |
|
||||
| UT-010 | Genre matching is case-insensitive | JR-016 | **Passing** |
|
||||
| UT-011 | A Series rule valued `Guid.Empty` does not swallow every movie | JR-016 | **Passing** |
|
||||
| UT-012 | `TryRegister` returns `false` when File Transformation is absent | JR-023 | **Passing** |
|
||||
| UT-013 | …and **warns** naming the install URL, with no "falling back" claim | JR-023 | **Passing** |
|
||||
| UT-014 | Overlay disabled ⇒ `index.html` returned unchanged | JR-023 | **Passing** |
|
||||
| UT-015 | Null contents return empty rather than throwing — this callback runs on every page another plugin serves | JR-023 | **Passing** |
|
||||
|
||||
All 11 execute and pass. The suite was also checked to **fail** on a mutation —
|
||||
removing the newline-stripping from `RemoveInjection` fails UT-001 and nothing
|
||||
else — because a suite that has only ever passed is not evidence that it tests
|
||||
anything.
|
||||
All 15 execute and pass. The suite was also checked to **fail** on two separate
|
||||
mutations, because a suite that has only ever passed is not evidence that it
|
||||
tests anything: removing the newline-stripping from `RemoveInjection` fails
|
||||
UT-001 alone, and downgrading the missing-dependency warning to `Information`
|
||||
fails UT-013 alone. In both cases the blast radius was one test, and the source
|
||||
was restored and re-verified.
|
||||
|
||||
`JR` is flat rather than split by theme. The plugin is one deployable with one
|
||||
audience, and the thematic grouping lives in the section headings below, where it
|
||||
@@ -95,7 +101,7 @@ coordinated `schema_version` bumps (SR-003).
|
||||
| JR-020 | Pause overlay: injected client script queries `jray?t=` and renders the scene's cast | **PR-001** | High | Done |
|
||||
| JR-021 | **jRay never injects into `index.html` on disk.** File Transformation is a hard dependency; there is no on-disk fallback. The only permitted write is JR-022's removal | PR-004 | High | **Done** |
|
||||
| JR-022 | Migration: remove any on-disk patch left by an earlier jRay, identified by the `<!-- jray-overlay -->` marker | PR-004 | High | **Done** (UT-001…006) |
|
||||
| JR-023 | Absent the dependency, disable **only** the overlay and say so in the log and the config page; never bundle the assembly | PR-004 | Medium | In Progress |
|
||||
| JR-023 | Absent the dependency, disable **only** the overlay and say so in the log and the config page; never bundle the assembly | PR-004 | Medium | **Done** (UT-012…015; config page is T4) |
|
||||
| JR-024 | Actor names and all server-supplied strings render as **text, never markup** | SR-004 | High | Done |
|
||||
|
||||
## Manifest exchange client (JR-025 … JR-037)
|
||||
@@ -188,7 +194,7 @@ thing when read across repos; reusing T3 for a live tier here would make a
|
||||
cross-repo reader count live-only requirements as covered.
|
||||
|
||||
`Jellyfin.Plugin.JRay.Tests` (xUnit, in the solution) carries the T1 tier. It
|
||||
builds clean alongside the plugin and all 11 tests pass.
|
||||
builds clean alongside the plugin and all 15 tests pass.
|
||||
|
||||
### Running the suite on a box without the web runtime
|
||||
|
||||
@@ -201,16 +207,26 @@ work anywhere, and both are load-bearing rather than incidental:
|
||||
runtime the plugin does not otherwise need.
|
||||
- **`DisableTransitiveFrameworkReferences=true`.** The plugin
|
||||
framework-references `Microsoft.AspNetCore.App` through `Jellyfin.Controller`,
|
||||
and that flows into anything referencing it — so the test host would demand a
|
||||
web runtime even for tests that touch no web type. .NET resolves assemblies
|
||||
lazily, so pure-logic types load fine without it.
|
||||
and that flows into anything referencing it — so the test host would otherwise
|
||||
demand a web runtime that no version of exists in the Arch repositories for
|
||||
.NET 9 (8 and 10 only).
|
||||
- **Explicit `Jellyfin.Controller` / `Jellyfin.Model` references.** The plugin
|
||||
sets `ExcludeAssets=runtime` on both, because at run time the *server* supplies
|
||||
them and shipping copies would risk loading a second, different
|
||||
`MediaBrowser.Common`. The test host is not the server, so it must bring its
|
||||
own — hence the same packages without that exclusion, and only in the test
|
||||
project.
|
||||
|
||||
**This is a T1-tier decision, not a workaround to unwind.** These tests exercise
|
||||
string and rule logic; requiring an ASP.NET Core runtime to run them would be
|
||||
incidental coupling. **T2 — controllers and authorisation — genuinely needs that
|
||||
runtime**, and those tests belong in a second project that keeps the reference.
|
||||
Note that no ASP.NET Core 9 exists in the Arch repositories (8 and 10 only), so
|
||||
that project will lean on `RollForward` too.
|
||||
Those two together are what make it work: the first drops the demand for the web
|
||||
*framework*, the second supplies the Jellyfin *assemblies*. Cutting the framework
|
||||
reference alone is not enough — `ILogger` and `MediaBrowser.Common` live in the
|
||||
excluded assets, so anything beyond genuinely dependency-free logic fails to load
|
||||
with `FileNotFoundException` at run time rather than at build.
|
||||
|
||||
**T2 — controllers and authorisation — is still a separate matter**, since
|
||||
instantiating MVC types needs the ASP.NET Core runtime itself, not just its
|
||||
reference assemblies. Those tests belong in a second project that keeps the
|
||||
framework reference and leans on `RollForward` to reach the 10.0 runtime.
|
||||
|
||||
### Per-requirement verification plan
|
||||
|
||||
@@ -238,7 +254,7 @@ that project will lean on `RollForward` too.
|
||||
| JR-018 | T1 | `covered / (total - ignored)` | Item carrying two genres counts in both rows |
|
||||
| JR-021 | **static** | No code path *adds* the script tag to `index.html` | `scripts/checks/no-index-injection.sh`. Removal (JR-022) is the one permitted write, so the check is on injection, not on writing. Verified to **fail** on a reintroduced `Apply()` and on reintroduced `ReplaceLast` injection, not merely to pass today |
|
||||
| JR-022 | T1 | A marked legacy patch is removed; unmarked content untouched | Foreign plugin's injection left intact |
|
||||
| JR-023 | T1 + **T4** | Absent dependency disables only the overlay | Detection unit-testable; config-page display is live |
|
||||
| JR-023 | T1 + **T4** | Absent dependency disables only the overlay, and the log says so | Detection and log content covered by UT-012…015; the config-page banner is T4, verifiable only against a live server |
|
||||
| JR-024 | T1 | A name containing markup renders escaped | `<script>` in an actor name from a hostile server |
|
||||
| JR-025 | T1 | First result clearing the tier wins; disabled servers skipped | All servers fail; first server returns a below-tier match |
|
||||
| JR-026 | T1 | Server 2 queried only for episodes server 1 lacked | Bundle with a gap in the middle of a season |
|
||||
|
||||
Reference in New Issue
Block a user