In microbenchmarks, ensure flutter quits by sending q to stdin
#19400
Conversation
q to stdinq to stdin
There was a problem hiding this comment.
the comment here seems wrong now? maybe "in case..."
There was a problem hiding this comment.
I'm not sure which bit you're referring to - do you mean the comment on the .kill()?
We could remove the kill, but I think ultimately we want to fix the issue (it may require a VM change to allow snapshot validation so we can change the shell script to use exec) and go back to the kill. I can remove it for now and add some comments if you think that might make more sense though.
There was a problem hiding this comment.
I'm all for leaving the kill, just changing the comment to "in case flutter run doesn't quit automatically" or some such. The comment is wrong, now that we do quit it.
There was a problem hiding this comment.
Ah sorry, I see what you mean now. The original comment was referring to the behaviour of flutter run not quitting automatically, but I agree it was a bit misleading without context. I've updated now (and swapped the comment/ignore around to make it more readable).
|
I've rebased since there were conflicts from the constant change. @Hixie Happy for me to land this? |
|
LGTM |
|
Looks like a timeout in Gradle. I'll rebase on master since it was a week ago anyway, that'll probably make it go green. Thanks! |
This isn't perfect, but it seems unlikely this behaviour will change (and if it does, this will likely just silently do nothing) but it does reliably result in Flutter quitting, whereas without this it hangs around until it's either forcefully killed at the end of the entire run, or the next app install forces it to quit.
Fixes #19208.