-
Notifications
You must be signed in to change notification settings - Fork 28.9k
write test to convince self of lack of timing issue #58011
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@@ -1196,6 +1196,48 @@ void main() { | |||
}) async => mockVMService, | |||
})); | |||
|
|||
|
|||
// Don't use this outside of the test below. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know you don't like groups, but should we wrap this completer and the test to enforce not re-using it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, good point - this is the right place to use one
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM other than nit about group
Description
In #55864 a race condition was described where a done event is received before we finish connecting. This cannot happen, since
async
functions begin synchronously and the flagisWaitingForVm
is tripped immediately, keepingonDone
from exiting.Closes #55864
At any rate, this test will keep things working.