ringhash: fix a couple of flakes in e2e style tests #7784
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #7784 +/- ##
==========================================
- Coverage 81.74% 81.62% -0.12%
==========================================
Files 374 374
Lines 38149 38149
==========================================
- Hits 31185 31141 -44
- Misses 5689 5723 +34
- Partials 1275 1285 +10 |
| // We don't close these listeners here, to make sure ports are | ||
| // not reused across them, and across tests. | ||
| lis.Stop() | ||
| t.Cleanup(func() { lis.Close() }) |
There was a problem hiding this comment.
I was concerned if this would slow down our tests. But seems like we have ports ~ 49152 to 65535 available to use
There was a problem hiding this comment.
I haven't seen us running out of ports so far. So, I'm not worried about it at this point.
| go func() { lis.Accept() }() | ||
|
|
||
| // We don't close these listeners here, to make sure ports are | ||
| // not reused across them, and across tests. | ||
| lis.Stop() |
There was a problem hiding this comment.
These statements are basically contesting for the lock in RestartableListener.
Could we potentially avoid the lis.Accept call altogether and simply call stop directly?
There was a problem hiding this comment.
Without the call to Accept, connections from the client will hang instead of fail (which is what we want). In fact, I didn't have the Accept in there and I spent a while debugging the test, until I figured out that we need an Accept in there :(
|
GA checks are lost in limbo? |
Looks like force push is the way out. I will give that a try. |
easwars
force-pushed
the
ringhash_flakes
branch
from
3df417e to
300bd26
Compare
Fixes #7464
Fixes #7777
Summary of fixes:
t.Parallelin our tests, thego testcommand runs tests from different packages in parallel, and therefore the port could get picked by a test from a different package)RestartableListenerthat holds on to the port, but closes the connection as soon as it accepts one.cc.GetStateto verify that the state of the grpc client. I switched them to usetestutils.AwaitStateto make them more forgiving.testutils.AwaitStatein some places wherecc.GetStatemight do the job. I did this before the former uses less lines of code and does the same job anyways.RELEASE NOTES: none