Skip to content

chore: Speed up port-forward tests #3062

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jul 20, 2022
Merged

chore: Speed up port-forward tests #3062

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
feadf91
chore: Speed up port-forward tests
mafredri Jul 20, 2022
b9c233b
chore: Add t.Helper and ensure listener closure on error
mafredri Jul 20, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 27 additions & 22 deletions cli/portforward_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,21 +142,22 @@ func TestPortForward(t *testing.T) {
},
}

// Setup agent once to be shared between test-cases (avoid expensive
// non-parallel setup).
var (
client = coderdtest.New(t, &coderdtest.Options{IncludeProvisionerD: true})
user = coderdtest.CreateFirstUser(t, client)
_, workspace = runAgent(t, client, user.UserID)
)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note this change: We now share coderd/agent between tests. I didn't see any reason not-to.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this will be an interesting test of how useful our logging messages are, since we're sharing a logger among multiple things happening at once (just like in a real deployment 😰 )

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's an interesting point I did not think of. We'll also be sharing the parent *testing.T 😮. To be fair, with regards to test logging, I rarely find them helpful due to the verbosity / parallel execution. 😂


for _, c := range cases { //nolint:paralleltest // the `c := c` confuses the linter
c := c
// Avoid parallel test here because setupLocal reserves
// Delay parallel tests here because setupLocal reserves
// a free open port which is not guaranteed to be free
// after the listener closes.
//nolint:paralleltest
// between the listener closing and port-forward ready.
t.Run(c.name, func(t *testing.T) {
//nolint:paralleltest
t.Run("OnePort", func(t *testing.T) {
var (
client = coderdtest.New(t, &coderdtest.Options{IncludeProvisionerD: true})
user = coderdtest.CreateFirstUser(t, client)
_, workspace = runAgent(t, client, user.UserID)
p1 = setupTestListener(t, c.setupRemote(t))
)
p1 := setupTestListener(t, c.setupRemote(t))

// Create a flag that forwards from local to listener 1.
localAddress, localFlag := c.setupLocal(t)
Expand All @@ -176,6 +177,8 @@ func TestPortForward(t *testing.T) {
}()
waitForPortForwardReady(t, buf)

t.Parallel() // Port is reserved, enable parallel execution.

// Open two connections simultaneously and test them out of
// sync.
d := net.Dialer{Timeout: 3 * time.Second}
Expand All @@ -196,11 +199,8 @@ func TestPortForward(t *testing.T) {
//nolint:paralleltest
t.Run("TwoPorts", func(t *testing.T) {
var (
client = coderdtest.New(t, &coderdtest.Options{IncludeProvisionerD: true})
user = coderdtest.CreateFirstUser(t, client)
_, workspace = runAgent(t, client, user.UserID)
p1 = setupTestListener(t, c.setupRemote(t))
p2 = setupTestListener(t, c.setupRemote(t))
p1 = setupTestListener(t, c.setupRemote(t))
p2 = setupTestListener(t, c.setupRemote(t))
)

// Create a flags for listener 1 and listener 2.
Expand All @@ -223,6 +223,8 @@ func TestPortForward(t *testing.T) {
}()
waitForPortForwardReady(t, buf)

t.Parallel() // Port is reserved, enable parallel execution.

// Open a connection to both listener 1 and 2 simultaneously and
// then test them out of order.
d := net.Dialer{Timeout: 3 * time.Second}
Expand All @@ -246,10 +248,6 @@ func TestPortForward(t *testing.T) {
//nolint:paralleltest
t.Run("TCP2Unix", func(t *testing.T) {
var (
client = coderdtest.New(t, &coderdtest.Options{IncludeProvisionerD: true})
user = coderdtest.CreateFirstUser(t, client)
_, workspace = runAgent(t, client, user.UserID)

// Find the TCP and Unix cases so we can use their setupLocal and
// setupRemote methods respectively.
tcpCase = cases[0]
Expand Down Expand Up @@ -278,6 +276,8 @@ func TestPortForward(t *testing.T) {
}()
waitForPortForwardReady(t, buf)

t.Parallel() // Port is reserved, enable parallel execution.

// Open two connections simultaneously and test them out of
// sync.
d := net.Dialer{Timeout: 3 * time.Second}
Expand All @@ -299,9 +299,6 @@ func TestPortForward(t *testing.T) {
//nolint:paralleltest
t.Run("All", func(t *testing.T) {
var (
client = coderdtest.New(t, &coderdtest.Options{IncludeProvisionerD: true})
user = coderdtest.CreateFirstUser(t, client)
_, workspace = runAgent(t, client, user.UserID)
// These aren't fixed size because we exclude Unix on Windows.
dials = []addr{}
flags = []string{}
Expand Down Expand Up @@ -339,6 +336,8 @@ func TestPortForward(t *testing.T) {
}()
waitForPortForwardReady(t, buf)

t.Parallel() // Port is reserved, enable parallel execution.

// Open connections to all items in the "dial" array.
var (
d = net.Dialer{Timeout: 3 * time.Second}
Expand Down Expand Up @@ -425,6 +424,8 @@ func runAgent(t *testing.T, client *codersdk.Client, userID uuid.UUID) ([]coders
// setupTestListener starts accepting connections and echoing a single packet.
// Returns the listener and the listen port or Unix path.
func setupTestListener(t *testing.T, l net.Listener) string {
t.Helper()

// Wait for listener to completely exit before releasing.
done := make(chan struct{})
t.Cleanup(func() {
Expand All @@ -440,6 +441,7 @@ func setupTestListener(t *testing.T, l net.Listener) string {
for {
c, err := l.Accept()
if err != nil {
_ = l.Close()
return
}

Expand Down Expand Up @@ -479,6 +481,7 @@ func testAccept(t *testing.T, c net.Conn) {
}

func assertReadPayload(t *testing.T, r io.Reader, payload []byte) {
t.Helper()
b := make([]byte, len(payload)+16)
n, err := r.Read(b)
assert.NoError(t, err, "read payload")
Expand All @@ -487,12 +490,14 @@ func assertReadPayload(t *testing.T, r io.Reader, payload []byte) {
}

func assertWritePayload(t *testing.T, w io.Writer, payload []byte) {
t.Helper()
n, err := w.Write(payload)
assert.NoError(t, err, "write payload")
assert.Equal(t, len(payload), n, "payload length does not match")
}

func waitForPortForwardReady(t *testing.T, output *threadSafeBuffer) {
t.Helper()
for i := 0; i < 100; i++ {
time.Sleep(250 * time.Millisecond)

Expand Down