Await some unawaited futures #654
Conversation
lib/src/executable.dart
Outdated
| signalSubscription.cancel(); | ||
| signalSubscription = null; | ||
| stdinLines.cancel(immediate: true); | ||
| await stdinLines.cancel(immediate: true); |
There was a problem hiding this comment.
I don't think this should matter, and it would be nice to get the runner started closing as soon as possible. Also, since this is in the signal-handling infrastructure, it's definitely not causing bot issues.
| _onSuiteAddedController.close(); | ||
| _suiteController.close(); | ||
| await _onSuiteAddedController.close(); | ||
| await _suiteController.close(); |
There was a problem hiding this comment.
I think these are the most likely candidates for the source of the flakiness. It's possible that there was a race condition where their events hadn't fully propagated, which would mean that the close() calls below didn't hit all the relevant tests.
It would be an interesting experiment to try awaiting only these and seeing if that clears up the flakiness.
There was a problem hiding this comment.
I removed this await and did a couple Travis runs. Unfortunately I could not get into the flaky state.
nex3
left a comment
nex3
left a comment
There was a problem hiding this comment.
Since it looks like this clears up the flakiness, I'm fine landing it as-is. There are almost certainly some false positives in here, but I don't think it's anything that's likely to affect performance.
|
Hmmm. Removing that await caused one of the tests to time out. I'm going to try and verify that is the root cause of the flakiness. |
grouma
changed the title
3 participants
A handful of these obviously should be awaited, namely the file creation ones. However, many of these do not obviously need to be awaited.