chore: Update pion/udp and improve parallel/non-parallel tests (#7164)

* test(all): Improve and fix subtests with parallell/nonparallel parents

* chore: Update pion/udp to fix buffer close

cli/portforward_test.go

@@ -21,10 +21,8 @@ import (
 	"github.com/coder/coder/testutil"
 )
 
+//nolint:tparallel,paralleltest // Subtests require setup that must not be done in parallel.
 func TestPortForward(t *testing.T) {
-	t.Parallel()
-	t.Skip("These tests flake... a lot. It seems related to the Tailscale change, but all other tests pass...")
-
 	t.Run("None", func(t *testing.T) {
 		t.Parallel()
 
@@ -116,106 +114,102 @@ func TestPortForward(t *testing.T) {
 		workspace = runAgent(t, client, user.UserID)
 	)
 
-	for _, c := range cases { //nolint:paralleltest // the `c := c` confuses the linter
+	for _, c := range cases {
 		c := c
 		// Delay parallel tests here because setupLocal reserves
 		// a free open port which is not guaranteed to be free
 		// between the listener closing and port-forward ready.
-		t.Run(c.name, func(t *testing.T) {
-			t.Run("OnePort", func(t *testing.T) {
-				p1 := setupTestListener(t, c.setupRemote(t))
+		t.Run(c.name+"_OnePort", func(t *testing.T) {
+			p1 := setupTestListener(t, c.setupRemote(t))
 
-				// Create a flag that forwards from local to listener 1.
-				localAddress, localFlag := c.setupLocal(t)
-				flag := fmt.Sprintf(c.flag, localFlag, p1)
+			// Create a flag that forwards from local to listener 1.
+			localAddress, localFlag := c.setupLocal(t)
+			flag := fmt.Sprintf(c.flag, localFlag, p1)
 
-				// Launch port-forward in a goroutine so we can start dialing
-				// the "local" listener.
-				inv, root := clitest.New(t, "-v", "port-forward", workspace.Name, flag)
-				clitest.SetupConfig(t, client, root)
-				pty := ptytest.New(t)
-				inv.Stdin = pty.Input()
-				inv.Stdout = pty.Output()
-				inv.Stderr = pty.Output()
-				ctx, cancel := context.WithCancel(context.Background())
-				defer cancel()
-				errC := make(chan error)
-				go func() {
-					errC <- inv.WithContext(ctx).Run()
-				}()
-				pty.ExpectMatch("Ready!")
+			// Launch port-forward in a goroutine so we can start dialing
+			// the "local" listener.
+			inv, root := clitest.New(t, "-v", "port-forward", workspace.Name, flag)
+			clitest.SetupConfig(t, client, root)
+			pty := ptytest.New(t)
+			inv.Stdin = pty.Input()
+			inv.Stdout = pty.Output()
+			inv.Stderr = pty.Output()
+			ctx, cancel := context.WithCancel(context.Background())
+			defer cancel()
+			errC := make(chan error)
+			go func() {
+				errC <- inv.WithContext(ctx).Run()
+			}()
+			pty.ExpectMatch("Ready!")
 
-				t.Parallel() // Port is reserved, enable parallel execution.
+			t.Parallel() // Port is reserved, enable parallel execution.
 
-				// Open two connections simultaneously and test them out of
-				// sync.
-				d := net.Dialer{Timeout: testutil.WaitShort}
-				c1, err := d.DialContext(ctx, c.network, localAddress)
-				require.NoError(t, err, "open connection 1 to 'local' listener")
-				defer c1.Close()
-				c2, err := d.DialContext(ctx, c.network, localAddress)
-				require.NoError(t, err, "open connection 2 to 'local' listener")
-				defer c2.Close()
-				testDial(t, c2)
-				testDial(t, c1)
+			// Open two connections simultaneously and test them out of
+			// sync.
+			d := net.Dialer{Timeout: testutil.WaitShort}
+			c1, err := d.DialContext(ctx, c.network, localAddress)
+			require.NoError(t, err, "open connection 1 to 'local' listener")
+			defer c1.Close()
+			c2, err := d.DialContext(ctx, c.network, localAddress)
+			require.NoError(t, err, "open connection 2 to 'local' listener")
+			defer c2.Close()
+			testDial(t, c2)
+			testDial(t, c1)
 
-				cancel()
-				err = <-errC
-				require.ErrorIs(t, err, context.Canceled)
-			})
+			cancel()
+			err = <-errC
+			require.ErrorIs(t, err, context.Canceled)
+		})
 
-			//nolint:paralleltest
-			t.Run("TwoPorts", func(t *testing.T) {
-				var (
-					p1 = setupTestListener(t, c.setupRemote(t))
-					p2 = setupTestListener(t, c.setupRemote(t))
-				)
+		t.Run(c.name+"_TwoPorts", func(t *testing.T) {
+			var (
+				p1 = setupTestListener(t, c.setupRemote(t))
+				p2 = setupTestListener(t, c.setupRemote(t))
+			)
 
-				// Create a flags for listener 1 and listener 2.
-				localAddress1, localFlag1 := c.setupLocal(t)
-				localAddress2, localFlag2 := c.setupLocal(t)
-				flag1 := fmt.Sprintf(c.flag, localFlag1, p1)
-				flag2 := fmt.Sprintf(c.flag, localFlag2, p2)
+			// Create a flags for listener 1 and listener 2.
+			localAddress1, localFlag1 := c.setupLocal(t)
+			localAddress2, localFlag2 := c.setupLocal(t)
+			flag1 := fmt.Sprintf(c.flag, localFlag1, p1)
+			flag2 := fmt.Sprintf(c.flag, localFlag2, p2)
 
-				// Launch port-forward in a goroutine so we can start dialing
-				// the "local" listeners.
-				inv, root := clitest.New(t, "-v", "port-forward", workspace.Name, flag1, flag2)
-				clitest.SetupConfig(t, client, root)
-				pty := ptytest.New(t)
-				inv.Stdin = pty.Input()
-				inv.Stdout = pty.Output()
-				inv.Stderr = pty.Output()
-				ctx, cancel := context.WithCancel(context.Background())
-				defer cancel()
-				errC := make(chan error)
-				go func() {
-					errC <- inv.WithContext(ctx).Run()
-				}()
-				pty.ExpectMatch("Ready!")
+			// Launch port-forward in a goroutine so we can start dialing
+			// the "local" listeners.
+			inv, root := clitest.New(t, "-v", "port-forward", workspace.Name, flag1, flag2)
+			clitest.SetupConfig(t, client, root)
+			pty := ptytest.New(t)
+			inv.Stdin = pty.Input()
+			inv.Stdout = pty.Output()
+			inv.Stderr = pty.Output()
+			ctx, cancel := context.WithCancel(context.Background())
+			defer cancel()
+			errC := make(chan error)
+			go func() {
+				errC <- inv.WithContext(ctx).Run()
+			}()
+			pty.ExpectMatch("Ready!")
 
-				t.Parallel() // Port is reserved, enable parallel execution.
+			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: testutil.WaitShort}
-				c1, err := d.DialContext(ctx, c.network, localAddress1)
-				require.NoError(t, err, "open connection 1 to 'local' listener 1")
-				defer c1.Close()
-				c2, err := d.DialContext(ctx, c.network, localAddress2)
-				require.NoError(t, err, "open connection 2 to 'local' listener 2")
-				defer c2.Close()
-				testDial(t, c2)
-				testDial(t, c1)
+			// Open a connection to both listener 1 and 2 simultaneously and
+			// then test them out of order.
+			d := net.Dialer{Timeout: testutil.WaitShort}
+			c1, err := d.DialContext(ctx, c.network, localAddress1)
+			require.NoError(t, err, "open connection 1 to 'local' listener 1")
+			defer c1.Close()
+			c2, err := d.DialContext(ctx, c.network, localAddress2)
+			require.NoError(t, err, "open connection 2 to 'local' listener 2")
+			defer c2.Close()
+			testDial(t, c2)
+			testDial(t, c1)
 
-				cancel()
-				err = <-errC
-				require.ErrorIs(t, err, context.Canceled)
-			})
+			cancel()
+			err = <-errC
+			require.ErrorIs(t, err, context.Canceled)
 		})
 	}
 
 	// Test doing TCP and UDP at the same time.
-	//nolint:paralleltest
 	t.Run("All", func(t *testing.T) {
 		var (
 			dials = []addr{}

cli/server_test.go

@@ -104,35 +104,9 @@ func TestReadGitAuthProvidersFromEnv(t *testing.T) {
 	})
 }
 
-// This cannot be ran in parallel because it uses a signal.
-// nolint:tparallel,paralleltest
 func TestServer(t *testing.T) {
-	t.Run("Production", func(t *testing.T) {
-		if runtime.GOOS != "linux" || testing.Short() {
-			// Skip on non-Linux because it spawns a PostgreSQL instance.
-			t.SkipNow()
-		}
-		connectionURL, closeFunc, err := postgres.Open()
-		require.NoError(t, err)
-		defer closeFunc()
+	t.Parallel()
 
-		// Postgres + race detector + CI = slow.
-		ctx := testutil.Context(t, testutil.WaitSuperLong*3)
-
-		inv, cfg := clitest.New(t,
-			"server",
-			"--http-address", ":0",
-			"--access-url", "http://example.com",
-			"--postgres-url", connectionURL,
-			"--cache-dir", t.TempDir(),
-		)
-		clitest.Start(t, inv.WithContext(ctx))
-		accessURL := waitAccessURL(t, cfg)
-		client := codersdk.New(accessURL)
-
-		_, err = client.CreateFirstUser(ctx, coderdtest.FirstUserParams)
-		require.NoError(t, err)
-	})
 	t.Run("BuiltinPostgres", func(t *testing.T) {
 		t.Parallel()
 		if testing.Short() {
@@ -855,38 +829,6 @@ func TestServer(t *testing.T) {
 		})
 	})
 
-	// This cannot be ran in parallel because it uses a signal.
-	//nolint:paralleltest
-	t.Run("Shutdown", func(t *testing.T) {
-		if runtime.GOOS == "windows" {
-			// Sending interrupt signal isn't supported on Windows!
-			t.SkipNow()
-		}
-		ctx, cancelFunc := context.WithCancel(context.Background())
-		defer cancelFunc()
-
-		root, cfg := clitest.New(t,
-			"server",
-			"--in-memory",
-			"--http-address", ":0",
-			"--access-url", "http://example.com",
-			"--provisioner-daemons", "1",
-			"--cache-dir", t.TempDir(),
-		)
-		serverErr := make(chan error, 1)
-		go func() {
-			serverErr <- root.WithContext(ctx).Run()
-		}()
-		_ = waitAccessURL(t, cfg)
-		currentProcess, err := os.FindProcess(os.Getpid())
-		require.NoError(t, err)
-		err = currentProcess.Signal(os.Interrupt)
-		require.NoError(t, err)
-		// We cannot send more signals here, because it's possible Coder
-		// has already exited, which could cause the test to fail due to interrupt.
-		err = <-serverErr
-		require.NoError(t, err)
-	})
 	t.Run("TracerNoLeak", func(t *testing.T) {
 		t.Parallel()
 
@@ -1518,6 +1460,66 @@ func TestServer(t *testing.T) {
 	})
 }
 
+//nolint:tparallel,paralleltest // This test spawns or connects to an existing PostgreSQL instance.
+func TestServer_Production(t *testing.T) {
+	if runtime.GOOS != "linux" || testing.Short() {
+		// Skip on non-Linux because it spawns a PostgreSQL instance.
+		t.SkipNow()
+	}
+	connectionURL, closeFunc, err := postgres.Open()
+	require.NoError(t, err)
+	defer closeFunc()
+
+	// Postgres + race detector + CI = slow.
+	ctx := testutil.Context(t, testutil.WaitSuperLong*3)
+
+	inv, cfg := clitest.New(t,
+		"server",
+		"--http-address", ":0",
+		"--access-url", "http://example.com",
+		"--postgres-url", connectionURL,
+		"--cache-dir", t.TempDir(),
+	)
+	clitest.Start(t, inv.WithContext(ctx))
+	accessURL := waitAccessURL(t, cfg)
+	client := codersdk.New(accessURL)
+
+	_, err = client.CreateFirstUser(ctx, coderdtest.FirstUserParams)
+	require.NoError(t, err)
+}
+
+//nolint:tparallel,paralleltest // This test cannot be run in parallel due to signal handling.
+func TestServer_Shutdown(t *testing.T) {
+	if runtime.GOOS == "windows" {
+		// Sending interrupt signal isn't supported on Windows!
+		t.SkipNow()
+	}
+	ctx, cancelFunc := context.WithCancel(context.Background())
+	defer cancelFunc()
+
+	root, cfg := clitest.New(t,
+		"server",
+		"--in-memory",
+		"--http-address", ":0",
+		"--access-url", "http://example.com",
+		"--provisioner-daemons", "1",
+		"--cache-dir", t.TempDir(),
+	)
+	serverErr := make(chan error, 1)
+	go func() {
+		serverErr <- root.WithContext(ctx).Run()
+	}()
+	_ = waitAccessURL(t, cfg)
+	currentProcess, err := os.FindProcess(os.Getpid())
+	require.NoError(t, err)
+	err = currentProcess.Signal(os.Interrupt)
+	require.NoError(t, err)
+	// We cannot send more signals here, because it's possible Coder
+	// has already exited, which could cause the test to fail due to interrupt.
+	err = <-serverErr
+	require.NoError(t, err)
+}
+
 func generateTLSCertificate(t testing.TB, commonName ...string) (certPath, keyPath string) {
 	dir := t.TempDir()
 

cli/templatepush_test.go

@@ -156,8 +156,6 @@ func TestTemplatePush(t *testing.T) {
 		require.Equal(t, "example", templateVersions[1].Name)
 	})
 
-	// This test modifies the working directory.
-	//nolint:paralleltest
 	t.Run("UseWorkingDir", func(t *testing.T) {
 		t.Parallel()