test: Increase disconnectTimeout to reduce test flakes #26
Conversation
WebRTC uses UDP, which means a network connection is never open or closed. It uses timeouts to determine connection state; on a slow CI runner, these timeouts could be reached. Increasing this timeout should reduce flakes, but is unlikely to remove this flake entirely.
Codecov Report
@@ Coverage Diff @@
## main #26 +/- ##
==========================================
+ Coverage 71.31% 71.47% +0.15%
==========================================
Files 37 37
Lines 1311 1311
Branches 7 7
==========================================
+ Hits 935 937 +2
+ Misses 302 300 -2
Partials 74 74
Continue to review full report at Codecov.
|
| @@ -23,7 +23,7 @@ import ( | |||
| ) | |||
|
|
|||
| const ( | |||
| disconnectedTimeout = time.Millisecond * 200 | |||
There was a problem hiding this comment.
thought (non-blocking): It's not clear to me why it would timeout at all given the connections are local 🤔
There was a problem hiding this comment.
When CPU is contended in CI... this flake occurred because it took >200ms to negotiate the connection (which is ridiculously slow execution)!
There was a problem hiding this comment.
Glad this improves the reliability of the tests! Increasing timeouts alone always makes me nervous (because code always finds a way of running just slow enough to still hit the timeout...) but cleaning up the servers with sch.Close() seems like it will help, too
Thanks for investigating these flaky test runs!
|
Also looks like one we could backport to |
WebRTC uses UDP, which means a network connection is never open or closed. It uses timeouts to determine connection state; on a slow CI runner, these timeouts could be reached. Increasing this timeout should reduce flakes, but is unlikely to remove this flake entirely.