Assert that runApp is called in the same zone as binding.ensureInitialized #117113
Conversation
flutter-dashboard
bot
added
a: tests
|
Looks like the checks are unhappy. Could you take a look? |
Hixie
force-pushed
the
zones
branch
3 times, most recently
from
46b2dd5 to
2a13282
Compare
|
This pull request executed golden file tests, but it has not been updated in a while (20+ days). Test results from Gold expire after as many days, so this pull request will need to be updated with a fresh commit in order to get results from Gold. For more guidance, visit Writing a golden file test for Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
Hixie
force-pushed
the
zones
branch
4 times, most recently
from
de79301 to
27161b9
Compare
Hixie
force-pushed
the
zones
branch
2 times, most recently
from
19458af to
b35e2f6
Compare
Hixie
removed
the
f: material design
|
This should be ready for review now. |
dev/tracing_tests/test/common.dart
Outdated
There was a problem hiding this comment.
For my own understanding: What do these tests do with zones that required disabling the zone check? From scanning through the tests, it wasn't immediately clear to me.
There was a problem hiding this comment.
I suspect that test introduces a Zone to catch (async) errors? But I didn't spend much time trying to track it down, to be honest.
dev/tracing_tests/test/common.dart
Outdated
There was a problem hiding this comment.
Maybe give this a name that makes people feel bad if they were to use it in a newly written test to avoid that this pattern spreads further? Something like LegacyTestBinding, maybe?
There was a problem hiding this comment.
ZoneIgnoringTestBinding?
There was a problem hiding this comment.
assert that the thrown error actually contains some of the expected information to make sure we get the right one here?
There was a problem hiding this comment.
done (and hey, it was already the right one this time)
|
Thanks for the comments; applied the fixes. Will land on green. |
auto-submit
bot
removed
the
autosubmit
|
auto label is removed for flutter/flutter, pr: 117113, due to - The status or check suite Google testing has failed. Please fix the issues identified (or deflake) before re-applying this label. |
|
Google tests found that I'd missed the driver binding when it comes to skipping the zone check. Trying again with the driver binding skipping the zone check. |
Hixie
force-pushed
the
zones
branch
2 times, most recently
from
f2bf857 to
696d9d8
Compare
Hixie
force-pushed
the
zones
branch
4 times, most recently
from
2dc3bc3 to
4161eff
Compare
|
@goderbauer I've walked this back a bit. Now it's a warning with a flag that lets you make it fatal, rather than an error. I did this because the google tests convinced me that making it fatal was too potentially breaking. I suggest we consider switching the default for the flag to true after a few more stable releases, once people have had enough advance warning. |
|
auto label is removed for flutter/flutter, pr: 117113, due to - The status or check suite Mac web_tool_tests has failed. Please fix the issues identified (or deflake) before re-applying this label. |
auto-submit
bot
removed
the
autosubmit
auto-submit
bot
removed
the
autosubmit
|
auto label is removed for flutter/flutter, pr: 117113, due to - The status or check suite tool_tests-general-linux has failed. Please fix the issues identified (or deflake) before re-applying this label. |
There was a problem hiding this comment.
| /// are encouraged to resolve any issues that case the [debugCheckZone] message | |
| /// are encouraged to resolve any issues that cause the [debugCheckZone] message |
3 participants
Fixes #94123