Fix race conditions in invoker_test - #6905
Conversation
|
Hi! Thanks for the pull request. Please ensure that this change is linked to an issue by mentioning an issue number in the description of the pull request. If this pull request would close the issue, please put the word 'Fixes' before the issue number somewhere in the pull request body. If this is a tiny change like fixing a typo, feel free to ignore this message. |
|
I have a couple of questions about your changes, but they are mainly for my own learning. Lifting listener initialization to the test makes sense to me. |
|
Note: the Projectsv2 flakey test is being fixed in #6909 |
mislav
left a comment
There was a problem hiding this comment.
Looks great. Thanks for working on this!
Please merge at your own discretion, but feel free to squash-merge for history simplification.
|
I noticed something interesting while testing this branch in a codespace: port |
|
@cmbrose Thanks for this! Do you think this PR also addresses the intermittent failure that happened for the 2nd-to-last commit in this branch? |
|
@mislav it should - the very last commit is meant to fix it, although the commit message doesn't really explain that 😅 Definitely let me know if they start popping up again though! |
Called out by @mislav over here the invoker tests don't properly wait for the listener to spin up, or for the grpc server to spin down. This creates some races in the invoker tests where new listeners may fail to bind as previous tests haven't fully cleaned up their goroutines.