fix: Isolate HTTP transports in parallel tests to prevent connection issues#3529
Conversation
- Ensure each test instance uses a dedicated HTTP transport instead of sharing the default transport. - Prevents race conditions caused by CloseIdleConnections() closing connections in the shared pool. - Improves test stability by avoiding intermittent connection broken errors. References: - Related discussion: google#3527 Signed-off-by: atilsensalduz <atil.sensalduz@gmail.com>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3529 +/- ##
==========================================
- Coverage 91.30% 91.28% -0.02%
==========================================
Files 184 184
Lines 16143 16143
==========================================
- Hits 14739 14736 -3
- Misses 1230 1231 +1
- Partials 174 176 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
gmlewis
left a comment
There was a problem hiding this comment.
Thank you, @atilsensalduz!
LGTM.
Awaiting second LGTM+Approval from any other contributor to this repo before merging.
|
@gmlewis can I review it? |
Absolutely! Thank you, @Abiji-2020! We welcome all contributions. |
Abiji-2020
left a comment
There was a problem hiding this comment.
✅ Looks Good.
I am just curious that can we reduce the timeout ?
Thank you, @Abiji-2020! The timeout here is sufficiently large such that it should NEVER trigger... and if it ever does, then something is seriously wrong. I think it is good to keep the 30-second timeout so that if it ever does take that long, then it will be pretty obvious and will alert us that it needs investigation as to why. So let's please leave it large like this for now. |
Summary
This PR addresses an issue where parallel tests share the default HTTP transport, leading to intermittent "connection broken" errors due to race conditions in connection reuse.
Root Cause
Tests create their own clients and servers via
setup(t), but they unintentionally share Go's default transport when no explicit transport is specified. As a result:server.Close()triggersCloseIdleConnections()on the default transport.Solution
References