Skip to content

Commit ec0cc6b

Browse files
mafredrimafredri
committed
test(all): Improve and fix subtests with parallell/nonparallel parents
1 parent 25c1e45 commit ec0cc6b

File tree

7 files changed

+132
-137
lines changed

7 files changed

+132
-137
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: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,8 @@ import (
2121
"github.com/coder/coder/testutil"
2222
)
2323

24+
//nolint:paralleltest // Non-parallel subtests.
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,7 +114,7 @@ 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
@@ -164,7 +162,6 @@ func TestPortForward(t *testing.T) {
164162
require.ErrorIs(t, err, context.Canceled)
165163
})
166164

167-
//nolint:paralleltest
168165
t.Run("TwoPorts", func(t *testing.T) {
169166
var (
170167
p1 = setupTestListener(t, c.setupRemote(t))
@@ -215,7 +212,6 @@ func TestPortForward(t *testing.T) {
215212
}
216213

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

‎cli/server_test.go

Lines changed: 60 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -104,35 +104,7 @@ func TestReadGitAuthProvidersFromEnv(t *testing.T) {
104104
})
105105
}
106106

107-
// This cannot be ran in parallel because it uses a signal.
108-
// nolint:tparallel,paralleltest
109107
func TestServer(t *testing.T) {
110-
t.Run("Production", func(t *testing.T) {
111-
if runtime.GOOS != "linux" || testing.Short() {
112-
// Skip on non-Linux because it spawns a PostgreSQL instance.
113-
t.SkipNow()
114-
}
115-
connectionURL, closeFunc, err := postgres.Open()
116-
require.NoError(t, err)
117-
defer closeFunc()
118-
119-
// Postgres + race detector + CI = slow.
120-
ctx := testutil.Context(t, testutil.WaitSuperLong*3)
121-
122-
inv, cfg := clitest.New(t,
123-
"server",
124-
"--http-address", ":0",
125-
"--access-url", "http://example.com",
126-
"--postgres-url", connectionURL,
127-
"--cache-dir", t.TempDir(),
128-
)
129-
clitest.Start(t, inv.WithContext(ctx))
130-
accessURL := waitAccessURL(t, cfg)
131-
client := codersdk.New(accessURL)
132-
133-
_, err = client.CreateFirstUser(ctx, coderdtest.FirstUserParams)
134-
require.NoError(t, err)
135-
})
136108
t.Run("BuiltinPostgres", func(t *testing.T) {
137109
t.Parallel()
138110
if testing.Short() {
@@ -855,38 +827,6 @@ func TestServer(t *testing.T) {
855827
})
856828
})
857829

858-
// This cannot be ran in parallel because it uses a signal.
859-
//nolint:paralleltest
860-
t.Run("Shutdown", func(t *testing.T) {
861-
if runtime.GOOS == "windows" {
862-
// Sending interrupt signal isn't supported on Windows!
863-
t.SkipNow()
864-
}
865-
ctx, cancelFunc := context.WithCancel(context.Background())
866-
defer cancelFunc()
867-
868-
root, cfg := clitest.New(t,
869-
"server",
870-
"--in-memory",
871-
"--http-address", ":0",
872-
"--access-url", "http://example.com",
873-
"--provisioner-daemons", "1",
874-
"--cache-dir", t.TempDir(),
875-
)
876-
serverErr := make(chan error, 1)
877-
go func() {
878-
serverErr <- root.WithContext(ctx).Run()
879-
}()
880-
_ = waitAccessURL(t, cfg)
881-
currentProcess, err := os.FindProcess(os.Getpid())
882-
require.NoError(t, err)
883-
err = currentProcess.Signal(os.Interrupt)
884-
require.NoError(t, err)
885-
// We cannot send more signals here, because it's possible Coder
886-
// has already exited, which could cause the test to fail due to interrupt.
887-
err = <-serverErr
888-
require.NoError(t, err)
889-
})
890830
t.Run("TracerNoLeak", func(t *testing.T) {
891831
t.Parallel()
892832

@@ -1493,6 +1433,66 @@ func TestServer(t *testing.T) {
14931433
})
14941434
}
14951435

1436+
//nolint:paralleltest // This test spawns or connects to an existing PostgreSQL instance.
1437+
func TestServer_Production(t *testing.T) {
1438+
if runtime.GOOS != "linux" || testing.Short() {
1439+
// Skip on non-Linux because it spawns a PostgreSQL instance.
1440+
t.SkipNow()
1441+
}
1442+
connectionURL, closeFunc, err := postgres.Open()
1443+
require.NoError(t, err)
1444+
defer closeFunc()
1445+
1446+
// Postgres + race detector + CI = slow.
1447+
ctx := testutil.Context(t, testutil.WaitSuperLong*3)
1448+
1449+
inv, cfg := clitest.New(t,
1450+
"server",
1451+
"--http-address", ":0",
1452+
"--access-url", "http://example.com",
1453+
"--postgres-url", connectionURL,
1454+
"--cache-dir", t.TempDir(),
1455+
)
1456+
clitest.Start(t, inv.WithContext(ctx))
1457+
accessURL := waitAccessURL(t, cfg)
1458+
client := codersdk.New(accessURL)
1459+
1460+
_, err = client.CreateFirstUser(ctx, coderdtest.FirstUserParams)
1461+
require.NoError(t, err)
1462+
}
1463+
1464+
//nolint:paralleltest // This test cannot be run in parallel due to signal handling.
1465+
func TestServer_Shutdown(t *testing.T) {
1466+
if runtime.GOOS == "windows" {
1467+
// Sending interrupt signal isn't supported on Windows!
1468+
t.SkipNow()
1469+
}
1470+
ctx, cancelFunc := context.WithCancel(context.Background())
1471+
defer cancelFunc()
1472+
1473+
root, cfg := clitest.New(t,
1474+
"server",
1475+
"--in-memory",
1476+
"--http-address", ":0",
1477+
"--access-url", "http://example.com",
1478+
"--provisioner-daemons", "1",
1479+
"--cache-dir", t.TempDir(),
1480+
)
1481+
serverErr := make(chan error, 1)
1482+
go func() {
1483+
serverErr <- root.WithContext(ctx).Run()
1484+
}()
1485+
_ = waitAccessURL(t, cfg)
1486+
currentProcess, err := os.FindProcess(os.Getpid())
1487+
require.NoError(t, err)
1488+
err = currentProcess.Signal(os.Interrupt)
1489+
require.NoError(t, err)
1490+
// We cannot send more signals here, because it's possible Coder
1491+
// has already exited, which could cause the test to fail due to interrupt.
1492+
err = <-serverErr
1493+
require.NoError(t, err)
1494+
}
1495+
14961496
func generateTLSCertificate(t testing.TB, commonName ...string) (certPath, keyPath string) {
14971497
dir := t.TempDir()
14981498

‎cli/templatepush_test.go

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -156,8 +156,6 @@ func TestTemplatePush(t *testing.T) {
156156
require.Equal(t, "example", templateVersions[1].Name)
157157
})
158158

159-
// This test modifies the working directory.
160-
//nolint:paralleltest
161159
t.Run("UseWorkingDir", func(t *testing.T) {
162160
t.Parallel()
163161

‎coderd/users_test.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1570,7 +1570,6 @@ func TestPaginatedUsers(t *testing.T) {
15701570
{name: "username search", limit: 3, allUsers: specialUsers, opt: usernameSearch},
15711571
{name: "username search", limit: 3, allUsers: specialUsers, opt: usernameSearch},
15721572
}
1573-
//nolint:paralleltest // Does not detect range value.
15741573
for _, tt := range tests {
15751574
tt := tt
15761575
t.Run(fmt.Sprintf("%s %d", tt.name, tt.limit), func(t *testing.T) {

‎cryptorand/strings_test.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,6 @@ func TestStringCharset(t *testing.T) {
9191
},
9292
}
9393

94-
//nolint:paralleltest
9594
for _, test := range tests {
9695
test := test
9796
t.Run(test.Name, func(t *testing.T) {

‎scaletest/agentconn/run_test.go

Lines changed: 42 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -58,44 +58,6 @@ func Test_Runner(t *testing.T) {
5858
require.NotContains(t, logStr, "Waiting for ")
5959
})
6060

61-
//nolint:paralleltest // Measures timing as part of the test.
62-
t.Run("Direct+Hold", func(t *testing.T) {
63-
client, agentID := setupRunnerTest(t)
64-
65-
runner := agentconn.NewRunner(client, agentconn.Config{
66-
AgentID: agentID,
67-
ConnectionMode: agentconn.ConnectionModeDirect,
68-
HoldDuration: httpapi.Duration(testutil.WaitShort),
69-
})
70-
71-
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
72-
defer cancel()
73-
74-
logs := bytes.NewBuffer(nil)
75-
start := time.Now()
76-
err := runner.Run(ctx, "1", logs)
77-
logStr := logs.String()
78-
t.Log("Runner logs:\n\n" + logStr)
79-
require.NoError(t, err)
80-
81-
require.WithinRange(t,
82-
time.Now(),
83-
start.Add(testutil.WaitShort-time.Second),
84-
start.Add(testutil.WaitShort+5*time.Second),
85-
)
86-
87-
require.Contains(t, logStr, "Opening connection to workspace agent")
88-
require.Contains(t, logStr, "Using direct connection")
89-
require.Contains(t, logStr, "Disco ping attempt 1/10...")
90-
require.Contains(t, logStr, "Direct connection check 1/30...")
91-
require.Contains(t, logStr, "Connection established")
92-
require.Contains(t, logStr, "Verify connection attempt 1/30...")
93-
require.Contains(t, logStr, "Connection verified")
94-
require.NotContains(t, logStr, "Performing initial service connections")
95-
require.NotContains(t, logStr, "Starting connection loops")
96-
require.Contains(t, logStr, fmt.Sprintf("Waiting for %s", testutil.WaitShort))
97-
})
98-
9961
t.Run("Derp+ServicesNoHold", func(t *testing.T) {
10062
t.Parallel()
10163

@@ -143,8 +105,49 @@ func Test_Runner(t *testing.T) {
143105
require.EqualValues(t, 1, service1Count())
144106
require.EqualValues(t, 1, service2Count())
145107
})
108+
}
109+
110+
//nolint:paralleltest // Measures timing as part of the test.
111+
func Test_Runner_Timing(t *testing.T) {
112+
//nolint:paralleltest
113+
t.Run("Direct+Hold", func(t *testing.T) {
114+
client, agentID := setupRunnerTest(t)
115+
116+
runner := agentconn.NewRunner(client, agentconn.Config{
117+
AgentID: agentID,
118+
ConnectionMode: agentconn.ConnectionModeDirect,
119+
HoldDuration: httpapi.Duration(testutil.WaitShort),
120+
})
121+
122+
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
123+
defer cancel()
124+
125+
logs := bytes.NewBuffer(nil)
126+
start := time.Now()
127+
err := runner.Run(ctx, "1", logs)
128+
logStr := logs.String()
129+
t.Log("Runner logs:\n\n" + logStr)
130+
require.NoError(t, err)
131+
132+
require.WithinRange(t,
133+
time.Now(),
134+
start.Add(testutil.WaitShort-time.Second),
135+
start.Add(testutil.WaitShort+5*time.Second),
136+
)
137+
138+
require.Contains(t, logStr, "Opening connection to workspace agent")
139+
require.Contains(t, logStr, "Using direct connection")
140+
require.Contains(t, logStr, "Disco ping attempt 1/10...")
141+
require.Contains(t, logStr, "Direct connection check 1/30...")
142+
require.Contains(t, logStr, "Connection established")
143+
require.Contains(t, logStr, "Verify connection attempt 1/30...")
144+
require.Contains(t, logStr, "Connection verified")
145+
require.NotContains(t, logStr, "Performing initial service connections")
146+
require.NotContains(t, logStr, "Starting connection loops")
147+
require.Contains(t, logStr, fmt.Sprintf("Waiting for %s", testutil.WaitShort))
148+
})
146149

147-
//nolint:paralleltest // Measures timing as part of the test.
150+
//nolint:paralleltest
148151
t.Run("Derp+Hold+Services", func(t *testing.T) {
149152
client, agentID := setupRunnerTest(t)
150153
service1URL, service1Count := testServer(t)

0 commit comments

Comments
 (0)