Merge pull request 'test: report which corpora a run actually had (#16 remedy 2)' (#19) from fix/corpus-report into main

Reviewed-on: #19

.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,

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<PathBuf>) {
+    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<PathBuf> {
+    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");
+}