[web] robustify safaridriver launch sequence (#162919)

Improve safaridriver launch sequence as follows:

- Fix retry: the previous version would call `_startDriverProcess`
recursively from within a listener to the stderr output. But by then the
outer `_startDriverProcess` call would have completed its future, so the
retry mechanism would kick in while tests are already running.
- Wait for `safaridriver` server process to start listening and
responding to WebDriver commands (using `/status` as the smoke test).
- Smoke-test the driver session by attempting to list all windows
(`WebDriver.windows`).
- When `safaridriver` fails to pass all of the above checks, both close
the session and kill the `safaridriver` process. Killing the process
alone leaves Safari windows open. Closing the session alone leaves
`safaridriver` processes open.
- When tests are finished use `quit()` instead of `window.close()`,
because the latter does not close the session.
This commit is contained in:
Yegor 2025-02-10 13:27:40 -08:00 committed by GitHub
parent 01c2866780
commit c5fbe83809
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 169 additions and 106 deletions

View File

@ -89,10 +89,11 @@ abstract class ProcessStep implements PipelineStep {
}
class _PipelineStepFailure {
_PipelineStepFailure(this.step, this.error);
_PipelineStepFailure(this.step, this.error, this.stackTrace);
final PipelineStep step;
final Object error;
final StackTrace stackTrace;
}
/// Executes a sequence of asynchronous tasks, typically as part of a build/test
@ -133,8 +134,8 @@ class Pipeline {
_currentStepFuture = step.run();
try {
await _currentStepFuture;
} catch (e) {
failures.add(_PipelineStepFailure(step, e));
} catch (error, stackTrace) {
failures.add(_PipelineStepFailure(step, error, stackTrace));
} finally {
_currentStep = null;
}
@ -145,7 +146,7 @@ class Pipeline {
_status = PipelineStatus.error;
print('Pipeline experienced the following failures:');
for (final _PipelineStepFailure failure in failures) {
print(' "${failure.step.description}": ${failure.error}');
print(' "${failure.step.description}": ${failure.error}\n${failure.stackTrace}');
}
throw ToolExit('Test pipeline failed.');
}

View File

@ -7,11 +7,21 @@ import 'dart:convert';
import 'dart:io';
import 'package:test_api/backend.dart';
import 'package:webdriver/async_io.dart' show WebDriver, createDriver;
import 'browser.dart';
import 'webdriver_browser.dart';
/// Provides an environment for the desktop variant of Safari running on macOS.
class SafariMacOsEnvironment extends WebDriverBrowserEnvironment {
class SafariMacOsEnvironment extends BrowserEnvironment {
static const Duration _waitBetweenRetries = Duration(seconds: 1);
static const int _maxRetryCount = 5;
late int _portNumber;
late Process _driverProcess;
Uri get _driverUri => Uri(scheme: 'http', host: 'localhost', port: _portNumber);
WebDriver? webDriver;
@override
final String name = 'Safari macOS';
@ -21,21 +31,34 @@ class SafariMacOsEnvironment extends WebDriverBrowserEnvironment {
@override
String get packageTestConfigurationYamlFile => 'dart_test_safari.yaml';
@override
Uri get driverUri => Uri(scheme: 'http', host: 'localhost', port: portNumber);
late Process _driverProcess;
int _retryCount = 0;
static const int _waitBetweenRetryInSeconds = 1;
static const int _maxRetryCount = 10;
@override
Future<Process> spawnDriverProcess() =>
Process.start('safaridriver', <String>['-p', portNumber.toString()]);
@override
Future<void> prepare() async {
await _startDriverProcess();
int retryCount = 0;
while (true) {
try {
if (retryCount > 0) {
print('Retry #$retryCount');
}
retryCount += 1;
await _startDriverProcess();
return;
} catch (error, stackTrace) {
if (retryCount < _maxRetryCount) {
print('''
Failed to start safaridriver:
Error: $error
$stackTrace
''');
print('Will try again.');
await Future<void>.delayed(_waitBetweenRetries);
} else {
print('Too many retries. Giving up.');
rethrow;
}
}
}
}
/// Pick an unused port and start `safaridriver` using that port.
@ -45,36 +68,130 @@ class SafariMacOsEnvironment extends WebDriverBrowserEnvironment {
/// again with a different port. Wait [_waitBetweenRetryInSeconds] seconds
/// between retries. Try up to [_maxRetryCount] times.
Future<void> _startDriverProcess() async {
_retryCount += 1;
if (_retryCount > 1) {
await Future<void>.delayed(const Duration(seconds: _waitBetweenRetryInSeconds));
}
portNumber = await pickUnusedPort();
_portNumber = await pickUnusedPort();
print('Starting safaridriver on port $_portNumber');
print('Attempt $_retryCount to start safaridriver on port $portNumber');
try {
_driverProcess = await Process.start('safaridriver', <String>['-p', _portNumber.toString()]);
_driverProcess = await spawnDriverProcess();
_driverProcess.stdout.transform(utf8.decoder).transform(const LineSplitter()).listen((
String log,
) {
print('[safaridriver] $log');
});
_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.stderr.transform(utf8.decoder).transform(const LineSplitter()).listen((
String error,
) {
print('[safaridriver][error] $error');
});
await _waitForSafariDriverServerReady();
// Smoke-test the web driver process by connecting to it and asking for a
// list of windows. It doesn't matter how many windows there are.
webDriver = await createDriver(
uri: _driverUri,
desired: <String, dynamic>{'browserName': packageTestRuntime.identifier},
);
await webDriver!.windows.toList();
} catch (_) {
print('safaridriver failed to start.');
final badDriver = webDriver;
webDriver = null; // let's not keep faulty driver around
if (badDriver != null) {
// This means the launch process got to a point where a WebDriver
// instance was created, but it failed the smoke test. To make sure no
// stray driver sessions are left hanging, try to close the session.
try {
// The method is called "quit" but all it does is close the session.
//
// See: https://www.w3.org/TR/webdriver2/#delete-session
await badDriver.quit();
} catch (error, stackTrace) {
// Just print. Do not rethrow. The attempt to close the session is
// only a best-effort thing.
print('''
Failed to close driver session. Will try to kill the safaridriver process.
Error: $error
$stackTrace
''');
}
}
});
_driverProcess.stdout.transform(utf8.decoder).transform(const LineSplitter()).listen((
String log,
) {
print('[Webdriver] $log');
});
// Try to kill gracefully using SIGTERM first.
_driverProcess.kill();
await _driverProcess.exitCode.timeout(
const Duration(seconds: 2),
onTimeout: () async {
// If the process fails to exit gracefully in a reasonable amount of
// time, kill it forcefully.
print('safaridriver failed to exit normally. Killing with SIGKILL.');
_driverProcess.kill(ProcessSignal.sigkill);
return 0;
},
);
// Rethrow the error to allow the caller to retry, if need be.
rethrow;
}
}
/// The Safari Driver process cannot instantly spawn a server, so this function
/// attempts to connect to the server in a loop until it succeeds.
///
/// A healthy driver process is expected to respond to a `GET /status` HTTP
/// request with `{value: {ready: true}}` JSON response.
///
/// See also: https://www.w3.org/TR/webdriver2/#status
Future<void> _waitForSafariDriverServerReady() async {
// Wait just a tiny bit before connecting for the very first time because
// frequently safaridriver isn't quick enough to bring up the server.
//
// 100ms seems enough in most cases, but feel free to revisit this.
await Future<void>.delayed(const Duration(milliseconds: 100));
int retryCount = 0;
while (true) {
retryCount += 1;
final httpClient = HttpClient();
try {
final request = await httpClient.get('localhost', _portNumber, '/status');
final response = await request.close();
final stringData = await response.transform(utf8.decoder).join();
final jsonResponse = json.decode(stringData) as Map<String, Object?>;
final value = jsonResponse['value']! as Map<String, Object?>;
final ready = value['ready']! as bool;
if (ready) {
break;
}
} catch (_) {
if (retryCount < 10) {
print('safaridriver not ready yet. Waiting...');
await Future<void>.delayed(const Duration(milliseconds: 100));
} else {
print(
'safaridriver failed to reach ready state in a reasonable amount of time. Giving up.',
);
rethrow;
}
}
}
}
@override
Future<Browser> launchBrowserInstance(Uri url, {bool debug = false}) async {
return WebDriverBrowser(webDriver!, url);
}
@override
Future<void> cleanup() async {
// WebDriver.quit() is not called here, because that's done in
// WebDriverBrowser.close().
_driverProcess.kill();
}
}

View File

@ -3,80 +3,25 @@
// found in the LICENSE file.
import 'dart:async';
import 'dart:convert';
import 'dart:io';
import 'dart:math';
import 'package:image/image.dart';
import 'package:webdriver/async_io.dart' show WebDriver, createDriver;
import 'package:webdriver/async_io.dart' show WebDriver;
import 'browser.dart';
abstract class WebDriverBrowserEnvironment extends BrowserEnvironment {
late int portNumber;
late final Process _driverProcess;
Future<Process> spawnDriverProcess();
Uri get driverUri;
/// Finds and returns an unused port on the test host in the local port range.
Future<int> 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);
final int port = socket.port;
await socket.close();
return port;
}
@override
Future<void> prepare() async {
portNumber = await pickUnusedPort();
_driverProcess = await spawnDriverProcess();
_driverProcess.stderr.transform(utf8.decoder).transform(const LineSplitter()).listen((
String error,
) {
print('[Webdriver][Error] $error');
});
_driverProcess.stdout.transform(utf8.decoder).transform(const LineSplitter()).listen((
String log,
) {
print('[Webdriver] $log');
});
}
@override
Future<void> cleanup() async {
_driverProcess.kill();
}
@override
Future<Browser> launchBrowserInstance(Uri url, {bool debug = false}) async {
while (true) {
try {
final WebDriver driver = await createDriver(
uri: driverUri,
desired: <String, dynamic>{'browserName': packageTestRuntime.identifier},
);
return WebDriverBrowser(driver, url);
} on SocketException {
// Sometimes we may try to connect before the web driver port is ready.
// So we should retry here. Note that if there was some issue with the
// webdriver process, we may loop infinitely here, so we're relying on
// the test timeout to kill us if it takes too long to connect.
print('Failed to connect to webdriver process. Retrying in 100 ms');
await Future<void>.delayed(const Duration(milliseconds: 100));
} catch (exception) {
rethrow;
}
}
}
/// Finds and returns an unused port on the test host in the local port range.
Future<int> 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);
final int port = socket.port;
await socket.close();
return port;
}
/// A browser managed through a WebDriver connection.
class WebDriverBrowser extends Browser {
WebDriverBrowser(this._driver, this._url) {
_driver.get(_url);
@ -104,7 +49,7 @@ class WebDriverBrowser extends Browser {
_shouldStopActivating = true;
await _activateLoopFuture;
await (await _driver.window).close();
await _driver.quit();
if (!_onExitCompleter.isCompleted) {
_onExitCompleter.complete();
}