From 12c74c7cf1f6247a854c073ba94d8b6511bc3b5c Mon Sep 17 00:00:00 2001 From: Victoria Ashworth Date: Thu, 7 Dec 2023 12:08:11 -0600 Subject: [PATCH 1/2] Retry when safaridriver fails --- ci/builders/linux_web_engine.json | 10 ++--- lib/web_ui/dev/generate_builder_json.dart | 2 +- lib/web_ui/dev/safari_macos.dart | 51 +++++++++++++++++++++++ lib/web_ui/dev/webdriver_browser.dart | 6 +-- 4 files changed, 60 insertions(+), 9 deletions(-) diff --git a/ci/builders/linux_web_engine.json b/ci/builders/linux_web_engine.json index 8df13d4fb1d7a..1e6a18f7d525c 100644 --- a/ci/builders/linux_web_engine.json +++ b/ci/builders/linux_web_engine.json @@ -1095,7 +1095,7 @@ "recipe": "engine_v2/tester_engine", "drone_dimensions": [ "device_type=none", - "os=Mac-12" + "os=Mac-13" ], "gclient_variables": { "download_android_deps": false @@ -1127,7 +1127,7 @@ "recipe": "engine_v2/tester_engine", "drone_dimensions": [ "device_type=none", - "os=Mac-12" + "os=Mac-13" ], "gclient_variables": { "download_android_deps": false @@ -1159,7 +1159,7 @@ "recipe": "engine_v2/tester_engine", "drone_dimensions": [ "device_type=none", - "os=Mac-12" + "os=Mac-13" ], "gclient_variables": { "download_android_deps": false @@ -1191,7 +1191,7 @@ "recipe": "engine_v2/tester_engine", "drone_dimensions": [ "device_type=none", - "os=Mac-12" + "os=Mac-13" ], "gclient_variables": { "download_android_deps": false @@ -1223,7 +1223,7 @@ "recipe": "engine_v2/tester_engine", "drone_dimensions": [ "device_type=none", - "os=Mac-12" + "os=Mac-13" ], "gclient_variables": { "download_android_deps": false diff --git a/lib/web_ui/dev/generate_builder_json.dart b/lib/web_ui/dev/generate_builder_json.dart index 4cfa562334cda..bd7d1438d7165 100644 --- a/lib/web_ui/dev/generate_builder_json.dart +++ b/lib/web_ui/dev/generate_builder_json.dart @@ -116,7 +116,7 @@ Iterable _getAllTestSteps(List suites) { // TODO(jacksongardner): Stop filtering to Mac-12 after macOS 13 issues are fixed: // https://github.com/flutter/flutter/issues/136274, // https://github.com/flutter/flutter/issues/136279 - ..._getTestStepsForPlatform(suites, 'Mac', specificOS: 'Mac-12', (TestSuite suite) => + ..._getTestStepsForPlatform(suites, 'Mac', specificOS: 'Mac-13', (TestSuite suite) => suite.runConfig.browser == BrowserName.safari ), ..._getTestStepsForPlatform(suites, 'Windows', (TestSuite suite) => diff --git a/lib/web_ui/dev/safari_macos.dart b/lib/web_ui/dev/safari_macos.dart index bb0727b8ec865..b7fbd62f3a81b 100644 --- a/lib/web_ui/dev/safari_macos.dart +++ b/lib/web_ui/dev/safari_macos.dart @@ -3,6 +3,7 @@ // found in the LICENSE file. import 'dart:async'; +import 'dart:convert'; import 'dart:io'; import 'package:test_api/src/backend/runtime.dart'; @@ -23,6 +24,56 @@ class SafariMacOsEnvironment extends WebDriverBrowserEnvironment { @override Uri get driverUri => Uri(scheme: 'http', host: 'localhost', port: portNumber); + late Process _driverProcess; + int _retryCount = 0; + static const int _waitBetweenRetry = 1; + static const int _maxRetryCount = 5; + @override Future spawnDriverProcess() => Process.start('safaridriver', ['-p', portNumber.toString()]); + + @override + Future prepare() async { + await _startDriverProcess(); + } + + /// Pick an unused port and start `safaridriver` using that port. + /// + /// On macOS 13, starting `safaridriver` can be flakey so if it returns an + /// "Operation not permitted" error, kill the `safaridriver` process and try again. + Future _startDriverProcess() async { + _retryCount += 1; + if (_retryCount > 1) { + await Future.delayed(const Duration(seconds: _waitBetweenRetry)); + } + portNumber = await pickUnusedPort(); + + print('Attempt $_retryCount to start safaridriver on port $portNumber'); + + _driverProcess = await spawnDriverProcess(); + + _driverProcess.stderr + .transform(utf8.decoder) + .transform(const LineSplitter()) + .listen((String error) { + print('[Webdriver][Error] $error'); + if (_retryCount > _maxRetryCount) { + print('[Webdriver][Error] Failed to start after $_maxRetryCount tries.'); + } else if (error.contains('Operation not permitted')) { + _driverProcess.kill(); + _startDriverProcess(); + } + }); + _driverProcess.stdout + .transform(utf8.decoder) + .transform(const LineSplitter()) + .listen((String log) { + print('[Webdriver] $log'); + }); + } + + @override + Future cleanup() async { + _driverProcess.kill(); + } } diff --git a/lib/web_ui/dev/webdriver_browser.dart b/lib/web_ui/dev/webdriver_browser.dart index ef27394127fa7..024a04580a40b 100644 --- a/lib/web_ui/dev/webdriver_browser.dart +++ b/lib/web_ui/dev/webdriver_browser.dart @@ -13,14 +13,14 @@ import 'package:webdriver/async_io.dart' show WebDriver, createDriver; import 'browser.dart'; abstract class WebDriverBrowserEnvironment extends BrowserEnvironment { - late final int portNumber; + late int portNumber; late final Process _driverProcess; Future spawnDriverProcess(); Uri get driverUri; /// Finds and returns an unused port on the test host in the local port range. - Future _pickUnusedPort() async { + Future pickUnusedPort() async { // Use bind to allocate an unused port, then unbind from that port to // make it available for use. final ServerSocket socket = await ServerSocket.bind('localhost', 0); @@ -33,7 +33,7 @@ abstract class WebDriverBrowserEnvironment extends BrowserEnvironment { @override Future prepare() async { - portNumber = await _pickUnusedPort(); + portNumber = await pickUnusedPort(); _driverProcess = await spawnDriverProcess(); From c2a7edf781a18aa7e18fc46ebd2043198432a211 Mon Sep 17 00:00:00 2001 From: Victoria Ashworth Date: Thu, 7 Dec 2023 12:47:19 -0600 Subject: [PATCH 2/2] revert config and update retry count --- ci/builders/linux_web_engine.json | 10 +++++----- lib/web_ui/dev/generate_builder_json.dart | 2 +- lib/web_ui/dev/safari_macos.dart | 12 +++++++----- 3 files changed, 13 insertions(+), 11 deletions(-) diff --git a/ci/builders/linux_web_engine.json b/ci/builders/linux_web_engine.json index 1e6a18f7d525c..8df13d4fb1d7a 100644 --- a/ci/builders/linux_web_engine.json +++ b/ci/builders/linux_web_engine.json @@ -1095,7 +1095,7 @@ "recipe": "engine_v2/tester_engine", "drone_dimensions": [ "device_type=none", - "os=Mac-13" + "os=Mac-12" ], "gclient_variables": { "download_android_deps": false @@ -1127,7 +1127,7 @@ "recipe": "engine_v2/tester_engine", "drone_dimensions": [ "device_type=none", - "os=Mac-13" + "os=Mac-12" ], "gclient_variables": { "download_android_deps": false @@ -1159,7 +1159,7 @@ "recipe": "engine_v2/tester_engine", "drone_dimensions": [ "device_type=none", - "os=Mac-13" + "os=Mac-12" ], "gclient_variables": { "download_android_deps": false @@ -1191,7 +1191,7 @@ "recipe": "engine_v2/tester_engine", "drone_dimensions": [ "device_type=none", - "os=Mac-13" + "os=Mac-12" ], "gclient_variables": { "download_android_deps": false @@ -1223,7 +1223,7 @@ "recipe": "engine_v2/tester_engine", "drone_dimensions": [ "device_type=none", - "os=Mac-13" + "os=Mac-12" ], "gclient_variables": { "download_android_deps": false diff --git a/lib/web_ui/dev/generate_builder_json.dart b/lib/web_ui/dev/generate_builder_json.dart index bd7d1438d7165..4cfa562334cda 100644 --- a/lib/web_ui/dev/generate_builder_json.dart +++ b/lib/web_ui/dev/generate_builder_json.dart @@ -116,7 +116,7 @@ Iterable _getAllTestSteps(List suites) { // TODO(jacksongardner): Stop filtering to Mac-12 after macOS 13 issues are fixed: // https://github.com/flutter/flutter/issues/136274, // https://github.com/flutter/flutter/issues/136279 - ..._getTestStepsForPlatform(suites, 'Mac', specificOS: 'Mac-13', (TestSuite suite) => + ..._getTestStepsForPlatform(suites, 'Mac', specificOS: 'Mac-12', (TestSuite suite) => suite.runConfig.browser == BrowserName.safari ), ..._getTestStepsForPlatform(suites, 'Windows', (TestSuite suite) => diff --git a/lib/web_ui/dev/safari_macos.dart b/lib/web_ui/dev/safari_macos.dart index b7fbd62f3a81b..5e5329a934bb3 100644 --- a/lib/web_ui/dev/safari_macos.dart +++ b/lib/web_ui/dev/safari_macos.dart @@ -26,8 +26,8 @@ class SafariMacOsEnvironment extends WebDriverBrowserEnvironment { late Process _driverProcess; int _retryCount = 0; - static const int _waitBetweenRetry = 1; - static const int _maxRetryCount = 5; + static const int _waitBetweenRetryInSeconds = 1; + static const int _maxRetryCount = 10; @override Future spawnDriverProcess() => Process.start('safaridriver', ['-p', portNumber.toString()]); @@ -39,12 +39,14 @@ class SafariMacOsEnvironment extends WebDriverBrowserEnvironment { /// Pick an unused port and start `safaridriver` using that port. /// - /// On macOS 13, starting `safaridriver` can be flakey so if it returns an - /// "Operation not permitted" error, kill the `safaridriver` process and try again. + /// On macOS 13, starting `safaridriver` can be flaky so if it returns an + /// "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 _startDriverProcess() async { _retryCount += 1; if (_retryCount > 1) { - await Future.delayed(const Duration(seconds: _waitBetweenRetry)); + await Future.delayed(const Duration(seconds: _waitBetweenRetryInSeconds)); } portNumber = await pickUnusedPort();