diff --git a/crates/sylpheed-formats/examples/default_owners.rs b/crates/sylpheed-formats/examples/default_owners.rs index c1f64db2..820b6882 100644 --- a/crates/sylpheed-formats/examples/default_owners.rs +++ b/crates/sylpheed-formats/examples/default_owners.rs @@ -25,8 +25,12 @@ fn is_value(s: &str) -> bool { } fn main() { - let root = std::env::var("SYLPHEED_DISC") - .unwrap_or_else(|_| "/home/fabi/RE - Project Sylpheed/sylph_extract".into()); + // No hardcoded fallback: it made SYLPHEED_DISC look like a control while + // one machine's directory layout decided the outcome (#16). + let Ok(root) = std::env::var("SYLPHEED_DISC") else { + eprintln!("set SYLPHEED_DISC to the extracted disc root"); + std::process::exit(2); + }; let wanted: Vec = std::env::args().skip(1).collect(); let arc = PakArchive::open(std::path::Path::new(&root).join("dat/GP_MAIN_GAME_E.pak")).unwrap(); diff --git a/crates/sylpheed-formats/examples/defaulted_fields.rs b/crates/sylpheed-formats/examples/defaulted_fields.rs index 3a554eea..379bf08d 100644 --- a/crates/sylpheed-formats/examples/defaulted_fields.rs +++ b/crates/sylpheed-formats/examples/defaulted_fields.rs @@ -31,9 +31,14 @@ fn is_value(s: &str) -> bool { } fn main() { + // argv[1], else SYLPHEED_DISC. No hardcoded fallback -- see #16. let root = std::env::args() .nth(1) - .unwrap_or_else(|| "/home/fabi/RE - Project Sylpheed/sylph_extract".into()); + .or_else(|| std::env::var("SYLPHEED_DISC").ok()) + .unwrap_or_else(|| { + eprintln!("usage: defaulted_fields [paks...] (or set SYLPHEED_DISC)"); + std::process::exit(2); + }); let paks: Vec = std::env::args().skip(2).collect(); let paks = if paks.is_empty() { vec![ diff --git a/crates/sylpheed-formats/tests/caption_families_disc.rs b/crates/sylpheed-formats/tests/caption_families_disc.rs index f344c998..d3f4f199 100644 --- a/crates/sylpheed-formats/tests/caption_families_disc.rs +++ b/crates/sylpheed-formats/tests/caption_families_disc.rs @@ -4,31 +4,11 @@ //! `build_caption_text` generalises the key parser to all eight. use std::collections::BTreeMap; -use std::path::{Path, PathBuf}; use sylpheed_formats::{movie_subtitle, PakArchive}; -fn disc_root() -> Option { - if let Ok(p) = std::env::var("SYLPHEED_DISC") { - let p = PathBuf::from(p); - if p.join("dat").is_dir() { - return Some(p); - } - } - let d = Path::new( - "/home/fabi/RE - Project Sylpheed/Project Sylpheed - Arc of Deception (USA, Europe) (En,Ja)", - ); - d.join("dat").is_dir().then(|| d.to_path_buf()) -} - -macro_rules! skip_without_disc { - ($root:ident) => { - let Some($root) = disc_root() else { - eprintln!("SKIP: set SYLPHEED_DISC"); - return; - }; - }; -} +mod common; +use common::skip_without_disc; #[test] fn all_eight_caption_families_are_read() { diff --git a/crates/sylpheed-formats/tests/common/mod.rs b/crates/sylpheed-formats/tests/common/mod.rs new file mode 100644 index 00000000..649e016f --- /dev/null +++ b/crates/sylpheed-formats/tests/common/mod.rs @@ -0,0 +1,79 @@ +//! 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 { + 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 { + 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 { + 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; diff --git a/crates/sylpheed-formats/tests/corpus_report.rs b/crates/sylpheed-formats/tests/corpus_report.rs index cc6221f6..e69da0b2 100644 --- a/crates/sylpheed-formats/tests/corpus_report.rs +++ b/crates/sylpheed-formats/tests/corpus_report.rs @@ -3,94 +3,62 @@ //! # Why this exists //! //! `cargo test --workspace` reports **the same tally whether or not the disc -//! corpus was exercised** — see issue #16. Measured: the disc suites ran on a -//! developer desktop (1 936 s, `mesh_consistency_disc` alone 1 220 s) and -//! skipped on CI (2.4 s total), and *both* reported `207 passed / 0 failed / -//! 14 ignored` across 30 suites. +//! corpus was exercised** — issue #16. Measured on `main` @ `4ca0b8e`: this +//! desktop ran the disc suites (`mesh_consistency_disc` alone **1 215 s**) and +//! CI skipped them (2.4 s total), and *both* reported +//! `209 passed / 0 failed / 14 ignored` across 31 suites. //! -//! Two mechanisms compound, and either alone would be survivable: +//! Two mechanisms compound: //! -//! 1. **A skip is a passing test.** The disc suites do `eprintln!("SKIP: …")` -//! and return early from a test that still passes, so a skipped suite and a -//! fully exercised one both score `1 passed`. The totals are invariant. -//! 2. **The message is invisible.** `cargo test` captures a *passing* test's -//! output, so neither log contains a `SKIP:` line. The absence of one proves -//! nothing, which makes the obvious check useless too. +//! 1. **A skip is a passing test.** The gated suites print `SKIP:` and return +//! early from a test that still passes, so a skipped suite and a fully +//! exercised one both score `1 passed`. +//! 2. **The message is invisible.** `cargo test` captures a passing test's +//! output, so neither log contains a `SKIP:` line. //! //! And `14 ignored` cannot help: `#[ignore]` is static, so that column is the //! literal count of attributes in the source and cannot move at runtime. //! -//! This test is the fix for the *report*, not for the control. It always runs, -//! never fails, and records what was actually available — to a file, because a -//! passing test's stdout is captured and would be invisible in exactly the CI -//! log that needs it. +//! This test fixes the *report*. `tests/common/mod.rs` fixes the *control*. use std::fmt::Write as _; use std::io::Write as _; -use std::path::{Path, PathBuf}; +use std::path::PathBuf; -/// Resolution mirrors the per-suite helpers exactly. If one of those changes, -/// this drifts — which is itself an argument for the shared helper in #16's -/// remedy (3). -fn resolve( - env: &str, - fallback: &str, - want_dir_child: Option<&str>, - want_file: bool, -) -> (String, Option) { - let ok = |p: &Path| -> bool { - match (want_dir_child, want_file) { - (Some(child), _) => p.join(child).is_dir(), - (None, true) => p.is_file(), - (None, false) => p.is_dir(), - } - }; - if let Ok(v) = std::env::var(env) { - let p = PathBuf::from(&v); - if ok(&p) { - return (format!("PRESENT via ${env}"), Some(p)); - } - return (format!("${env} is set but does not resolve: {v}"), None); - } - let p = PathBuf::from(fallback); - if ok(&p) { - // The important case. ${env} is unset, yet the corpus resolved anyway, - // so the suites run because of this machine's directory layout — a - // property invisible in the command and in the output. - return ( - format!("PRESENT via the HARDCODED fallback, NOT ${env}"), - Some(p), - ); - } - ( - "ABSENT — the gated suites will self-skip and still count as passed".into(), - None, - ) -} +mod common; -fn target_dir() -> Option { - let exe = std::env::current_exe().ok()?; - exe.ancestors() - .find(|a| a.file_name().is_some_and(|n| n == "target")) - .map(PathBuf::from) +/// Describe one corpus. Resolution is delegated to `common`, so this cannot +/// drift from what the suites themselves do — the previous version duplicated +/// the resolution logic and said so in its own comments. +fn status(var: &str, resolved: Option) -> (String, Option) { + match (std::env::var(var), resolved) { + (Ok(_), Some(p)) => (format!("PRESENT via ${var}"), Some(p)), + // Set but unusable is NOT the same as absent, and wants a different + // fix: a typo or a moved directory rather than a machine without the + // corpus. Reporting them alike would send someone hunting the wrong one. + (Ok(v), None) => (format!("${var} is set but does not resolve: {v}"), None), + (Err(_), _) => ( + format!("ABSENT — ${var} unset; its suites self-skip and still count as passed"), + None, + ), + } } #[test] fn corpus_report() { - let corpora = [ - ("SYLPHEED_DISC", "/home/fabi/RE - Project Sylpheed/Project Sylpheed - Arc of Deception (USA, Europe) (En,Ja)", Some("dat"), false), - ("SYLPHEED_RES3D", "/home/fabi/RE - Project Sylpheed/sylph_extract/hidden/resource3d", None, false), - ("SYLPHEED_ISO", "/home/fabi/RE - Project Sylpheed/Project Sylpheed - Arc of Deception (USA, Europe) (En,Ja).iso", None, true), + let rows = [ + status("SYLPHEED_DISC", common::disc_root()), + status("SYLPHEED_RES3D", common::res3d_dir()), + status("SYLPHEED_ISO", common::iso_path()), ]; let mut out = String::from("test corpus report (issue #16)\n"); let mut any = false; - for (env, fallback, child, file) in corpora { - let (status, path) = resolve(env, fallback, child, file); + for (text, path) in &rows { any |= path.is_some(); - let _ = writeln!(out, " {env:<15} {status}"); + let _ = writeln!(out, " {text}"); if let Some(p) = path { - let _ = writeln!(out, " {:<15} -> {}", "", p.display()); + let _ = writeln!(out, " -> {}", p.display()); } } let _ = writeln!( @@ -107,15 +75,14 @@ fn corpus_report() { ); } - // Captured for a passing test, so it is only visible with --show-output or - // --nocapture. Kept anyway: it is the natural place to look locally. + // Captured for a passing test, so visible only with --show-output. Kept + // because it is the natural place to look locally. println!("{out}"); // The channel that survives capture, and the one CI reads. if let Some(dir) = target_dir() { let _ = std::fs::write(dir.join("sylpheed-corpus-report.txt"), &out); } - // Gitea and GitHub both honour this; it puts the block in the run summary. if let Ok(p) = std::env::var("GITHUB_STEP_SUMMARY") { if let Ok(mut f) = std::fs::OpenOptions::new() .create(true) @@ -127,31 +94,30 @@ fn corpus_report() { } } -/// The ABSENT branch is the one CI takes, and it cannot be reached on a machine -/// that has the corpora — so it is exercised directly here rather than shipped -/// unrun. Same for the "set but does not resolve" branch, which is what a typo -/// in the env var produces. -#[test] -fn resolve_renders_every_branch() { - let missing = "/nonexistent/sylpheed/corpus"; - - // env unset + fallback missing -> ABSENT - std::env::remove_var("SYLPHEED_TEST_PROBE"); - let (status, path) = resolve("SYLPHEED_TEST_PROBE", missing, None, false); - assert!(status.starts_with("ABSENT"), "{status}"); - assert!(path.is_none()); - - // env set to something that does not resolve -> named as such, NOT absent, - // because those two states want different fixes. - std::env::set_var("SYLPHEED_TEST_PROBE", missing); - let (status, path) = resolve("SYLPHEED_TEST_PROBE", missing, None, false); - assert!(status.contains("is set but does not resolve"), "{status}"); - assert!(path.is_none()); - - // env set and valid -> attributed to the env var, not the fallback - std::env::set_var("SYLPHEED_TEST_PROBE", env!("CARGO_MANIFEST_DIR")); - let (status, path) = resolve("SYLPHEED_TEST_PROBE", missing, None, false); - assert_eq!(status, "PRESENT via $SYLPHEED_TEST_PROBE"); - assert!(path.is_some()); - std::env::remove_var("SYLPHEED_TEST_PROBE"); +fn target_dir() -> Option { + let exe = std::env::current_exe().ok()?; + exe.ancestors() + .find(|a| a.file_name().is_some_and(|n| n == "target")) + .map(PathBuf::from) +} + +/// The three states must render distinctly, and CI only ever exercises one of +/// them — so they are driven directly here rather than shipped unrun. +#[test] +fn status_renders_every_state() { + let probe = "SYLPHEED_TEST_PROBE"; + std::env::remove_var(probe); + let (text, path) = status(probe, None); + assert!(text.starts_with("ABSENT"), "{text}"); + assert!(path.is_none()); + + std::env::set_var(probe, "/nonexistent/sylpheed/corpus"); + let (text, _) = status(probe, None); + assert!(text.contains("is set but does not resolve"), "{text}"); + + std::env::set_var(probe, env!("CARGO_MANIFEST_DIR")); + let (text, path) = status(probe, Some(PathBuf::from(env!("CARGO_MANIFEST_DIR")))); + assert_eq!(text, format!("PRESENT via ${probe}")); + assert!(path.is_some()); + std::env::remove_var(probe); } diff --git a/crates/sylpheed-formats/tests/idxd_records_disc.rs b/crates/sylpheed-formats/tests/idxd_records_disc.rs index 3c84e01d..48bf42d8 100644 --- a/crates/sylpheed-formats/tests/idxd_records_disc.rs +++ b/crates/sylpheed-formats/tests/idxd_records_disc.rs @@ -13,27 +13,8 @@ use std::path::{Path, PathBuf}; use sylpheed_formats::hash::tag_hash; use sylpheed_formats::{IdxdObject, PakArchive}; -fn disc_root() -> Option { - if let Ok(p) = std::env::var("SYLPHEED_DISC") { - let p = PathBuf::from(p); - if p.join("dat").is_dir() { - return Some(p); - } - } - let default = Path::new( - "/home/fabi/RE - Project Sylpheed/Project Sylpheed - Arc of Deception (USA, Europe) (En,Ja)", - ); - default.join("dat").is_dir().then(|| default.to_path_buf()) -} - -macro_rules! skip_without_disc { - ($root:ident) => { - let Some($root) = disc_root() else { - eprintln!("SKIP: extracted disc not found (set SYLPHEED_DISC to enable)"); - return; - }; - }; -} +mod common; +use common::skip_without_disc; /// Every `.pak` on the disc, recursively. /// diff --git a/crates/sylpheed-formats/tests/ixud_records_disc.rs b/crates/sylpheed-formats/tests/ixud_records_disc.rs index c833ad57..bd465dc4 100644 --- a/crates/sylpheed-formats/tests/ixud_records_disc.rs +++ b/crates/sylpheed-formats/tests/ixud_records_disc.rs @@ -8,27 +8,8 @@ use std::path::{Path, PathBuf}; use sylpheed_formats::hash::ixud_hash_str; use sylpheed_formats::{IxudObject, PakArchive}; -fn disc_root() -> Option { - if let Ok(p) = std::env::var("SYLPHEED_DISC") { - let p = PathBuf::from(p); - if p.join("dat").is_dir() { - return Some(p); - } - } - let d = Path::new( - "/home/fabi/RE - Project Sylpheed/Project Sylpheed - Arc of Deception (USA, Europe) (En,Ja)", - ); - d.join("dat").is_dir().then(|| d.to_path_buf()) -} - -macro_rules! skip_without_disc { - ($root:ident) => { - let Some($root) = disc_root() else { - eprintln!("SKIP: set SYLPHEED_DISC"); - return; - }; - }; -} +mod common; +use common::skip_without_disc; fn all_paks(root: &Path) -> Vec { let mut out = Vec::new(); diff --git a/crates/sylpheed-formats/tests/mesh_consistency_disc.rs b/crates/sylpheed-formats/tests/mesh_consistency_disc.rs index cc0b73a4..0e6a4ea0 100644 --- a/crates/sylpheed-formats/tests/mesh_consistency_disc.rs +++ b/crates/sylpheed-formats/tests/mesh_consistency_disc.rs @@ -11,25 +11,12 @@ //! rather than as a snapshot of the bug. use std::collections::BTreeMap; -use std::path::{Path, PathBuf}; +use std::path::PathBuf; use sylpheed_formats::mesh::Xbg7Model; -fn disc_root() -> Option { - if let Ok(p) = std::env::var("SYLPHEED_DISC") { - let p = PathBuf::from(p); - if p.join("dat").is_dir() { - return Some(p); - } - } - let default = Path::new( - "/home/fabi/RE - Project Sylpheed/Project Sylpheed - Arc of Deception (USA, Europe) (En,Ja)", - ); - if default.join("dat").is_dir() { - return Some(default.to_path_buf()); - } - None -} +mod common; +use common::disc_root; /// Rounded (w, h, d) of a model's own geometry. fn span(m: &Xbg7Model) -> Option<[i64; 3]> { diff --git a/crates/sylpheed-formats/tests/mesh_disc.rs b/crates/sylpheed-formats/tests/mesh_disc.rs index ea4ec8df..19d1aa72 100644 --- a/crates/sylpheed-formats/tests/mesh_disc.rs +++ b/crates/sylpheed-formats/tests/mesh_disc.rs @@ -9,16 +9,8 @@ use std::path::PathBuf; use sylpheed_formats::mesh::{material_groups, node_transforms, submesh_albedos, Xbg7Model}; -fn res3d_dir() -> Option { - if let Ok(p) = std::env::var("SYLPHEED_RES3D") { - let p = PathBuf::from(p); - if p.is_dir() { - return Some(p); - } - } - let default = PathBuf::from("/home/fabi/RE - Project Sylpheed/sylph_extract/hidden/resource3d"); - default.is_dir().then_some(default) -} +mod common; +use common::res3d_dir; #[test] #[ignore = "requires extracted disc models — set SYLPHEED_RES3D"] @@ -378,10 +370,11 @@ fn hero_ship_grouped_pool_decodes() { #[ignore] fn stage_models_decode() { use sylpheed_formats::mesh::Xbg7Model; - let dir = std::env::var("SYLPHEED_RES3D").unwrap_or_else(|_| { - "/home/fabi/RE - Project Sylpheed/sylph_extract/hidden/resource3d".to_string() - }); - let path = format!("{dir}/Stage_S10.xpr"); + let Some(dir) = res3d_dir() else { + eprintln!("SKIP: set SYLPHEED_RES3D to the extracted resource3d directory"); + return; + }; + let path = format!("{}/Stage_S10.xpr", dir.display()); let bytes = std::fs::read(&path).expect("read Stage_S10"); let models = Xbg7Model::stage_models(&bytes); for m in &models { @@ -423,9 +416,10 @@ fn stage_models_decode() { fn stage_models_sweep() { use std::time::Instant; use sylpheed_formats::mesh::Xbg7Model; - let dir = std::env::var("SYLPHEED_RES3D").unwrap_or_else(|_| { - "/home/fabi/RE - Project Sylpheed/sylph_extract/hidden/resource3d".to_string() - }); + let Some(dir) = res3d_dir() else { + eprintln!("SKIP: set SYLPHEED_RES3D to the extracted resource3d directory"); + return; + }; let mut names: Vec<_> = std::fs::read_dir(&dir) .unwrap() .filter_map(|e| e.ok().map(|e| e.file_name().into_string().unwrap())) @@ -434,7 +428,7 @@ fn stage_models_sweep() { names.sort(); let mut tot = 0usize; for n in &names { - let bytes = std::fs::read(format!("{dir}/{n}")).unwrap(); + let bytes = std::fs::read(format!("{}/{n}", dir.display())).unwrap(); let t0 = Instant::now(); let models = Xbg7Model::stage_models(&bytes); let dt = t0.elapsed().as_millis(); @@ -455,10 +449,11 @@ fn stage_models_sweep() { #[ignore] fn stage_models_quality_audit() { use sylpheed_formats::mesh::Xbg7Model; - let dir = std::env::var("SYLPHEED_RES3D").unwrap_or_else(|_| { - "/home/fabi/RE - Project Sylpheed/sylph_extract/hidden/resource3d".to_string() - }); - let bytes = std::fs::read(format!("{dir}/Stage_S07.xpr")).unwrap(); + let Some(dir) = res3d_dir() else { + eprintln!("SKIP: set SYLPHEED_RES3D to the extracted resource3d directory"); + return; + }; + let bytes = std::fs::read(format!("{}/Stage_S07.xpr", dir.display())).unwrap(); let models = Xbg7Model::stage_models(&bytes); let (mut small, mut mid, mut huge, mut dupnames) = (0, 0, 0, 0); let mut seen = std::collections::HashSet::new(); diff --git a/crates/sylpheed-formats/tests/movie_manifest_disc.rs b/crates/sylpheed-formats/tests/movie_manifest_disc.rs index bdea2406..36d03a68 100644 --- a/crates/sylpheed-formats/tests/movie_manifest_disc.rs +++ b/crates/sylpheed-formats/tests/movie_manifest_disc.rs @@ -7,10 +7,8 @@ use sylpheed_formats::movie_manifest; use sylpheed_formats::slb::VoiceLang; use sylpheed_formats::PakArchive; -fn disc_root() -> Option { - let p = PathBuf::from(std::env::var("SYLPHEED_DISC").ok()?); - p.join("dat").is_dir().then_some(p) -} +mod common; +use common::disc_root; /// Read the manifest + `eng\sounds.tbl` out of `tables.pak`. fn load_manifest_and_sounds(root: &PathBuf) -> (Vec, Vec) { diff --git a/crates/sylpheed-formats/tests/movie_subtitle_disc.rs b/crates/sylpheed-formats/tests/movie_subtitle_disc.rs index 760874f5..6b0640c6 100644 --- a/crates/sylpheed-formats/tests/movie_subtitle_disc.rs +++ b/crates/sylpheed-formats/tests/movie_subtitle_disc.rs @@ -1,15 +1,11 @@ //! Real-disc test for the movie subtitle chain. Skipped when the extracted disc //! is absent (set `SYLPHEED_DISC` to the extract root to enable). -use std::path::PathBuf; - use sylpheed_formats::movie_subtitle::{self, SubLang}; use sylpheed_formats::PakArchive; -fn disc_root() -> Option { - let p = PathBuf::from(std::env::var("SYLPHEED_DISC").ok()?); - p.join("dat").is_dir().then_some(p) -} +mod common; +use common::disc_root; #[test] fn resolves_english_radio_subtitles() { diff --git a/crates/sylpheed-formats/tests/pak_idxd_disc.rs b/crates/sylpheed-formats/tests/pak_idxd_disc.rs index b7e14d61..d433d51f 100644 --- a/crates/sylpheed-formats/tests/pak_idxd_disc.rs +++ b/crates/sylpheed-formats/tests/pak_idxd_disc.rs @@ -1,40 +1,16 @@ //! Integration tests against the real extracted Project Sylpheed disc. //! //! These are **skipped** (pass as no-ops) when the extracted disc is not present, -//! so the suite still runs on machines/CI without the game. Point at the disc via -//! the `SYLPHEED_DISC` env var, or drop it at the default dev path below. - -use std::path::{Path, PathBuf}; +//! so the suite still runs on machines/CI without the game. Point at the disc +//! with the `SYLPHEED_DISC` env var; there is no path fallback (see #16). use sylpheed_formats::texture::X360Texture; use sylpheed_formats::{IdxdObject, PakArchive}; +mod common; +use common::skip_without_disc; + /// Locate the extracted disc root, or `None` to skip. -fn disc_root() -> Option { - if let Ok(p) = std::env::var("SYLPHEED_DISC") { - let p = PathBuf::from(p); - if p.join("dat").is_dir() { - return Some(p); - } - } - let default = Path::new( - "/home/fabi/RE - Project Sylpheed/Project Sylpheed - Arc of Deception (USA, Europe) (En,Ja)", - ); - if default.join("dat").is_dir() { - return Some(default.to_path_buf()); - } - None -} - -macro_rules! skip_without_disc { - ($root:ident) => { - let Some($root) = disc_root() else { - eprintln!("SKIP: extracted disc not found (set SYLPHEED_DISC to enable)"); - return; - }; - }; -} - #[test] fn deftables_header_and_first_entry() { skip_without_disc!(root); diff --git a/crates/sylpheed-formats/tests/slb_disc.rs b/crates/sylpheed-formats/tests/slb_disc.rs index e17b4e56..c584335b 100644 --- a/crates/sylpheed-formats/tests/slb_disc.rs +++ b/crates/sylpheed-formats/tests/slb_disc.rs @@ -9,10 +9,8 @@ use sylpheed_formats::hash::name_hash; use sylpheed_formats::slb::{self, VoiceLang}; use sylpheed_formats::PakArchive; -fn disc_root() -> Option { - let p = PathBuf::from(std::env::var("SYLPHEED_DISC").ok()?); - p.join("dat").is_dir().then_some(p) -} +mod common; +use common::disc_root; /// Read `[off, off+size)` from `dat/sound.p00..` (segments concatenated). fn read_range(root: &PathBuf, mut off: u64, size: usize) -> Vec { diff --git a/crates/sylpheed-formats/tests/slb_leading_segment_disc.rs b/crates/sylpheed-formats/tests/slb_leading_segment_disc.rs index c5e69f6b..a9204206 100644 --- a/crates/sylpheed-formats/tests/slb_leading_segment_disc.rs +++ b/crates/sylpheed-formats/tests/slb_leading_segment_disc.rs @@ -5,31 +5,12 @@ //! `RIFF`. The banks that looked fine were the ones whose leading segment is //! silence. One rule, two outcomes. -use std::path::{Path, PathBuf}; +use std::path::Path; use sylpheed_formats::{slb, PakArchive}; -fn disc_root() -> Option { - if let Ok(p) = std::env::var("SYLPHEED_DISC") { - let p = PathBuf::from(p); - if p.join("dat").is_dir() { - return Some(p); - } - } - let default = Path::new( - "/home/fabi/RE - Project Sylpheed/Project Sylpheed - Arc of Deception (USA, Europe) (En,Ja)", - ); - default.join("dat").is_dir().then(|| default.to_path_buf()) -} - -macro_rules! skip_without_disc { - ($root:ident) => { - let Some($root) = disc_root() else { - eprintln!("SKIP: set SYLPHEED_DISC"); - return; - }; - }; -} +mod common; +use common::skip_without_disc; fn bank(root: &Path, n: u32) -> Vec { let snd = PakArchive::open(root.join("dat/sound.pak")).expect("sound.pak"); diff --git a/crates/sylpheed-formats/tests/texture_disc.rs b/crates/sylpheed-formats/tests/texture_disc.rs index 6e552ae9..0c8525ac 100644 --- a/crates/sylpheed-formats/tests/texture_disc.rs +++ b/crates/sylpheed-formats/tests/texture_disc.rs @@ -1,28 +1,15 @@ //! Integration test: run the XPR2 texture pipeline against REAL `.xpr` files //! read directly from the retail disc image, reproducing exactly what the //! viewer's texture-preview path does (`identify_format` → `X360Texture:: -//! from_xpr2`). Skipped unless `SYLPHEED_ISO` points at the disc (or the -//! default dev path exists). +//! from_xpr2`). Skipped unless `SYLPHEED_ISO` points at the disc image. //! //! Run: `cargo test -p sylpheed-formats --test texture_disc -- --ignored --nocapture` -use std::path::PathBuf; - use sylpheed_formats::texture::X360Texture; use sylpheed_formats::vfs::identify_format; -fn iso_path() -> Option { - if let Ok(p) = std::env::var("SYLPHEED_ISO") { - let p = PathBuf::from(p); - if p.is_file() { - return Some(p); - } - } - let default = PathBuf::from( - "/home/fabi/RE - Project Sylpheed/Project Sylpheed - Arc of Deception (USA, Europe) (En,Ja).iso", - ); - default.is_file().then_some(default) -} +mod common; +use common::iso_path; #[tokio::test] #[ignore = "requires the retail ISO — set SYLPHEED_ISO"] diff --git a/crates/sylpheed-formats/tests/ui_focus_kind_disc.rs b/crates/sylpheed-formats/tests/ui_focus_kind_disc.rs index b2498463..2ef79795 100644 --- a/crates/sylpheed-formats/tests/ui_focus_kind_disc.rs +++ b/crates/sylpheed-formats/tests/ui_focus_kind_disc.rs @@ -17,25 +17,12 @@ use std::path::{Path, PathBuf}; use sylpheed_formats::{pak::PakArchive, ratc, ui_layout}; +mod common; +use common::disc_root; + const DECL_TABLE_AT: usize = 0x20; const DECL_ENTRY: usize = 60; -fn disc_root() -> Option { - if let Ok(p) = std::env::var("SYLPHEED_DISC") { - let p = PathBuf::from(p); - if p.join("dat").is_dir() { - return Some(p); - } - } - let default = Path::new( - "/home/fabi/RE - Project Sylpheed/Project Sylpheed - Arc of Deception (USA, Europe) (En,Ja)", - ); - if default.join("dat").is_dir() { - return Some(default.to_path_buf()); - } - None -} - fn for_each_build(root: &Path, mut f: impl FnMut(&str, &[u8])) { let mut paks: Vec = std::fs::read_dir(root.join("dat")) .expect("dat/") diff --git a/crates/sylpheed-formats/tests/ui_header_time_disc.rs b/crates/sylpheed-formats/tests/ui_header_time_disc.rs index d9eb8e0b..3d7e7c65 100644 --- a/crates/sylpheed-formats/tests/ui_header_time_disc.rs +++ b/crates/sylpheed-formats/tests/ui_header_time_disc.rs @@ -16,21 +16,8 @@ use std::path::{Path, PathBuf}; use sylpheed_formats::{pak::PakArchive, ratc, ui_layout}; -fn disc_root() -> Option { - if let Ok(p) = std::env::var("SYLPHEED_DISC") { - let p = PathBuf::from(p); - if p.join("dat").is_dir() { - return Some(p); - } - } - let default = Path::new( - "/home/fabi/RE - Project Sylpheed/Project Sylpheed - Arc of Deception (USA, Europe) (En,Ja)", - ); - if default.join("dat").is_dir() { - return Some(default.to_path_buf()); - } - None -} +mod common; +use common::disc_root; fn for_each_build(root: &Path, mut f: impl FnMut(&str, &[u8])) { let mut paks: Vec = std::fs::read_dir(root.join("dat")) diff --git a/crates/sylpheed-formats/tests/ui_opt_link_disc.rs b/crates/sylpheed-formats/tests/ui_opt_link_disc.rs index d1c54833..2dcd0d1b 100644 --- a/crates/sylpheed-formats/tests/ui_opt_link_disc.rs +++ b/crates/sylpheed-formats/tests/ui_opt_link_disc.rs @@ -17,21 +17,8 @@ use std::path::{Path, PathBuf}; use sylpheed_formats::{pak::PakArchive, ratc, ui_layout}; -fn disc_root() -> Option { - if let Ok(p) = std::env::var("SYLPHEED_DISC") { - let p = PathBuf::from(p); - if p.join("dat").is_dir() { - return Some(p); - } - } - let default = Path::new( - "/home/fabi/RE - Project Sylpheed/Project Sylpheed - Arc of Deception (USA, Europe) (En,Ja)", - ); - if default.join("dat").is_dir() { - return Some(default.to_path_buf()); - } - None -} +mod common; +use common::disc_root; fn for_each_build(root: &Path, mut f: impl FnMut(&str, &[u8])) { let mut paks: Vec = std::fs::read_dir(root.join("dat")) diff --git a/crates/sylpheed-formats/tests/ui_paint_order_disc.rs b/crates/sylpheed-formats/tests/ui_paint_order_disc.rs index 7f280a32..2804792b 100644 --- a/crates/sylpheed-formats/tests/ui_paint_order_disc.rs +++ b/crates/sylpheed-formats/tests/ui_paint_order_disc.rs @@ -21,30 +21,8 @@ use sylpheed_formats::{ ui_layout::{self, ComposeOptions}, }; -fn disc_root() -> Option { - if let Ok(p) = std::env::var("SYLPHEED_DISC") { - let p = PathBuf::from(p); - if p.join("dat").is_dir() { - return Some(p); - } - } - let default = Path::new( - "/home/fabi/RE - Project Sylpheed/Project Sylpheed - Arc of Deception (USA, Europe) (En,Ja)", - ); - if default.join("dat").is_dir() { - return Some(default.to_path_buf()); - } - None -} - -macro_rules! skip_without_disc { - ($root:ident) => { - let Some($root) = disc_root() else { - eprintln!("SKIP: extracted disc not found (set SYLPHEED_DISC to enable)"); - return; - }; - }; -} +mod common; +use common::skip_without_disc; /// Every parseable screen build on the disc, as (pak name, bundle bytes). fn builds(root: &Path) -> Vec<(String, Vec)> { diff --git a/crates/sylpheed-formats/tests/ui_prm_primitives_disc.rs b/crates/sylpheed-formats/tests/ui_prm_primitives_disc.rs index 9b8e4357..6e3e0198 100644 --- a/crates/sylpheed-formats/tests/ui_prm_primitives_disc.rs +++ b/crates/sylpheed-formats/tests/ui_prm_primitives_disc.rs @@ -9,21 +9,8 @@ use std::path::{Path, PathBuf}; use sylpheed_formats::{pak::PakArchive, ratc, ui_layout}; -fn disc_root() -> Option { - if let Ok(p) = std::env::var("SYLPHEED_DISC") { - let p = PathBuf::from(p); - if p.join("dat").is_dir() { - return Some(p); - } - } - let default = Path::new( - "/home/fabi/RE - Project Sylpheed/Project Sylpheed - Arc of Deception (USA, Europe) (En,Ja)", - ); - if default.join("dat").is_dir() { - return Some(default.to_path_buf()); - } - None -} +mod common; +use common::disc_root; fn for_each_build(root: &Path, mut f: impl FnMut(&str, &[u8])) { let mut paks: Vec = std::fs::read_dir(root.join("dat")) diff --git a/crates/sylpheed-formats/tests/ui_screen_vs_fragment_disc.rs b/crates/sylpheed-formats/tests/ui_screen_vs_fragment_disc.rs index 9e8bcec7..f2d63c37 100644 --- a/crates/sylpheed-formats/tests/ui_screen_vs_fragment_disc.rs +++ b/crates/sylpheed-formats/tests/ui_screen_vs_fragment_disc.rs @@ -21,21 +21,8 @@ use std::path::{Path, PathBuf}; use sylpheed_formats::{pak::PakArchive, ratc, ui_layout}; -fn disc_root() -> Option { - if let Ok(p) = std::env::var("SYLPHEED_DISC") { - let p = PathBuf::from(p); - if p.join("dat").is_dir() { - return Some(p); - } - } - let default = Path::new( - "/home/fabi/RE - Project Sylpheed/Project Sylpheed - Arc of Deception (USA, Europe) (En,Ja)", - ); - if default.join("dat").is_dir() { - return Some(default.to_path_buf()); - } - None -} +mod common; +use common::disc_root; fn for_each_build(root: &Path, mut f: impl FnMut(&str, &[u8])) { let mut paks: Vec = std::fs::read_dir(root.join("dat")) diff --git a/crates/sylpheed-formats/tests/ui_surfaces_disc.rs b/crates/sylpheed-formats/tests/ui_surfaces_disc.rs index 5acb1e5f..fdd1a024 100644 --- a/crates/sylpheed-formats/tests/ui_surfaces_disc.rs +++ b/crates/sylpheed-formats/tests/ui_surfaces_disc.rs @@ -10,30 +10,8 @@ use std::path::{Path, PathBuf}; use sylpheed_formats::{lsta, pak::PakArchive, ratc, t8ad}; -fn disc_root() -> Option { - if let Ok(p) = std::env::var("SYLPHEED_DISC") { - let p = PathBuf::from(p); - if p.join("dat").is_dir() { - return Some(p); - } - } - let default = Path::new( - "/home/fabi/RE - Project Sylpheed/Project Sylpheed - Arc of Deception (USA, Europe) (En,Ja)", - ); - if default.join("dat").is_dir() { - return Some(default.to_path_buf()); - } - None -} - -macro_rules! skip_without_disc { - ($root:ident) => { - let Some($root) = disc_root() else { - eprintln!("SKIP: extracted disc not found (set SYLPHEED_DISC to enable)"); - return; - }; - }; -} +mod common; +use common::skip_without_disc; /// Every entry of every pak, plus every RATC child, as raw bytes. fn for_each_blob(root: &Path, mut f: impl FnMut(&str, &str, &[u8])) { diff --git a/crates/sylpheed-formats/tests/unit_layout_disc.rs b/crates/sylpheed-formats/tests/unit_layout_disc.rs index 40012e83..575c2a1e 100644 --- a/crates/sylpheed-formats/tests/unit_layout_disc.rs +++ b/crates/sylpheed-formats/tests/unit_layout_disc.rs @@ -11,6 +11,9 @@ use sylpheed_formats::idxd::IdxdObject; use sylpheed_formats::pak::PakArchive; use sylpheed_formats::unit_layout::{fields, Kind}; +mod common; +use common::disc_root; + /// Live objects identified in the dump, with the disc record each one is. /// `bf001` is here because full-record agreement is what identified it: the /// four-value signature also fitted `UN_be005_ADAN_SpaceFortress`, which @@ -31,19 +34,6 @@ const IDENTIFIED: &[(&str, &str)] = &[ const DUMP: &str = include_str!("../../../docs/re/captures/stage02-live-unit-definitions-deep.txt"); -fn disc_root() -> Option { - if let Ok(p) = std::env::var("SYLPHEED_DISC") { - if std::path::Path::new(&p).join("dat").is_dir() { - return Some(p); - } - } - let d = "/home/fabi/RE - Project Sylpheed/Project Sylpheed - Arc of Deception (USA, Europe) (En,Ja)"; - std::path::Path::new(d) - .join("dat") - .is_dir() - .then(|| d.to_string()) -} - /// `va -> offset -> value`, from the dump's `addr +off hex u32 f32` columns. fn live() -> BTreeMap> { let mut out: BTreeMap> = BTreeMap::new(); @@ -70,7 +60,8 @@ fn mapped_fields_match_the_disc_records() { eprintln!("SKIP: extracted disc not found (set SYLPHEED_DISC to enable)"); return; }; - let pak = PakArchive::open(format!("{disc}/dat/GP_MAIN_GAME_E.pak")).expect("main pak"); + let pak = + PakArchive::open(format!("{}/dat/GP_MAIN_GAME_E.pak", disc.display())).expect("main pak"); let live = live(); let floats: Vec<_> = fields() .into_iter() diff --git a/justfile b/justfile index c79d5829..4034b45f 100644 --- a/justfile +++ b/justfile @@ -71,6 +71,28 @@ sniff-unknown: test: cargo test --workspace +# Run the disc-backed suites. They are gated on the environment ALONE now (#16): +# no source file hardcodes a path any more, so nothing runs by accident because +# a machine happens to have a directory. Put your paths in `.env` (already +# gitignored as a local dev override): +# +# SYLPHEED_DISC="/path/to/extracted/disc" # the dir containing dat/ +# +# QUOTE the values. These paths contain spaces, and an unquoted `VAR=a b c` +# is parsed as "run command b with VAR=a" -- it fails silently into ABSENT. +# SYLPHEED_RES3D=/path/to/hidden/resource3d +# SYLPHEED_ISO=/path/to/game.iso +# +# Read the corpus block the run prints -- the pass/fail tally is identical +# whether or not the corpus was exercised, which is the whole of #16. +test-disc: + #!/usr/bin/env bash + set -euo pipefail + [ -f .env ] && set -a && . ./.env && set +a + cargo test --workspace + echo "--- corpus report ---" + cat target/sylpheed-corpus-report.txt + # Run tests including ISO integration tests (requires SYLPHEED_ISO env var) test-integration: SYLPHEED_ISO=./game.iso cargo test --workspace -- --include-ignored