Skip to content

Commit c11caf3

Browse files
johnstcnkylecarbs
johnstcn
authored and
kylecarbs
committed
chore: avoid concurrent usage of t.FailNow (#1683)
* chore: golangci: add linter rule to report usage of t.FailNow inside goroutines * chore: avoid t.FailNow in goroutines to appease the race detector
1 parent 458c755 commit c11caf3

27 files changed

+120
-74
lines changed

‎.golangci.yaml

+2
Original file line numberDiff line numberDiff line change
@@ -201,6 +201,8 @@ run:
201201
concurrency: 4
202202
skip-dirs:
203203
- node_modules
204+
skip-files:
205+
- scripts/rules.go
204206
timeout: 5m
205207

206208
# Over time, add more and more linters from

‎agent/agent_test.go

+9-8
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020
"github.com/pion/udp"
2121
"github.com/pion/webrtc/v3"
2222
"github.com/pkg/sftp"
23+
"github.com/stretchr/testify/assert"
2324
"github.com/stretchr/testify/require"
2425
"go.uber.org/goleak"
2526
"golang.org/x/crypto/ssh"
@@ -119,7 +120,7 @@ func TestAgent(t *testing.T) {
119120
done := make(chan struct{})
120121
go func() {
121122
conn, err := local.Accept()
122-
require.NoError(t, err)
123+
assert.NoError(t, err)
123124
_ = conn.Close()
124125
close(done)
125126
}()
@@ -367,7 +368,7 @@ func setupSSHCommand(t *testing.T, beforeArgs []string, afterArgs []string) *exe
367368
return
368369
}
369370
ssh, err := agentConn.SSH()
370-
require.NoError(t, err)
371+
assert.NoError(t, err)
371372
go io.Copy(conn, ssh)
372373
go io.Copy(ssh, conn)
373374
}
@@ -409,7 +410,7 @@ func setupAgent(t *testing.T, metadata agent.Metadata, ptyTimeout time.Duration)
409410
})
410411
api := proto.NewDRPCPeerBrokerClient(provisionersdk.Conn(client))
411412
stream, err := api.NegotiateConnection(context.Background())
412-
require.NoError(t, err)
413+
assert.NoError(t, err)
413414
conn, err := peerbroker.Dial(stream, []webrtc.ICEServer{}, &peer.ConnOptions{
414415
Logger: slogtest.Make(t, nil),
415416
})
@@ -444,13 +445,13 @@ func testAccept(t *testing.T, c net.Conn) {
444445
func assertReadPayload(t *testing.T, r io.Reader, payload []byte) {
445446
b := make([]byte, len(payload)+16)
446447
n, err := r.Read(b)
447-
require.NoError(t, err, "read payload")
448-
require.Equal(t, len(payload), n, "read payload length does not match")
449-
require.Equal(t, payload, b[:n])
448+
assert.NoError(t, err, "read payload")
449+
assert.Equal(t, len(payload), n, "read payload length does not match")
450+
assert.Equal(t, payload, b[:n])
450451
}
451452

452453
func assertWritePayload(t *testing.T, w io.Writer, payload []byte) {
453454
n, err := w.Write(payload)
454-
require.NoError(t, err, "write payload")
455-
require.Equal(t, len(payload), n, "payload length does not match")
455+
assert.NoError(t, err, "write payload")
456+
assert.Equal(t, len(payload), n, "payload length does not match")
456457
}

‎cli/cliui/agent_test.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import (
66
"time"
77

88
"github.com/spf13/cobra"
9-
"github.com/stretchr/testify/require"
9+
"github.com/stretchr/testify/assert"
1010
"go.uber.org/atomic"
1111

1212
"github.com/coder/coder/cli/cliui"
@@ -43,7 +43,7 @@ func TestAgent(t *testing.T) {
4343
go func() {
4444
defer close(done)
4545
err := cmd.Execute()
46-
require.NoError(t, err)
46+
assert.NoError(t, err)
4747
}()
4848
ptty.ExpectMatch("lost connection")
4949
disconnected.Store(true)

‎cli/cliui/prompt_test.go

+7-6
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"time"
1111

1212
"github.com/spf13/cobra"
13+
"github.com/stretchr/testify/assert"
1314
"github.com/stretchr/testify/require"
1415

1516
"github.com/coder/coder/cli/cliui"
@@ -27,7 +28,7 @@ func TestPrompt(t *testing.T) {
2728
resp, err := newPrompt(ptty, cliui.PromptOptions{
2829
Text: "Example",
2930
}, nil)
30-
require.NoError(t, err)
31+
assert.NoError(t, err)
3132
msgChan <- resp
3233
}()
3334
ptty.ExpectMatch("Example")
@@ -44,7 +45,7 @@ func TestPrompt(t *testing.T) {
4445
Text: "Example",
4546
IsConfirm: true,
4647
}, nil)
47-
require.NoError(t, err)
48+
assert.NoError(t, err)
4849
doneChan <- resp
4950
}()
5051
ptty.ExpectMatch("Example")
@@ -80,7 +81,7 @@ func TestPrompt(t *testing.T) {
8081
cliui.AllowSkipPrompt(cmd)
8182
cmd.SetArgs([]string{"-y"})
8283
})
83-
require.NoError(t, err)
84+
assert.NoError(t, err)
8485
doneChan <- resp
8586
}()
8687

@@ -101,7 +102,7 @@ func TestPrompt(t *testing.T) {
101102
resp, err := newPrompt(ptty, cliui.PromptOptions{
102103
Text: "Example",
103104
}, nil)
104-
require.NoError(t, err)
105+
assert.NoError(t, err)
105106
doneChan <- resp
106107
}()
107108
ptty.ExpectMatch("Example")
@@ -117,7 +118,7 @@ func TestPrompt(t *testing.T) {
117118
resp, err := newPrompt(ptty, cliui.PromptOptions{
118119
Text: "Example",
119120
}, nil)
120-
require.NoError(t, err)
121+
assert.NoError(t, err)
121122
doneChan <- resp
122123
}()
123124
ptty.ExpectMatch("Example")
@@ -133,7 +134,7 @@ func TestPrompt(t *testing.T) {
133134
resp, err := newPrompt(ptty, cliui.PromptOptions{
134135
Text: "Example",
135136
}, nil)
136-
require.NoError(t, err)
137+
assert.NoError(t, err)
137138
doneChan <- resp
138139
}()
139140
ptty.ExpectMatch("Example")

‎cli/cliui/provisionerjob_test.go

+4-4
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import (
99
"time"
1010

1111
"github.com/spf13/cobra"
12-
"github.com/stretchr/testify/require"
12+
"github.com/stretchr/testify/assert"
1313

1414
"github.com/coder/coder/cli/cliui"
1515
"github.com/coder/coder/coderd/database"
@@ -90,9 +90,9 @@ func TestProvisionerJob(t *testing.T) {
9090
go func() {
9191
<-test.Next
9292
currentProcess, err := os.FindProcess(os.Getpid())
93-
require.NoError(t, err)
93+
assert.NoError(t, err)
9494
err = currentProcess.Signal(os.Interrupt)
95-
require.NoError(t, err)
95+
assert.NoError(t, err)
9696
<-test.Next
9797
test.JobMutex.Lock()
9898
test.Job.Status = codersdk.ProvisionerJobCanceled
@@ -150,7 +150,7 @@ func newProvisionerJob(t *testing.T) provisionerJobTest {
150150
defer close(done)
151151
err := cmd.ExecuteContext(context.Background())
152152
if err != nil {
153-
require.ErrorIs(t, err, cliui.Canceled)
153+
assert.ErrorIs(t, err, cliui.Canceled)
154154
}
155155
}()
156156
t.Cleanup(func() {

‎cli/cliui/resources_test.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import (
44
"testing"
55
"time"
66

7-
"github.com/stretchr/testify/require"
7+
"github.com/stretchr/testify/assert"
88

99
"github.com/coder/coder/cli/cliui"
1010
"github.com/coder/coder/coderd/database"
@@ -32,7 +32,7 @@ func TestWorkspaceResources(t *testing.T) {
3232
}}, cliui.WorkspaceResourcesOptions{
3333
WorkspaceName: "example",
3434
})
35-
require.NoError(t, err)
35+
assert.NoError(t, err)
3636
close(done)
3737
}()
3838
ptty.ExpectMatch("coder ssh example")
@@ -85,7 +85,7 @@ func TestWorkspaceResources(t *testing.T) {
8585
HideAgentState: false,
8686
HideAccess: false,
8787
})
88-
require.NoError(t, err)
88+
assert.NoError(t, err)
8989
close(done)
9090
}()
9191
ptty.ExpectMatch("google_compute_disk.root")

‎cli/cliui/select_test.go

+2-1
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"testing"
66

77
"github.com/spf13/cobra"
8+
"github.com/stretchr/testify/assert"
89
"github.com/stretchr/testify/require"
910

1011
"github.com/coder/coder/cli/cliui"
@@ -21,7 +22,7 @@ func TestSelect(t *testing.T) {
2122
resp, err := newSelect(ptty, cliui.SelectOptions{
2223
Options: []string{"First", "Second"},
2324
})
24-
require.NoError(t, err)
25+
assert.NoError(t, err)
2526
msgChan <- resp
2627
}()
2728
require.Equal(t, "First", <-msgChan)

‎cli/configssh_test.go

+3-2
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"testing"
1212

1313
"github.com/google/uuid"
14+
"github.com/stretchr/testify/assert"
1415
"github.com/stretchr/testify/require"
1516

1617
"cdr.dev/slog/sloggers/slogtest"
@@ -96,7 +97,7 @@ func TestConfigSSH(t *testing.T) {
9697
return
9798
}
9899
ssh, err := agentConn.SSH()
99-
require.NoError(t, err)
100+
assert.NoError(t, err)
100101
go io.Copy(conn, ssh)
101102
go io.Copy(ssh, conn)
102103
}
@@ -120,7 +121,7 @@ func TestConfigSSH(t *testing.T) {
120121
go func() {
121122
defer close(doneChan)
122123
err := cmd.Execute()
123-
require.NoError(t, err)
124+
assert.NoError(t, err)
124125
}()
125126
<-doneChan
126127

‎cli/create_test.go

+6-6
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ func TestCreate(t *testing.T) {
4545
go func() {
4646
defer close(doneChan)
4747
err := cmd.Execute()
48-
require.NoError(t, err)
48+
assert.NoError(t, err)
4949
}()
5050
matches := []string{
5151
"Confirm create", "yes",
@@ -126,7 +126,7 @@ func TestCreate(t *testing.T) {
126126
go func() {
127127
defer done()
128128
err := cmd.ExecuteContext(cmdCtx)
129-
require.NoError(t, err)
129+
assert.NoError(t, err)
130130
}()
131131
// No pty interaction needed since we use the -y skip prompt flag
132132
<-cmdCtx.Done()
@@ -149,7 +149,7 @@ func TestCreate(t *testing.T) {
149149
go func() {
150150
defer close(doneChan)
151151
err := cmd.Execute()
152-
require.NoError(t, err)
152+
assert.NoError(t, err)
153153
}()
154154
matches := []string{
155155
"Specify a name", "my-workspace",
@@ -187,7 +187,7 @@ func TestCreate(t *testing.T) {
187187
go func() {
188188
defer close(doneChan)
189189
err := cmd.Execute()
190-
require.NoError(t, err)
190+
assert.NoError(t, err)
191191
}()
192192

193193
matches := []string{
@@ -231,7 +231,7 @@ func TestCreate(t *testing.T) {
231231
go func() {
232232
defer close(doneChan)
233233
err := cmd.Execute()
234-
require.NoError(t, err)
234+
assert.NoError(t, err)
235235
}()
236236

237237
matches := []string{
@@ -272,7 +272,7 @@ func TestCreate(t *testing.T) {
272272
go func() {
273273
defer close(doneChan)
274274
err := cmd.Execute()
275-
require.EqualError(t, err, "Parameter value absent in parameter file for \"region\"!")
275+
assert.EqualError(t, err, "Parameter value absent in parameter file for \"region\"!")
276276
}()
277277
<-doneChan
278278
removeTmpDirUntilSuccess(t, tempDir)

‎cli/delete_test.go

+6-2
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
11
package cli_test
22

33
import (
4+
"io"
45
"testing"
56

6-
"github.com/stretchr/testify/require"
7+
"github.com/stretchr/testify/assert"
78

89
"github.com/coder/coder/cli/clitest"
910
"github.com/coder/coder/coderd/coderdtest"
@@ -29,7 +30,10 @@ func TestDelete(t *testing.T) {
2930
go func() {
3031
defer close(doneChan)
3132
err := cmd.Execute()
32-
require.NoError(t, err)
33+
// When running with the race detector on, we sometimes get an EOF.
34+
if err != nil {
35+
assert.ErrorIs(t, err, io.EOF)
36+
}
3337
}()
3438
pty.ExpectMatch("Cleaning Up")
3539
<-doneChan

‎cli/login_test.go

+5-4
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"context"
55
"testing"
66

7+
"github.com/stretchr/testify/assert"
78
"github.com/stretchr/testify/require"
89

910
"github.com/coder/coder/cli/clitest"
@@ -35,7 +36,7 @@ func TestLogin(t *testing.T) {
3536
go func() {
3637
defer close(doneChan)
3738
err := root.Execute()
38-
require.NoError(t, err)
39+
assert.NoError(t, err)
3940
}()
4041

4142
matches := []string{
@@ -71,7 +72,7 @@ func TestLogin(t *testing.T) {
7172
go func() {
7273
defer close(doneChan)
7374
err := root.ExecuteContext(ctx)
74-
require.ErrorIs(t, err, context.Canceled)
75+
assert.ErrorIs(t, err, context.Canceled)
7576
}()
7677

7778
matches := []string{
@@ -106,7 +107,7 @@ func TestLogin(t *testing.T) {
106107
go func() {
107108
defer close(doneChan)
108109
err := root.Execute()
109-
require.NoError(t, err)
110+
assert.NoError(t, err)
110111
}()
111112

112113
pty.ExpectMatch("Paste your token here:")
@@ -131,7 +132,7 @@ func TestLogin(t *testing.T) {
131132
defer close(doneChan)
132133
err := root.ExecuteContext(ctx)
133134
// An error is expected in this case, since the login wasn't successful:
134-
require.Error(t, err)
135+
assert.Error(t, err)
135136
}()
136137

137138
pty.ExpectMatch("Paste your token here:")

‎cli/logout_test.go

+2-1
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package cli_test
33
import (
44
"testing"
55

6+
"github.com/stretchr/testify/assert"
67
"github.com/stretchr/testify/require"
78

89
"github.com/coder/coder/cli/clitest"
@@ -25,7 +26,7 @@ func TestLogout(t *testing.T) {
2526
go func() {
2627
defer close(doneChan)
2728
err := root.Execute()
28-
require.NoError(t, err)
29+
assert.NoError(t, err)
2930
}()
3031

3132
pty.ExpectMatch("Paste your token here:")

0 commit comments

Comments
 (0)