Repository navigation
Make flutter test have a 3s timeout. - #3366
eseidelGoogle wants to merge 1 commit into
Conversation
| print(line); | ||
| }); | ||
| finalize(); | ||
| throw new TimeoutException("Test took too long", _kPlatformTimeout); |
There was a problem hiding this comment.
Maybe "Test timed out. This may indicate a syntax error in the test (try using "flutter analyze" on the test). If an exception occurred, it will have been printed above this message."
(Or below this message, I forget in what order they print out.)
|
Travis is showing failures for a lot of the tests. Maybe some bad interaction with how we run the tests in parallel? When are you starting the timer, relative to when a particular test starts running? Ideally we wouldn't use a timer at all, and we'd immediately fail when the child process fails. |
|
My approach appears to be wrong. I attempted to add an exitCode listener to the process, but the process never exits on failure. I also added print statements to the platform_plugin's handleDone methods on streams and those are never called in failure. It looks like the child process is somehow swallowing the syntax errors and not reporting them to the parent. I added some prints to package:test's vm's catchIsolateErrors() function, and it looks like we're not hitting that error handler in the syntax error case. I suspect that it may due to how we've wired up our error handling for the isolate: |
|
I'm pretty sure the bug here is that Flutter's isolates not sending all expected errors through I'm not quite sure how to write a minimal test of Flutter's Isolate.current.addErrorListener handling, but I suspect that we'd find that we don't call errorListeners for syntax errors. FYI @johnmccutchan @nex3 |
|
So this change is wrong. I believe the correct change is to make flutter pass https://github.com/dart-lang/sdk/blob/master/tests/isolate/handle_error2_test.dart. I'll update the original bug. |
2 participants
Fixes #2182
@Hixie @devoncarew @nex3