511d9974
fix: waitexit drains the last bytes before believing the exit
a73x 2026-08-10 08:23
Commit message
test/e2e.sh
| Old | New | ||
|---|---|---|---|
| @@ -233,6 +233,11 @@ cleanup() { | |||
| 233 | "$OUT.doctored" "$OUT.doctored.render" "$OUT.doctored.dump" \ | 233 | "$OUT.doctored" "$OUT.doctored.render" "$OUT.doctored.dump" \ |
| 234 | "$OUT.doctored.render.n" "$OUT.doctored.dump.n" "$OUT.doctored.diff" \ | 234 | "$OUT.doctored.render.n" "$OUT.doctored.dump.n" "$OUT.doctored.diff" \ |
| 235 | "$OUT.doctored.rvt" "$OUT.doctored.dvt" | 235 | "$OUT.doctored.rvt" "$OUT.doctored.dvt" |
| 236 | # The M12 fixture controls, spelled from $OUT rather than from $PCLOG: | ||
| 237 | # that variable is assigned mid-file, so on a failure above it "${PCLOG:-}.2" | ||
| 238 | # would expand to a bare ".2" — an rm target in the CWD, not a temp file. | ||
| 239 | rm -f "$OUT.pc" "$OUT.pc.err" "$OUT.pc2" "$OUT.pc2.err" "$OUT.pc3" "$OUT.pc3.err" \ | ||
| 240 | "$OUT.pc.log" "$OUT.pc.log.2" "$OUT.pc.log.3" | ||
| 236 | # The convergence files a FAILING assert_converged leaves behind | 241 | # The convergence files a FAILING assert_converged leaves behind |
| 237 | # (.render/.dump/.rvt/.dvt/.diff for that capture) are deliberately not | 242 | # (.render/.dump/.rvt/.dvt/.diff for that capture) are deliberately not |
| 238 | # chased here: on a failing run they are the evidence. | 243 | # chased here: on a failing run they are the evidence. |
| @@ -1535,12 +1540,38 @@ expect never-going-to-match 500 | |||
| 1535 | EOF | 1540 | EOF |
| 1536 | RC=$? | 1541 | RC=$? |
| 1537 | set -e | 1542 | set -e |
| 1538 | [ "$RC" -ne 0 ] || { | 1543 | # The literal, not merely nonzero: 3 is EXIT_TIMEOUT, and the tp1/tp2 |
| 1539 | echo "e2e FAIL: ptyclient expect control did not fire on an impossible needle"; exit 1; } | 1544 | # scenarios read that code to tell a missed needle apart from a client that |
| 1545 | # died or a script the fixture refused. A bare -ne 0 would pass just as | ||
| 1546 | # happily on a usage error that never ran the script at all. | ||
| 1547 | [ "$RC" -eq 3 ] || { | ||
| 1548 | echo "e2e FAIL: ptyclient expect control exited $RC, want 3 (EXIT_TIMEOUT)"; cat "$PCLOG.2"; exit 1; } | ||
| 1540 | grep -q "did not arrive" "$PCLOG.2" || { | 1549 | grep -q "did not arrive" "$PCLOG.2" || { |
| 1541 | echo "e2e FAIL: ptyclient timeout fired but never said what it saw"; cat "$PCLOG.2"; exit 1; } | 1550 | echo "e2e FAIL: ptyclient timeout fired but never said what it saw"; cat "$PCLOG.2"; exit 1; } |
| 1542 | rm -f "$OUT.pc" "$OUT.pc.err" "$OUT.pc2" "$OUT.pc2.err" "$PCLOG" "$PCLOG.2" | 1551 | # The stderr split, end to end: the client's predict stats must land in the |
| 1543 | ok "ptyclient controls: pty echo roundtrips, impossible expect fails loudly" | 1552 | # .err sibling and NOT in the capture the convergence machinery diffs. Only |
| 1553 | # a child that writes to BOTH streams can tell a working split from a child | ||
| 1554 | # that simply never wrote to stderr. | ||
| 1555 | set +e | ||
| 1556 | "$PTYCLIENT" --cols 80 --rows 24 --out "$OUT.pc3" --err "$OUT.pc3.err" -- \ | ||
| 1557 | /bin/sh -c 'printf pc-stdout; printf pc-stderr 1>&2' > "$PCLOG.3" 2>&1 <<'EOF' | ||
| 1558 | expect pc-stdout 10000 | ||
| 1559 | waitexit 10000 | ||
| 1560 | EOF | ||
| 1561 | RC=$? | ||
| 1562 | set -e | ||
| 1563 | [ "$RC" -eq 0 ] || { | ||
| 1564 | echo "e2e FAIL: ptyclient stderr-split leg exited $RC:"; cat "$PCLOG.3"; exit 1; } | ||
| 1565 | grep -q "pc-stdout" "$OUT.pc3" || { | ||
| 1566 | echo "e2e FAIL: ptyclient capture missing the child's stdout"; cat -v "$OUT.pc3"; exit 1; } | ||
| 1567 | ! grep -q "pc-stderr" "$OUT.pc3" || { | ||
| 1568 | echo "e2e FAIL: the child's stderr leaked into the capture"; cat -v "$OUT.pc3"; exit 1; } | ||
| 1569 | grep -q "pc-stderr" "$OUT.pc3.err" || { | ||
| 1570 | echo "e2e FAIL: the child's stderr reached neither the capture nor --err" | ||
| 1571 | cat -v "$OUT.pc3.err"; exit 1; } | ||
| 1572 | rm -f "$OUT.pc" "$OUT.pc.err" "$OUT.pc2" "$OUT.pc2.err" "$OUT.pc3" "$OUT.pc3.err" \ | ||
| 1573 | "$PCLOG" "$PCLOG.2" "$PCLOG.3" | ||
| 1574 | ok "ptyclient controls: pty echo roundtrips, impossible expect fails loudly, stderr stays off the capture" | ||
| 1544 | 1575 | ||
| 1545 | # The pins. Literals, not variables set from counting something else — | 1576 | # The pins. Literals, not variables set from counting something else — |
| 1546 | # "assert the literal, never the constant the code under test reads" | 1577 | # "assert the literal, never the constant the code under test reads" |
test/ptyclient.zig
| Old | New | ||
|---|---|---|---|
| @@ -2,8 +2,10 @@ | |||
| 2 | //! owns, so `isatty()` answers yes and the tty-gated branches open. Driven | 2 | //! owns, so `isatty()` answers yes and the tty-gated branches open. Driven |
| 3 | //! by a line-oriented script on stdin; everything read from the master | 3 | //! by a line-oriented script on stdin; everything read from the master |
| 4 | //! tees into --out so the convergence machinery consumes the same capture | 4 | //! tees into --out so the convergence machinery consumes the same capture |
| 5 | //! files non-tty scenarios produce. Spec: docs/superpowers/specs/ | 5 | //! files non-tty scenarios produce. Capture convention: the master is read |
| 6 | //! 2026-08-10-m12-ptyclient-design.md. | 6 | //! only while a verb is running, so a script MUST end with `waitexit` — end |
| 7 | //! it on an expect and --out holds only what had arrived by that match. | ||
| 8 | //! Spec: docs/superpowers/specs/2026-08-10-m12-ptyclient-design.md. | ||
| 7 | const std = @import("std"); | 9 | const std = @import("std"); |
| 8 | const Pty = @import("pty").Pty; | 10 | const Pty = @import("pty").Pty; |
| 9 | 11 | ||
| @@ -293,15 +295,29 @@ pub fn main() !void { | |||
| 293 | }, | 295 | }, |
| 294 | .waitexit => |deadline_ms| { | 296 | .waitexit => |deadline_ms| { |
| 295 | const start = std.time.milliTimestamp(); | 297 | const start = std.time.milliTimestamp(); |
| 296 | const deadline: i64 = @intCast(deadline_ms); | ||
| 297 | while (true) { | 298 | while (true) { |
| 298 | const alive = try drain(alloc, &pty, out, &exp); | 299 | const alive = try drain(alloc, &pty, out, &exp); |
| 299 | if (pty.checkExited()) |status| { | 300 | if (pty.checkExited()) |status| { |
| 301 | // The child can write its last bytes and exit in the | ||
| 302 | // window between drain's poll and this waitpid; they | ||
| 303 | // are sitting in the pty buffer, and dropping them | ||
| 304 | // truncates a capture that assert_converged compares | ||
| 305 | // byte for byte — silently, with the fixture still | ||
| 306 | // reporting success. Read to EIO before believing the | ||
| 307 | // exit. Bounded, because anything OTHER than the client | ||
| 308 | // still holding the slave keeps the fd open forever, | ||
| 309 | // and a hung suite is worse than a short capture. | ||
| 310 | while (try drain(alloc, &pty, out, &exp)) { | ||
| 311 | var fds = [_]std.posix.pollfd{ | ||
| 312 | .{ .fd = pty.master, .events = std.posix.POLL.IN, .revents = 0 }, | ||
| 313 | }; | ||
| 314 | if (try std.posix.poll(&fds, 50) == 0) break; | ||
| 315 | } | ||
| 300 | if (status != 0) | 316 | if (status != 0) |
| 301 | fatal(@intCast(@min(status, 255)), "client exited {d}", .{status}); | 317 | fatal(@intCast(@min(status, 255)), "client exited {d}", .{status}); |
| 302 | break; | 318 | break; |
| 303 | } | 319 | } |
| 304 | if (std.time.milliTimestamp() - start > deadline) | 320 | if (std.time.milliTimestamp() - start > deadline_ms) |
| 305 | fatal(EXIT_TIMEOUT, "verb {d}: client still running after {d}ms", .{ verb_no, deadline_ms }); | 321 | fatal(EXIT_TIMEOUT, "verb {d}: client still running after {d}ms", .{ verb_no, deadline_ms }); |
| 306 | if (alive) { | 322 | if (alive) { |
| 307 | var fds = [_]std.posix.pollfd{ | 323 | var fds = [_]std.posix.pollfd{ |