Skip to content

Commit 02ee724

Browse files
code-ashercode-asher
authored
fix: do terminal emulation in reconnecting pty tests (coder#9114)
It looks like it is possible for screen to use control sequences instead of literal newlines which fails the tests. This reuses the existing readUntil function used in other pty tests.
1 parent 7499930 commit 02ee724

File tree

4 files changed

+97
-109
lines changed

4 files changed

+97
-109
lines changed

‎agent/agent_test.go

Lines changed: 15 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package agent_test
22

33
import (
4-
"bufio"
54
"bytes"
65
"context"
76
"encoding/json"
@@ -1588,10 +1587,6 @@ func TestAgent_Startup(t *testing.T) {
15881587
})
15891588
}
15901589

1591-
const ansi = "[\u001B\u009B][[\\]()#;?]*(?:(?:(?:[a-zA-Z\\d]*(?:;[a-zA-Z\\d]*)*)?\u0007)|(?:(?:\\d{1,4}(?:;\\d{0,4})*)?[\\dA-PRZcf-ntqry=><~]))"
1592-
1593-
var re = regexp.MustCompile(ansi)
1594-
15951590
//nolint:paralleltest // This test sets an environment variable.
15961591
func TestAgent_ReconnectingPTY(t *testing.T) {
15971592
if runtime.GOOS == "windows" {
@@ -1635,17 +1630,14 @@ func TestAgent_ReconnectingPTY(t *testing.T) {
16351630
//nolint:dogsled
16361631
conn, _, _, _, _ := setupAgent(t, agentsdk.Manifest{}, 0)
16371632
id := uuid.New()
1638-
netConn1, err := conn.ReconnectingPTY(ctx, id, 100, 100, "bash")
1633+
netConn1, err := conn.ReconnectingPTY(ctx, id, 80, 80, "bash")
16391634
require.NoError(t, err)
16401635
defer netConn1.Close()
16411636

1642-
scanner1 := bufio.NewScanner(netConn1)
1643-
16441637
// A second simultaneous connection.
1645-
netConn2, err := conn.ReconnectingPTY(ctx, id, 100, 100, "bash")
1638+
netConn2, err := conn.ReconnectingPTY(ctx, id, 80, 80, "bash")
16461639
require.NoError(t, err)
16471640
defer netConn2.Close()
1648-
scanner2 := bufio.NewScanner(netConn2)
16491641

16501642
// Brief pause to reduce the likelihood that we send keystrokes while
16511643
// the shell is simultaneously sending a prompt.
@@ -1658,17 +1650,6 @@ func TestAgent_ReconnectingPTY(t *testing.T) {
16581650
_, err = netConn1.Write(data)
16591651
require.NoError(t, err)
16601652

1661-
hasLine := func(scanner *bufio.Scanner, matcher func(string) bool) bool {
1662-
for scanner.Scan() {
1663-
line := scanner.Text()
1664-
t.Logf("bash tty stdout = %s", re.ReplaceAllString(line, ""))
1665-
if matcher(line) {
1666-
return true
1667-
}
1668-
}
1669-
return false
1670-
}
1671-
16721653
matchEchoCommand := func(line string) bool {
16731654
return strings.Contains(line, "echo test")
16741655
}
@@ -1683,25 +1664,23 @@ func TestAgent_ReconnectingPTY(t *testing.T) {
16831664
}
16841665

16851666
// Once for typing the command...
1686-
require.True(t, hasLine(scanner1, matchEchoCommand), "find echo command")
1667+
require.NoError(t, testutil.ReadUntil(ctx, t, netConn1, matchEchoCommand), "find echo command")
16871668
// And another time for the actual output.
1688-
require.True(t, hasLine(scanner1, matchEchoOutput), "find echo output")
1669+
require.NoError(t, testutil.ReadUntil(ctx, t, netConn1, matchEchoOutput), "find echo output")
16891670

16901671
// Same for the other connection.
1691-
require.True(t, hasLine(scanner2, matchEchoCommand), "find echo command")
1692-
require.True(t, hasLine(scanner2, matchEchoOutput), "find echo output")
1672+
require.NoError(t, testutil.ReadUntil(ctx, t, netConn2, matchEchoCommand), "find echo command")
1673+
require.NoError(t, testutil.ReadUntil(ctx, t, netConn2, matchEchoOutput), "find echo output")
16931674

16941675
_ = netConn1.Close()
16951676
_ = netConn2.Close()
1696-
netConn3, err := conn.ReconnectingPTY(ctx, id, 100, 100, "bash")
1677+
netConn3, err := conn.ReconnectingPTY(ctx, id, 80, 80, "bash")
16971678
require.NoError(t, err)
16981679
defer netConn3.Close()
16991680

1700-
scanner3 := bufio.NewScanner(netConn3)
1701-
17021681
// Same output again!
1703-
require.True(t, hasLine(scanner3, matchEchoCommand), "find echo command")
1704-
require.True(t, hasLine(scanner3, matchEchoOutput), "find echo output")
1682+
require.NoError(t, testutil.ReadUntil(ctx, t, netConn3, matchEchoCommand), "find echo command")
1683+
require.NoError(t, testutil.ReadUntil(ctx, t, netConn3, matchEchoOutput), "find echo output")
17051684

17061685
// Exit should cause the connection to close.
17071686
data, err = json.Marshal(codersdk.ReconnectingPTYRequest{
@@ -1712,26 +1691,19 @@ func TestAgent_ReconnectingPTY(t *testing.T) {
17121691
require.NoError(t, err)
17131692

17141693
// Once for the input and again for the output.
1715-
require.True(t, hasLine(scanner3, matchExitCommand), "find exit command")
1716-
require.True(t, hasLine(scanner3, matchExitOutput), "find exit output")
1694+
require.NoError(t, testutil.ReadUntil(ctx, t, netConn3, matchExitCommand), "find exit command")
1695+
require.NoError(t, testutil.ReadUntil(ctx, t, netConn3, matchExitOutput), "find exit output")
17171696

17181697
// Wait for the connection to close.
1719-
for scanner3.Scan() {
1720-
line := scanner3.Text()
1721-
t.Logf("bash tty stdout = %s", re.ReplaceAllString(line, ""))
1722-
}
1698+
require.ErrorIs(t, testutil.ReadUntil(ctx, t, netConn3, nil), io.EOF)
17231699

17241700
// Try a non-shell command. It should output then immediately exit.
1725-
netConn4, err := conn.ReconnectingPTY(ctx, uuid.New(), 100, 100, "echo test")
1701+
netConn4, err := conn.ReconnectingPTY(ctx, uuid.New(), 80, 80, "echo test")
17261702
require.NoError(t, err)
17271703
defer netConn4.Close()
17281704

1729-
scanner4 := bufio.NewScanner(netConn4)
1730-
require.True(t, hasLine(scanner4, matchEchoOutput), "find echo output")
1731-
for scanner4.Scan() {
1732-
line := scanner4.Text()
1733-
t.Logf("bash tty stdout = %s", re.ReplaceAllString(line, ""))
1734-
}
1705+
require.NoError(t, testutil.ReadUntil(ctx, t, netConn4, matchEchoOutput), "find echo output")
1706+
require.ErrorIs(t, testutil.ReadUntil(ctx, t, netConn3, nil), io.EOF)
17351707
})
17361708
}
17371709
}

‎coderd/workspaceapps/apptest/apptest.go

Lines changed: 11 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ import (
1212
"net/http/httputil"
1313
"net/url"
1414
"path"
15-
"regexp"
1615
"runtime"
1716
"strconv"
1817
"strings"
@@ -32,10 +31,6 @@ import (
3231
"github.com/coder/coder/testutil"
3332
)
3433

35-
const ansi = "[\u001B\u009B][[\\]()#;?]*(?:(?:(?:[a-zA-Z\\d]*(?:;[a-zA-Z\\d]*)*)?\u0007)|(?:(?:\\d{1,4}(?:;\\d{0,4})*)?[\\dA-PRZcf-ntqry=><~]))"
36-
37-
var re = regexp.MustCompile(ansi)
38-
3934
// Run runs the entire workspace app test suite against deployments minted
4035
// by the provided factory.
4136
//
@@ -70,8 +65,8 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
7065
testReconnectingPTY(ctx, t, client, codersdk.WorkspaceAgentReconnectingPTYOpts{
7166
AgentID: appDetails.Agent.ID,
7267
Reconnect: uuid.New(),
73-
Height: 80,
74-
Width: 80,
68+
Height: 100,
69+
Width: 100,
7570
Command: "bash",
7671
})
7772
})
@@ -104,8 +99,8 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
10499
testReconnectingPTY(ctx, t, unauthedAppClient, codersdk.WorkspaceAgentReconnectingPTYOpts{
105100
AgentID: appDetails.Agent.ID,
106101
Reconnect: uuid.New(),
107-
Height: 80,
108-
Width: 80,
102+
Height: 100,
103+
Width: 100,
109104
Command: "bash",
110105
SignedToken: issueRes.SignedToken,
111106
})
@@ -1407,16 +1402,6 @@ func (r *fakeStatsReporter) Report(_ context.Context, stats []workspaceapps.Stat
14071402
}
14081403

14091404
func testReconnectingPTY(ctx context.Context, t *testing.T, client *codersdk.Client, opts codersdk.WorkspaceAgentReconnectingPTYOpts) {
1410-
hasLine := func(scanner *bufio.Scanner, matcher func(string) bool) bool {
1411-
for scanner.Scan() {
1412-
line := scanner.Text()
1413-
t.Logf("bash tty stdout = %s", re.ReplaceAllString(line, ""))
1414-
if matcher(line) {
1415-
return true
1416-
}
1417-
}
1418-
return false
1419-
}
14201405
matchEchoCommand := func(line string) bool {
14211406
return strings.Contains(line, "echo test")
14221407
}
@@ -1437,13 +1422,12 @@ func testReconnectingPTY(ctx context.Context, t *testing.T, client *codersdk.Cli
14371422
// First attempt to resize the TTY.
14381423
// The websocket will close if it fails!
14391424
data, err := json.Marshal(codersdk.ReconnectingPTYRequest{
1440-
Height: 250,
1441-
Width: 250,
1425+
Height: 80,
1426+
Width: 80,
14421427
})
14431428
require.NoError(t, err)
14441429
_, err = conn.Write(data)
14451430
require.NoError(t, err)
1446-
scanner := bufio.NewScanner(conn)
14471431

14481432
// Brief pause to reduce the likelihood that we send keystrokes while
14491433
// the shell is simultaneously sending a prompt.
@@ -1456,8 +1440,8 @@ func testReconnectingPTY(ctx context.Context, t *testing.T, client *codersdk.Cli
14561440
_, err = conn.Write(data)
14571441
require.NoError(t, err)
14581442

1459-
require.True(t, hasLine(scanner, matchEchoCommand), "find echo command")
1460-
require.True(t, hasLine(scanner, matchEchoOutput), "find echo output")
1443+
require.NoError(t, testutil.ReadUntil(ctx, t, conn, matchEchoCommand), "find echo command")
1444+
require.NoError(t, testutil.ReadUntil(ctx, t, conn, matchEchoOutput), "find echo output")
14611445

14621446
// Exit should cause the connection to close.
14631447
data, err = json.Marshal(codersdk.ReconnectingPTYRequest{
@@ -1468,12 +1452,9 @@ func testReconnectingPTY(ctx context.Context, t *testing.T, client *codersdk.Cli
14681452
require.NoError(t, err)
14691453

14701454
// Once for the input and again for the output.
1471-
require.True(t, hasLine(scanner, matchExitCommand), "find exit command")
1472-
require.True(t, hasLine(scanner, matchExitOutput), "find exit output")
1455+
require.NoError(t, testutil.ReadUntil(ctx, t, conn, matchExitCommand), "find exit command")
1456+
require.NoError(t, testutil.ReadUntil(ctx, t, conn, matchExitOutput), "find exit output")
14731457

14741458
// Ensure the connection closes.
1475-
for scanner.Scan() {
1476-
line := scanner.Text()
1477-
t.Logf("bash tty stdout = %s", re.ReplaceAllString(line, ""))
1478-
}
1459+
require.ErrorIs(t, testutil.ReadUntil(ctx, t, conn, nil), io.EOF)
14791460
}

‎pty/start_test.go

Lines changed: 1 addition & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,9 @@ import (
55
"context"
66
"fmt"
77
"io"
8-
"strings"
98
"testing"
109
"time"
1110

12-
"github.com/hinshun/vt10x"
1311
"github.com/stretchr/testify/assert"
1412
"github.com/stretchr/testify/require"
1513

@@ -73,7 +71,7 @@ func Test_Start_truncation(t *testing.T) {
7371
n := 1
7472
for n <= countEnd {
7573
want := fmt.Sprintf("%d", n)
76-
err := readUntil(ctx, t, want, pc.OutputReader())
74+
err := testutil.ReadUntilString(ctx, t, want, pc.OutputReader())
7775
assert.NoError(t, err, "want: %s", want)
7876
if err != nil {
7977
return
@@ -141,36 +139,3 @@ func Test_Start_cancel_context(t *testing.T) {
141139
t.Error("cmd.Wait() timed out")
142140
}
143141
}
144-
145-
// readUntil reads one byte at a time until we either see the string we want, or the context expires
146-
func readUntil(ctx context.Context, t *testing.T, want string, r io.Reader) error {
147-
// output can contain virtual terminal sequences, so we need to parse these
148-
// to correctly interpret getting what we want.
149-
term := vt10x.New(vt10x.WithSize(80, 80))
150-
readErrs := make(chan error, 1)
151-
for {
152-
b := make([]byte, 1)
153-
go func() {
154-
_, err := r.Read(b)
155-
readErrs <- err
156-
}()
157-
select {
158-
case err := <-readErrs:
159-
if err != nil {
160-
t.Logf("err: %v\ngot: %v", err, term)
161-
return err
162-
}
163-
term.Write(b)
164-
case <-ctx.Done():
165-
return ctx.Err()
166-
}
167-
got := term.String()
168-
lines := strings.Split(got, "\n")
169-
for _, line := range lines {
170-
if strings.TrimSpace(line) == want {
171-
t.Logf("want: %v\n got:%v", want, line)
172-
return nil
173-
}
174-
}
175-
}
176-
}

‎testutil/pty.go

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
package testutil
2+
3+
import (
4+
"context"
5+
"io"
6+
"strings"
7+
"testing"
8+
9+
"github.com/hinshun/vt10x"
10+
)
11+
12+
// ReadUntilString emulates a terminal and reads one byte at a time until we
13+
// either see the string we want, or the context expires. The PTY must be sized
14+
// 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 {
17+
return strings.TrimSpace(line) == want
18+
})
19+
}
20+
21+
// ReadUntil emulates a terminal and reads one byte at a time until the matcher
22+
// returns true or the context expires. If the matcher is nil, read until EOF.
23+
// 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))
28+
readErrs := make(chan error, 1)
29+
defer func() {
30+
// 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()
33+
lines := strings.Split(got, "\n")
34+
for _, line := range lines {
35+
if strings.TrimSpace(line) != "" {
36+
t.Logf("got: %v", line)
37+
}
38+
}
39+
}()
40+
for {
41+
b := make([]byte, 1)
42+
go func() {
43+
_, err := r.Read(b)
44+
readErrs <- err
45+
}()
46+
select {
47+
case err := <-readErrs:
48+
if err != nil {
49+
return err
50+
}
51+
_, err = term.Write(b)
52+
if err != nil {
53+
return err
54+
}
55+
case <-ctx.Done():
56+
return ctx.Err()
57+
}
58+
if matcher == nil {
59+
// A nil matcher means to read until EOF.
60+
continue
61+
}
62+
got := term.String()
63+
lines := strings.Split(got, "\n")
64+
for _, line := range lines {
65+
if matcher(line) {
66+
return nil
67+
}
68+
}
69+
}
70+
}

0 commit comments

Comments
 (0)