test: readiness signals replace sleep-based timing (N4)

- signal tests use a marker-file readiness signal: the child's command
  is 'echo ready > <marker>; exec sleep 60', the test polls
  wait_for_file(marker, 5s) — marker exists = the shell exec'd, so the
  signal lands on the real target regardless of machine load. Applied
  in tests/pty.rs (both signal tests), tests/pipe.rs (SIGTERM), and
  the src/local unit tests; wait_for_file lives in tests/common.
- cancel-cleanup post-action sleeps became bounded polls for the
  child's death (kill(pid,0) -> ESRCH, 5s deadline) — faster and
  flake-proof in both directions.
- resize/cat-stdin tests need no readiness signal at all: the adapter's
  input pump processes chunks in order — the sleeps there were pure
  latency (integration suites now ~40ms, was 200-270ms).
This commit is contained in:
glm-5.3-flash committed 2026-09-05 07:21:30 +00:00
1 parent a73484203e
commit cdd6893046
6 files changed
+241 -61

No files matched your search

+27 -3
View File
@@ -535,7 +535,7 @@ Highlights:
| L1 | channels `input` ignored | decide drop-vs-pass-through | small | low | ✅ resolved (2026-09-05) |
| L3 | `open_via_channels` 0% covered | end-to-end channels consumer test | medium | low | ✅ resolved (2026-09-05) |
| L6 | pty bridge error paths untested | targeted error-path tests | medium | low | ✅ resolved (2026-09-05) |
| N4 | sleep-based timing | readiness signals | small | low | open |
| N4 | sleep-based timing | readiness signals | small | low | ✅ resolved (2026-09-05) |
| N6 | MSRV unverified | CI MSRV job or bump | small | none | open |
### Resolution (2026-08-17, commit `9944153`)
@@ -652,14 +652,38 @@ warn + return, with no panic. `local/pty.rs` line coverage
documented-unreachable arms plus `StdinSink`'s in-flight-parking path
(a single write cannot fill the 64-slot channel).
### Resolution (2026-09-05, N4 — readiness signals replace fixed sleeps)
The signal tests now use a **marker-file readiness signal** (the
pattern the cancel-cleanup tests already used): the child's command is
`echo ready > <marker>; exec sleep 60`, and the test polls
`wait_for_file(marker, 5s)` — the marker existing means the shell
exec'd, so the signal lands on the real target regardless of machine
load. Applied in `tests/pty.rs` (both signal tests), `tests/pipe.rs`
(SIGTERM test), and the `src/local/` unit tests (`signal_int_kills_child`,
`signal_reaches_process_group_child`, `unknown_signal_*`,
`signal_term_kills_child`). `wait_for_file` lives in
`tests/common/mod.rs`; `src/local/` copies are private test helpers
(separate compilation units).
The fixed **post-action** sleeps in the cancel-cleanup tests (wait
after drop, then probe once) became **bounded polls** for the child's
death (`kill(pid, 0)` → ESRCH) with a 5 s deadline — faster and
flake-proof in both directions.
The resize/cat-stdin tests needed no readiness signal at all: the
adapter's input pump processes chunks in order and `resize` is safe
whenever the control handle exists — the sleeps there were pure
latency. Integration suites now finish in ~40 ms (was 200-270 ms
each).
### Remaining (open)
- **N4** — sleep-based timing in signal/cancel tests.
- **N6** — MSRV unverified.
### Recommended Order (remaining)
1. **N4 + N6** — test hardening and MSRV; defer until CI exists.
1. **N6** — MSRV; defer until CI exists.
---
+67 -21
View File
@@ -373,11 +373,50 @@ mod tests {
assert_eq!(String::from_utf8_lossy(&err), "err\n");
}
fn rand_seed() -> u64 {
use std::time::{SystemTime, UNIX_EPOCH};
SystemTime::now()
.duration_since(UNIX_EPOCH)
.map(|d| d.as_nanos() as u64)
.unwrap_or(0)
}
/// Readiness signal (review #001 N4): the child writes `ready` to
/// the marker file right before `exec sleep`, so the test knows
/// the exec'd process exists — no fixed pre-signal sleep.
async fn wait_marker(marker: &std::path::Path) {
let deadline = tokio::time::Instant::now() + std::time::Duration::from_secs(5);
loop {
if marker.exists() {
let _ = std::fs::remove_file(marker);
return;
}
if tokio::time::Instant::now() >= deadline {
panic!("child never became ready (marker {:?} missing)", marker);
}
tokio::time::sleep(std::time::Duration::from_millis(10)).await;
}
}
fn marker_path(name: &str) -> std::path::PathBuf {
std::env::temp_dir().join(format!(
"alktty_pipe_{name}_{}_{}.txt",
std::process::id(),
rand_seed()
))
}
#[cfg(unix)]
#[tokio::test]
async fn signal_term_kills_child() {
let handle =
allocate_pipe(sh_cmd(&["sleep", "60"]), None, HashMap::new()).expect("allocate");
let marker = marker_path("sigterm_ready");
let cmd = sh_cmd(&[
"sh",
"-c",
&format!("echo ready > '{}'; exec sleep 60", marker.display()),
]);
let handle = allocate_pipe(cmd, None, HashMap::new()).expect("allocate");
wait_marker(&marker).await;
let control = handle.control.clone().expect("control present");
control.signal("TERM");
let code = handle.exit_code.await.expect("exit");
@@ -401,10 +440,11 @@ mod tests {
&format!("echo $$ > '{}'; exec sleep 60", pid_file.display()),
]);
let handle = allocate_pipe(cmd, None, HashMap::new()).expect("allocate");
// Wait for the shell to write its pid.
for _ in 0..100 {
if pid_file.exists() {
break;
// Wait for the shell to write its pid (readiness signal, N4).
let deadline = tokio::time::Instant::now() + std::time::Duration::from_secs(5);
while !pid_file.exists() {
if tokio::time::Instant::now() >= deadline {
panic!("pid file never written");
}
tokio::time::sleep(std::time::Duration::from_millis(10)).await;
}
@@ -414,11 +454,19 @@ mod tests {
// Drop the handle without awaiting exit_code — the ADR-056 guard
// must kill the child.
drop(handle);
// Give the kernel a moment to deliver SIGKILL and reap.
tokio::time::sleep(std::time::Duration::from_millis(200)).await;
// kill(pid, 0) returns ESRCH (no such process) when the child is gone.
let alive = unsafe { libc::kill(pid, 0) } == 0;
assert!(!alive, "child (pid={pid}) should be killed after drop");
// Poll for the child's death (pid_file written before exec; the
// kill + reap completes quickly).
let deadline = tokio::time::Instant::now() + std::time::Duration::from_secs(5);
loop {
let alive = unsafe { libc::kill(pid, 0) } == 0;
if !alive {
break;
}
if tokio::time::Instant::now() >= deadline {
panic!("child (pid={pid}) should be killed after drop");
}
tokio::time::sleep(std::time::Duration::from_millis(20)).await;
}
}
#[tokio::test]
@@ -432,8 +480,14 @@ mod tests {
#[cfg(unix)]
#[tokio::test]
async fn unknown_signal_falls_back_to_sigkill() {
let handle =
allocate_pipe(sh_cmd(&["sleep", "60"]), None, HashMap::new()).expect("allocate");
let marker = marker_path("unknown_sig_ready");
let cmd = sh_cmd(&[
"sh",
"-c",
&format!("echo ready > '{}'; exec sleep 60", marker.display()),
]);
let handle = allocate_pipe(cmd, None, HashMap::new()).expect("allocate");
wait_marker(&marker).await;
let control = handle.control.clone().expect("control present");
// Unknown name → SIGKILL fallback (equivalent to start_kill()).
control.signal("NOSUCH");
@@ -458,12 +512,4 @@ mod tests {
"expected AllocFailed"
);
}
fn rand_seed() -> u64 {
use std::time::{SystemTime, UNIX_EPOCH};
SystemTime::now()
.duration_since(UNIX_EPOCH)
.map(|d| d.as_nanos() as u64)
.unwrap_or(0)
}
}
+48 -8
View File
@@ -566,6 +566,39 @@ mod tests {
env
}
/// Readiness signal (review #001 N4): the child writes `ready` to
/// the marker file right before `exec sleep`, so the test knows
/// the exec'd process exists — no fixed pre-signal sleep.
async fn wait_marker(marker: &std::path::Path) {
let deadline = tokio::time::Instant::now() + std::time::Duration::from_secs(5);
loop {
if marker.exists() {
let _ = std::fs::remove_file(marker);
return;
}
if tokio::time::Instant::now() >= deadline {
panic!("child never became ready (marker {:?} missing)", marker);
}
tokio::time::sleep(std::time::Duration::from_millis(10)).await;
}
}
fn marker_path(name: &str) -> std::path::PathBuf {
std::env::temp_dir().join(format!(
"alktty_pty_{name}_{}_{}.txt",
std::process::id(),
nanos_seed()
))
}
fn nanos_seed() -> u64 {
use std::time::{SystemTime, UNIX_EPOCH};
SystemTime::now()
.duration_since(UNIX_EPOCH)
.expect("clock went backwards")
.as_nanos() as u64
}
#[tokio::test(flavor = "multi_thread", worker_threads = 4)]
async fn happy_path_echo_exits_zero() {
let handle = allocate_pty(
@@ -631,16 +664,17 @@ mod tests {
#[cfg(unix)]
#[tokio::test(flavor = "multi_thread", worker_threads = 4)]
async fn signal_int_kills_child() {
let marker = marker_path("sigint_ready");
let cmd = format!("echo ready > '{}'; exec sleep 60", marker.display());
let handle = allocate_pty(
term(),
vec!["sleep".to_string(), "60".to_string()],
vec!["bash".to_string(), "-c".to_string(), cmd],
None,
env_default(),
)
.expect("allocate");
let control = handle.control.as_ref().expect("control");
// Give the child a moment to actually exec sleep.
tokio::time::sleep(std::time::Duration::from_millis(150)).await;
wait_marker(&marker).await;
control.signal("INT");
let code = tokio::time::timeout(std::time::Duration::from_secs(5), handle.exit_code)
.await
@@ -655,17 +689,20 @@ mod tests {
#[cfg(unix)]
#[tokio::test(flavor = "multi_thread", worker_threads = 4)]
async fn signal_reaches_process_group_child() {
// bash -c "sleep 60" — sleep is a child of bash. The group signal
// must reach sleep too (REQ-TTY-02). bash exits when its child does.
// bash -c 'echo ready > marker; sleep 60' — sleep is a child of
// bash. The group signal must reach sleep too (REQ-TTY-02). bash
// exits when its child does.
let marker = marker_path("pgroup_ready");
let cmd = format!("echo ready > '{}'; sleep 60", marker.display());
let handle = allocate_pty(
term(),
vec!["bash".to_string(), "-c".to_string(), "sleep 60".to_string()],
vec!["bash".to_string(), "-c".to_string(), cmd],
None,
env_default(),
)
.expect("allocate");
let control = handle.control.as_ref().expect("control");
tokio::time::sleep(std::time::Duration::from_millis(200)).await;
wait_marker(&marker).await;
control.signal("INT");
let code = tokio::time::timeout(std::time::Duration::from_secs(5), handle.exit_code)
.await
@@ -712,14 +749,17 @@ mod tests {
#[cfg(unix)]
#[tokio::test(flavor = "multi_thread", worker_threads = 4)]
async fn unknown_signal_falls_back_to_child_killer() {
let marker = marker_path("unknown_sig_ready");
let cmd = format!("echo ready > '{}'; exec sleep 60", marker.display());
let handle = allocate_pty(
term(),
vec!["sleep".to_string(), "60".to_string()],
vec!["bash".to_string(), "-c".to_string(), cmd],
None,
env_default(),
)
.expect("allocate");
let control = handle.control.as_ref().expect("control");
wait_marker(&marker).await;
// "NOSUCH" is not a known signal name → falls back to
// ChildKiller::kill (SIGHUP). The child should die.
control.signal("NOSUCH");
+21 -2
View File
@@ -16,6 +16,7 @@
use std::collections::HashMap;
use std::sync::Arc;
use std::time::Duration;
use alkcall::core::auth::Identity;
use alktty::adapter::drive_session;
@@ -297,6 +298,24 @@ pub fn nanos_seed() -> u64 {
use std::time::{SystemTime, UNIX_EPOCH};
SystemTime::now()
.duration_since(UNIX_EPOCH)
.map(|d| d.as_nanos() as u64)
.unwrap_or(0)
.expect("clock went backwards")
.as_nanos() as u64
}
/// Poll until `path` exists (or `timeout` elapses), sleeping 10 ms
/// between probes. This is the readiness signal for tests that need
/// the child to have reached a state (e.g. `exec`'d) before acting on
/// it — replaces fixed pre-action sleeps, which fail spuriously on a
/// loaded machine (review #001 N4). Returns `false` on timeout.
pub async fn wait_for_file(path: &std::path::Path, timeout: Duration) -> bool {
let deadline = tokio::time::Instant::now() + timeout;
loop {
if path.exists() {
return true;
}
if tokio::time::Instant::now() >= deadline {
return false;
}
tokio::time::sleep(Duration::from_millis(10)).await;
}
}
+25 -8
View File
@@ -77,18 +77,31 @@ async fn pipe_separate_stderr() {
let _ = server.await;
}
/// 11. Signal (SIGTERM, Unix): negotiate `cmd:["sleep","60"]`, send
/// 11. Signal (SIGTERM, Unix): negotiate `sh -c 'echo ready > <marker>;
/// exec sleep 60'`, wait for the marker (the child has exec'd), send
/// `signal:"TERM"`, await exit, assert signal-terminated.
#[cfg(unix)]
#[tokio::test(flavor = "multi_thread", worker_threads = 4)]
async fn pipe_signal_sigterm_kills_child() {
let marker = std::env::temp_dir().join(format!(
"alktty_pipe_sigterm_ready_{}_{}.txt",
std::process::id(),
nanos_seed()
));
let cmd = format!("echo ready > '{}'; exec sleep 60", marker.display());
let backend = Arc::new(LocalTtyBackend::new());
let (mut client, server) = spawn_session("local", backend);
client
.write_negotiation(negotiate_pipe_json("local", &["sleep", "60"]).as_str())
.write_negotiation(negotiate_pipe_json("local", &["sh", "-c", cmd.as_str()]).as_str())
.await;
tokio::time::sleep(Duration::from_millis(150)).await;
// Readiness signal (N4): the child exec'd `sleep` once the marker
// file exists — no fixed sleep.
assert!(
common::wait_for_file(&marker, Duration::from_secs(5)).await,
"child never became ready"
);
let _ = std::fs::remove_file(&marker);
client
.write_control(br#"{"type":"signal","name":"TERM"}"#)
@@ -133,19 +146,23 @@ async fn pipe_cancel_cleanup_kills_child_no_orphan() {
let pid: i32 = pid_str.trim().parse().expect("pid parses");
let _ = std::fs::remove_file(&pid_file);
tokio::time::sleep(Duration::from_millis(150)).await;
drop(client);
server.abort();
let _ = server.await;
// Poll for the child's death (the pid file is written before exec,
// so the kill lands on the exec'd process; bounded, no fixed sleep).
let mut alive = true;
for _ in 0..100 {
let deadline = tokio::time::Instant::now() + Duration::from_secs(5);
while alive {
let r = unsafe { libc::kill(pid, 0) };
if r != 0 && std::io::Error::last_os_error().raw_os_error() == Some(libc::ESRCH) {
alive = false;
break;
}
if tokio::time::Instant::now() >= deadline {
break;
}
tokio::time::sleep(Duration::from_millis(20)).await;
}
assert!(!alive, "child (pid={pid}) should be killed after cancel");
@@ -164,8 +181,8 @@ async fn pipe_resize_noop() {
.write_negotiation(negotiate_pipe_json("local", &["cat"]).as_str())
.await;
tokio::time::sleep(Duration::from_millis(150)).await;
// No readiness sleep needed (N4): the adapter's input pump
// processes chunks in order, and PipeControl::resize is a no-op.
client
.write_control(br#"{"type":"resize","cols":120,"rows":40}"#)
.await;
+53 -19
View File
@@ -61,8 +61,9 @@ async fn pty_interactive_cat_round_trip() {
.write_negotiation(negotiate_pty_json("local", &["cat"]).as_str())
.await;
tokio::time::sleep(Duration::from_millis(200)).await;
// `cat` with no stdin to close doesn't need a readiness signal:
// eof just makes the backend's stdin close, cat exits, and the
// reader drains. No sleep needed (N4).
client.write_chunk(STREAM_STDIN, b"ping\n").await;
client.write_control(br#"{"type":"eof"}"#).await;
@@ -89,8 +90,9 @@ async fn pty_resize_no_error() {
.write_negotiation(negotiate_pty_json("local", &["cat"]).as_str())
.await;
tokio::time::sleep(Duration::from_millis(150)).await;
// No readiness sleep needed (N4): the adapter's input pump
// processes chunks in order, and `PtyControl::resize` is safe to
// call whenever the control handle exists.
client
.write_control(br#"{"type":"resize","cols":120,"rows":40}"#)
.await;
@@ -105,19 +107,33 @@ async fn pty_resize_no_error() {
let _ = server.await;
}
/// 4. Signal (SIGINT, Unix): negotiate `sleep 60`, send `signal:"INT"`,
/// await the exit chunk. Assert exit code is signal-terminated (non-zero,
/// negative on Unix). Assert the child is reaped (no zombie).
/// 4. Signal (SIGINT, Unix): negotiate `bash -c 'echo ready > <marker>;
/// exec sleep 60'`, wait for the marker file (the child has exec'd),
/// send `signal:"INT"`, await the exit chunk. Assert exit code is
/// signal-terminated (non-zero, negative on Unix). Assert the child is
/// reaped (no zombie).
#[cfg(unix)]
#[tokio::test(flavor = "multi_thread", worker_threads = 4)]
async fn pty_signal_sigint_kills_child() {
let marker = std::env::temp_dir().join(format!(
"alktty_pty_sigint_ready_{}_{}.txt",
std::process::id(),
nanos_seed()
));
let cmd = format!("echo ready > '{}'; exec sleep 60", marker.display());
let backend = Arc::new(LocalTtyBackend::new());
let (mut client, server) = spawn_session("local", backend);
client
.write_negotiation(negotiate_pty_json("local", &["sleep", "60"]).as_str())
.write_negotiation(negotiate_pty_json("local", &["bash", "-c", cmd.as_str()]).as_str())
.await;
tokio::time::sleep(Duration::from_millis(200)).await;
// Readiness signal (N4): the child exec'd `sleep` once the marker
// file exists — no fixed sleep.
assert!(
common::wait_for_file(&marker, Duration::from_secs(5)).await,
"child never became ready"
);
let _ = std::fs::remove_file(&marker);
client
.write_control(br#"{"type":"signal","name":"INT"}"#)
@@ -134,19 +150,33 @@ async fn pty_signal_sigint_kills_child() {
let _ = server.await;
}
/// 5. Process-group signal (Unix): negotiate `bash -c "sleep 60"`,
/// send `signal:"INT"`, assert the `sleep` child also receives the
/// signal (the process group is targeted — REQ-TTY-02).
/// 5. Process-group signal (Unix): negotiate `bash -c 'echo ready >
/// <marker>; sleep 60'` (sleep is a child of bash, bash stays in the
/// foreground waiting), wait for the marker, send `signal:"INT"`,
/// assert the `sleep` child also receives the signal (the process
/// group is targeted — REQ-TTY-02).
#[cfg(unix)]
#[tokio::test(flavor = "multi_thread", worker_threads = 4)]
async fn pty_signal_reaches_process_group_child() {
let marker = std::env::temp_dir().join(format!(
"alktty_pty_pgroup_ready_{}_{}.txt",
std::process::id(),
nanos_seed()
));
let cmd = format!("echo ready > '{}'; sleep 60", marker.display());
let backend = Arc::new(LocalTtyBackend::new());
let (mut client, server) = spawn_session("local", backend);
client
.write_negotiation(negotiate_pty_json("local", &["bash", "-c", "sleep 60"]).as_str())
.write_negotiation(negotiate_pty_json("local", &["bash", "-c", cmd.as_str()]).as_str())
.await;
tokio::time::sleep(Duration::from_millis(250)).await;
// Readiness signal (N4): bash has parsed the script and started
// `sleep` once the marker exists.
assert!(
common::wait_for_file(&marker, Duration::from_secs(5)).await,
"child never became ready"
);
let _ = std::fs::remove_file(&marker);
client
.write_control(br#"{"type":"signal","name":"INT"}"#)
@@ -174,8 +204,8 @@ async fn pty_stdin_eof_zero_length_chunk() {
.write_negotiation(negotiate_pty_json("local", &["cat"]).as_str())
.await;
tokio::time::sleep(Duration::from_millis(150)).await;
// No readiness sleep needed (N4): the input pump processes the
// sentinel in order.
client.write_chunk(STREAM_STDIN, b"").await;
let (_out, _err, code) = client
@@ -215,19 +245,23 @@ async fn pty_cancel_cleanup_kills_child_no_orphan() {
let pid: i32 = pid_str.trim().parse().expect("pid parses");
let _ = std::fs::remove_file(&pid_file);
tokio::time::sleep(Duration::from_millis(150)).await;
drop(client);
server.abort();
let _ = server.await;
// Poll for the child's death (the pid file is written before exec,
// so the kill lands on the exec'd process; bounded, no fixed sleep).
let mut alive = true;
for _ in 0..100 {
let deadline = tokio::time::Instant::now() + Duration::from_secs(5);
while alive {
let r = unsafe { libc::kill(pid, 0) };
if r != 0 && std::io::Error::last_os_error().raw_os_error() == Some(libc::ESRCH) {
alive = false;
break;
}
if tokio::time::Instant::now() >= deadline {
break;
}
tokio::time::sleep(Duration::from_millis(20)).await;
}
assert!(!alive, "child (pid={pid}) should be killed after cancel");