Fabian Hamm
15d51b30ac
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>
80 lines
3.3 KiB
Rust
80 lines
3.3 KiB
Rust
//! One place that decides where the disc corpora are — issue #16, remedy (3).
|
|
//!
|
|
//! # What this replaces
|
|
//!
|
|
//! Seventeen files under `tests/` each defined their own `disc_root()`, and they
|
|
//! had **already drifted into five variants**. Four were the same thing written
|
|
//! four ways (differing only in return type and style). The fifth —
|
|
//! `movie_manifest_disc`, `movie_subtitle_disc`, `slb_disc` — did something
|
|
//! materially different: it honoured `SYLPHEED_DISC` **and nothing else**.
|
|
//!
|
|
//! So one function name meant two different things in one directory, which is
|
|
//! the same "one name, several meanings" defect #16 identifies in
|
|
//! `SYLPHEED_DISC` itself and in `#[ignore]`.
|
|
//!
|
|
//! # Why the env var, and no fallback
|
|
//!
|
|
//! The fourteen copies with a fallback hardcoded one machine's absolute layout:
|
|
//!
|
|
//! ```text
|
|
//! /home/fabi/RE - Project Sylpheed/Project Sylpheed - Arc of Deception (USA, Europe) (En,Ja)
|
|
//! ```
|
|
//!
|
|
//! That made `unset SYLPHEED_DISC` a no-op there: whether the disc suites ran
|
|
//! was a property of *the machine's directory layout*, invisible in the command
|
|
//! and in the output. The env var looked like a control and was not one.
|
|
//!
|
|
//! This module adopts the behaviour three of those files already had, rather
|
|
//! than inventing a new one: **the environment decides, always.** Point
|
|
//! `SYLPHEED_DISC` at the extracted disc and the suites run; leave it unset and
|
|
//! they skip. Same command, same answer, on every machine.
|
|
//!
|
|
//! `just test-disc` reads `.env` (already gitignored as a local dev override)
|
|
//! so no absolute path has to live in the source tree again.
|
|
//!
|
|
//! # This does not fix the tally
|
|
//!
|
|
//! A skipped suite still counts as `passed` — `#[ignore]` is static and cannot
|
|
//! move at runtime. That is why `tests/corpus_report.rs` exists: it prints which
|
|
//! corpora resolved, and it is the thing to read. This module only makes the
|
|
//! *control* honest, so that report can now say `PRESENT via $SYLPHEED_DISC`
|
|
//! and mean it.
|
|
// `tests/common/mod.rs` is compiled into EVERY integration-test binary, and each
|
|
// one uses only the resolver (and maybe the macro) it needs. Without these, every
|
|
// binary warns about the parts it did not use.
|
|
#![allow(dead_code, unused_macros, unused_imports)]
|
|
|
|
use std::path::PathBuf;
|
|
|
|
/// The extracted disc root — the directory containing `dat/`.
|
|
pub fn disc_root() -> Option<PathBuf> {
|
|
let p = PathBuf::from(std::env::var("SYLPHEED_DISC").ok()?);
|
|
p.join("dat").is_dir().then_some(p)
|
|
}
|
|
|
|
/// The extracted `resource3d` directory (`Stage_SNN.xpr` models).
|
|
pub fn res3d_dir() -> Option<PathBuf> {
|
|
let p = PathBuf::from(std::env::var("SYLPHEED_RES3D").ok()?);
|
|
p.is_dir().then_some(p)
|
|
}
|
|
|
|
/// The retail ISO image itself, not a directory.
|
|
pub fn iso_path() -> Option<PathBuf> {
|
|
let p = PathBuf::from(std::env::var("SYLPHEED_ISO").ok()?);
|
|
p.is_file().then_some(p)
|
|
}
|
|
|
|
/// Bind the disc root or return from the test.
|
|
///
|
|
/// The early return keeps the test *passing*, which is why the tally cannot
|
|
/// distinguish a skip from a real run — see `corpus_report.rs`.
|
|
macro_rules! skip_without_disc {
|
|
($root:ident) => {
|
|
let Some($root) = crate::common::disc_root() else {
|
|
eprintln!("SKIP: set SYLPHEED_DISC to the extracted disc root");
|
|
return;
|
|
};
|
|
};
|
|
}
|
|
pub(crate) use skip_without_disc;
|