fix(cli): keep inspect's expected errors out of the artifact dir

CHANGELOG.md

@@ -100,6 +100,14 @@ reads that marker.
   however short the waits that follow (#593). Each wait restarted that
   grace, so waits shorter than it never saw EOF, and a send to the dead
   child was not refused.
+- `termlens inspect` no longer leaves screens in `TERMLENS_ARTIFACT_DIR`
+  for a run that ended normally (#596). It takes the library's `Eof` and
+  `Timeout` as outcomes — a program that exits, one still running at the
+  deadline — and the library records every error that carries a screen, so
+  `inspect true` wrote a failure screen the report action then listed among
+  the failing tests. The program under inspection still gets the variable
+  when it inherits the environment. The library's documentation of the
+  variable now says an expected error is recorded too.
 - `termlens` no longer panics with exit code 101 (#594). A closed or full
   stderr (`2>&1 | true`, `2>/dev/full`) made every diagnostic and the
   `inspect` trailer panic; the write is now checked, and a trailer that cannot

crates/termlens-cli/src/main.rs

@@ -681,6 +681,15 @@ fn shows_something(screen: &termlens::Screen) -> bool {
 }
 
 fn inspect(args: Vec<OsString>) -> ExitCode {
+    // The library writes every screen-carrying error to
+    // `TERMLENS_ARTIFACT_DIR`, which is right for a failing test. Here a
+    // program that exits (`Eof`) or is still running at the deadline
+    // (`Timeout`) is a run that ended normally, so the variable is taken
+    // out of this process's environment before anything reads it — no
+    // thread exists yet — and handed to the program itself when it
+    // inherits the environment (#596).
+    let artifact_dir = std::env::var_os("TERMLENS_ARTIFACT_DIR");
+    std::env::remove_var("TERMLENS_ARTIFACT_DIR");
     let mut args = args.into_iter().peekable();
     let mut size = (80u16, 24u16);
     let mut timeout = Duration::from_secs(5);
@@ -776,6 +785,8 @@ fn inspect(args: Vec<OsString>) -> ExitCode {
         if let Some(path) = std::env::var_os("PATH") {
             builder = builder.env("PATH", path);
         }
+    } else if let Some(dir) = artifact_dir {
+        builder = builder.env("TERMLENS_ARTIFACT_DIR", dir);
     }
     for (key, value) in env {
         builder = builder.env(key, value);

crates/termlens-cli/tests/cli.rs

@@ -2102,3 +2102,57 @@ fn render_empty_separate_out_value_names_the_flag() -> termlens::Result<()> {
     assert_eq!(stderr, "termlens: --out needs a PATH argument\n");
     Ok(())
 }
+
+/// `inspect` takes the library's errors as outcomes — a program that exits
+/// without painting is `Eof`, one still running at the deadline `Timeout` —
+/// and the library writes every such error to `TERMLENS_ARTIFACT_DIR`. A
+/// run that ended normally left "failing test" screens there (#596). A
+/// program that inherits the environment is still handed the variable.
+#[test]
+#[cfg_attr(windows, ignore = "the program under inspection is a POSIX shell")]
+fn inspect_leaves_the_artifact_dir_alone_on_a_normal_run() {
+    use std::process::Command;
+    let dir = std::path::Path::new(env!("CARGO_TARGET_TMPDIR"))
+        .join(format!("inspect-artifacts-{}", std::process::id()));
+    std::fs::create_dir_all(&dir).expect("an artifact directory");
+    let inspect = |args: &[&str]| {
+        Command::new(env!("CARGO_BIN_EXE_termlens"))
+            .args(args)
+            .env("TERMLENS_ARTIFACT_DIR", &dir)
+            .output()
+            .expect("termlens runs")
+    };
+    for args in [
+        &["inspect", "true"][..],
+        &[
+            "inspect",
+            "--timeout",
+            "1",
+            "sh",
+            "-c",
+            "printf hi; sleep 30",
+        ][..],
+    ] {
+        let out = inspect(args);
+        assert_eq!(out.status.code(), Some(0), "termlens {args:?}: {out:?}");
+        let left: Vec<_> = std::fs::read_dir(&dir)
+            .expect("the directory is readable")
+            .map(|entry| entry.expect("an entry").file_name())
+            .collect();
+        assert!(left.is_empty(), "termlens {args:?} left {left:?}");
+    }
+    let out = inspect(&[
+        "inspect",
+        "--inherit-env",
+        "sh",
+        "-c",
+        "printf '%s' \"${TERMLENS_ARTIFACT_DIR##*/}\"",
+    ]);
+    let shown = String::from_utf8_lossy(&out.stdout);
+    let name = dir
+        .file_name()
+        .expect("a name")
+        .to_string_lossy()
+        .into_owned();
+    assert!(shown.contains(&name), "the program saw: {shown}");
+}

crates/termlens/examples/inspect.rs

@@ -167,6 +167,15 @@ fn fail(message: &str) -> ExitCode {
 }
 
 fn main() -> ExitCode {
+    // The library writes every screen-carrying error to
+    // `TERMLENS_ARTIFACT_DIR`, which is right for a failing test. Here a
+    // program that exits (`Eof`) or is still running at the deadline
+    // (`Timeout`) is a run that ended normally, so the variable is taken
+    // out of this process's environment before anything reads it — no
+    // thread exists yet — and handed to the program itself when it
+    // inherits the environment (#596).
+    let artifact_dir = std::env::var_os("TERMLENS_ARTIFACT_DIR");
+    std::env::remove_var("TERMLENS_ARTIFACT_DIR");
     // `args_os`: `args` panics on an argument that is not UTF-8 (#594).
     let mut args = std::env::args_os().skip(1).peekable();
 
@@ -265,6 +274,8 @@ fn main() -> ExitCode {
         if let Some(path) = std::env::var_os("PATH") {
             builder = builder.env("PATH", path);
         }
+    } else if let Some(dir) = artifact_dir {
+        builder = builder.env("TERMLENS_ARTIFACT_DIR", dir);
     }
     for (key, value) in env {
         builder = builder.env(key, value);

crates/termlens/src/error.rs

@@ -23,7 +23,10 @@ pub type Result<T, E = Error> = std::result::Result<T, E>;
 /// written there as it is embedded: `<test>-<n>.screen.json` with the
 /// `serde` feature, `<test>-<n>.screen.txt` (the `with_styles` rendering,
 /// which [`Screen::parse`] reads back) without, where `<test>` is the
-/// current thread's name — under `cargo test`, the test's path. Unset, the
+/// current thread's name — under `cargo test`, the test's path. Every such
+/// error is recorded, an expected one included: a test that asserts a
+/// timeout or EOF leaves that screen in the directory too, so set the
+/// variable for the steps whose failures you want to see (#596). Unset, the
 /// hook is one environment read and nothing else; insta's `.snap.new`
 /// files stay where insta puts them. This repository's `report` action
 /// (`.github/actions/report`) renders that directory into a pull