test: make $SYLPHEED_DISC an actual control, not a decoration (#16 remedy 3) #22
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: fabi/Sylpheed#22
Reference in New Issue
Block a user
Delete Branch "fix/corpus-control"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Implements remedy (3) of #16: make the corpus env vars an actual control.
The defect, restated precisely
unset SYLPHEED_DISCdid not disable the disc-backed suites on the machinethat has the disc — every
disc_root()fell back to a hardcoded absolute paththat exists there. The variable looked like a control and was not one. Same
for
$SYLPHEED_RES3Dand$SYLPHEED_ISO.What the refactor actually found
The 17
disc_root()copies were not 17 copies of one function. They had driftedinto five variants, and one differed semantically:
movie_manifest_disc,movie_subtitle_discandslb_discalready honoured$SYLPHEED_DISCandnothing else, while the other 14 fell back to the hardcoded path.
So the same function name already meant two different things inside one
directory — a third instance of the exact shape #16 is about. That also settled
the design question: the shared helper adopts the env-only behaviour those
three already had, rather than inventing a sixth variant.
Changes
disc_root()definitionscorpus_report.rsreports onThree of the 16 were inline in
mesh_disc.rs, in no resolver at all, and twowere in
examples/— a remedy scoped only totests/*_disc.rswould havemissed them.
Two module docs still described the fallback after it was deleted (the same
defect in prose):
texture_discclaimed "or the default dev path exists" andpak_idxd_discsaid "or drop it at the default dev path below". Both now saywhat the code does.
Verification — both directions, on the machine that has the corpus
This is the only machine where the refactor can be falsified, because it is
the only one where the old fallback used to resolve.
ABSENT — $SYLPHEED_DISC unsetPRESENT via $SYLPHEED_DISCIdentical tallies, opposite corpus states, ~10 000× apart in wall clock.
mesh_consistency_discstill decodes for ~20 min.That the two runs are indistinguishable by tally is not a flaw in the test —
it is #16, and it is why PR #19 added the corpus block. Read the block.
cargo fmt --all -- --checkclean; no new compiler warnings.Scope note: remedy (1) is deliberately not here
#[ignore]+--ignoredwas considered and rejected. Remedy (3) already movesthe mode from the machine's filesystem into the environment, which is the
property #16 asks for. And
#[ignore]already carries three meanings inthis directory (corpus-absent ×10, known-failing ×1, bare ×3 — the 14 ignored).
Overloading it a fourth time would re-create the very defect being fixed.
Happy to do (1) as a follow-up if you'd rather have
--ignoredas the gate, butit should then come with disambiguating the three existing meanings first.
Note for review
CI here runs with the corpus ABSENT, so its green means "parser-only
coverage" — exactly as the corpus block will state in the job log. The
disc-backed evidence is (B) above and cannot be reproduced by the runner.
Refs #16
Re-verified on the final tree
The table in the description was measured before the last cleanup commit
content (unused imports, doc-comment reattachment, the two stale module docs).
Those edits are non-semantic, but that is an inference, and this repo has a
standing lesson about inferring instead of counting — so I re-ran the disc
suites against the exact tree at
64f3df7:mesh_consistency_discat 1234.03 s vs 1235.53 s on the pre-cleanup run— 0.1% apart, i.e. noise. Both directions now hold on the final tree, so the
description's table stands as written.
Nothing blocking merge from my side.
Note for whoever reads this PR's CI: the runner has no corpus, so its green
means parser-only coverage — its own corpus block will say
ABSENTin the joblog. The disc-backed evidence above is local and the runner structurally cannot
reproduce it.
🤖 Generated with Claude Code
Before this commit, `unset SYLPHEED_DISC` did not disable the disc-backed suites on the machine that has the disc: every `disc_root()` fell back to a hardcoded absolute path that exists on this box. The env var looked like a control and was not one. Same for $SYLPHEED_RES3D and $SYLPHEED_ISO. Replace the duplicated resolvers with one `tests/common/mod.rs`: - 17 local `disc_root()` definitions -> 1 - 7 copies of the skip macro -> 1 (`skip_without_disc!` and siblings) - 16 hardcoded absolute paths -> 0 executable ones (3 of those were inline in `mesh_disc.rs`, in no resolver at all, and 2 were in `examples/`) - `corpus_report.rs` now reports on the SAME resolver the suites use, instead of a second copy of the logic its own comments flagged as a drift risk. The 17 copies had already drifted into FIVE variants, and they were not all the same function. `movie_manifest_disc`, `movie_subtitle_disc` and `slb_disc` honoured $SYLPHEED_DISC and nothing else, while the other 14 fell back. So one name already meant two things -- a third instance of the shape #16 is about. The shared helper adopts the env-only behaviour those three already had, rather than inventing a sixth variant. Two module docs still described the fallback after it was deleted, which is the same defect in prose: `texture_disc` claimed "or the default dev path exists" and `pak_idxd_disc` said "or drop it at the default dev path below". Both now say what the code does. Verified both ways on the machine that HAS the corpus, which is the only place this refactor can be falsified: A env unset -> "ABSENT -- $SYLPHEED_DISC unset; its suites self-skip" suites=31 passed=209 failed=0 ignored=14, slowest 0.12s B env set -> "PRESENT via $SYLPHEED_DISC" (all three corpora) suites=31 passed=209 failed=0 ignored=14, slowest 1235.53s (mesh_consistency_disc) Identical tallies, opposite corpus states, ~10000x apart in wall clock. (A) is new behaviour -- it was previously unreachable here. (B) proves nothing broke. `just test-disc` sources `.env` (already gitignored) for the set case. Note the quoting trap documented there: the corpus paths contain spaces, and an unquoted `VAR=a b c` parses as "run command `b`", failing silently into ABSENT -- which looks exactly like a working skip. Remedy (1) (`#[ignore]` + `--ignored`) is deliberately NOT done here: (3) already moves the mode from the filesystem into the environment, and `#[ignore]` already carries three meanings in this directory (corpus-absent, known-failing, bare). Overloading it a fourth time would re-create the defect. `cargo fmt --all --check` clean; no new compiler warnings. Refs #16 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>