-
Notifications
You must be signed in to change notification settings - Fork 6k
Listen for uncaught exceptions during loading of a web test suite in Chrome #55166
Listen for uncaught exceptions during loading of a web test suite in Chrome #55166
Conversation
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption, contact "@test-exemption-reviewer" in the #hackers channel in Discord (don't just cc them here, they won't see it!). If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. The test exemption team is a small volunteer group, so all reviewers should feel empowered to ask for tests, without delegating that responsibility entirely to the test exemption group. |
cc @osa1 |
Thanks for doing this @jason-simmons, this should save so much time. I'm curious how the failures are reported. Do I see it in the terminal when running the tests as usual via the |
Yes - if an uncaught exception happens while loading the suite then |
…Chrome Without this the test runner will hang if the compiled test is invalid and unable to execute.
d8bf74a
to
008fdea
Compare
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.
controller!.suite | ||
]; | ||
if (_browser.onUncaughtException != null) { | ||
futures.add(_browser.onUncaughtException!.then<RunnerSuite>( |
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.
I think I'd mildly prefer _browser.onUncaughtException
to be a callback function. It would clean up this code. For example, you could simply do:
_browser.onUncaughtException = controller!.onError;
And then you wouldn't need to manage a list of futures.
That said, I don't know what kind of concurrency is going on here. Perhaps, the futures can already be completed by the time this code is reached, and so a callback will have no effect.
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 code is waiting for the completion of either:
controller!.suite
(which completes when the suite has been loaded)_browser.onUncaughtException
(which completes if the browser sees an uncaught exception during loading)
If the suite loading future completes, then this function will return the suite. If the exception future completes, then this function will throw the exception.
…155843) flutter/engine@7c603de...f21f2b2 2024-09-27 [email protected] [docs] Fix broken links in docs/ (flutter/engine#55350) 2024-09-27 [email protected] Roll Skia from e77818421e91 to 7efc11f2ea9e (6 revisions) (flutter/engine#55489) 2024-09-27 [email protected] Listen for uncaught exceptions during loading of a web test suite in Chrome (flutter/engine#55166) 2024-09-27 98614782+auto-submit[bot]@users.noreply.github.com Reverts "[Impeller] hash even less stuff per frame. (#55092)" (flutter/engine#55491) 2024-09-27 [email protected] [web] Update builder json generator to reflect recent changes (flutter/engine#55307) 2024-09-27 [email protected] [Impeller] hash even less stuff per frame. (flutter/engine#55092) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC [email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
…lutter#155843) flutter/engine@7c603de...f21f2b2 2024-09-27 [email protected] [docs] Fix broken links in docs/ (flutter/engine#55350) 2024-09-27 [email protected] Roll Skia from e77818421e91 to 7efc11f2ea9e (6 revisions) (flutter/engine#55489) 2024-09-27 [email protected] Listen for uncaught exceptions during loading of a web test suite in Chrome (flutter/engine#55166) 2024-09-27 98614782+auto-submit[bot]@users.noreply.github.com Reverts "[Impeller] hash even less stuff per frame. (flutter#55092)" (flutter/engine#55491) 2024-09-27 [email protected] [web] Update builder json generator to reflect recent changes (flutter/engine#55307) 2024-09-27 [email protected] [Impeller] hash even less stuff per frame. (flutter/engine#55092) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC [email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Without this the test runner will hang if the compiled test is invalid and unable to execute.