From 4057bee897da644b38c22228ea77c508e9fbfd37 Mon Sep 17 00:00:00 2001 From: Fabian Hamm Date: Wed, 9 Sep 2026 07:39:49 +0200 Subject: [PATCH] test: report which corpora a run actually had (#16) `cargo test --workspace` reports the same tally whether the disc corpus was exercised or entirely absent. Measured: the disc suites RAN on a developer desktop (1936 s, mesh_consistency_disc alone 1220 s) and SKIPPED on CI (2.4 s total) -- and both reported 207 passed / 0 failed / 14 ignored across 30 suites. Two mechanisms compound: * a skip is a PASSING test. The gated suites `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. * 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. Ask what this check would still report if the corpus were entirely absent, and the answer is 207/0/14. This is #16's remedy (2) -- the only one that touches the REPORT, which is the defect. Remedies (1) and (3) improve the control and are left open. Adds `tests/corpus_report.rs`: always runs, never fails, resolves all three corpora exactly as the per-suite helpers do, and records what was available. It writes to a FILE rather than relying on stdout, because a passing test's stdout is captured and would be invisible in exactly the CI log that needs it; the workflow then prints that file. It also appends to GITHUB_STEP_SUMMARY when set. Run here, it immediately shows the thing the issue is about -- all three corpora resolve through the HARDCODED fallback, so SYLPHEED_DISC is not controlling anything on this machine: SYLPHEED_DISC PRESENT via the HARDCODED fallback, NOT $SYLPHEED_DISC SYLPHEED_RES3D PRESENT via the HARDCODED fallback, NOT $SYLPHEED_RES3D SYLPHEED_ISO PRESENT via the HARDCODED fallback, NOT $SYLPHEED_ISO The ABSENT branch is the one CI takes and cannot be reached on a machine that has the corpora, so `resolve_renders_every_branch` exercises it directly rather than shipping it unrun -- along with "set but does not resolve", which is what a typo in the env var produces and which is deliberately reported as a DIFFERENT state from absent, since the two want different fixes. Verified: `cargo fmt --all -- --check` clean. Clippy is unchanged by this (a test target; CI's `cargo clippy --workspace` does not build test cfg) -- it fails identically on unmodified main here with `only_used_in_recursion` at vfs.rs:85, which is the rustc 1.90.0 vs the runner's 1.98.1 divergence, i.e. #15, not this. Refs #16 Co-Authored-By: Claude Opus 5 --- .github/workflows/ci.yml | 11 ++ .../sylpheed-formats/tests/corpus_report.rs | 157 ++++++++++++++++++ 2 files changed, 168 insertions(+) create mode 100644 crates/sylpheed-formats/tests/corpus_report.rs 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..0e071da6 --- /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 ( + "PRESENT via the HARDCODED fallback (not ${env})".replace("${env}", 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"); +}