From 140f8d86153ab3f52c98c8147783bfa1196eb9e6 Mon Sep 17 00:00:00 2001 From: Vyncint Ng <115854244+vyncint@users.noreply.github.com> Date: Wed, 16 Sep 2026 20:24:13 +0700 Subject: [PATCH 1/2] fix(test): wait for the panic message, not for the word "panicked" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The test I added in #311 waits for `contains("panicked")` and then asserts the message. The Rust runtime writes the location line and the message as separate writes, so the predicate is true one line before the message exists. It went red on the `features` job with the grid holding TUI drawing here thread 'main' (7206) panicked at fixtures/emit/src/main.rs:281:37: and nothing under it — the assertion's own output is the evidence. Every wait now names the message. That is rule 3 of the wait-semantics contract in docs/DESIGN.md §2: name the last thing the application paints, so that its truth implies the rest arrived. "panicked" is asserted instead of waited on, since it cannot be absent once the line after it is there. Not reproducible here: 60 runs of the pre-fix test at --test-threads 8 all passed on this machine, which is why it reached main. The fixed test is 60/60 too, so the local runs prove nothing either way; the CI grid above is the measurement, and stress.yml is the instrument. Signed-off-by: Vyncint Ng <115854244+vyncint@users.noreply.github.com> --- crates/termlens/tests/process.rs | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/crates/termlens/tests/process.rs b/crates/termlens/tests/process.rs index 37d7b84..1ef87d6 100644 --- a/crates/termlens/tests/process.rs +++ b/crates/termlens/tests/process.rs @@ -166,6 +166,16 @@ fn a_panicking_child_puts_its_message_on_the_screen() -> termlens::Result<()> { // Wide enough that the message is one row: the runtime's own // `panicked at :` line wraps on a narrow grid, and a // wrapped needle is a test about the width, not about the panic. + // + // And every wait below names the *message*, never the word "panicked". + // The runtime writes the location line and the message as separate + // writes, so `contains("panicked")` is true one line before the message + // exists — which went red on CI with the grid holding `panicked at + // …:281:37:` and nothing under it. Waiting for the message is rule 3 of + // the wait-semantics contract (docs/DESIGN.md §2): name the last thing + // the application paints, so that its truth implies the rest arrived. + // "panicked" is then asserted rather than waited on — it cannot be + // absent once the line after it is there. let wide = || { Terminal::builder() .size(100, 10) @@ -174,8 +184,9 @@ fn a_panicking_child_puts_its_message_on_the_screen() -> termlens::Result<()> { // 1. A plain child, which is what the README's table is about. let mut t = common::spawn_emit(wide(), &["drew this ", "--panic", "plain panic here"])?; - t.wait_until(|s| s.contains("panicked"))?; + t.wait_until(|s| s.contains("plain panic here"))?; let s = t.screen(); + assert!(s.contains("panicked"), "the word the README promises: {s}"); assert!( s.contains("plain panic here"), "the message reaches the grid: {s}" @@ -201,10 +212,10 @@ fn a_panicking_child_puts_its_message_on_the_screen() -> termlens::Result<()> { "boom in the alt screen", ], )?; - t.wait_until(|s| s.contains("panicked"))?; + t.wait_until(|s| s.contains("boom in the alt screen"))?; let s = t.screen(); + assert!(s.contains("panicked"), "{s}"); assert!(s.alternate_screen(), "nothing tore it down: {s}"); - assert!(s.contains("boom in the alt screen"), "{s}"); assert!( s.contains("TUI drawing here"), "the message joins the frame it died on: {s}" @@ -225,10 +236,10 @@ fn a_panicking_child_puts_its_message_on_the_screen() -> termlens::Result<()> { "boom after teardown", ], )?; - t.wait_until(|s| s.contains("panicked"))?; + t.wait_until(|s| s.contains("boom after teardown"))?; let s = t.screen(); + assert!(s.contains("panicked"), "{s}"); assert!(!s.alternate_screen(), "the child left it: {s}"); - assert!(s.contains("boom after teardown"), "{s}"); assert!( !s.contains("TUI drawing here"), "what the alternate screen held went with it: {s}" From f86a0156a576f180a281785ec2d01f524b860628 Mon Sep 17 00:00:00 2001 From: Vyncint Ng <115854244+vyncint@users.noreply.github.com> Date: Wed, 16 Sep 2026 21:14:41 +0700 Subject: [PATCH 2/2] fix(test): a refused stdin operand may close the pipe first MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `with_stdin` wrote the input with `.expect("write to the child's stdin")`. `diff - -` refuses its arguments before reading anything, so the child can exit and close the pipe before that write lands, and the helper panicked on a broken pipe — the faster the refusal, the likelier it is. write to the child's stdin: Os { code: 32, kind: BrokenPipe, … } Found by stress.yml on main, on both the 4- and 8-thread Linux shards, at iterations 13 and 15 of 25. It never reproduced on an idle machine, where the bytes reach the pipe buffer before the child can exit. A broken pipe is a result, not a failure: the callers assert the child's exit code and stderr, and whether it chose to read the input is the child's business. Proven both ways by forcing the race with a sleep before the write — the old helper fails with exactly the message above, the new one passes. Signed-off-by: Vyncint Ng <115854244+vyncint@users.noreply.github.com> --- crates/termlens-cli/tests/cli.rs | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/crates/termlens-cli/tests/cli.rs b/crates/termlens-cli/tests/cli.rs index 36535c1..f7f68cf 100644 --- a/crates/termlens-cli/tests/cli.rs +++ b/crates/termlens-cli/tests/cli.rs @@ -363,12 +363,20 @@ fn with_stdin(args: &[&str], input: &str) -> std::process::Output { .stderr(Stdio::piped()) .spawn() .expect("spawn termlens"); - child - .stdin - .take() - .expect("a stdin pipe") - .write_all(input.as_bytes()) - .expect("write to the child's stdin"); + // A broken pipe here is a *result*, not a failure. `diff - -` refuses its + // arguments before reading anything, so the child can exit and close the + // pipe before this write lands — the faster the refusal, the likelier it + // is. The stress workflow found it on both Linux shards while it never + // reproduced locally, because on an idle machine the bytes reach the + // pipe buffer first. The child's exit code and stderr are what the + // callers assert; whether it read the input is the child's business. + let mut pipe = child.stdin.take().expect("a stdin pipe"); + match pipe.write_all(input.as_bytes()) { + Ok(()) => {} + Err(e) if e.kind() == std::io::ErrorKind::BrokenPipe => {} + Err(e) => panic!("write to the child's stdin: {e}"), + } + drop(pipe); child.wait_with_output().expect("the child's output") }