test: pty bridge late-signal fallback chain; document unreachable error arms (L6)
- signal_after_child_exit_takes_both_kill_fallbacks: a late signal (after the child exited) takes kill(-pgid) fail -> kill(pid) fail -> warn + return, with no panic — the reachable part of the REQ-TTY-02 fallback chain. - the remaining bridge error arms are documented-unreachable through the public path (per the review's disposition), with per-arm reasoning in the module doc: try_clone_reader (dup failure), reader read error (EIO -> EOF mapped), take_writer (second-take only), writer write/flush (externally-closed fd), waiter wait() (already-reaped child). The ADR-055 §4 -1 sentinel is covered at the adapter level (exit_error_sends_minus_one) — it also arises when the oneshot drops on kill-on-cancel. - local/pty.rs line coverage 80.85% -> 86.01%; total 94.48%.
This commit is contained in:
@@ -534,7 +534,7 @@ Highlights:
|
|||||||
| M1 | negotiation-rejection frame unhandled | implement disambiguation read | medium | medium (wire-facing) | ✅ resolved |
|
| M1 | negotiation-rejection frame unhandled | implement disambiguation read | medium | medium (wire-facing) | ✅ resolved |
|
||||||
| L1 | channels `input` ignored | decide drop-vs-pass-through | small | low | ✅ resolved (2026-09-05) |
|
| 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) |
|
| 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 | open |
|
| 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 | open |
|
||||||
| N6 | MSRV unverified | CI MSRV job or bump | small | none | open |
|
| N6 | MSRV unverified | CI MSRV job or bump | small | none | open |
|
||||||
|
|
||||||
@@ -627,16 +627,39 @@ data flow). The channels harness (`wire_client_and_server`) moved to
|
|||||||
producer path (`register_openable` + `drive_session_pre_negotiated`
|
producer path (`register_openable` + `drive_session_pre_negotiated`
|
||||||
through alkcall's channels stack).
|
through alkcall's channels stack).
|
||||||
|
|
||||||
|
### Resolution (2026-09-05, L6 — pty bridge error paths)
|
||||||
|
|
||||||
|
Per the review's own disposition ("the `try_clone_reader`/`take_writer`
|
||||||
|
failure paths can be left as documented-unreachable if the publisher
|
||||||
|
agrees"), the bridge's error arms are now **documented-unreachable**
|
||||||
|
(module doc in `local/pty.rs` §"Bridge error paths"), with the
|
||||||
|
reasoning per arm: `try_clone_reader` needs a dup failure (exhausted
|
||||||
|
fd table — not deterministically forceable); a reader read error has
|
||||||
|
no trigger (EIO → EOF is mapped); `take_writer` only fails on a second
|
||||||
|
take (the bridge takes it once); a writer write/flush error requires
|
||||||
|
an externally-closed fd; the waiter `wait()` failure needs an
|
||||||
|
already-reaped child (the bridge never calls `try_wait`). The
|
||||||
|
ADR-055 §4 `-1` sentinel the review wanted tested is covered at the
|
||||||
|
adapter level (`exit_error_sends_minus_one` — the sentinel also arises
|
||||||
|
when the oneshot drops on kill-on-cancel, exercised by the
|
||||||
|
cancel-cleanup tests).
|
||||||
|
|
||||||
|
The reachable fallback chain *is* now tested:
|
||||||
|
`signal_after_child_exit_takes_both_kill_fallbacks` — a late signal
|
||||||
|
(after the child exited) takes `kill(-pgid)` fail → `kill(pid)` fail →
|
||||||
|
warn + return, with no panic. `local/pty.rs` line coverage
|
||||||
|
80.85% → 86.01%; the remaining uncovered lines are exactly the
|
||||||
|
documented-unreachable arms plus `StdinSink`'s in-flight-parking path
|
||||||
|
(a single write cannot fill the 64-slot channel).
|
||||||
|
|
||||||
### Remaining (open)
|
### Remaining (open)
|
||||||
|
|
||||||
- **L6** — pty bridge error paths untested.
|
|
||||||
- **N4** — sleep-based timing in signal/cancel tests.
|
- **N4** — sleep-based timing in signal/cancel tests.
|
||||||
- **N6** — MSRV unverified.
|
- **N6** — MSRV unverified.
|
||||||
|
|
||||||
### Recommended Order (remaining)
|
### Recommended Order (remaining)
|
||||||
|
|
||||||
1. **L6** — pty bridge error paths; medium effort.
|
1. **N4 + N6** — test hardening and MSRV; defer until CI exists.
|
||||||
2. **N4 + N6** — test hardening and MSRV; defer until CI exists.
|
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
|
|||||||
@@ -34,6 +34,33 @@
|
|||||||
//! guard (`Option::take()`), so the subsequent `Drop` is a no-op. The
|
//! guard (`Option::take()`), so the subsequent `Drop` is a no-op. The
|
||||||
//! waiter thread reaps the killed child via its blocking `wait()`, so
|
//! waiter thread reaps the killed child via its blocking `wait()`, so
|
||||||
//! there is no zombie. See `tty-local.md` §"Cancel-Cleanup (ADR-056)".
|
//! there is no zombie. See `tty-local.md` §"Cancel-Cleanup (ADR-056)".
|
||||||
|
//!
|
||||||
|
//! # Bridge error paths (review #001 L6)
|
||||||
|
//!
|
||||||
|
//! The three-thread bridge's error arms are documented-unreachable
|
||||||
|
//! through the public path, per the review's disposition:
|
||||||
|
//!
|
||||||
|
//! - **reader `try_clone_reader` failure** — the master fd is open and
|
||||||
|
//! owned by this allocation; `dup` failure requires an exhausted
|
||||||
|
//! fd table, which no test can force deterministically.
|
||||||
|
//! - **reader read error** — the `PtyFd` `Read` impl maps `EIO`
|
||||||
|
//! (slave closed) to EOF, the only realistic master read error; any
|
||||||
|
//! other error kind has no trigger from the public path.
|
||||||
|
//! - **writer `take_writer` failure** — portable-pty 0.9 fails this
|
||||||
|
//! only on the *second* take (`took_writer` flag); the bridge takes
|
||||||
|
//! it exactly once.
|
||||||
|
//! - **writer write/flush failure** — the master fd is open and the
|
||||||
|
//! writer thread is the sole writer; a write error requires an
|
||||||
|
//! externally-closed fd the bridge never observes.
|
||||||
|
//! - **waiter `wait()` failure** — `std::process::Child::wait` fails
|
||||||
|
//! only if the child was already reaped (the bridge never calls
|
||||||
|
//! `try_wait`) — unreachable through the public path. The ADR-055 §4
|
||||||
|
//! `-1` sentinel it would produce is covered at the adapter level
|
||||||
|
//! (`exit_error_sends_minus_one`): the sentinel also arises when the
|
||||||
|
//! oneshot is dropped (kill-on-cancel path, exercised by the
|
||||||
|
//! cancel-cleanup tests). The late-signal fallback chain
|
||||||
|
//! (`kill(-pgid)` fail → `kill(pid)` fail → warn) is tested by
|
||||||
|
//! `signal_after_child_exit_takes_both_kill_fallbacks`.
|
||||||
|
|
||||||
use std::collections::HashMap;
|
use std::collections::HashMap;
|
||||||
use std::future::Future;
|
use std::future::Future;
|
||||||
@@ -702,4 +729,27 @@ mod tests {
|
|||||||
.expect("exit_code");
|
.expect("exit_code");
|
||||||
assert_ne!(code, 0, "fallback kill should terminate the child: {code}");
|
assert_ne!(code, 0, "fallback kill should terminate the child: {code}");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[cfg(unix)]
|
||||||
|
#[tokio::test(flavor = "multi_thread", worker_threads = 4)]
|
||||||
|
async fn signal_after_child_exit_takes_both_kill_fallbacks() {
|
||||||
|
// The signal path's fallback chain (REQ-TTY-02): `kill(-pgid, sig)`
|
||||||
|
// fails once the child (and its group) is gone, then the
|
||||||
|
// `kill(pid, sig)` fallback fails too, and the call degrades to a
|
||||||
|
// warn + return — no panic, no cascade. A late signal (the client
|
||||||
|
// signals after the process already exited) must be safe.
|
||||||
|
let handle = allocate_pty(
|
||||||
|
term(),
|
||||||
|
vec!["echo".to_string(), "done".to_string()],
|
||||||
|
None,
|
||||||
|
env_default(),
|
||||||
|
)
|
||||||
|
.expect("allocate");
|
||||||
|
let control = handle.control.clone().expect("control");
|
||||||
|
let code = handle.exit_code.await.expect("exit_code");
|
||||||
|
assert_eq!(code, 0);
|
||||||
|
|
||||||
|
control.signal("INT");
|
||||||
|
control.signal("NOSUCH");
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user