From f964af3bf33b4da1cd3f02d209c91673cab0cdc2 Mon Sep 17 00:00:00 2001 From: Alexander Thomas Date: Thu, 5 May 2022 21:49:56 +0000 Subject: [PATCH] [test_runner] Add web driver service This changes safaridriver to be running whenever test.py runs any safari configuration. The service exists when all tests are done. The process can only be used by a single browser but supports many consecutive sessions. Bug: b/208186791 Cq-Include-Trybots: luci.dart.try:dart2js-strong-mac-x64-safari-try Change-Id: I7c1d5910f8c97ae97c1806a07251df918f139e0a Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/243720 Reviewed-by: William Hesse Commit-Queue: Alexander Thomas --- .../lib/src/browser_controller.dart | 63 +++++----------- pkg/test_runner/lib/src/service/service.dart | 11 +++ .../lib/src/service/web_driver_service.dart | 73 +++++++++++++++++++ .../lib/src/test_configurations.dart | 45 +++++++----- .../test/browser_controller_test.dart | 10 ++- 5 files changed, 135 insertions(+), 67 deletions(-) create mode 100644 pkg/test_runner/lib/src/service/service.dart create mode 100644 pkg/test_runner/lib/src/service/web_driver_service.dart diff --git a/pkg/test_runner/lib/src/browser_controller.dart b/pkg/test_runner/lib/src/browser_controller.dart index d7853383969..a6c8b8a9375 100644 --- a/pkg/test_runner/lib/src/browser_controller.dart +++ b/pkg/test_runner/lib/src/browser_controller.dart @@ -12,6 +12,7 @@ import 'package:webdriver/io.dart'; import 'android.dart'; import 'configuration.dart'; import 'path.dart'; +import 'service/web_driver_service.dart'; import 'utils.dart'; typedef BrowserDoneCallback = void Function(BrowserTestOutput output); @@ -75,7 +76,8 @@ abstract class Browser { browser = Chrome(configuration.browserLocation); break; case Runtime.safari: - browser = Safari(); + var service = WebDriverService.fromRuntime(Runtime.safari); + browser = Safari(service.port); break; case Runtime.ie9: case Runtime.ie10: @@ -262,20 +264,15 @@ abstract class Browser { abstract class WebDriverBrowser extends Browser { WebDriver _driver; + final int _port; + final Map _desiredCapabilities; - String get driverExecutable; - List get driverArguments; - Map get desiredCapabilities; + WebDriverBrowser(this._port, this._desiredCapabilities); @override Future start(String url) async { _logEvent('Starting $this browser on: $url'); - var port = await _findUnusedPort(); - if (!await startBrowserProcess( - driverExecutable, ['--port', '$port', ...driverArguments])) { - return false; - } - await _createDriver(port); + await _createDriver(); await _driver.get(url); try { _logEvent('Got version: ${await version}'); @@ -286,14 +283,14 @@ abstract class WebDriverBrowser extends Browser { return true; } - Future _createDriver(int port) async { + Future _createDriver() async { for (var i = 5; i >= 0; i--) { // Give the driver process some time to be ready to accept connections. await Future.delayed(const Duration(seconds: 1)); try { _driver = await createDriver( - uri: Uri.parse('http://localhost:$port/'), - desired: desiredCapabilities); + uri: Uri.parse('http://localhost:$_port/'), + desired: _desiredCapabilities); } catch (error) { if (i > 0) { _logEvent( @@ -308,33 +305,10 @@ abstract class WebDriverBrowser extends Browser { } } - /// Returns a port that is probably, but not definitely, not in use. - /// - /// This has a built-in race condition: another process may bind this port at - /// any time after this call has returned. - static Future _findUnusedPort() async { - int port; - ServerSocket socket; - try { - socket = await ServerSocket.bind(InternetAddress.loopbackIPv6, 0, - v6Only: true); - } on SocketException { - socket = await ServerSocket.bind(InternetAddress.loopbackIPv4, 0); - } - port = socket.port; - await socket.close(); - return port; - } - @override Future close() async { - try { - await _driver?.quit(); - // Give the driver process some time to be quit the browser. - await Future.delayed(const Duration(seconds: 1)); - } finally { - process?.kill(); - } + await _driver?.quit(); + // Give the driver process some time to be quit the browser. return true; } } @@ -343,14 +317,11 @@ class Safari extends WebDriverBrowser { /// We get the safari version by parsing a version file static const versionFile = '/Applications/Safari.app/Contents/version.plist'; - @override - final driverExecutable = '/usr/bin/safaridriver'; - @override - final driverArguments = []; - @override - final desiredCapabilities = { - 'browserName': 'safari', - }; + Safari(int port) + : super(port, { + 'browserName': 'safari', + }); + @override Future get version async { // Example of the file: diff --git a/pkg/test_runner/lib/src/service/service.dart b/pkg/test_runner/lib/src/service/service.dart new file mode 100644 index 00000000000..25ac0b70e2a --- /dev/null +++ b/pkg/test_runner/lib/src/service/service.dart @@ -0,0 +1,11 @@ +// Copyright (c) 2022, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +enum ServiceState { + created, + starting, + running, + failed, + stopped, +} diff --git a/pkg/test_runner/lib/src/service/web_driver_service.dart b/pkg/test_runner/lib/src/service/web_driver_service.dart new file mode 100644 index 00000000000..9719116b081 --- /dev/null +++ b/pkg/test_runner/lib/src/service/web_driver_service.dart @@ -0,0 +1,73 @@ +// Copyright (c) 2022, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +import 'dart:async'; +import 'dart:io'; + +import 'package:smith/smith.dart'; + +import '../test_progress.dart'; +import 'service.dart'; + +const safariDriverPort = 7055; + +class WebDriverService extends EventListener { + static final _instances = {}; + static final supportedRuntimes = { + Runtime.safari, + }; + + final String _driverExecutable; + final List _driverArguments; + Future _started; + Process _process; + + ServiceState state = ServiceState.created; + final int port; + + WebDriverService(this._driverExecutable, this._driverArguments, this.port); + + Future start() { + if (_started != null) { + return _started; + } + return _started = () async { + state = ServiceState.starting; + try { + _process = await Process.start( + _driverExecutable, ['--port', '$port', ..._driverArguments]); + _process.exitCode.then((exitCode) { + if (state != ServiceState.stopped) { + state = ServiceState.failed; + print('$runtimeType stopped unexpectedly: $exitCode'); + } + }); + state = ServiceState.running; + print('Started $runtimeType on port $port'); + } catch (error) { + state = ServiceState.failed; + print('Failed to start $runtimeType: $error'); + rethrow; + } + }(); + } + + @override + void allDone() { + state = ServiceState.stopped; + _process?.kill(); + } + + factory WebDriverService.fromRuntime(Runtime runtime) { + return _instances.putIfAbsent(runtime, () { + switch (runtime) { + case Runtime.safari: + return WebDriverService( + '/usr/bin/safaridriver', [], safariDriverPort); + default: + throw ArgumentError.value(runtime, 'runtime', 'Unsupported runtime'); + } + }); + } +} diff --git a/pkg/test_runner/lib/src/test_configurations.dart b/pkg/test_runner/lib/src/test_configurations.dart index 6d7f1f88b2a..e5157fec0ca 100644 --- a/pkg/test_runner/lib/src/test_configurations.dart +++ b/pkg/test_runner/lib/src/test_configurations.dart @@ -13,6 +13,7 @@ import 'configuration.dart'; import 'fuchsia.dart'; import 'path.dart'; import 'process_queue.dart'; +import 'service/web_driver_service.dart'; import 'terminal.dart'; import 'test_progress.dart'; import 'test_suite.dart'; @@ -89,8 +90,9 @@ Future testConfigurations(List configurations) async { var runningBrowserTests = configurations.any((config) => config.runtime.isBrowser); - var serverFutures = []; + var eventListeners = []; var testSuites = []; + var serverFutures = []; var maxBrowserProcesses = maxProcesses; if (configurations.length > 1 && (configurations[0].testServerPort != 0 || @@ -101,9 +103,13 @@ Future testConfigurations(List configurations) async { exit(1); } + var services = {}; for (var configuration in configurations) { if (!listTests && !listStatusFiles && runningBrowserTests) { serverFutures.add(configuration.startServers()); + if (WebDriverService.supportedRuntimes.contains(configuration.runtime)) { + services.add(WebDriverService.fromRuntime(configuration.runtime)); + } } if (configuration.runtime.isIE) { @@ -170,6 +176,11 @@ Future testConfigurations(List configurations) async { } } + for (var service in services) { + serverFutures.add(service.start()); + eventListeners.add(service); + } + // If we only need to print out status files for test suites // we return from running here and just print. if (firstConf.listStatusFiles) { @@ -194,8 +205,6 @@ Future testConfigurations(List configurations) async { } } - var eventListener = []; - // We don't print progress if we list tests. if (progress != Progress.silent && !listTests) { var formatter = Formatter.normal; @@ -204,53 +213,53 @@ Future testConfigurations(List configurations) async { formatter = Formatter.color; } - eventListener.add(SummaryPrinter()); + eventListeners.add(SummaryPrinter()); if (!firstConf.silentFailures) { - eventListener.add(TestFailurePrinter(formatter)); + eventListeners.add(TestFailurePrinter(formatter)); } if (firstConf.printPassingStdout) { - eventListener.add(PassingStdoutPrinter(formatter)); + eventListeners.add(PassingStdoutPrinter(formatter)); } var indicator = ProgressIndicator.fromProgress(progress, startTime, formatter); - if (indicator != null) eventListener.add(indicator); + if (indicator != null) eventListeners.add(indicator); if (printTiming) { - eventListener.add(TimingPrinter(startTime)); + eventListeners.add(TimingPrinter(startTime)); } - eventListener.add(SkippedCompilationsPrinter()); + eventListeners.add(SkippedCompilationsPrinter()); if (progress == Progress.status) { - eventListener.add(TimedProgressPrinter()); + eventListeners.add(TimedProgressPrinter()); } if (firstConf.reportFailures) { - eventListener.add(FailedTestsPrinter()); + eventListeners.add(FailedTestsPrinter()); } - eventListener.add(ResultCountPrinter(formatter)); + eventListeners.add(ResultCountPrinter(formatter)); } if (firstConf.writeResults) { - eventListener.add(ResultWriter(firstConf.outputDirectory)); + eventListeners.add(ResultWriter(firstConf.outputDirectory)); } if (firstConf.copyCoreDumps) { - eventListener.add(UnexpectedCrashLogger()); + eventListeners.add(UnexpectedCrashLogger()); } // The only progress indicator when listing tests should be the // the summary printer. if (listTests) { - eventListener.add(SummaryPrinter(jsonOnly: reportInJson)); + eventListeners.add(SummaryPrinter(jsonOnly: reportInJson)); } else { if (!firstConf.cleanExit) { - eventListener.add(ExitCodeSetter()); + eventListeners.add(ExitCodeSetter()); } - eventListener.add(IgnoredTestMonitor()); + eventListeners.add(IgnoredTestMonitor()); } // If any of the configurations need to access android devices we'll first @@ -270,5 +279,5 @@ Future testConfigurations(List configurations) async { // [firstConf] is needed here, because the ProcessQueue uses some settings. ProcessQueue(firstConf, maxProcesses, maxBrowserProcesses, testSuites, - eventListener, allTestsFinished, verbose, adbDevicePool); + eventListeners, allTestsFinished, verbose, adbDevicePool); } diff --git a/pkg/test_runner/test/browser_controller_test.dart b/pkg/test_runner/test/browser_controller_test.dart index c801de2d39b..b9738e0aa34 100644 --- a/pkg/test_runner/test/browser_controller_test.dart +++ b/pkg/test_runner/test/browser_controller_test.dart @@ -5,8 +5,9 @@ import 'dart:io'; import 'package:expect/expect.dart'; - +import 'package:smith/smith.dart'; import 'package:test_runner/src/browser_controller.dart'; +import 'package:test_runner/src/service/web_driver_service.dart'; void main() async { if (Platform.environment.containsKey('CHROME_PATH')) { @@ -31,8 +32,11 @@ Future testFirefox() { return testBrowser(Firefox(Platform.environment['FIREFOX_PATH'])); } -Future testSafari() { - return testBrowser(Safari()); +Future testSafari() async { + var service = WebDriverService.fromRuntime(Runtime.safari); + await service.start(); + await testBrowser(Safari(service.port)); + service.allDone(); } Future testBrowser(Browser browser) async {