feat(agent): add SSH session test driver#26135
Draft
mafredri wants to merge 1 commit into
Draft
Conversation
Replace fragile bufio.Scanner and blind-write SSH session test patterns with a deterministic, composable driver: agenttest.WrapSession yields a Session that starts a Command or a login Shell over a PTY. Reads are ctx-bounded via testutil.WaitBuffer, and a write to an exited process returns a *ProcessError carrying the exit code and captured stderr, the diagnosable form of the bare EOF in coder/internal#1560. agenttest.ShellEnvInfo forces a deterministic shell (/bin/sh, cmd.exe) whose prompt is a sentinel marker, so interactive-shell tests no longer depend on the host shell or hand-rolled prompt detection. Test-only; no production change. Verified on Linux and Windows.
e65c1c7 to
d44bd12
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
PR 3 of the SSH session test redesign (issue coder/internal#1560), following
#26102 (merged). Test-only; no
production change.
#1560is the class of flaky SSH session tests that drive a session with abufio.Scannerread loop and blindFprintfwrites. When the remote processdies, the write surfaces a bare
io.EOFwith no exit code or stderr, so thefailure is undiagnosable, and unbounded reads hang.
This adds a deterministic, composable replacement in
agent/agenttest.API
agenttest.WrapSession(t, *gossh.Session)returns aSession; chain.PTY()then
.Command(ctx, cmd)or.Shell(ctx).Processimplementsio.ReadWriteCloser:Writefeeds stdin (logged tot),Readconsumes stdout (io.EOFon exit),Closecloses stdin. PlusReadUntil(ctx, token),Signal,Wait,Status,Stderr.*ProcessErrorcarrying theexit code and captured stderr, never a bare EOF.
Shelladds prompt policy:Shell(ctx)returns after the first prompt;Run/ReadPrompt/Banner.agenttest.ShellEnvInfo(t, ...)forces a deterministic shell (/bin/sh,cmd.exe) whose prompt isPromptMarker, so interactive tests stopdepending on the host shell or hand-rolled prompt detection.
Reads use
testutil.WaitBufferfor ctx-bounded matching;gosshdoes thestream copying (
session.Stdout/Stderr), so the driver carries no copygoroutines of its own.
Testing
Verified on Linux and Windows (a live
windows-2022-class host). Windows skipsare deliberate and documented: no reliable non-PTY line echo (
findstrrejectspiped stdin), and the agent does not deliver SSH signals to Windows processes.
The PTY
Shellpath is verified against ConPTY/cmd.exe.Design decisions
usershelltestpackage. The testEnvInfoerlives inagenttestbecause all consumers (
agent_test,agentssh_test,cli/ssh_test) areexternal
_testpackages, so importingagenttestcauses no cycle. Aseparate package was unjustified structure.
Processis anio.ReadWriteCloser. Earlier drafts tookctxonWrite/Close/Signaland hand-rolled stdin/stdout copy goroutines. Thisversion conforms to the stdlib interfaces (composes with
io.Copy,io.WriteString,io.ReadAll), logs writes, and keepsctxonly where itis honored (
Wait,ReadUntil,Run).returning an error, matching the test-helper contract.
*ProcessErrorisreserved for the stdin path, which is the actual chore: add container image to releases page #1560 scenario.
agenttest.New'sEnvInfo(and themigration of existing tests) is left to PR 4 so it lands with the consumers
it serves; PR 3 is opt-in via
o.EnvInfo = agenttest.ShellEnvInfo(t).cmd.exeunder ConPTYtakes
CR(notLF) to submit a line; the marker is matched as a substringon a wide PTY with ANSI stripped from returned text; the
Signaltest usesexec sleepso the signal targets the long-running process rather than aresident shell with an orphaned child.