Deflake proxy unit tests
What type of PR is this? (check all applicable)
- [ ] Bug Fix
This PR is best viewed ignoring whitespace (a couple functions just got indented) - https://github.com/gorilla/websocket/pull/982/files?w=1
Description
I noticed a flake on main unit tests at https://app.circleci.com/pipelines/github/gorilla/websocket/424/workflows/e37b2052-9796-4d0e-b3f6-9c6f72980e3f/jobs/1529?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-checks-link&utm_content=summary
Dug in and found a few tweaks needed in the unit tests from https://github.com/gorilla/websocket/pull/978
This PR:
- Correctly closes the test proxy server
- Correctly handles any buffered data when hijacking the connection in the test handler
- Fixes a panic if the upgrade failed inside the test websocket handler (was happening due to not handling the buffered data)
- Switches from calling http.Error after Upgrade (which doesn't work) to logging errors to the test logger
Added/updated tests?
- [x] Yes, fixed unit test flakes
Run verifications and test
go test -race ./...
ok github.com/gorilla/websocket 2.616s
? github.com/gorilla/websocket/examples/autobahn [no test files]
? github.com/gorilla/websocket/examples/chat [no test files]
? github.com/gorilla/websocket/examples/command [no test files]
? github.com/gorilla/websocket/examples/filewatch [no test files]
On main:
$ go test -race -c .
$ stress ./websocket.test
/var/folders/37/ns7gt60104scfm9fvg02p1jh00kjgb/T/go-stress-20250320T105828-994893878
2025/03/20 10:58:28 http: superfluous response.WriteHeader call from github.com/gorilla/websocket.init.func2 (client_proxy_server_test.go:640)
2025/03/20 10:58:28 http: panic serving 127.0.0.1:55001: runtime error: invalid memory address or nil pointer dereference
goroutine 15 [running]:
net/http.(*conn).serve.func1()
/Users/liggitt/.gimme/versions/go1.24.0.darwin.arm64/src/net/http/server.go:1947 +0xe4
panic({0x104d71fa0?, 0x1050a4500?})
/Users/liggitt/.gimme/versions/go1.24.0.darwin.arm64/src/runtime/panic.go:787 +0x124
github.com/gorilla/websocket.(*Conn).Close(...)
/Users/liggitt/go/src/github.com/gorilla/websocket/conn.go:344
panic({0x104d71fa0?, 0x1050a4500?})
/Users/liggitt/.gimme/versions/go1.24.0.darwin.arm64/src/runtime/panic.go:787 +0x124
github.com/gorilla/websocket.(*Conn).beginMessage(0x0, 0xc0000fecc0, 0x2)
/Users/liggitt/go/src/github.com/gorilla/websocket/conn.go:488 +0x30
github.com/gorilla/websocket.(*Conn).NextWriter(0x0, 0x2)
/Users/liggitt/go/src/github.com/gorilla/websocket/conn.go:529 +0x78
github.com/gorilla/websocket.init.func2({0x104ded610, 0xc0002860e0}, 0xc0000f4640)
/Users/liggitt/go/src/github.com/gorilla/websocket/client_proxy_server_test.go:644 +0x1b4
net/http.HandlerFunc.ServeHTTP(0x104de5880, {0x104ded610, 0xc0002860e0}, 0xc0000f4640)
/Users/liggitt/.gimme/versions/go1.24.0.darwin.arm64/src/net/http/server.go:2294 +0x4c
net/http.serverHandler.ServeHTTP({0xc0000fea50?}, {0x104ded610, 0xc0002860e0}, 0xc0000f4640)
/Users/liggitt/.gimme/versions/go1.24.0.darwin.arm64/src/net/http/server.go:3301 +0x29c
net/http.(*conn).serve(0xc0000d4990, {0x104ded928, 0xc0000fe930})
/Users/liggitt/.gimme/versions/go1.24.0.darwin.arm64/src/net/http/server.go:2102 +0xeb8
created by net/http.(*Server).Serve in goroutine 37
/Users/liggitt/.gimme/versions/go1.24.0.darwin.arm64/src/net/http/server.go:3454 +0x678
--- FAIL: TestHTTPProxyWithNetDialContext (0.00s)
client_proxy_server_test.go:151: websocket dial error: unexpected EOF
2025/03/20 10:58:29 http: TLS handshake error from 127.0.0.1:
…
1m0s: 217 runs so far, 6 failures (2.76%), 10 active
With this PR:
$ go test -race -c .
$ stress ./websocket.test
5s: 10 runs so far, 0 failures, 10 active
10s: 30 runs so far, 0 failures, 10 active
15s: 48 runs so far, 0 failures, 10 active
20s: 66 runs so far, 0 failures, 10 active
25s: 83 runs so far, 0 failures, 10 active
30s: 93 runs so far, 0 failures, 10 active
35s: 111 runs so far, 0 failures, 10 active
40s: 131 runs so far, 0 failures, 10 active
45s: 151 runs so far, 0 failures, 10 active
50s: 169 runs so far, 0 failures, 10 active
55s: 187 runs so far, 0 failures, 10 active
1m0s: 205 runs so far, 0 failures, 10 active
1m5s: 223 runs so far, 0 failures, 10 active
1m10s: 241 runs so far, 0 failures, 10 active
1m15s: 254 runs so far, 0 failures, 10 active
1m20s: 268 runs so far, 0 failures, 10 active
1m25s: 286 runs so far, 0 failures, 10 active
1m30s: 304 runs so far, 0 failures, 10 active
1m35s: 322 runs so far, 0 failures, 10 active
1m40s: 340 runs so far, 0 failures, 10 active
1m45s: 358 runs so far, 0 failures, 10 active
1m50s: 376 runs so far, 0 failures, 10 active
1m55s: 394 runs so far, 0 failures, 10 active
2m0s: 412 runs so far, 0 failures, 10 active
2m5s: 435 runs so far, 0 failures, 10 active
2m10s: 455 runs so far, 0 failures, 10 active
2m15s: 474 runs so far, 0 failures, 10 active
2m20s: 492 runs so far, 0 failures, 10 active
2m25s: 510 runs so far, 0 failures, 10 active
2m30s: 528 runs so far, 0 failures, 10 active
...
cc @seans3 @aojea
cc @AlexVulaj
/lgtm
/lgtm
Thanks for the clean-up and fixes.
@AlexVulaj friendly ping on this as a follow-up to https://github.com/gorilla/websocket/pull/978