This repository was archived by the owner on Feb 25, 2025. It is now read-only.
Retry when safaridriver fails #48791
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
eyebrowsoffire
left a comment
eyebrowsoffire
left a comment
There was a problem hiding this comment.
Thank you so much for doing this!
| /// "Operation not permitted" error, kill the `safaridriver` process and try | ||
| /// again with a different port. Wait [_waitBetweenRetryInSeconds] seconds | ||
| /// between retries. Try up to [_maxRetryCount] times. | ||
| Future<void> _startDriverProcess() async { |
Contributor
There was a problem hiding this comment.
Is there a reason this isn't just the implementation of spawnDriverProcess rather than overriding prepare?
Contributor
Author
There was a problem hiding this comment.
Not really, mostly just to mimic the prepare function in webdriver_browser.dart. Like picking the port and starting the stderr/stdout listeners happens in prepare rather than the spawnDriverProcess. I suppose I could have just used the prepare function rather that _startDriverProcess. What would you suggest?
^^ I read the question wrong.
Yes, in webdriver_browser.dart prepare, it sets the _driverProcess to the result of spawnDriverProcess, but we want to call spawnDriverProcess and update _driverProcess multiple times depending on the stderr from _driverProcess. We could move the _startDriverProcess to webdriver_browser.dart's prepare, but I wasn't sure how other implementations of WebDriverBrowserEnvironment are used so I thought it safer to make the logic exclusive to SafariMacOsEnvironment.
Contributor
There was a problem hiding this comment.
I see. Thanks for the clarification!
eyebrowsoffire
approved these changes
Dec 7, 2023
Contributor
eyebrowsoffire
left a comment
eyebrowsoffire
left a comment
There was a problem hiding this comment.
LGTM! Thanks again!
| /// "Operation not permitted" error, kill the `safaridriver` process and try | ||
| /// again with a different port. Wait [_waitBetweenRetryInSeconds] seconds | ||
| /// between retries. Try up to [_maxRetryCount] times. | ||
| Future<void> _startDriverProcess() async { |
Contributor
There was a problem hiding this comment.
I see. Thanks for the clarification!
vashworth
added
the
autosubmit
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Dec 7, 2023
auto-submit bot
pushed a commit
to flutter/flutter
that referenced
this pull request
Dec 7, 2023
flutter/engine@82de334...6d9b2fb 2023-12-07 jason-simmons@users.noreply.github.com Revert Dart SDK back to be8a95b6717d (flutter/engine#48799) 2023-12-07 15619084+vashworth@users.noreply.github.com Retry when safaridriver fails (flutter/engine#48791) 2023-12-07 skia-flutter-autoroll@skia.org Roll Skia from f49ec3e6c1b1 to b541f668f531 (2 revisions) (flutter/engine#48796) 2023-12-07 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Update clang revision from 00396e6a1a0b7 to b3a9e8f7c0afb00." (flutter/engine#48802) 2023-12-07 chinmaygarde@google.com Update clang revision from 00396e6a1a0b7 to b3a9e8f7c0afb00. (flutter/engine#48705) 2023-12-07 skia-flutter-autoroll@skia.org Roll Skia from c68e050e9198 to f49ec3e6c1b1 (1 revision) (flutter/engine#48790) 2023-12-07 skia-flutter-autoroll@skia.org Roll Skia from af998f66ae73 to c68e050e9198 (1 revision) (flutter/engine#48788) 2023-12-07 jason-simmons@users.noreply.github.com Reland "Replace use of Fontmgr::RefDefault with explicit creation calls" (flutter/engine#48764) 2023-12-07 skia-flutter-autoroll@skia.org Roll Skia from 308f3d9bef2c to af998f66ae73 (1 revision) (flutter/engine#48784) 2023-12-07 skia-flutter-autoroll@skia.org Roll Skia from 2b33ff5642c3 to 308f3d9bef2c (1 revision) (flutter/engine#48777) 2023-12-07 skia-flutter-autoroll@skia.org Roll Dart SDK from 4b22e6430c20 to b6d5e010d2c5 (1 revision) (flutter/engine#48776) 2023-12-07 skia-flutter-autoroll@skia.org Roll Skia from 0753f680c573 to 2b33ff5642c3 (1 revision) (flutter/engine#48772) 2023-12-07 skia-flutter-autoroll@skia.org Roll Skia from 0f1822e9137a to 0753f680c573 (1 revision) (flutter/engine#48771) 2023-12-07 skia-flutter-autoroll@skia.org Roll Skia from dbcf508d1dce to 0f1822e9137a (1 revision) (flutter/engine#48769) 2023-12-07 skia-flutter-autoroll@skia.org Roll Dart SDK from be8a95b6717d to 4b22e6430c20 (1 revision) (flutter/engine#48768) 2023-12-07 skia-flutter-autoroll@skia.org Roll Skia from 8ebf43ba1c09 to dbcf508d1dce (1 revision) (flutter/engine#48766) 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 chinmaygarde@google.com,rmistry@google.com,zra@google.com 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
vashworth
added a commit
to vashworth/engine
that referenced
this pull request
Dec 14, 2023
Starting `safaridriver` is flakey sometimes on macOS 13. It will occasionally error with "Operation not permitted". As a workaround, if it fails with that message, retry starting `safaridriver`. Fixes flutter/flutter#136972. Example of fix on macOS 13 bot: https://ci.chromium.org/ui/p/flutter/builders/try/Mac%20Engine%20Drone/564967/overview Note: The test is still failing due to flutter/flutter#136279, but you can see it first has error "Operation not permitted" and retries and connects on second attempt. [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Starting
safaridriveris flakey sometimes on macOS 13. It will occasionally error with "Operation not permitted". As a workaround, if it fails with that message, retry startingsafaridriver.Fixes flutter/flutter#136972.
Example of fix on macOS 13 bot: https://ci.chromium.org/ui/p/flutter/builders/try/Mac%20Engine%20Drone/564967/overview
Note: The test is still failing due to flutter/flutter#136279, but you can see it first has error "Operation not permitted" and retries and connects on second attempt.
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.