-
Notifications
You must be signed in to change notification settings - Fork 29.8k
Stop logging on iOS process exit #66293
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
| run.kill(); | ||
| }); | ||
| return passedTest && failedTest && skippedTest && finishedMessage | ||
| return passedTest && failedTest && skippedTest && finishedMessage && printMessage && writelnMessage |
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.
This fails on stable (neither line is printed).
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.
👏
| ..listen(receivedLogLines.add); | ||
|
|
||
| expect(await iosDeployDebugger.launchAndAttach(), isTrue); | ||
| await logLines.toList(); |
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.
you could also await logLines.last
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.
Hm, it doesn't like that:
dart:async Stream.last
test/general.shard/ios/ios_deploy_test.dart 117:24 main.<fn>.<fn>.<fn>
===== asynchronous gap ===========================
dart:async _asyncThenWrapperHelper
test/general.shard/ios/ios_deploy_test.dart main.<fn>.<fn>.<fn>
dart:async runZoned
test/src/common.dart 207:14 testWithoutContext.<fn>
test/src/common.dart 177:18 test.<fn>
Bad state: No element
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.
Ahh, because it is empty...
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.
toList seems easier than Listen/onDone
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.
Getting the streams to work in these tests was 3x more work than the actual change.
jonahwilliams
left a comment
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 with nit
|
Dependency #66092 was reverted. |
|
Closing, this was included with #66399. |
Description
Detect when an
exit()s and don't pass on further logging to the log reader. This pretty much never happens on iOS apps because the App Store will reject apps that exit themselves manually, but I did see it while I was writing integration tests.Related Issues
Follow up on #66092.
Tests
adblogging). Run on iOS, let's stop regressing logging on that platform...Checklist
///).flutter analyze --flutter-repo) does not report any problems on my PR.Breaking Change