a73x

1ad4be3e

test: DELAY_MS is pinned, in the unit test and on the wire

a73x   2026-08-08 16:43

Commit message
test: DELAY_MS is pinned, in the unit test and on the wire

The env test asserted only the unset case, so it returned the default
whatever the lookup did — and the default is 150, which is what the
scenarios would silently have measured if the lookup ever broke. Splitting
parseDelay from the getenv wrapper makes the interpretation testable on its
own (std has no portable setenv, so a test going through the environment
can only ever reach the branch that returns the default no matter how badly
the lookup is broken).

The other half is on the wire, where it matters: the line-mode scenario
snapshots 450ms after the keystroke, which the 600ms round trip cannot have
answered but the default's 300ms would have. Empty-stringing the lookup now
fails e2e rather than quietly measuring a path twice as fast as claimed.

Choosing that delay turned up a real ceiling, recorded in the comment
because it is a property and not a test artefact: at 400ms each way the
burst scenario dropped to confirmed=1. delaypipe delays each chunk
serially, so a burst's later keystrokes queue behind the earlier ones and
age while they wait — past the overlay's 1000ms expiry bound, which then
retired them exactly as designed. Prediction survives a burst only while
the round trip plus queueing stays inside that bound.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
test/delaypipe.zig
Old New
@@ -45,12 +45,21 @@ fn writeAll(fd: std.posix.fd_t, data: []const u8) !void {
45 while (i < data.len) i += try std.posix.write(fd, data[i..]); 45 while (i < data.len) i += try std.posix.write(fd, data[i..]);
46 } 46 }
47 47
48 /// `DELAY_MS` from the environment, or the default. An unparseable value is 48 /// Interpret a `DELAY_MS` value. Absent or unparseable is the default: this
49 /// the default too: this is a test helper, and failing to start would look 49 /// is a test helper, and failing to start would look like a broken transport
50 /// like a broken transport rather than a typo. 50 /// rather than a typo.
51 ///
52 /// Split from the lookup so the interpretation is testable on its own —
53 /// std has no portable setenv, so a test that went through the environment
54 /// could only ever exercise the unset case, which is the one branch that
55 /// returns the default no matter how badly the lookup is broken.
56 pub fn parseDelay(raw: ?[]const u8) u64 {
57 const s = raw orelse return default_delay_ms;
58 return std.fmt.parseInt(u64, s, 10) catch default_delay_ms;
59 }
60
51 pub fn delayFromEnv() u64 { 61 pub fn delayFromEnv() u64 {
52 const raw = std.posix.getenv("DELAY_MS") orelse return default_delay_ms; 62 return parseDelay(std.posix.getenv("DELAY_MS"));
53 return std.fmt.parseInt(u64, raw, 10) catch default_delay_ms;
54 } 63 }
55 64
56 pub fn main() void { 65 pub fn main() void {
@@ -125,8 +134,23 @@ test "a payload larger than one write survives the crossing in order" {
125 try std.testing.expectEqualStrings("alpha-beta-gamma", got.items); 134 try std.testing.expectEqualStrings("alpha-beta-gamma", got.items);
126 } 135 }
127 136
128 test "DELAY_MS is read from the environment, and a bad value is not fatal" { 137 test "DELAY_MS is honoured when set, and never fatal when it is nonsense" {
129 // Nothing set in the test environment, so this is the default path — 138 // The case that matters: a value that is NOT the default, because a
130 // the one a bare `delaypipe` in a pipeline takes. 139 // lookup that silently failed would fall back to the default and every
140 // measurement taken through this pipe would quietly be of the wrong
141 // path. e2e types into a 400ms path and asserts the round trip is
142 // inconsistent with 150, which is the other half of this.
143 try std.testing.expectEqual(@as(u64, 250), parseDelay("250"));
144 try std.testing.expectEqual(@as(u64, 0), parseDelay("0"));
145
146 // Nonsense is the default rather than a failure to start: a helper that
147 // refused to run would read as a broken transport in the suite that
148 // used it, which is a much longer way round to finding a typo.
149 try std.testing.expectEqual(default_delay_ms, parseDelay("abc"));
150 try std.testing.expectEqual(default_delay_ms, parseDelay(""));
151 try std.testing.expectEqual(default_delay_ms, parseDelay("-5"));
152
153 // Unset, which is what a bare `delaypipe` in a pipeline takes.
154 try std.testing.expectEqual(default_delay_ms, parseDelay(null));
131 try std.testing.expectEqual(default_delay_ms, delayFromEnv()); 155 try std.testing.expectEqual(default_delay_ms, delayFromEnv());
132 } 156 }
test/e2e.sh
Old New
@@ -688,10 +688,23 @@ rm -f "$OUT.q" "$OUT.qc" "$OUT.qr" "$OUT.qa" "$OUT.qk" "$QKEY" "$QKEY.bad" "$QKE
688 688
689 # Delay per direction. The round trip is twice this, and every assertion 689 # Delay per direction. The round trip is twice this, and every assertion
690 # below about "before the daemon could have answered" is measured against 690 # below about "before the daemon could have answered" is measured against
691 # it. Deliberately larger than the plan's 150ms: the margin between "the 691 # it. Chosen against three constraints at once, which is why it is not a
692 # prediction is painted" and "the echo could have arrived" is what keeps 692 # round number picked for looks:
693 # this scenario from being a race, and 300ms each way makes that margin 693 #
694 # half a second rather than a tenth. 694 # - Larger than delaypipe's built-in default of 150, and far enough from
695 # it to be told apart. The snapshot below is taken 450ms after the
696 # keystroke: the echo cannot have returned over this 600ms round trip,
697 # but WOULD have over the 300ms one the default gives. A DELAY_MS lookup
698 # that silently stopped working — leaving every scenario here measuring
699 # a path twice as fast as it claimed — fails that assertion rather than
700 # passing quietly.
701 # - Small enough that the round trip stays clear of the overlay's 1000ms
702 # expiry bound. This is a real ceiling, not a test artefact: delaypipe
703 # delays each chunk serially, so a burst's later keystrokes queue behind
704 # the earlier ones and age while they wait. At 400ms each way the burst
705 # scenario went to confirmed=1 — the predictions were expiring before
706 # their echo could arrive, exactly as the overlay is designed to do.
707 # - Leaving margin on both sides, so neither is a race under load.
695 PDELAY=300 708 PDELAY=300
696 709
697 "$MUXD" run --sock "$SOCK5" --shell /bin/cat > "$OUT.p1" 2>&1 & 710 "$MUXD" run --sock "$SOCK5" --shell /bin/cat > "$OUT.p1" 2>&1 &
@@ -709,9 +722,10 @@ set +e
709 > "$OUT.p1" 2>&1 & 722 > "$OUT.p1" 2>&1 &
710 P1PID=$! 723 P1PID=$!
711 set -e 724 set -e
712 # Snapshot the client's output a quarter second after the keystroke — long 725 # Snapshot the client output 450ms after the keystroke: well
713 # before the 600ms round trip could bring the pty's own echo back. 726 # before this 600ms round trip could bring the pty own echo back, and well
714 sleep 2.25 727 # AFTER the 300ms round trip delaypipe default of 150 would have given.
728 sleep 2.45
715 cp "$OUT.p1" "$OUT.p1.early" 2>/dev/null || true 729 cp "$OUT.p1" "$OUT.p1.early" 2>/dev/null || true
716 set +e 730 set +e
717 wait "$P1PID" 731 wait "$P1PID"
@@ -723,15 +737,15 @@ set -e
723 # This is the whole claim of the milestone, as an effect rather than a 737 # This is the whole claim of the milestone, as an effect rather than a
724 # counter: the character was on the screen while it was still in flight. 738 # counter: the character was on the screen while it was still in flight.
725 grep -q "$(printf '\033\[4mz')" "$OUT.p1.early" || { 739 grep -q "$(printf '\033\[4mz')" "$OUT.p1.early" || {
726 echo "e2e FAIL: no predicted glyph 250ms after the keystroke (RTT is $((PDELAY * 2))ms)" 740 echo "e2e FAIL: no predicted glyph 450ms after the keystroke (RTT is $((PDELAY * 2))ms)"
727 echo "--- early snapshot ---"; cat -v "$OUT.p1.early"; exit 1; 741 echo "--- early snapshot ---"; cat -v "$OUT.p1.early"; exit 1;
728 } 742 }
729 # ...and the daemon's own answer was NOT there yet, which is what makes the 743 # ...and the daemon's own answer was NOT there yet, which is what makes the
730 # line above mean anything. A delta paints a row with EL(2) before its 744 # line above mean anything. A delta paints a row with EL(2) before its
731 # content; the prediction never does. 745 # content; the prediction never does.
732 if grep -q "$(printf '\033\[2K\033\[0mz')" "$OUT.p1.early"; then 746 if grep -q "$(printf '\033\[2K\033\[0mz')" "$OUT.p1.early"; then
733 echo "e2e FAIL: the pty's echo arrived within 250ms; the delay pipe is not delaying," 747 echo "e2e FAIL: the echo arrived within 450ms, so the path is not $((PDELAY * 2))ms;"
734 echo " so the assertion above proves nothing about prediction" 748 echo " either DELAY_MS was ignored (default is 150) or delaypipe is not delaying"
735 exit 1 749 exit 1
736 fi 750 fi
737 want_stat "$OUT.p1" contradicted 0 "line mode" 751 want_stat "$OUT.p1" contradicted 0 "line mode"