Skip to content

Commit e3d7149

Browse files
stevenhHalloTschuess
stevenh
and
HalloTschuess
authored
fix: race conditions in tests (#38)
Fix the race conditions in test and simplify the use of newServer. Fix race on socket close with data reads. This ignores connection reset by peer errors when a quit has already successfully been processed. Handle duplicate closes due to clean up in ssh mux to prevent spurious invalid failures. Also: * Fix missing t.Helper() and remove nolint directives. * Remove some more unneeded test server == nil checks. * Remove unneeded sshConn from sshServerShell. * Update dependencies. Co-authored-by: HalloTschuess <[email protected]>
1 parent a81b447 commit e3d7149

File tree

9 files changed

+204
-175
lines changed

9 files changed

+204
-175
lines changed

‎basic_cmds_test.go

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,6 @@ import (
99

1010
func TestCmdsBasic(t *testing.T) {
1111
s := newServer(t)
12-
if s == nil {
13-
return
14-
}
1512
defer func() {
1613
assert.NoError(t, s.Close())
1714
}()
@@ -28,11 +25,7 @@ func TestCmdsBasic(t *testing.T) {
2825
}
2926

3027
func TestCmdsBasicSSH(t *testing.T) {
31-
s := newServer(t)
32-
if s == nil {
33-
return
34-
}
35-
s.useSSH = true
28+
s := newServer(t, useSSH())
3629
defer func() {
3730
assert.NoError(t, s.Close())
3831
}()
@@ -48,7 +41,8 @@ func TestCmdsBasicSSH(t *testing.T) {
4841
testCmdsBasic(t, c)
4942
}
5043

51-
func testCmdsBasic(t *testing.T, c *Client) { //nolint: thelper
44+
func testCmdsBasic(t *testing.T, c *Client) {
45+
t.Helper()
5246
auth := func(t *testing.T) {
5347
t.Helper()
5448
if err := c.Login("user", "pass"); !assert.NoError(t, err) {

‎client.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -314,7 +314,7 @@ func (c *Client) Close() error {
314314

315315
if err != nil {
316316
return err
317-
} else if err2 != nil {
317+
} else if err2 != nil && !strings.HasSuffix(err2.Error(), "connection reset by peer") {
318318
return fmt.Errorf("client: close: %w", err2)
319319
}
320320

‎client_test.go

Lines changed: 9 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,6 @@ import (
1111

1212
func TestClient(t *testing.T) {
1313
s := newServer(t)
14-
if s == nil {
15-
return
16-
}
1714
defer func() {
1815
assert.NoError(t, s.Close())
1916
}()
@@ -41,11 +38,7 @@ func TestClient(t *testing.T) {
4138
}
4239

4340
func TestClientSSH(t *testing.T) {
44-
s := newServer(t)
45-
if s == nil {
46-
return
47-
}
48-
s.useSSH = true
41+
s := newServer(t, useSSH())
4942
defer func() {
5043
assert.NoError(t, s.Close())
5144
}()
@@ -93,9 +86,6 @@ func TestClientOptionError(t *testing.T) {
9386

9487
func TestClientDisconnect(t *testing.T) {
9588
s := newServer(t)
96-
if s == nil {
97-
return
98-
}
9989
defer func() {
10090
assert.NoError(t, s.Close())
10191
}()
@@ -112,11 +102,7 @@ func TestClientDisconnect(t *testing.T) {
112102
}
113103

114104
func TestClientDisconnectSSH(t *testing.T) {
115-
s := newServer(t)
116-
if s == nil {
117-
return
118-
}
119-
s.useSSH = true
105+
s := newServer(t, useSSH())
120106
defer func() {
121107
assert.NoError(t, s.Close())
122108
}()
@@ -134,9 +120,6 @@ func TestClientDisconnectSSH(t *testing.T) {
134120

135121
func TestClientWriteFail(t *testing.T) {
136122
s := newServer(t)
137-
if s == nil {
138-
return
139-
}
140123
defer func() {
141124
assert.NoError(t, s.Close())
142125
}()
@@ -173,9 +156,6 @@ func TestClientDialFailSSH(t *testing.T) {
173156

174157
func TestClientTimeout(t *testing.T) {
175158
s := newServer(t)
176-
if s == nil {
177-
return
178-
}
179159
defer func() {
180160
assert.NoError(t, s.Close())
181161
}()
@@ -191,11 +171,7 @@ func TestClientTimeout(t *testing.T) {
191171
}
192172

193173
func TestClientTimeoutSSH(t *testing.T) {
194-
s := newServer(t)
195-
if s == nil {
196-
return
197-
}
198-
s.useSSH = true
174+
s := newServer(t, useSSH())
199175
defer func() {
200176
assert.NoError(t, s.Close())
201177
}()
@@ -212,9 +188,6 @@ func TestClientTimeoutSSH(t *testing.T) {
212188

213189
func TestClientDeadline(t *testing.T) {
214190
s := newServer(t)
215-
if s == nil {
216-
return
217-
}
218191
defer func() {
219192
assert.NoError(t, s.Close())
220193
}()
@@ -235,11 +208,7 @@ func TestClientDeadline(t *testing.T) {
235208
}
236209

237210
func TestClientDeadlineSSH(t *testing.T) {
238-
s := newServer(t)
239-
if s == nil {
240-
return
241-
}
242-
s.useSSH = true
211+
s := newServer(t, useSSH())
243212
defer func() {
244213
assert.NoError(t, s.Close())
245214
}()
@@ -260,12 +229,7 @@ func TestClientDeadlineSSH(t *testing.T) {
260229
}
261230

262231
func TestClientNoHeader(t *testing.T) {
263-
s := newServerStopped(t)
264-
if s == nil {
265-
return
266-
}
267-
s.noHeader = true
268-
s.Start()
232+
s := newServer(t, noHeader())
269233
defer func() {
270234
assert.NoError(t, s.Close())
271235
}()
@@ -280,12 +244,7 @@ func TestClientNoHeader(t *testing.T) {
280244
}
281245

282246
func TestClientNoBanner(t *testing.T) {
283-
s := newServerStopped(t)
284-
if s == nil {
285-
return
286-
}
287-
s.noBanner = true
288-
s.Start()
247+
s := newServer(t, noBanner())
289248
defer func() {
290249
assert.NoError(t, s.Close())
291250
}()
@@ -300,12 +259,7 @@ func TestClientNoBanner(t *testing.T) {
300259
}
301260

302261
func TestClientFailConn(t *testing.T) {
303-
s := newServerStopped(t)
304-
if s == nil {
305-
return
306-
}
307-
s.failConn = true
308-
s.Start()
262+
s := newServer(t, failConn())
309263
defer func() {
310264
assert.NoError(t, s.Close())
311265
}()
@@ -320,12 +274,7 @@ func TestClientFailConn(t *testing.T) {
320274
}
321275

322276
func TestClientFailConnSSH(t *testing.T) {
323-
s := newServerStopped(t)
324-
if s == nil {
325-
return
326-
}
327-
s.failConn = true
328-
s.Start()
277+
s := newServer(t, failConn())
329278
defer func() {
330279
assert.NoError(t, s.Close())
331280
}()
@@ -340,12 +289,7 @@ func TestClientFailConnSSH(t *testing.T) {
340289
}
341290

342291
func TestClientBadHeader(t *testing.T) {
343-
s := newServerStopped(t)
344-
if s == nil {
345-
return
346-
}
347-
s.badHeader = true
348-
s.Start()
292+
s := newServer(t, badHeader())
349293
defer func() {
350294
assert.NoError(t, s.Close())
351295
}()

‎connection.go

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -106,10 +106,16 @@ func (c *sshConnection) Write(p []byte) (n int, err error) {
106106
// Close implements io.Closer.
107107
func (c *sshConnection) Close() error {
108108
var err error
109-
if err2 := c.channel.Close(); err2 != nil && !errors.Is(err2, io.EOF) {
109+
// In both cases we ignore errors which don't have any value.
110+
if err2 := c.channel.Close(); err2 != nil &&
111+
!errors.Is(err2, io.EOF) &&
112+
!strings.HasSuffix(err2.Error(), "connection reset by peer") {
110113
err = err2
111114
}
112-
if err2 := c.Conn.Close(); err2 != nil && !errors.Is(err2, net.ErrClosed) {
115+
if err2 := c.Conn.Close(); err2 != nil &&
116+
err == nil &&
117+
!errors.Is(err2, net.ErrClosed) &&
118+
!strings.HasSuffix(err2.Error(), "connection reset by peer") {
113119
err = err2
114120
}
115121
return err

‎connection_test.go

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,16 +20,17 @@ func TestConnection(t *testing.T) {
2020
newServer func(*testing.T) *server
2121
}{
2222
"legacyConnection": {
23-
conn: new(legacyConnection),
24-
newServer: newServer,
23+
conn: new(legacyConnection),
24+
newServer: func(t *testing.T) *server {
25+
t.Helper()
26+
return newServer(t)
27+
},
2528
},
2629
"sshConnection": {
2730
conn: &sshConnection{config: sshClientTestConfig},
2831
newServer: func(t *testing.T) *server {
2932
t.Helper()
30-
s := newServer(t)
31-
s.useSSH = true
32-
return s
33+
return newServer(t, useSSH())
3334
},
3435
},
3536
}

‎go.mod

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,5 @@ go 1.13
55
require (
66
github.com/mitchellh/mapstructure v1.5.0
77
github.com/stretchr/testify v1.4.0
8-
golang.org/x/crypto v0.0.0-20220829220503-c86fa9a7ed90
9-
golang.org/x/sys v0.0.0-20220829200755-d48e67d00261 // indirect
8+
golang.org/x/crypto v0.3.0
109
)

‎go.sum

Lines changed: 27 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -7,18 +7,37 @@ github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZN
77
github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
88
github.com/stretchr/testify v1.4.0 h1:2E4SXV/wtOkTonXsotYi4li6zVWxYlZuYNCXe9XRJyk=
99
github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4=
10-
golang.org/x/crypto v0.0.0-20220829220503-c86fa9a7ed90 h1:Y/gsMcFOcR+6S6f3YeMKl5g+dZMEWqcz5Czj/GWYbkM=
11-
golang.org/x/crypto v0.0.0-20220829220503-c86fa9a7ed90/go.mod h1:IxCIyHEi3zRg3s0A5j5BB6A9Jmi73HwBIUl50j+osU4=
12-
golang.org/x/net v0.0.0-20211112202133-69e39bad7dc2/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y=
10+
github.com/yuin/goldmark v1.4.13/go.mod h1:6yULJ656Px+3vBD8DxQVa3kxgyrAnzto9xy5taEt/CY=
11+
golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=
12+
golang.org/x/crypto v0.0.0-20210921155107-089bfa567519/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc=
13+
golang.org/x/crypto v0.3.0 h1:a06MkbcxBrEFc0w0QIZWXrH/9cCX6KJyWbBOIwAn+7A=
14+
golang.org/x/crypto v0.3.0/go.mod h1:hebNnKkNXi2UzZN1eVRvBB7co0a+JxK6XbPiWVs/3J4=
15+
golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91VN4djpZkiMVwK6gcyfeH4XE8wZrZaV4=
16+
golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s=
17+
golang.org/x/net v0.0.0-20210226172049-e18ecbb05110/go.mod h1:m0MpNAwzfU5UDzcl9v0D8zg8gWTRqZa9RBIspLL5mdg=
18+
golang.org/x/net v0.0.0-20220722155237-a158d28d115b/go.mod h1:XRhObCWvk6IyKnWLug+ECip1KBveYUHfp+8e9klMJ9c=
19+
golang.org/x/net v0.2.0/go.mod h1:KqCZLdyyvdV855qA2rE3GC2aiw5xGR5TEjj8smXukLY=
20+
golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
21+
golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
22+
golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
1323
golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
14-
golang.org/x/sys v0.0.0-20210423082822-04245dca01da/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
1524
golang.org/x/sys v0.0.0-20210615035016-665e8c7367d1/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
16-
golang.org/x/sys v0.0.0-20220829200755-d48e67d00261 h1:v6hYoSR9T5oet+pMXwUWkbiVqx/63mlHjefrHmxwfeY=
17-
golang.org/x/sys v0.0.0-20220829200755-d48e67d00261/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
18-
golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1 h1:v+OssWQX+hTHEmOBgwxdZxK4zHq3yOs8F9J7mk0PY8E=
25+
golang.org/x/sys v0.0.0-20220520151302-bc2c85ada10a/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
26+
golang.org/x/sys v0.0.0-20220722155257-8c9f86f7a55f/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
27+
golang.org/x/sys v0.2.0 h1:ljd4t30dBnAvMZaQCevtY0xLLD0A+bRZXbgLMLU1F/A=
28+
golang.org/x/sys v0.2.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
1929
golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo=
20-
golang.org/x/text v0.3.6/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
30+
golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8=
31+
golang.org/x/term v0.2.0 h1:z85xZCsEl7bi/KwbNADeBYoOP0++7W1ipu+aGnpwzRM=
32+
golang.org/x/term v0.2.0/go.mod h1:TVmDHMZPmdnySmBfhjOoOdhjzdE1h4u1VwSiw2l1Nuc=
33+
golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
34+
golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
35+
golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ=
36+
golang.org/x/text v0.4.0/go.mod h1:mrYo+phRRbMaCq/xk9113O4dZlRixOauAjOtrjsXDZ8=
2137
golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
38+
golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo=
39+
golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc=
40+
golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
2241
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM=
2342
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
2443
gopkg.in/yaml.v2 v2.2.2 h1:ZCJp+EgiOT7lHqUV2J862kp8Qj64Jo6az82+3Td9dZw=

0 commit comments

Comments
 (0)