Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commitc8ed213

Browse files
mafredrijohnstcn
andauthored
fix: Guard against CLI cmd running after test exit (#1658)
* fix: Guard against CLI cmd running after test exit* fix: cli: avoid calling t.FailNow in non-test-main goroutine* fix: cli: server_test: avoid calling t.FailNow outside main goroutine* fix: cli: clitest_test: avoid calling t.FailNow outside main goroutine* fix: cli: list_test: avoid calling t.FailNow outside main goroutine* fix: TestGitSSH use-of-t-after-exit* fix: TestGitSSH "too many authentication failures"Due to local SSH keys being given* chore: clitest: fix TestCli* chore: Simplify TestTemplateInitCo-authored-by: Cian Johnston <cian@coder.com>
1 parentfa957d6 commitc8ed213

File tree

7 files changed

+87
-86
lines changed

7 files changed

+87
-86
lines changed

‎cli/agent_test.go

Lines changed: 21 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -49,13 +49,12 @@ func TestWorkspaceAgent(t *testing.T) {
4949
cmd,_:=clitest.New(t,"agent","--auth","azure-instance-identity","--agent-url",client.URL.String())
5050
ctx,cancelFunc:=context.WithCancel(context.Background())
5151
defercancelFunc()
52+
errC:=make(chanerror)
5253
gofunc() {
53-
// A linting error occurs for weakly typing the context value here,
54-
// but it seems reasonable for a one-off test.
55-
// nolint
56-
ctx=context.WithValue(ctx,"azure-client",metadataClient)
57-
err:=cmd.ExecuteContext(ctx)
58-
require.NoError(t,err)
54+
// A linting error occurs for weakly typing the context value here.
55+
//nolint // The above seems reasonable for a one-off test.
56+
ctx:=context.WithValue(ctx,"azure-client",metadataClient)
57+
errC<-cmd.ExecuteContext(ctx)
5958
}()
6059
coderdtest.AwaitWorkspaceAgents(t,client,workspace.LatestBuild.ID)
6160
resources,err:=client.WorkspaceResourcesByBuild(ctx,workspace.LatestBuild.ID)
@@ -66,6 +65,8 @@ func TestWorkspaceAgent(t *testing.T) {
6665
_,err=dialer.Ping()
6766
require.NoError(t,err)
6867
cancelFunc()
68+
err=<-errC
69+
require.NoError(t,err)
6970
})
7071

7172
t.Run("AWS",func(t*testing.T) {
@@ -103,13 +104,12 @@ func TestWorkspaceAgent(t *testing.T) {
103104
cmd,_:=clitest.New(t,"agent","--auth","aws-instance-identity","--agent-url",client.URL.String())
104105
ctx,cancelFunc:=context.WithCancel(context.Background())
105106
defercancelFunc()
107+
errC:=make(chanerror)
106108
gofunc() {
107-
// A linting error occurs for weakly typing the context value here,
108-
// but it seems reasonable for a one-off test.
109-
// nolint
110-
ctx=context.WithValue(ctx,"aws-client",metadataClient)
111-
err:=cmd.ExecuteContext(ctx)
112-
require.NoError(t,err)
109+
// A linting error occurs for weakly typing the context value here.
110+
//nolint // The above seems reasonable for a one-off test.
111+
ctx:=context.WithValue(ctx,"aws-client",metadataClient)
112+
errC<-cmd.ExecuteContext(ctx)
113113
}()
114114
coderdtest.AwaitWorkspaceAgents(t,client,workspace.LatestBuild.ID)
115115
resources,err:=client.WorkspaceResourcesByBuild(ctx,workspace.LatestBuild.ID)
@@ -120,6 +120,8 @@ func TestWorkspaceAgent(t *testing.T) {
120120
_,err=dialer.Ping()
121121
require.NoError(t,err)
122122
cancelFunc()
123+
err=<-errC
124+
require.NoError(t,err)
123125
})
124126

125127
t.Run("GoogleCloud",func(t*testing.T) {
@@ -157,13 +159,12 @@ func TestWorkspaceAgent(t *testing.T) {
157159
cmd,_:=clitest.New(t,"agent","--auth","google-instance-identity","--agent-url",client.URL.String())
158160
ctx,cancelFunc:=context.WithCancel(context.Background())
159161
defercancelFunc()
162+
errC:=make(chanerror)
160163
gofunc() {
161-
// A linting error occurs for weakly typing the context value here,
162-
// but it seems reasonable for a one-off test.
163-
// nolint
164-
ctx=context.WithValue(ctx,"gcp-client",metadata)
165-
err:=cmd.ExecuteContext(ctx)
166-
require.NoError(t,err)
164+
// A linting error occurs for weakly typing the context value here.
165+
//nolint // The above seems reasonable for a one-off test.
166+
ctx:=context.WithValue(ctx,"gcp-client",metadata)
167+
errC<-cmd.ExecuteContext(ctx)
167168
}()
168169
coderdtest.AwaitWorkspaceAgents(t,client,workspace.LatestBuild.ID)
169170
resources,err:=client.WorkspaceResourcesByBuild(ctx,workspace.LatestBuild.ID)
@@ -174,5 +175,7 @@ func TestWorkspaceAgent(t *testing.T) {
174175
_,err=dialer.Ping()
175176
require.NoError(t,err)
176177
cancelFunc()
178+
err=<-errC
179+
require.NoError(t,err)
177180
})
178181
}

‎cli/clitest/clitest_test.go

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ package clitest_test
33
import (
44
"testing"
55

6-
"github.com/stretchr/testify/require"
76
"go.uber.org/goleak"
87

98
"github.com/coder/coder/cli/clitest"
@@ -25,8 +24,7 @@ func TestCli(t *testing.T) {
2524
cmd.SetIn(pty.Input())
2625
cmd.SetOut(pty.Output())
2726
gofunc() {
28-
err:=cmd.Execute()
29-
require.NoError(t,err)
27+
_=cmd.Execute()
3028
}()
3129
pty.ExpectMatch("coder")
3230
}

‎cli/gitssh_test.go

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -63,9 +63,9 @@ func TestGitSSH(t *testing.T) {
6363
clitest.SetupConfig(t,agentClient,root)
6464
ctx,cancelFunc:=context.WithCancel(context.Background())
6565
defercancelFunc()
66+
agentErrC:=make(chanerror)
6667
gofunc() {
67-
err:=cmd.ExecuteContext(ctx)
68-
require.NoError(t,err)
68+
agentErrC<-cmd.ExecuteContext(ctx)
6969
}()
7070

7171
coderdtest.AwaitWorkspaceAgents(t,client,workspace.LatestBuild.ID)
@@ -85,23 +85,30 @@ func TestGitSSH(t *testing.T) {
8585
returnssh.KeysEqual(publicKey,key)
8686
})
8787
varincint64
88+
sshErrC:=make(chanerror)
8889
gofunc() {
8990
// as long as we get a successful session we don't care if the server errors
9091
_=ssh.Serve(l,func(s ssh.Session) {
9192
atomic.AddInt64(&inc,1)
9293
t.Log("got authenticated session")
93-
err:=s.Exit(0)
94-
require.NoError(t,err)
94+
sshErrC<-s.Exit(0)
9595
},publicKeyOption)
9696
}()
9797

9898
// start ssh session
9999
addr,ok:=l.Addr().(*net.TCPAddr)
100100
require.True(t,ok)
101101
// set to agent config dir
102-
cmd,_=clitest.New(t,"gitssh","--agent-url",agentClient.URL.String(),"--agent-token",agentToken,"--",fmt.Sprintf("-p%d",addr.Port),"-o","StrictHostKeyChecking=no","127.0.0.1")
103-
err=cmd.ExecuteContext(context.Background())
102+
gitsshCmd,_:=clitest.New(t,"gitssh","--agent-url",agentClient.URL.String(),"--agent-token",agentToken,"--",fmt.Sprintf("-p%d",addr.Port),"-o","StrictHostKeyChecking=no","-o","IdentitiesOnly=yes","127.0.0.1")
103+
err=gitsshCmd.ExecuteContext(context.Background())
104104
require.NoError(t,err)
105105
require.EqualValues(t,1,inc)
106+
107+
err=<-sshErrC
108+
require.NoError(t,err,"error in ssh session exit")
109+
110+
cancelFunc()
111+
err=<-agentErrC
112+
require.NoError(t,err,"error in agent execute")
106113
})
107114
}

‎cli/list_test.go

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
package cli_test
22

33
import (
4+
"context"
45
"testing"
6+
"time"
57

68
"github.com/stretchr/testify/require"
79

@@ -14,6 +16,8 @@ func TestList(t *testing.T) {
1416
t.Parallel()
1517
t.Run("Single",func(t*testing.T) {
1618
t.Parallel()
19+
ctx,cancelFunc:=context.WithTimeout(context.Background(),5*time.Second)
20+
defercancelFunc()
1721
client:=coderdtest.New(t,&coderdtest.Options{IncludeProvisionerD:true})
1822
user:=coderdtest.CreateFirstUser(t,client)
1923
version:=coderdtest.CreateTemplateVersion(t,client,user.OrganizationID,nil)
@@ -23,17 +27,16 @@ func TestList(t *testing.T) {
2327
coderdtest.AwaitWorkspaceBuildJob(t,client,workspace.LatestBuild.ID)
2428
cmd,root:=clitest.New(t,"ls")
2529
clitest.SetupConfig(t,client,root)
26-
doneChan:=make(chanstruct{})
2730
pty:=ptytest.New(t)
2831
cmd.SetIn(pty.Input())
2932
cmd.SetOut(pty.Output())
33+
errC:=make(chanerror)
3034
gofunc() {
31-
deferclose(doneChan)
32-
err:=cmd.Execute()
33-
require.NoError(t,err)
35+
errC<-cmd.ExecuteContext(ctx)
3436
}()
3537
pty.ExpectMatch(workspace.Name)
3638
pty.ExpectMatch("Running")
37-
<-doneChan
39+
cancelFunc()
40+
require.NoError(t,<-errC)
3841
})
3942
}

‎cli/server_test.go

Lines changed: 39 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ import (
1717
"os"
1818
"runtime"
1919
"strings"
20-
"sync"
2120
"testing"
2221
"time"
2322

@@ -45,12 +44,11 @@ func TestServer(t *testing.T) {
4544
require.NoError(t,err)
4645
defercloseFunc()
4746
ctx,cancelFunc:=context.WithCancel(context.Background())
48-
done:=make(chanstruct{})
47+
defercancelFunc()
4948
root,cfg:=clitest.New(t,"server","--address",":0","--postgres-url",connectionURL)
49+
errC:=make(chanerror)
5050
gofunc() {
51-
deferclose(done)
52-
err=root.ExecuteContext(ctx)
53-
require.ErrorIs(t,err,context.Canceled)
51+
errC<-root.ExecuteContext(ctx)
5452
}()
5553
varclient*codersdk.Client
5654
require.Eventually(t,func()bool {
@@ -71,8 +69,9 @@ func TestServer(t *testing.T) {
7169
})
7270
require.NoError(t,err)
7371
cancelFunc()
74-
<-done
72+
require.ErrorIs(t,<-errC,context.Canceled)
7573
})
74+
7675
t.Run("Development",func(t*testing.T) {
7776
t.Parallel()
7877
ctx,cancelFunc:=context.WithCancel(context.Background())
@@ -82,26 +81,12 @@ func TestServer(t *testing.T) {
8281

8382
root,cfg:=clitest.New(t,"server","--dev","--tunnel=false","--address",":0")
8483
varbuf strings.Builder
84+
errC:=make(chanerror)
8585
root.SetOutput(&buf)
86-
varwg sync.WaitGroup
87-
wg.Add(1)
8886
gofunc() {
89-
deferwg.Done()
90-
91-
err:=root.ExecuteContext(ctx)
92-
require.ErrorIs(t,err,context.Canceled)
93-
94-
// Verify that credentials were output to the terminal.
95-
assert.Contains(t,buf.String(),fmt.Sprintf("email: %s",wantEmail),"expected output %q; got no match",wantEmail)
96-
// Check that the password line is output and that it's non-empty.
97-
if_,after,found:=strings.Cut(buf.String(),"password: ");found {
98-
before,_,_:=strings.Cut(after,"\n")
99-
before=strings.Trim(before,"\r")// Ensure no control character is left.
100-
assert.NotEmpty(t,before,"expected non-empty password; got empty")
101-
}else {
102-
t.Error("expected password line output; got no match")
103-
}
87+
errC<-root.ExecuteContext(ctx)
10488
}()
89+
10590
vartokenstring
10691
require.Eventually(t,func()bool {
10792
varerrerror
@@ -119,8 +104,20 @@ func TestServer(t *testing.T) {
119104
require.NoError(t,err)
120105

121106
cancelFunc()
122-
wg.Wait()
107+
require.ErrorIs(t,<-errC,context.Canceled)
108+
109+
// Verify that credentials were output to the terminal.
110+
assert.Contains(t,buf.String(),fmt.Sprintf("email: %s",wantEmail),"expected output %q; got no match",wantEmail)
111+
// Check that the password line is output and that it's non-empty.
112+
if_,after,found:=strings.Cut(buf.String(),"password: ");found {
113+
before,_,_:=strings.Cut(after,"\n")
114+
before=strings.Trim(before,"\r")// Ensure no control character is left.
115+
assert.NotEmpty(t,before,"expected non-empty password; got empty")
116+
}else {
117+
t.Error("expected password line output; got no match")
118+
}
123119
})
120+
124121
// Duplicated test from "Development" above to test setting email/password via env.
125122
// Cannot run parallel due to os.Setenv.
126123
//nolint:paralleltest
@@ -136,18 +133,11 @@ func TestServer(t *testing.T) {
136133
root,cfg:=clitest.New(t,"server","--dev","--tunnel=false","--address",":0")
137134
varbuf strings.Builder
138135
root.SetOutput(&buf)
139-
varwg sync.WaitGroup
140-
wg.Add(1)
136+
errC:=make(chanerror)
141137
gofunc() {
142-
deferwg.Done()
143-
144-
err:=root.ExecuteContext(ctx)
145-
require.ErrorIs(t,err,context.Canceled)
146-
147-
// Verify that credentials were output to the terminal.
148-
assert.Contains(t,buf.String(),fmt.Sprintf("email: %s",wantEmail),"expected output %q; got no match",wantEmail)
149-
assert.Contains(t,buf.String(),fmt.Sprintf("password: %s",wantPassword),"expected output %q; got no match",wantPassword)
138+
errC<-root.ExecuteContext(ctx)
150139
}()
140+
151141
vartokenstring
152142
require.Eventually(t,func()bool {
153143
varerrerror
@@ -165,8 +155,12 @@ func TestServer(t *testing.T) {
165155
require.NoError(t,err)
166156

167157
cancelFunc()
168-
wg.Wait()
158+
require.ErrorIs(t,<-errC,context.Canceled)
159+
// Verify that credentials were output to the terminal.
160+
assert.Contains(t,buf.String(),fmt.Sprintf("email: %s",wantEmail),"expected output %q; got no match",wantEmail)
161+
assert.Contains(t,buf.String(),fmt.Sprintf("password: %s",wantPassword),"expected output %q; got no match",wantPassword)
169162
})
163+
170164
t.Run("TLSBadVersion",func(t*testing.T) {
171165
t.Parallel()
172166
ctx,cancelFunc:=context.WithCancel(context.Background())
@@ -202,10 +196,12 @@ func TestServer(t *testing.T) {
202196
certPath,keyPath:=generateTLSCertificate(t)
203197
root,cfg:=clitest.New(t,"server","--dev","--tunnel=false","--address",":0",
204198
"--tls-enable","--tls-cert-file",certPath,"--tls-key-file",keyPath)
199+
errC:=make(chanerror)
205200
gofunc() {
206-
err:=root.ExecuteContext(ctx)
207-
require.ErrorIs(t,err,context.Canceled)
201+
errC<-root.ExecuteContext(ctx)
208202
}()
203+
204+
// Verify HTTPS
209205
varaccessURLRawstring
210206
require.Eventually(t,func()bool {
211207
varerrerror
@@ -226,6 +222,9 @@ func TestServer(t *testing.T) {
226222
}
227223
_,err=client.HasFirstUser(ctx)
228224
require.NoError(t,err)
225+
226+
cancelFunc()
227+
require.ErrorIs(t,<-errC,context.Canceled)
229228
})
230229
// This cannot be ran in parallel because it uses a signal.
231230
//nolint:paralleltest
@@ -284,14 +283,12 @@ func TestServer(t *testing.T) {
284283
ctx,cancelFunc:=context.WithCancel(context.Background())
285284
defercancelFunc()
286285
root,_:=clitest.New(t,"server","--dev","--tunnel=false","--address",":0","--trace=true")
287-
done:=make(chanstruct{})
286+
errC:=make(chanerror)
288287
gofunc() {
289-
deferclose(done)
290-
err:=root.ExecuteContext(ctx)
291-
require.ErrorIs(t,err,context.Canceled)
288+
errC<-root.ExecuteContext(ctx)
292289
}()
293290
cancelFunc()
294-
<-done
291+
require.ErrorIs(t,<-errC,context.Canceled)
295292
require.Error(t,goleak.Find())
296293
})
297294
}

‎cli/templateinit_test.go

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -16,16 +16,11 @@ func TestTemplateInit(t *testing.T) {
1616
t.Parallel()
1717
tempDir:=t.TempDir()
1818
cmd,_:=clitest.New(t,"templates","init",tempDir)
19-
doneChan:=make(chanstruct{})
2019
pty:=ptytest.New(t)
2120
cmd.SetIn(pty.Input())
2221
cmd.SetOut(pty.Output())
23-
gofunc() {
24-
deferclose(doneChan)
25-
err:=cmd.Execute()
26-
require.NoError(t,err)
27-
}()
28-
<-doneChan
22+
err:=cmd.Execute()
23+
require.NoError(t,err)
2924
files,err:=os.ReadDir(tempDir)
3025
require.NoError(t,err)
3126
require.Greater(t,len(files),0)

‎cli/userlist_test.go

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,15 +16,13 @@ func TestUserList(t *testing.T) {
1616
coderdtest.CreateFirstUser(t,client)
1717
cmd,root:=clitest.New(t,"users","list")
1818
clitest.SetupConfig(t,client,root)
19-
doneChan:=make(chanstruct{})
2019
pty:=ptytest.New(t)
2120
cmd.SetIn(pty.Input())
2221
cmd.SetOut(pty.Output())
22+
errC:=make(chanerror)
2323
gofunc() {
24-
deferclose(doneChan)
25-
err:=cmd.Execute()
26-
require.NoError(t,err)
24+
errC<-cmd.Execute()
2725
}()
26+
require.NoError(t,<-errC)
2827
pty.ExpectMatch("coder.com")
29-
<-doneChan
3028
}

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp