Skip to content

Commit 70e481e

Browse files
spikecurtisspikecurtis
authored
fix: use terminal emulator that keeps state in ReconnectingPTY tests (#9765)
* Add more pty diagnostics for terminal parsing Signed-off-by: Spike Curtis <spike@coder.com> * print escaped strings Signed-off-by: Spike Curtis <spike@coder.com> * Only log on failure - heisenbug? Signed-off-by: Spike Curtis <spike@coder.com> * use the terminal across matches to keep cursor & contents state Signed-off-by: Spike Curtis <spike@coder.com> * Only log bytes if we're not expecting EOF Signed-off-by: Spike Curtis <spike@coder.com> --------- Signed-off-by: Spike Curtis <spike@coder.com>
1 parent 269b1c5 commit 70e481e

File tree

4 files changed

+68
-32
lines changed

4 files changed

+68
-32
lines changed

‎agent/agent_test.go

+15-11
Original file line numberDiff line numberDiff line change
@@ -1669,13 +1669,15 @@ func TestAgent_ReconnectingPTY(t *testing.T) {
16691669
}
16701670

16711671
// Once for typing the command...
1672-
require.NoError(t, testutil.ReadUntil(ctx, t, netConn1, matchEchoCommand), "find echo command")
1672+
tr1 := testutil.NewTerminalReader(t, netConn1)
1673+
require.NoError(t, tr1.ReadUntil(ctx, matchEchoCommand), "find echo command")
16731674
// And another time for the actual output.
1674-
require.NoError(t, testutil.ReadUntil(ctx, t, netConn1, matchEchoOutput), "find echo output")
1675+
require.NoError(t, tr1.ReadUntil(ctx, matchEchoOutput), "find echo output")
16751676

16761677
// Same for the other connection.
1677-
require.NoError(t, testutil.ReadUntil(ctx, t, netConn2, matchEchoCommand), "find echo command")
1678-
require.NoError(t, testutil.ReadUntil(ctx, t, netConn2, matchEchoOutput), "find echo output")
1678+
tr2 := testutil.NewTerminalReader(t, netConn2)
1679+
require.NoError(t, tr2.ReadUntil(ctx, matchEchoCommand), "find echo command")
1680+
require.NoError(t, tr2.ReadUntil(ctx, matchEchoOutput), "find echo output")
16791681

16801682
_ = netConn1.Close()
16811683
_ = netConn2.Close()
@@ -1684,8 +1686,9 @@ func TestAgent_ReconnectingPTY(t *testing.T) {
16841686
defer netConn3.Close()
16851687

16861688
// Same output again!
1687-
require.NoError(t, testutil.ReadUntil(ctx, t, netConn3, matchEchoCommand), "find echo command")
1688-
require.NoError(t, testutil.ReadUntil(ctx, t, netConn3, matchEchoOutput), "find echo output")
1689+
tr3 := testutil.NewTerminalReader(t, netConn3)
1690+
require.NoError(t, tr3.ReadUntil(ctx, matchEchoCommand), "find echo command")
1691+
require.NoError(t, tr3.ReadUntil(ctx, matchEchoOutput), "find echo output")
16891692

16901693
// Exit should cause the connection to close.
16911694
data, err = json.Marshal(codersdk.ReconnectingPTYRequest{
@@ -1696,19 +1699,20 @@ func TestAgent_ReconnectingPTY(t *testing.T) {
16961699
require.NoError(t, err)
16971700

16981701
// Once for the input and again for the output.
1699-
require.NoError(t, testutil.ReadUntil(ctx, t, netConn3, matchExitCommand), "find exit command")
1700-
require.NoError(t, testutil.ReadUntil(ctx, t, netConn3, matchExitOutput), "find exit output")
1702+
require.NoError(t, tr3.ReadUntil(ctx, matchExitCommand), "find exit command")
1703+
require.NoError(t, tr3.ReadUntil(ctx, matchExitOutput), "find exit output")
17011704

17021705
// Wait for the connection to close.
1703-
require.ErrorIs(t, testutil.ReadUntil(ctx, t, netConn3, nil), io.EOF)
1706+
require.ErrorIs(t, tr3.ReadUntil(ctx, nil), io.EOF)
17041707

17051708
// Try a non-shell command. It should output then immediately exit.
17061709
netConn4, err := conn.ReconnectingPTY(ctx, uuid.New(), 80, 80, "echo test")
17071710
require.NoError(t, err)
17081711
defer netConn4.Close()
17091712

1710-
require.NoError(t, testutil.ReadUntil(ctx, t, netConn4, matchEchoOutput), "find echo output")
1711-
require.ErrorIs(t, testutil.ReadUntil(ctx, t, netConn3, nil), io.EOF)
1713+
tr4 := testutil.NewTerminalReader(t, netConn4)
1714+
require.NoError(t, tr4.ReadUntil(ctx, matchEchoOutput), "find echo output")
1715+
require.ErrorIs(t, tr4.ReadUntil(ctx, nil), io.EOF)
17121716
})
17131717
}
17141718
}

‎coderd/workspaceapps/apptest/apptest.go

+6-5
Original file line numberDiff line numberDiff line change
@@ -1428,8 +1428,9 @@ func testReconnectingPTY(ctx context.Context, t *testing.T, client *codersdk.Cli
14281428
_, err = conn.Write(data)
14291429
require.NoError(t, err)
14301430

1431-
require.NoError(t, testutil.ReadUntil(ctx, t, conn, matchEchoCommand), "find echo command")
1432-
require.NoError(t, testutil.ReadUntil(ctx, t, conn, matchEchoOutput), "find echo output")
1431+
tr := testutil.NewTerminalReader(t, conn)
1432+
require.NoError(t, tr.ReadUntil(ctx, matchEchoCommand), "find echo command")
1433+
require.NoError(t, tr.ReadUntil(ctx, matchEchoOutput), "find echo output")
14331434

14341435
// Exit should cause the connection to close.
14351436
data, err = json.Marshal(codersdk.ReconnectingPTYRequest{
@@ -1440,9 +1441,9 @@ func testReconnectingPTY(ctx context.Context, t *testing.T, client *codersdk.Cli
14401441
require.NoError(t, err)
14411442

14421443
// Once for the input and again for the output.
1443-
require.NoError(t, testutil.ReadUntil(ctx, t, conn, matchExitCommand), "find exit command")
1444-
require.NoError(t, testutil.ReadUntil(ctx, t, conn, matchExitOutput), "find exit output")
1444+
require.NoError(t, tr.ReadUntil(ctx, matchExitCommand), "find exit command")
1445+
require.NoError(t, tr.ReadUntil(ctx, matchExitOutput), "find exit output")
14451446

14461447
// Ensure the connection closes.
1447-
require.ErrorIs(t, testutil.ReadUntil(ctx, t, conn, nil), io.EOF)
1448+
require.ErrorIs(t, tr.ReadUntil(ctx, nil), io.EOF)
14481449
}

‎pty/start_test.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -67,11 +67,11 @@ func Test_Start_truncation(t *testing.T) {
6767
readDone := make(chan struct{})
6868
go func() {
6969
defer close(readDone)
70-
// avoid buffered IO so that we can precisely control how many bytes to read.
70+
terminalReader := testutil.NewTerminalReader(t, pc.OutputReader())
7171
n := 1
7272
for n <= countEnd {
7373
want := fmt.Sprintf("%d", n)
74-
err := testutil.ReadUntilString(ctx, t, want, pc.OutputReader())
74+
err := terminalReader.ReadUntilString(ctx, want)
7575
assert.NoError(t, err, "want: %s", want)
7676
if err != nil {
7777
return

‎testutil/pty.go

+45-14
Original file line numberDiff line numberDiff line change
@@ -6,49 +6,80 @@ import (
66
"strings"
77
"testing"
88

9+
"golang.org/x/xerrors"
10+
911
"github.com/hinshun/vt10x"
1012
)
1113

14+
// TerminalReader emulates a terminal and allows matching output. It's important in cases where we
15+
// can get control sequences to parse them correctly, and keep the state of the terminal across the
16+
// lifespan of the PTY, since some control sequences are relative to the current cursor position.
17+
type TerminalReader struct {
18+
t *testing.T
19+
r io.Reader
20+
term vt10x.Terminal
21+
}
22+
23+
func NewTerminalReader(t *testing.T, r io.Reader) *TerminalReader {
24+
return &TerminalReader{
25+
t: t,
26+
r: r,
27+
term: vt10x.New(vt10x.WithSize(80, 80)),
28+
}
29+
}
30+
1231
// ReadUntilString emulates a terminal and reads one byte at a time until we
1332
// either see the string we want, or the context expires. The PTY must be sized
1433
// to 80x80 or there could be unexpected results.
15-
func ReadUntilString(ctx context.Context, t *testing.T, want string, r io.Reader) error {
16-
return ReadUntil(ctx, t, r, func(line string) bool {
34+
func (tr *TerminalReader) ReadUntilString(ctx context.Context, want string) error {
35+
return tr.ReadUntil(ctx, func(line string) bool {
1736
return strings.TrimSpace(line) == want
1837
})
1938
}
2039

2140
// ReadUntil emulates a terminal and reads one byte at a time until the matcher
2241
// returns true or the context expires. If the matcher is nil, read until EOF.
2342
// The PTY must be sized to 80x80 or there could be unexpected results.
24-
func ReadUntil(ctx context.Context, t *testing.T, r io.Reader, matcher func(line string) bool) error {
25-
// output can contain virtual terminal sequences, so we need to parse these
26-
// to correctly interpret getting what we want.
27-
term := vt10x.New(vt10x.WithSize(80, 80))
43+
func (tr *TerminalReader) ReadUntil(ctx context.Context, matcher func(line string) bool) (retErr error) {
44+
readBytes := make([]byte, 0)
2845
readErrs := make(chan error, 1)
2946
defer func() {
3047
// Dump the terminal contents since they can be helpful for debugging, but
31-
// skip empty lines since much of the terminal will usually be blank.
32-
got := term.String()
48+
// trim empty lines since much of the terminal will usually be blank.
49+
got := tr.term.String()
3350
lines := strings.Split(got, "\n")
34-
for _, line := range lines {
35-
if strings.TrimSpace(line) != "" {
36-
t.Logf("got: %v", line)
51+
for i := range lines {
52+
if strings.TrimSpace(lines[i]) != "" {
53+
lines = lines[i:]
54+
break
3755
}
3856
}
57+
for i := len(lines) - 1; i >= 0; i-- {
58+
if strings.TrimSpace(lines[i]) != "" {
59+
lines = lines[:i+1]
60+
break
61+
}
62+
}
63+
gotTrimmed := strings.Join(lines, "\n")
64+
tr.t.Logf("Terminal contents:\n%s", gotTrimmed)
65+
// EOF is expected when matcher == nil
66+
if retErr != nil && !(xerrors.Is(retErr, io.EOF) && matcher == nil) {
67+
tr.t.Logf("Bytes Read: %q", string(readBytes))
68+
}
3969
}()
4070
for {
4171
b := make([]byte, 1)
4272
go func() {
43-
_, err := r.Read(b)
73+
_, err := tr.r.Read(b)
4474
readErrs <- err
4575
}()
4676
select {
4777
case err := <-readErrs:
4878
if err != nil {
4979
return err
5080
}
51-
_, err = term.Write(b)
81+
readBytes = append(readBytes, b...)
82+
_, err = tr.term.Write(b)
5283
if err != nil {
5384
return err
5485
}
@@ -59,7 +90,7 @@ func ReadUntil(ctx context.Context, t *testing.T, r io.Reader, matcher func(line
5990
// A nil matcher means to read until EOF.
6091
continue
6192
}
62-
got := term.String()
93+
got := tr.term.String()
6394
lines := strings.Split(got, "\n")
6495
for _, line := range lines {
6596
if matcher(line) {

0 commit comments

Comments
 (0)