NotificationsYou must be signed in to change notification settings
Fork3.6k
Star24.3k

Deflake proxy unit tests#982

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Open

liggitt wants to merge1 commit intogorilla:main

base:main

Choose a base branch

fromliggitt:deflake

Open

Deflake proxy unit tests#982

liggitt wants to merge1 commit intogorilla:mainfromliggitt:deflake

+113 −95

Conversation

Copy link

liggitt commentedMar 20, 2025•
edited by apoorvajagtap
Loading

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 onmain unit tests athttps://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#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?

Yes, fixed unit test flakes

Run verifications and test

go test -race ./...ok  github.com/gorilla/websocket2.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]

Onmain:

$ go test -race -c .$ stress ./websocket.test /var/folders/37/ns7gt60104scfm9fvg02p1jh00kjgb/T/go-stress-20250320T105828-9948938782025/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 dereferencegoroutine 15 [running]:net/http.(*conn).serve.func1()/Users/liggitt/.gimme/versions/go1.24.0.darwin.arm64/src/net/http/server.go:1947 +0xe4panic({0x104d71fa0?, 0x1050a4500?})/Users/liggitt/.gimme/versions/go1.24.0.darwin.arm64/src/runtime/panic.go:787 +0x124github.com/gorilla/websocket.(*Conn).Close(...)/Users/liggitt/go/src/github.com/gorilla/websocket/conn.go:344panic({0x104d71fa0?, 0x1050a4500?})/Users/liggitt/.gimme/versions/go1.24.0.darwin.arm64/src/runtime/panic.go:787 +0x124github.com/gorilla/websocket.(*Conn).beginMessage(0x0, 0xc0000fecc0, 0x2)/Users/liggitt/go/src/github.com/gorilla/websocket/conn.go:488 +0x30github.com/gorilla/websocket.(*Conn).NextWriter(0x0, 0x2)/Users/liggitt/go/src/github.com/gorilla/websocket/conn.go:529 +0x78github.com/gorilla/websocket.init.func2({0x104ded610, 0xc0002860e0}, 0xc0000f4640)/Users/liggitt/go/src/github.com/gorilla/websocket/client_proxy_server_test.go:644 +0x1b4net/http.HandlerFunc.ServeHTTP(0x104de5880, {0x104ded610, 0xc0002860e0}, 0xc0000f4640)/Users/liggitt/.gimme/versions/go1.24.0.darwin.arm64/src/net/http/server.go:2294 +0x4cnet/http.serverHandler.ServeHTTP({0xc0000fea50?}, {0x104ded610, 0xc0002860e0}, 0xc0000f4640)/Users/liggitt/.gimme/versions/go1.24.0.darwin.arm64/src/net/http/server.go:3301 +0x29cnet/http.(*conn).serve(0xc0000d4990, {0x104ded928, 0xc0000fe930})/Users/liggitt/.gimme/versions/go1.24.0.darwin.arm64/src/net/http/server.go:2102 +0xeb8created 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 EOF2025/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 active10s: 30 runs so far, 0 failures, 10 active15s: 48 runs so far, 0 failures, 10 active20s: 66 runs so far, 0 failures, 10 active25s: 83 runs so far, 0 failures, 10 active30s: 93 runs so far, 0 failures, 10 active35s: 111 runs so far, 0 failures, 10 active40s: 131 runs so far, 0 failures, 10 active45s: 151 runs so far, 0 failures, 10 active50s: 169 runs so far, 0 failures, 10 active55s: 187 runs so far, 0 failures, 10 active1m0s: 205 runs so far, 0 failures, 10 active1m5s: 223 runs so far, 0 failures, 10 active1m10s: 241 runs so far, 0 failures, 10 active1m15s: 254 runs so far, 0 failures, 10 active1m20s: 268 runs so far, 0 failures, 10 active1m25s: 286 runs so far, 0 failures, 10 active1m30s: 304 runs so far, 0 failures, 10 active1m35s: 322 runs so far, 0 failures, 10 active1m40s: 340 runs so far, 0 failures, 10 active1m45s: 358 runs so far, 0 failures, 10 active1m50s: 376 runs so far, 0 failures, 10 active1m55s: 394 runs so far, 0 failures, 10 active2m0s: 412 runs so far, 0 failures, 10 active2m5s: 435 runs so far, 0 failures, 10 active2m10s: 455 runs so far, 0 failures, 10 active2m15s: 474 runs so far, 0 failures, 10 active2m20s: 492 runs so far, 0 failures, 10 active2m25s: 510 runs so far, 0 failures, 10 active2m30s: 528 runs so far, 0 failures, 10 active...

cc@seans3 @aojea