From feadf911784270866f71ea8a3c306e3ab3121047 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Wed, 20 Jul 2022 16:08:22 +0300 Subject: [PATCH 1/2] chore: Speed up port-forward tests --- cli/portforward_test.go | 43 ++++++++++++++++++++--------------------- 1 file changed, 21 insertions(+), 22 deletions(-) diff --git a/cli/portforward_test.go b/cli/portforward_test.go index 6924a66f16950..f6d0a257c3ce9 100644 --- a/cli/portforward_test.go +++ b/cli/portforward_test.go @@ -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) + ) + 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) @@ -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} @@ -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. @@ -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} @@ -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] @@ -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} @@ -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{} @@ -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} From b9c233bd9330aa3d7fb2331d311671e95c9d2f94 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Wed, 20 Jul 2022 16:08:52 +0300 Subject: [PATCH 2/2] chore: Add t.Helper and ensure listener closure on error --- cli/portforward_test.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/cli/portforward_test.go b/cli/portforward_test.go index f6d0a257c3ce9..95d515b00856a 100644 --- a/cli/portforward_test.go +++ b/cli/portforward_test.go @@ -424,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() { @@ -439,6 +441,7 @@ func setupTestListener(t *testing.T, l net.Listener) string { for { c, err := l.Accept() if err != nil { + _ = l.Close() return } @@ -478,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") @@ -486,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)