Skip to content

Commit bf0fed4

Browse files
mafredrimafredri
authored
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
1 parent c6b2861 commit bf0fed4

File tree

9 files changed

+225
-232
lines changed

9 files changed

+225
-232
lines changed

‎agent/reaper/reaper_test.go

Lines changed: 28 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -30,42 +30,42 @@ func TestReap(t *testing.T) {
3030
// OK checks that's the reaper is successfully reaping
3131
// exited processes and passing the PIDs through the shared
3232
// channel.
33+
}
3334

34-
//nolint:paralleltest // Signal handling.
35-
t.Run("OK", func(t *testing.T) {
36-
pids := make(reap.PidCh, 1)
37-
err := reaper.ForkReap(
38-
reaper.WithPIDCallback(pids),
39-
// Provide some argument that immediately exits.
40-
reaper.WithExecArgs("/bin/sh", "-c", "exit 0"),
41-
)
42-
require.NoError(t, err)
35+
//nolint:paralleltest // Signal handling.
36+
func TestReap_OK(t *testing.T) {
37+
pids := make(reap.PidCh, 1)
38+
err := reaper.ForkReap(
39+
reaper.WithPIDCallback(pids),
40+
// Provide some argument that immediately exits.
41+
reaper.WithExecArgs("/bin/sh", "-c", "exit 0"),
42+
)
43+
require.NoError(t, err)
4344

44-
cmd := exec.Command("tail", "-f", "/dev/null")
45-
err = cmd.Start()
46-
require.NoError(t, err)
45+
cmd := exec.Command("tail", "-f", "/dev/null")
46+
err = cmd.Start()
47+
require.NoError(t, err)
4748

48-
cmd2 := exec.Command("tail", "-f", "/dev/null")
49-
err = cmd2.Start()
50-
require.NoError(t, err)
49+
cmd2 := exec.Command("tail", "-f", "/dev/null")
50+
err = cmd2.Start()
51+
require.NoError(t, err)
5152

52-
err = cmd.Process.Kill()
53-
require.NoError(t, err)
53+
err = cmd.Process.Kill()
54+
require.NoError(t, err)
5455

55-
err = cmd2.Process.Kill()
56-
require.NoError(t, err)
56+
err = cmd2.Process.Kill()
57+
require.NoError(t, err)
5758

58-
expectedPIDs := []int{cmd.Process.Pid, cmd2.Process.Pid}
59+
expectedPIDs := []int{cmd.Process.Pid, cmd2.Process.Pid}
5960

60-
for i := 0; i < len(expectedPIDs); i++ {
61-
select {
62-
case <-time.After(testutil.WaitShort):
63-
t.Fatalf("Timed out waiting for process")
64-
case pid := <-pids:
65-
require.Contains(t, expectedPIDs, pid)
66-
}
61+
for i := 0; i < len(expectedPIDs); i++ {
62+
select {
63+
case <-time.After(testutil.WaitShort):
64+
t.Fatalf("Timed out waiting for process")
65+
case pid := <-pids:
66+
require.Contains(t, expectedPIDs, pid)
6767
}
68-
})
68+
}
6969
}
7070

7171
//nolint:paralleltest // Signal handling.

‎cli/portforward_test.go

Lines changed: 88 additions & 94 deletions
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,8 @@ import (
2121
"github.com/coder/coder/testutil"
2222
)
2323

24+
//nolint:tparallel,paralleltest // Subtests require setup that must not be done in parallel.
2425
func TestPortForward(t *testing.T) {
25-
t.Parallel()
26-
t.Skip("These tests flake... a lot. It seems related to the Tailscale change, but all other tests pass...")
27-
2826
t.Run("None", func(t *testing.T) {
2927
t.Parallel()
3028

@@ -116,106 +114,102 @@ func TestPortForward(t *testing.T) {
116114
workspace = runAgent(t, client, user.UserID)
117115
)
118116

119-
for _, c := range cases { //nolint:paralleltest // the `c := c` confuses the linter
117+
for _, c := range cases {
120118
c := c
121119
// Delay parallel tests here because setupLocal reserves
122120
// a free open port which is not guaranteed to be free
123121
// between the listener closing and port-forward ready.
124-
t.Run(c.name, func(t *testing.T) {
125-
t.Run("OnePort", func(t *testing.T) {
126-
p1 := setupTestListener(t, c.setupRemote(t))
127-
128-
// Create a flag that forwards from local to listener 1.
129-
localAddress, localFlag := c.setupLocal(t)
130-
flag := fmt.Sprintf(c.flag, localFlag, p1)
131-
132-
// Launch port-forward in a goroutine so we can start dialing
133-
// the "local" listener.
134-
inv, root := clitest.New(t, "-v", "port-forward", workspace.Name, flag)
135-
clitest.SetupConfig(t, client, root)
136-
pty := ptytest.New(t)
137-
inv.Stdin = pty.Input()
138-
inv.Stdout = pty.Output()
139-
inv.Stderr = pty.Output()
140-
ctx, cancel := context.WithCancel(context.Background())
141-
defer cancel()
142-
errC := make(chan error)
143-
go func() {
144-
errC <- inv.WithContext(ctx).Run()
145-
}()
146-
pty.ExpectMatch("Ready!")
147-
148-
t.Parallel() // Port is reserved, enable parallel execution.
149-
150-
// Open two connections simultaneously and test them out of
151-
// sync.
152-
d := net.Dialer{Timeout: testutil.WaitShort}
153-
c1, err := d.DialContext(ctx, c.network, localAddress)
154-
require.NoError(t, err, "open connection 1 to 'local' listener")
155-
defer c1.Close()
156-
c2, err := d.DialContext(ctx, c.network, localAddress)
157-
require.NoError(t, err, "open connection 2 to 'local' listener")
158-
defer c2.Close()
159-
testDial(t, c2)
160-
testDial(t, c1)
161-
162-
cancel()
163-
err = <-errC
164-
require.ErrorIs(t, err, context.Canceled)
165-
})
122+
t.Run(c.name+"_OnePort", func(t *testing.T) {
123+
p1 := setupTestListener(t, c.setupRemote(t))
166124

167-
//nolint:paralleltest
168-
t.Run("TwoPorts", func(t *testing.T) {
169-
var (
170-
p1 = setupTestListener(t, c.setupRemote(t))
171-
p2 = setupTestListener(t, c.setupRemote(t))
172-
)
173-
174-
// Create a flags for listener 1 and listener 2.
175-
localAddress1, localFlag1 := c.setupLocal(t)
176-
localAddress2, localFlag2 := c.setupLocal(t)
177-
flag1 := fmt.Sprintf(c.flag, localFlag1, p1)
178-
flag2 := fmt.Sprintf(c.flag, localFlag2, p2)
179-
180-
// Launch port-forward in a goroutine so we can start dialing
181-
// the "local" listeners.
182-
inv, root := clitest.New(t, "-v", "port-forward", workspace.Name, flag1, flag2)
183-
clitest.SetupConfig(t, client, root)
184-
pty := ptytest.New(t)
185-
inv.Stdin = pty.Input()
186-
inv.Stdout = pty.Output()
187-
inv.Stderr = pty.Output()
188-
ctx, cancel := context.WithCancel(context.Background())
189-
defer cancel()
190-
errC := make(chan error)
191-
go func() {
192-
errC <- inv.WithContext(ctx).Run()
193-
}()
194-
pty.ExpectMatch("Ready!")
195-
196-
t.Parallel() // Port is reserved, enable parallel execution.
197-
198-
// Open a connection to both listener 1 and 2 simultaneously and
199-
// then test them out of order.
200-
d := net.Dialer{Timeout: testutil.WaitShort}
201-
c1, err := d.DialContext(ctx, c.network, localAddress1)
202-
require.NoError(t, err, "open connection 1 to 'local' listener 1")
203-
defer c1.Close()
204-
c2, err := d.DialContext(ctx, c.network, localAddress2)
205-
require.NoError(t, err, "open connection 2 to 'local' listener 2")
206-
defer c2.Close()
207-
testDial(t, c2)
208-
testDial(t, c1)
209-
210-
cancel()
211-
err = <-errC
212-
require.ErrorIs(t, err, context.Canceled)
213-
})
125+
// Create a flag that forwards from local to listener 1.
126+
localAddress, localFlag := c.setupLocal(t)
127+
flag := fmt.Sprintf(c.flag, localFlag, p1)
128+
129+
// Launch port-forward in a goroutine so we can start dialing
130+
// the "local" listener.
131+
inv, root := clitest.New(t, "-v", "port-forward", workspace.Name, flag)
132+
clitest.SetupConfig(t, client, root)
133+
pty := ptytest.New(t)
134+
inv.Stdin = pty.Input()
135+
inv.Stdout = pty.Output()
136+
inv.Stderr = pty.Output()
137+
ctx, cancel := context.WithCancel(context.Background())
138+
defer cancel()
139+
errC := make(chan error)
140+
go func() {
141+
errC <- inv.WithContext(ctx).Run()
142+
}()
143+
pty.ExpectMatch("Ready!")
144+
145+
t.Parallel() // Port is reserved, enable parallel execution.
146+
147+
// Open two connections simultaneously and test them out of
148+
// sync.
149+
d := net.Dialer{Timeout: testutil.WaitShort}
150+
c1, err := d.DialContext(ctx, c.network, localAddress)
151+
require.NoError(t, err, "open connection 1 to 'local' listener")
152+
defer c1.Close()
153+
c2, err := d.DialContext(ctx, c.network, localAddress)
154+
require.NoError(t, err, "open connection 2 to 'local' listener")
155+
defer c2.Close()
156+
testDial(t, c2)
157+
testDial(t, c1)
158+
159+
cancel()
160+
err = <-errC
161+
require.ErrorIs(t, err, context.Canceled)
162+
})
163+
164+
t.Run(c.name+"_TwoPorts", func(t *testing.T) {
165+
var (
166+
p1 = setupTestListener(t, c.setupRemote(t))
167+
p2 = setupTestListener(t, c.setupRemote(t))
168+
)
169+
170+
// Create a flags for listener 1 and listener 2.
171+
localAddress1, localFlag1 := c.setupLocal(t)
172+
localAddress2, localFlag2 := c.setupLocal(t)
173+
flag1 := fmt.Sprintf(c.flag, localFlag1, p1)
174+
flag2 := fmt.Sprintf(c.flag, localFlag2, p2)
175+
176+
// Launch port-forward in a goroutine so we can start dialing
177+
// the "local" listeners.
178+
inv, root := clitest.New(t, "-v", "port-forward", workspace.Name, flag1, flag2)
179+
clitest.SetupConfig(t, client, root)
180+
pty := ptytest.New(t)
181+
inv.Stdin = pty.Input()
182+
inv.Stdout = pty.Output()
183+
inv.Stderr = pty.Output()
184+
ctx, cancel := context.WithCancel(context.Background())
185+
defer cancel()
186+
errC := make(chan error)
187+
go func() {
188+
errC <- inv.WithContext(ctx).Run()
189+
}()
190+
pty.ExpectMatch("Ready!")
191+
192+
t.Parallel() // Port is reserved, enable parallel execution.
193+
194+
// Open a connection to both listener 1 and 2 simultaneously and
195+
// then test them out of order.
196+
d := net.Dialer{Timeout: testutil.WaitShort}
197+
c1, err := d.DialContext(ctx, c.network, localAddress1)
198+
require.NoError(t, err, "open connection 1 to 'local' listener 1")
199+
defer c1.Close()
200+
c2, err := d.DialContext(ctx, c.network, localAddress2)
201+
require.NoError(t, err, "open connection 2 to 'local' listener 2")
202+
defer c2.Close()
203+
testDial(t, c2)
204+
testDial(t, c1)
205+
206+
cancel()
207+
err = <-errC
208+
require.ErrorIs(t, err, context.Canceled)
214209
})
215210
}
216211

217212
// Test doing TCP and UDP at the same time.
218-
//nolint:paralleltest
219213
t.Run("All", func(t *testing.T) {
220214
var (
221215
dials = []addr{}

0 commit comments

Comments
 (0)