Skip to content

Commit 00a0cf2

Browse files
philzexedev-shelley
andcommitted
shelley/server: de-flake TestExecTerminal_ControlCharacters
Prompt: Run a demo server / Push with bin/q / Try again with —retries 2. Separately see if you can make the test not flaky. The control-character pty test intermittently failed in CI with empty "cat -v output". Root cause: the read loop wrapped each wsjson.Read in a 200ms cancellable sub-context. With coder/websocket, a Read whose context is cancelled fails the *whole connection* — so under CI CPU contention the first premature 200ms timeout (before the pty spawned and cat emitted any output) permanently poisoned the websocket; every subsequent read then errored and the buffer stayed empty, tripping the failure. Read on the single outer context instead (already bounding the whole test, now 15s), so a slow first byte just blocks rather than killing the socket. Also wait for the server's "attached" handshake before sending Ctrl-B so the pty (and cat -v) is guaranteed running and can't drop the keystroke. Verified: 120 iterations pass under heavy CPU load (GOMAXPROCS=1 with the box saturated by `yes` workers), where the symptom is reproducible. Co-authored-by: Shelley <shelley@exe.dev>
1 parent 93c705e commit 00a0cf2

1 file changed

Lines changed: 23 additions & 14 deletions

File tree

server/exec_terminal_test.go

Lines changed: 23 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -346,7 +346,14 @@ func TestExecTerminal_ControlCharacters(t *testing.T) {
346346
// Sending Ctrl-B (\x02) should appear as "^B" in the output.
347347
wsURL := "ws" + strings.TrimPrefix(server.URL, "http") + "/api/exec-ws?cmd=cat+-v"
348348

349-
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
349+
// Generous overall deadline: under heavy CI load the pty spawn + first
350+
// output can lag well past a second. We bound every Read on this single
351+
// context rather than a per-read sub-context — with coder/websocket a Read
352+
// whose context is cancelled fails the whole connection, so the old
353+
// 200ms-per-read pattern poisoned the socket on the first premature
354+
// timeout (every later Read then errored, yielding empty output). This
355+
// loop instead blocks on Read until data arrives or the deadline fires.
356+
ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second)
350357
defer cancel()
351358

352359
conn, _, err := websocket.Dial(ctx, wsURL, nil)
@@ -360,20 +367,23 @@ func TestExecTerminal_ControlCharacters(t *testing.T) {
360367
t.Fatalf("Failed to write init message: %v", err)
361368
}
362369

363-
// Send Ctrl-B (\x02) followed by newline to flush the line buffer.
364-
if err := wsjson.Write(ctx, conn, ExecMessage{Type: "input", Data: "\x02\n"}); err != nil {
365-
t.Fatalf("Failed to write input: %v", err)
366-
}
367-
368-
// Read output until we see ^B (cat -v notation for \x02).
370+
// Read output until we see ^B (cat -v notation for \x02). Wait for the
371+
// server's "attached" handshake before sending input so the pty (and
372+
// cat -v) is guaranteed to be running and won't drop the keystroke.
369373
var output strings.Builder
370-
deadline := time.Now().Add(3 * time.Second)
371-
for time.Now().Before(deadline) {
372-
readCtx, readCancel := context.WithTimeout(ctx, 200*time.Millisecond)
374+
sentInput := false
375+
for {
373376
var msg ExecMessage
374-
err := wsjson.Read(readCtx, conn, &msg)
375-
readCancel()
376-
if err != nil {
377+
if err := wsjson.Read(ctx, conn, &msg); err != nil {
378+
t.Errorf("Ctrl-B (\\x02) was not delivered through pty (read: %v); cat -v output: %q", err, output.String())
379+
return
380+
}
381+
if msg.Type == "attached" && !sentInput {
382+
// Send Ctrl-B (\x02) followed by newline to flush the line buffer.
383+
if err := wsjson.Write(ctx, conn, ExecMessage{Type: "input", Data: "\x02\n"}); err != nil {
384+
t.Fatalf("Failed to write input: %v", err)
385+
}
386+
sentInput = true
377387
continue
378388
}
379389
if msg.Type == "output" {
@@ -384,7 +394,6 @@ func TestExecTerminal_ControlCharacters(t *testing.T) {
384394
return // success
385395
}
386396
}
387-
t.Errorf("Ctrl-B (\\x02) was not delivered through pty; cat -v output: %q", output.String())
388397
}
389398

390399
// TestExecTerminal_ShelleyEnvVars confirms the websocket spawner exposes

0 commit comments

Comments
 (0)