Assert that runApp is called in the same zone as binding.ensureInitialized by Hixie · Pull Request #117113 · flutter/flutter

Hixie · 2022-12-14T22:40:06Z

goderbauer · 2022-12-14T23:27:54Z

Looks like the checks are unhappy. Could you take a look?

flutter-dashboard · 2023-01-04T00:18:33Z

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 package:flutter.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

Hixie · 2023-02-15T22:02:06Z

This should be ready for review now.

goderbauer

LGTM

goderbauer · 2023-02-15T23:31:50Z

dev/tracing_tests/test/common.dart

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.

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.

goderbauer · 2023-02-15T23:33:23Z

dev/tracing_tests/test/common.dart

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?

ZoneIgnoringTestBinding?

goderbauer · 2023-02-15T23:39:36Z

packages/flutter/test/foundation/binding_2_test.dart

assert that the thrown error actually contains some of the expected information to make sure we get the right one here?

done (and hey, it was already the right one this time)

Hixie · 2023-02-15T23:52:14Z

Thanks for the comments; applied the fixes. Will land on green.

auto-submit · 2023-02-16T08:16:43Z

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.

Hixie · 2023-02-17T18:55:43Z

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 · 2023-02-17T21:13:29Z

@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.

goderbauer

Still LGTM

auto-submit · 2023-02-17T23:59:23Z

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 · 2023-02-18T00:13:11Z

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.

cbatson

nit: typo

cbatson · 2023-02-28T04:05:26Z

packages/flutter/lib/src/foundation/binding.dart

Suggested change

/// are encouraged to resolve any issues that case the [debugCheckZone] message

/// are encouraged to resolve any issues that cause the [debugCheckZone] message

…lized Fixes flutter#94123

…reInitialized (#117113)" This reverts commit 96f927f.

…reInitialized (#117113)" (#122830) Revert "Assert that runApp is called in the same zone as binding.ensureInitialized"

…reInitialized (flutter/flutter#117113)

Hixie marked this pull request as ready for review December 14, 2022 22:46

Hixie mentioned this pull request Dec 14, 2022

Flutter framework does not warn when ensureInitialized is called in a different zone than runApp #94123

Closed

Hixie force-pushed the zones branch 3 times, most recently from 46b2dd5 to 2a13282 Compare December 15, 2022 00:17

Hixie force-pushed the zones branch 4 times, most recently from de79301 to 27161b9 Compare February 2, 2023 22:02

Hixie force-pushed the zones branch 2 times, most recently from 19458af to b35e2f6 Compare February 13, 2023 21:46

Hixie removed the f: material design flutter/packages/flutter/material repository. label Feb 15, 2023

goderbauer approved these changes Feb 15, 2023

View reviewed changes

Hixie force-pushed the zones branch from b35e2f6 to c17d2dd Compare February 15, 2023 23:51

Hixie added the autosubmit Merge PR when tree becomes green via auto submit App label Feb 15, 2023

auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Feb 16, 2023

Hixie force-pushed the zones branch from c17d2dd to f7283ee Compare February 17, 2023 18:43

Hixie force-pushed the zones branch 2 times, most recently from f2bf857 to 696d9d8 Compare February 17, 2023 21:05

Hixie force-pushed the zones branch 4 times, most recently from 2dc3bc3 to 4161eff Compare February 17, 2023 21:11

goderbauer approved these changes Feb 17, 2023

View reviewed changes

Hixie force-pushed the zones branch from 4161eff to e9c52ca Compare February 17, 2023 21:27

Hixie added the autosubmit Merge PR when tree becomes green via auto submit App label Feb 17, 2023

Hixie force-pushed the zones branch from e9c52ca to a793d71 Compare February 17, 2023 23:36

auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Feb 17, 2023

Hixie force-pushed the zones branch from a793d71 to 662c674 Compare February 18, 2023 00:07

Hixie added the autosubmit Merge PR when tree becomes green via auto submit App label Feb 18, 2023

auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Feb 18, 2023

cbatson reviewed Feb 28, 2023

View reviewed changes

goderbauer mentioned this pull request Feb 28, 2023

Unexpected uncaught exception passed to onError of runZonedGuarded() despite try/catch #121439

Closed

Assert that runApp is called in the same zone as binding.ensureInitia…

8cc7a2c

…lized Fixes flutter#94123

Hixie force-pushed the zones branch from 662c674 to 8cc7a2c Compare March 16, 2023 08:19

Hixie added the autosubmit Merge PR when tree becomes green via auto submit App label Mar 16, 2023

auto-submit bot merged commit 96f927f into flutter:master Mar 16, 2023

yaakovschectman added a commit that referenced this pull request Mar 16, 2023

Revert "Assert that runApp is called in the same zone as binding.ensu…

926955b

…reInitialized (#117113)" This reverts commit 96f927f.

yaakovschectman mentioned this pull request Mar 16, 2023

Revert "Assert that runApp is called in the same zone as binding.ensureInitialized" #122830

Merged

auto-submit bot mentioned this pull request Mar 16, 2023

Review request for Revert PR flutter/flutter#122830 #122831

Closed

auto-submit bot pushed a commit that referenced this pull request Mar 16, 2023

Revert "Assert that runApp is called in the same zone as binding.ensu…

5bea4d9

…reInitialized (#117113)" (#122830) Revert "Assert that runApp is called in the same zone as binding.ensureInitialized"

Hixie mentioned this pull request Mar 16, 2023

Assert that runApp is called in the same zone as binding.ensureInitialized #122836

Merged

engine-flutter-autoroll mentioned this pull request Mar 17, 2023

Roll Flutter from 67e5f663d075 to 53dfd23b043f (39 revisions) flutter/packages#3484

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 17, 2023

96f927f Assert that runApp is called in the same zone as binding.ensu…

077eb3a

…reInitialized (flutter/flutter#117113)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 10, 2023

96f927f Assert that runApp is called in the same zone as binding.ensu…

6700e9b

…reInitialized (flutter/flutter#117113)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 10, 2023

96f927f Assert that runApp is called in the same zone as binding.ensu…

09c4f76

…reInitialized (flutter/flutter#117113)

	/// are encouraged to resolve any issues that case the [debugCheckZone] message
	/// are encouraged to resolve any issues that cause the [debugCheckZone] message

Conversation

Hixie commented Dec 14, 2022

Uh oh!

goderbauer commented Dec 14, 2022

Uh oh!

flutter-dashboard bot commented Jan 4, 2023

Uh oh!

Hixie commented Feb 15, 2023

Uh oh!

goderbauer left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Hixie commented Feb 15, 2023

Uh oh!

auto-submit bot commented Feb 16, 2023

Uh oh!

Hixie commented Feb 17, 2023

Uh oh!

Hixie commented Feb 17, 2023

Uh oh!

goderbauer left a comment

Choose a reason for hiding this comment

Uh oh!

auto-submit bot commented Feb 17, 2023

Uh oh!

auto-submit bot commented Feb 18, 2023

Uh oh!

cbatson left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants