Skip to content

Commit a1576b2

Browse files
mafredrimafredri
committed
fix: Change uses of t.Cleanup -> defer in test bodies
Mixing t.Cleanup and defer can lead to unexpected order of execution.
1 parent 75ff579 commit a1576b2

15 files changed

+57
-58
lines changed

‎cli/configssh_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -109,19 +109,19 @@ func TestConfigSSH(t *testing.T) {
109109
agentCloser := agent.New(agentClient.ListenWorkspaceAgent, &agent.Options{
110110
Logger: slogtest.Make(t, nil),
111111
})
112-
t.Cleanup(func() {
112+
defer func() {
113113
_ = agentCloser.Close()
114-
})
114+
}()
115115
resources := coderdtest.AwaitWorkspaceAgents(t, client, workspace.LatestBuild.ID)
116116
agentConn, err := client.DialWorkspaceAgent(context.Background(), resources[0].Agents[0].ID, nil)
117117
require.NoError(t, err)
118118
defer agentConn.Close()
119119

120120
listener, err := net.Listen("tcp", "127.0.0.1:0")
121121
require.NoError(t, err)
122-
t.Cleanup(func() {
122+
defer func() {
123123
_ = listener.Close()
124-
})
124+
}()
125125
go func() {
126126
for {
127127
conn, err := listener.Accept()

‎cli/logout_test.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -152,19 +152,19 @@ func TestLogout(t *testing.T) {
152152
err = os.Chmod(string(config), 0500)
153153
require.NoError(t, err)
154154
}
155-
t.Cleanup(func() {
155+
defer func() {
156156
if runtime.GOOS == "windows" {
157157
// Closing the opened files for cleanup.
158158
err = urlFile.Close()
159-
require.NoError(t, err)
159+
assert.NoError(t, err)
160160
err = sessionFile.Close()
161-
require.NoError(t, err)
161+
assert.NoError(t, err)
162162
} else {
163163
// Setting the permissions back for cleanup.
164-
err = os.Chmod(string(config), 0700)
165-
require.NoError(t, err)
164+
err = os.Chmod(string(config), 0o700)
165+
assert.NoError(t, err)
166166
}
167-
})
167+
}()
168168

169169
logoutChan := make(chan struct{})
170170
logout, _ := clitest.New(t, "logout", "--global-config", string(config))

‎cli/portforward_test.go

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -405,11 +405,7 @@ func runAgent(t *testing.T, client *codersdk.Client, userID uuid.UUID) ([]coders
405405
clitest.SetupConfig(t, client, root)
406406
errC := make(chan error)
407407
agentCtx, agentCancel := context.WithCancel(ctx)
408-
t.Cleanup(func() {
409-
agentCancel()
410-
err := <-errC
411-
require.NoError(t, err)
412-
})
408+
defer agentCancel()
413409
go func() {
414410
errC <- cmd.ExecuteContext(agentCtx)
415411
}()
@@ -418,6 +414,9 @@ func runAgent(t *testing.T, client *codersdk.Client, userID uuid.UUID) ([]coders
418414
resources, err := client.WorkspaceResourcesByBuild(context.Background(), workspace.LatestBuild.ID)
419415
require.NoError(t, err)
420416

417+
err := <-errC
418+
require.NoError(t, err)
419+
421420
return resources, workspace
422421
}
423422

‎cli/server_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -251,7 +251,7 @@ func TestServer(t *testing.T) {
251251
snapshot <- ss
252252
})
253253
server := httptest.NewServer(r)
254-
t.Cleanup(server.Close)
254+
defer server.Close()
255255

256256
root, _ := clitest.New(t, "server", "--in-memory", "--address", ":0", "--telemetry", "--telemetry-url", server.URL)
257257
errC := make(chan error)

‎coderd/provisionerjobs_internal_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ func TestProvisionerJobLogs_Unit(t *testing.T) {
2828

2929
t.Run("QueryPubSubDupes", func(t *testing.T) {
3030
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
31-
t.Cleanup(cancel)
31+
defer cancel()
3232
logger := slogtest.Make(t, nil).Leveled(slog.LevelDebug)
3333
// mDB := mocks.NewStore(t)
3434
fDB := databasefake.New()
@@ -40,7 +40,7 @@ func TestProvisionerJobLogs_Unit(t *testing.T) {
4040
}
4141
api := New(&opts)
4242
server := httptest.NewServer(api.Handler)
43-
t.Cleanup(server.Close)
43+
defer server.Close()
4444
userID := uuid.New()
4545
keyID, keySecret, err := generateAPIKeyIDSecret()
4646
require.NoError(t, err)

‎coderd/provisionerjobs_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ func TestProvisionerJobLogs(t *testing.T) {
4141
coderdtest.AwaitWorkspaceBuildJob(t, client, workspace.LatestBuild.ID)
4242

4343
ctx, cancelFunc := context.WithCancel(context.Background())
44-
t.Cleanup(cancelFunc)
44+
defer cancelFunc()
4545
logs, err := client.WorkspaceBuildLogsAfter(ctx, workspace.LatestBuild.ID, before)
4646
require.NoError(t, err)
4747
for {
@@ -77,7 +77,7 @@ func TestProvisionerJobLogs(t *testing.T) {
7777
workspace := coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID)
7878
before := database.Now()
7979
ctx, cancelFunc := context.WithCancel(context.Background())
80-
t.Cleanup(cancelFunc)
80+
defer cancelFunc()
8181
logs, err := client.WorkspaceBuildLogsAfter(ctx, workspace.LatestBuild.ID, before)
8282
require.NoError(t, err)
8383
for {

‎coderd/templateversions_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -376,7 +376,7 @@ func TestTemplateVersionLogs(t *testing.T) {
376376
}},
377377
})
378378
ctx, cancelFunc := context.WithCancel(context.Background())
379-
t.Cleanup(cancelFunc)
379+
defer cancelFunc()
380380
logs, err := client.TemplateVersionLogsAfter(ctx, version.ID, before)
381381
require.NoError(t, err)
382382
for {

‎coderd/workspaceagents_test.go

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -107,15 +107,15 @@ func TestWorkspaceAgentListen(t *testing.T) {
107107
agentCloser := agent.New(agentClient.ListenWorkspaceAgent, &agent.Options{
108108
Logger: slogtest.Make(t, nil).Named("agent").Leveled(slog.LevelDebug),
109109
})
110-
t.Cleanup(func() {
110+
defer func() {
111111
_ = agentCloser.Close()
112-
})
112+
}()
113113
resources := coderdtest.AwaitWorkspaceAgents(t, client, workspace.LatestBuild.ID)
114114
conn, err := client.DialWorkspaceAgent(context.Background(), resources[0].Agents[0].ID, nil)
115115
require.NoError(t, err)
116-
t.Cleanup(func() {
116+
defer func() {
117117
_ = conn.Close()
118-
})
118+
}()
119119
_, err = conn.Ping()
120120
require.NoError(t, err)
121121
})
@@ -232,9 +232,9 @@ func TestWorkspaceAgentTURN(t *testing.T) {
232232
agentCloser := agent.New(agentClient.ListenWorkspaceAgent, &agent.Options{
233233
Logger: slogtest.Make(t, nil),
234234
})
235-
t.Cleanup(func() {
235+
defer func() {
236236
_ = agentCloser.Close()
237-
})
237+
}()
238238
resources := coderdtest.AwaitWorkspaceAgents(t, client, workspace.LatestBuild.ID)
239239
opts := &peer.ConnOptions{
240240
Logger: slogtest.Make(t, nil).Named("client"),
@@ -243,9 +243,9 @@ func TestWorkspaceAgentTURN(t *testing.T) {
243243
opts.SettingEngine.SetNetworkTypes([]webrtc.NetworkType{webrtc.NetworkTypeTCP4})
244244
conn, err := client.DialWorkspaceAgent(context.Background(), resources[0].Agents[0].ID, opts)
245245
require.NoError(t, err)
246-
t.Cleanup(func() {
246+
defer func() {
247247
_ = conn.Close()
248-
})
248+
}()
249249
_, err = conn.Ping()
250250
require.NoError(t, err)
251251
}
@@ -293,9 +293,9 @@ func TestWorkspaceAgentPTY(t *testing.T) {
293293
agentCloser := agent.New(agentClient.ListenWorkspaceAgent, &agent.Options{
294294
Logger: slogtest.Make(t, nil),
295295
})
296-
t.Cleanup(func() {
296+
defer func() {
297297
_ = agentCloser.Close()
298-
})
298+
}()
299299
resources := coderdtest.AwaitWorkspaceAgents(t, client, workspace.LatestBuild.ID)
300300

301301
conn, err := client.WorkspaceAgentReconnectingPTY(context.Background(), resources[0].Agents[0].ID, uuid.New(), 80, 80, "/bin/bash")

‎coderd/workspaceapps_test.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -24,16 +24,16 @@ func TestWorkspaceAppsProxyPath(t *testing.T) {
2424
// #nosec
2525
ln, err := net.Listen("tcp", ":0")
2626
require.NoError(t, err)
27+
defer func() {
28+
_ = ln.Close()
29+
}()
2730
server := http.Server{
2831
Handler: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
2932
w.WriteHeader(http.StatusOK)
3033
}),
3134
}
32-
t.Cleanup(func() {
33-
_ = server.Close()
34-
_ = ln.Close()
35-
})
3635
go server.Serve(ln)
36+
defer server.Close()
3737
tcpAddr, _ := ln.Addr().(*net.TCPAddr)
3838

3939
client := coderdtest.New(t, &coderdtest.Options{
@@ -78,9 +78,9 @@ func TestWorkspaceAppsProxyPath(t *testing.T) {
7878
agentCloser := agent.New(agentClient.ListenWorkspaceAgent, &agent.Options{
7979
Logger: slogtest.Make(t, nil),
8080
})
81-
t.Cleanup(func() {
81+
defer func() {
8282
_ = agentCloser.Close()
83-
})
83+
}()
8484
coderdtest.AwaitWorkspaceAgents(t, client, workspace.LatestBuild.ID)
8585
client.HTTPClient.CheckRedirect = func(req *http.Request, via []*http.Request) error {
8686
return http.ErrUseLastResponse

‎coderd/workspacebuilds_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -328,7 +328,7 @@ func TestWorkspaceBuildLogs(t *testing.T) {
328328
template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID)
329329
workspace := coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID)
330330
ctx, cancelFunc := context.WithCancel(context.Background())
331-
t.Cleanup(cancelFunc)
331+
defer cancelFunc()
332332
logs, err := client.WorkspaceBuildLogsAfter(ctx, workspace.LatestBuild.ID, before.Add(-time.Hour))
333333
require.NoError(t, err)
334334
for {

‎coderd/wsconncache/wsconncache_test.go

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -40,9 +40,9 @@ func TestCache(t *testing.T) {
4040
cache := wsconncache.New(func(r *http.Request, id uuid.UUID) (*agent.Conn, error) {
4141
return setupAgent(t, agent.Metadata{}, 0), nil
4242
}, 0)
43-
t.Cleanup(func() {
43+
defer func() {
4444
_ = cache.Close()
45-
})
45+
}()
4646
conn1, _, err := cache.Acquire(httptest.NewRequest(http.MethodGet, "/", nil), uuid.Nil)
4747
require.NoError(t, err)
4848
conn2, _, err := cache.Acquire(httptest.NewRequest(http.MethodGet, "/", nil), uuid.Nil)
@@ -56,9 +56,9 @@ func TestCache(t *testing.T) {
5656
called.Add(1)
5757
return setupAgent(t, agent.Metadata{}, 0), nil
5858
}, time.Microsecond)
59-
t.Cleanup(func() {
59+
defer func() {
6060
_ = cache.Close()
61-
})
61+
}()
6262
conn, release, err := cache.Acquire(httptest.NewRequest(http.MethodGet, "/", nil), uuid.Nil)
6363
require.NoError(t, err)
6464
release()
@@ -74,9 +74,9 @@ func TestCache(t *testing.T) {
7474
cache := wsconncache.New(func(r *http.Request, id uuid.UUID) (*agent.Conn, error) {
7575
return setupAgent(t, agent.Metadata{}, 0), nil
7676
}, time.Microsecond)
77-
t.Cleanup(func() {
77+
defer func() {
7878
_ = cache.Close()
79-
})
79+
}()
8080
conn, release, err := cache.Acquire(httptest.NewRequest(http.MethodGet, "/", nil), uuid.Nil)
8181
require.NoError(t, err)
8282
time.Sleep(time.Millisecond)
@@ -87,9 +87,9 @@ func TestCache(t *testing.T) {
8787
t.Parallel()
8888
random, err := net.Listen("tcp", "127.0.0.1:0")
8989
require.NoError(t, err)
90-
t.Cleanup(func() {
90+
defer func() {
9191
_ = random.Close()
92-
})
92+
}()
9393
tcpAddr, valid := random.Addr().(*net.TCPAddr)
9494
require.True(t, valid)
9595

@@ -98,17 +98,17 @@ func TestCache(t *testing.T) {
9898
w.WriteHeader(http.StatusOK)
9999
}),
100100
}
101-
t.Cleanup(func() {
101+
defer func() {
102102
_ = server.Close()
103-
})
103+
}()
104104
go server.Serve(random)
105105

106106
cache := wsconncache.New(func(r *http.Request, id uuid.UUID) (*agent.Conn, error) {
107107
return setupAgent(t, agent.Metadata{}, 0), nil
108108
}, time.Microsecond)
109-
t.Cleanup(func() {
109+
defer func() {
110110
_ = cache.Close()
111-
})
111+
}()
112112

113113
var wg sync.WaitGroup
114114
// Perform many requests in parallel to simulate
@@ -132,7 +132,7 @@ func TestCache(t *testing.T) {
132132
res := httptest.NewRecorder()
133133
proxy.ServeHTTP(res, req)
134134
res.Result().Body.Close()
135-
require.Equal(t, http.StatusOK, res.Result().StatusCode)
135+
assert.Equal(t, http.StatusOK, res.Result().StatusCode)
136136
}()
137137
}
138138
wg.Wait()

‎peerbroker/proxy_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,9 +43,9 @@ func TestProxy(t *testing.T) {
4343
Pubsub: pubsub,
4444
})
4545
require.NoError(t, err)
46-
t.Cleanup(func() {
46+
defer func() {
4747
_ = proxyCloser.Close()
48-
})
48+
}()
4949

5050
var wg sync.WaitGroup
5151
wg.Add(1)

‎provisioner/echo/serve_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,11 +25,11 @@ func TestEcho(t *testing.T) {
2525
// Create an in-memory provisioner to communicate with.
2626
client, server := provisionersdk.TransportPipe()
2727
ctx, cancelFunc := context.WithCancel(context.Background())
28-
t.Cleanup(func() {
28+
defer func() {
2929
_ = client.Close()
3030
_ = server.Close()
3131
cancelFunc()
32-
})
32+
}()
3333
go func() {
3434
err := echo.Serve(ctx, fs, &provisionersdk.ServeOptions{
3535
Listener: server,

‎provisioner/terraform/parse_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,11 +23,11 @@ func TestParse(t *testing.T) {
2323
// Create an in-memory provisioner to communicate with.
2424
client, server := provisionersdk.TransportPipe()
2525
ctx, cancelFunc := context.WithCancel(context.Background())
26-
t.Cleanup(func() {
26+
defer func() {
2727
_ = client.Close()
2828
_ = server.Close()
2929
cancelFunc()
30-
})
30+
}()
3131
go func() {
3232
err := terraform.Serve(ctx, &terraform.ServeOptions{
3333
ServeOptions: &provisionersdk.ServeOptions{

‎provisionersdk/agent_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ func TestAgentScript(t *testing.T) {
3535
render.Status(r, http.StatusOK)
3636
render.Data(rw, r, content)
3737
}))
38-
t.Cleanup(srv.Close)
38+
defer srv.Close()
3939
srvURL, err := url.Parse(srv.URL)
4040
require.NoError(t, err)
4141

0 commit comments

Comments
 (0)