diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a0d89ee6..30b1cf62 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -77,6 +77,17 @@ jobs: - name: Run tests run: cargo test --workspace + # The tally above cannot tell you what it verified. `cargo test` reports + # the same 207/0/14 whether the disc corpus was exercised or entirely + # absent -- a gated suite that skips still counts as passed, and the + # `ignored` column is a static count of `#[ignore]` attributes that cannot + # move at runtime. Issue #16. This prints what the run ACTUALLY had, from + # a file, because a passing test's stdout is captured and would be + # invisible in exactly this log. + - name: Report which corpora the tests actually had + if: always() + run: cat target/sylpheed-corpus-report.txt || echo "(no corpus report produced -- did corpus_report run?)" + # This step has never once executed on this codebase: the toolchain above # shipped without the component, so every run died on "not installed" # before clippy saw a line of source. Its result was never pass or fail, diff --git a/crates/sylpheed-formats/tests/corpus_report.rs b/crates/sylpheed-formats/tests/corpus_report.rs new file mode 100644 index 00000000..cc6221f6 --- /dev/null +++ b/crates/sylpheed-formats/tests/corpus_report.rs @@ -0,0 +1,157 @@ +//! Says, in the run's own output, which corpora this run actually had. +//! +//! # 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. +//! +//! Two mechanisms compound, and either alone would be survivable: +//! +//! 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. +//! +//! 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. + +use std::fmt::Write as _; +use std::io::Write as _; +use std::path::{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, + ) +} + +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) +} + +#[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 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); + any |= path.is_some(); + let _ = writeln!(out, " {env:<15} {status}"); + if let Some(p) = path { + let _ = writeln!(out, " {:<15} -> {}", "", p.display()); + } + } + let _ = writeln!( + out, + "\n The pass/fail/ignored tally does NOT distinguish these states: a gated\n \ + suite that skips still counts as passed, and `14 ignored` is a static\n \ + count of `#[ignore]` attributes. Read THIS block, not the tally, to know\n \ + what a run verified." + ); + if !any { + let _ = writeln!( + out, + " This run verified NO disc-backed behaviour. Parser-only coverage." + ); + } + + // 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. + 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) + .append(true) + .open(p) + { + let _ = writeln!(f, "### Test corpus\n\n```\n{out}\n```"); + } + } +} + +/// 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"); +}