alex/ephyra
Watch
1
0
Fork
You've already forked ephyra
 0
Code Issues 2 Pull requests Projects Releases Packages 1 Wiki Activity Actions

Thirty-five tests skip themselves and are reported as passed #137

New issue
Closed
opened 2026-08-14 08:16:32 +02:00 by Claude · 1 comment
Claude commented 2026-08-14 08:16:32 +02:00
Collaborator
Copy link

Thirty-five tests across seven files skip themselves by printing a note and returning, and nextest counts every one of them as passed:

let Some(scanned) = scan().await else { return };   // prints "skipped: no reference media"

Measured while reviewing #106: in a worktree without reference/snapshots/ copied in, 15 of the 35 were vacuous — they printed their note, returned, and were reported green. scan_enriched.rs's twelve run in 0.011 s each when vacuous and 2.3 s or more when real, so the difference is visible in the timing and nowhere in the result.

The workspace already has an honest idiom for this and uses it for nine other tests: a build-script cfg plus #[cfg_attr(…, ignore)], which nextest reports as skipped. Two idioms, one of which lies.

Why this is worth more than any single issue

Every milestone in this project has an entry about a check that reported success while measuring nothing:

  • M5: "A scenario that resolves no id is worse than a missing one: it reports success." Six scenarios were vacuous for a day.
  • M8, after the pin moved: bundle.json asked for an account that no longer existed and 10 of 43 responses were answered by neither server, and the run passed. It is held to "max_unanswered": 0 now for exactly that reason.
  • #111's visibility.json was built almost entirely out of refusals, so a regression returning None for every restricted account would have diffed clean.

This is the same failure one layer down, in the test runner rather than the harness — and it is the layer that is supposed to catch the others.

JELLYFIN_RS_REQUIRE_FIXTURES=1 exists and is the right lever: CI sets it, and under it a missing fixture fails instead of leaving a note. But it is opt-in, and a developer or an agent running cargo nextest run --workspace in a fresh worktree gets fifteen green tests that did nothing. That is where the port is actually written.

Where to start

The seven files that use the silent form, and crates/jellyfin-app/tests/scan_enriched.rs:105-115 for the shape of the gate. The nine honest ones are the model.

Two things to decide rather than assume:

  • Whether the env var should be the default rather than the escape. A fixture that is missing is nearly always a mistake; the case for tolerating it is a fresh clone with no reference instance, which xtask can detect for itself.
  • What a gate that cannot be satisfied should do. scan_enriched.rs's enriched_libraries() (added by #106) filters on a directory existing with no note at all and no env-var escape, so it is quiet even in CI. A gate with no way to make it loud is worse than one with an opt-in.

How to know it is done

A test that cannot run says so in a way nextest reports as skipped rather than passed, and a run with JELLYFIN_RS_REQUIRE_FIXTURES=1 in a worktree missing a fixture fails and names what is missing. Then the count in a green run means what it says — which is the only reason to keep counts in a close comment at all.

House rules

Tests run under cargo nextest; upstream's source is the checkout in upstream/ at the pin; generated artifacts are regenerated, never edited; comments say what the code does, not how it got there. Kill processes by PID after checking /proc/<pid>/cwd, never by pkill -f — the pattern matches the shell running it.

Thirty-five tests across seven files skip themselves by printing a note and returning, and **nextest counts every one of them as passed**: ```rust let Some(scanned) = scan().await else { return }; // prints "skipped: no reference media" ``` Measured while reviewing #106: in a worktree without `reference/snapshots/` copied in, **15 of the 35 were vacuous** — they printed their note, returned, and were reported green. `scan_enriched.rs`'s twelve run in 0.011 s each when vacuous and 2.3 s or more when real, so the difference is visible in the timing and nowhere in the result. The workspace already has an honest idiom for this and uses it for nine other tests: a build-script `cfg` plus `#[cfg_attr(…, ignore)]`, which nextest reports as **skipped**. Two idioms, one of which lies. ## Why this is worth more than any single issue Every milestone in this project has an entry about a check that reported success while measuring nothing: - M5: *"A scenario that resolves no id is worse than a missing one: it reports success."* Six scenarios were vacuous for a day. - M8, after the pin moved: `bundle.json` asked for an account that no longer existed and 10 of 43 responses were answered by neither server, and the run passed. It is held to `"max_unanswered": 0` now for exactly that reason. - #111's `visibility.json` was built almost entirely out of refusals, so a regression returning `None` for every restricted account would have diffed clean. This is the same failure one layer down, in the test runner rather than the harness — and it is the layer that is supposed to catch the others. `JELLYFIN_RS_REQUIRE_FIXTURES=1` exists and is the right lever: CI sets it, and under it a missing fixture fails instead of leaving a note. But it is opt-in, and a developer or an agent running `cargo nextest run --workspace` in a fresh worktree gets fifteen green tests that did nothing. That is where the port is actually written. ## Where to start The seven files that use the silent form, and `crates/jellyfin-app/tests/scan_enriched.rs:105-115` for the shape of the gate. The nine honest ones are the model. Two things to decide rather than assume: - **Whether the env var should be the default rather than the escape.** A fixture that is missing is nearly always a mistake; the case for tolerating it is a fresh clone with no reference instance, which `xtask` can detect for itself. - **What a gate that cannot be satisfied should do.** `scan_enriched.rs`'s `enriched_libraries()` (added by #106) filters on a directory existing with no note at all and no env-var escape, so it is quiet even in CI. A gate with no way to make it loud is worse than one with an opt-in. ## How to know it is done A test that cannot run says so in a way nextest reports as skipped rather than passed, and a run with `JELLYFIN_RS_REQUIRE_FIXTURES=1` in a worktree missing a fixture fails and names what is missing. Then the count in a green run means what it says — which is the only reason to keep counts in a close comment at all. ### House rules Tests run under `cargo nextest`; upstream's source is the checkout in `upstream/` **at the pin**; generated artifacts are regenerated, never edited; comments say what the code does, not how it got there. Kill processes by PID after checking `/proc/<pid>/cwd`, never by `pkill -f` — the pattern matches the shell running it.
Claude added
p1
 and removed
p2
 labels 2026-08-20 17:31:12 +02:00
Claude commented 2026-08-20 18:29:08 +02:00
Author
Collaborator
Copy link

Landed in 48dbd8a and 0c62072.

The gate moved to build time. crates/ephyra-fixtures names each fixture once —
the snapshot, the media, the enriched media, the upstream checkout, ffmpeg and
ffprobe on PATH — a build script per crate declares the cfg for whichever the
checkout has, and each gated test carries
#[cfg_attr(not(has_…), ignore = "…")].

Past the gate the fixture is required rather than probed: the helpers return the
path and assert, so the let Some(x) = f() else { return } that turned a
missing fixture into a green test is gone from all eight files.

The two things this wanted decided rather than written.

Whether the env var should be the default rather than the escape — neither, in
the end. The cfg makes the default honest, so EPHYRA_REQUIRE_FIXTURES no
longer has to carry that weight; it fails the build instead, naming what is
missing before a test runs. Louder than a per-test assert, and it cannot be
reached vacuously.

What a gate that cannot be satisfied should do — enriched_libraries() now
asserts all three sub-libraries rather than quietly dropping one, which is what
its own doc comment already said it wanted.

Measured, in a worktree with no reference/ copied in:

with fixtures without
passed 2516 2470
skipped 0 46

And EPHYRA_REQUIRE_FIXTURES=1 there fails the build with
needs reference/snapshots/default/config/data/jellyfin.db — run cargo xtask reference bootstrap.

One review round, which found two defects in the new crate, both fixed in
0c62072: declare_programs watched nothing, so installing ffmpeg never lifted
the ignore; and 28 tests that spawn ffprobe were gated only on the media, so a
machine with the fixtures but no ffmpeg build failed them rather than ignoring
them.

Two things left, neither in this issue's scope:

  • A fixtureless checkout recompiles four crates on every cargo invocation — a
    watched path that does not exist is dirty forever. It is the price of the
    ignore lifting when you provision one, and it was already true of
    ephyra-db and xtask.
  • scan_enriched.rs derives its scratch directory from process::id() alone,
    so its twelve tests share one path under a single-process runner. Safe under
    nextest, not under cargo test.
Landed in `48dbd8a` and `0c62072`. The gate moved to build time. `crates/ephyra-fixtures` names each fixture once — the snapshot, the media, the enriched media, the upstream checkout, `ffmpeg` and `ffprobe` on PATH — a build script per crate declares the cfg for whichever the checkout has, and each gated test carries `#[cfg_attr(not(has_…), ignore = "…")]`. Past the gate the fixture is required rather than probed: the helpers return the path and assert, so the `let Some(x) = f() else { return }` that turned a missing fixture into a green test is gone from all eight files. **The two things this wanted decided rather than written.** *Whether the env var should be the default rather than the escape* — neither, in the end. The cfg makes the default honest, so `EPHYRA_REQUIRE_FIXTURES` no longer has to carry that weight; it fails the **build** instead, naming what is missing before a test runs. Louder than a per-test assert, and it cannot be reached vacuously. *What a gate that cannot be satisfied should do* — `enriched_libraries()` now asserts all three sub-libraries rather than quietly dropping one, which is what its own doc comment already said it wanted. **Measured**, in a worktree with no `reference/` copied in: | | with fixtures | without | |---|---|---| | passed | 2516 | 2470 | | skipped | 0 | **46** | And `EPHYRA_REQUIRE_FIXTURES=1` there fails the build with `needs reference/snapshots/default/config/data/jellyfin.db — run cargo xtask reference bootstrap`. **One review round**, which found two defects in the new crate, both fixed in `0c62072`: `declare_programs` watched nothing, so installing ffmpeg never lifted the ignore; and 28 tests that spawn `ffprobe` were gated only on the media, so a machine with the fixtures but no ffmpeg build failed them rather than ignoring them. Two things left, neither in this issue's scope: - A fixtureless checkout recompiles four crates on every `cargo` invocation — a watched path that does not exist is dirty forever. It is the price of the ignore lifting when you provision one, and it was already true of `ephyra-db` and `xtask`. - `scan_enriched.rs` derives its scratch directory from `process::id()` alone, so its twelve tests share one path under a single-process runner. Safe under nextest, not under `cargo test`.
Sign in to join this conversation.
No Branch/Tag specified
Branches Tags
main
No results found.
Labels
Clear labels   
correctness
Diverges from upstream

  
encoding
Transcoding, ffmpeg, and the arg strings

  
http
Route behaviour

  
jobs
Scheduled tasks

  
operations
Installing a server and keeping one running

  
p1
Do this first

  
p2
After the p1s

  
p3
Real, and not urgent

  
performance
Speed or resource use

  
providers
Metadata from somewhere other than the file

  
scanner
The library scan

  
security
Who is allowed to see what

  
testing
Test coverage and harness

No labels
correctness
encoding
http
jobs
operations
p1
p2
p3
performance
providers
scanner
security
testing
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
alex/ephyra#137
No description provided.