[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 <whesse@google.com>
Commit-Queue: Alexander Thomas <athom@google.com>

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<String, dynamic> _desiredCapabilities;
 
-  String get driverExecutable;
-  List<String> get driverArguments;
-  Map<String, dynamic> get desiredCapabilities;
+  WebDriverBrowser(this._port, this._desiredCapabilities);
 
   @override
   Future<bool> 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<void> _createDriver(int port) async {
+  Future<void> _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<int> _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<bool> 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 = <String>[];
-  @override
-  final desiredCapabilities = <String, dynamic>{
-    'browserName': 'safari',
-  };
+  Safari(int port)
+      : super(port, {
+          'browserName': 'safari',
+        });
+
   @override
   Future<String> get version async {
     // Example of the file:

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,
+}

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 = <Runtime, WebDriverService>{};
+  static final supportedRuntimes = <Runtime>{
+    Runtime.safari,
+  };
+
+  final String _driverExecutable;
+  final List<String> _driverArguments;
+  Future<void> _started;
+  Process _process;
+
+  ServiceState state = ServiceState.created;
+  final int port;
+
+  WebDriverService(this._driverExecutable, this._driverArguments, this.port);
+
+  Future<void> 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');
+      }
+    });
+  }
+}

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<TestConfiguration> configurations) async {
   var runningBrowserTests =
       configurations.any((config) => config.runtime.isBrowser);
 
-  var serverFutures = <Future>[];
+  var eventListeners = <EventListener>[];
   var testSuites = <TestSuite>[];
+  var serverFutures = <Future>[];
   var maxBrowserProcesses = maxProcesses;
   if (configurations.length > 1 &&
       (configurations[0].testServerPort != 0 ||
@@ -101,9 +103,13 @@ Future testConfigurations(List<TestConfiguration> configurations) async {
     exit(1);
   }
 
+  var services = <WebDriverService>{};
   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<TestConfiguration> 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<TestConfiguration> configurations) async {
     }
   }
 
-  var eventListener = <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<TestConfiguration> 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<TestConfiguration> 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);
 }

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<void> testFirefox() {
   return testBrowser(Firefox(Platform.environment['FIREFOX_PATH']));
 }
 
-Future<void> testSafari() {
-  return testBrowser(Safari());
+Future<void> testSafari() async {
+  var service = WebDriverService.fromRuntime(Runtime.safari);
+  await service.start();
+  await testBrowser(Safari(service.port));
+  service.allDone();
 }
 
 Future<void> testBrowser(Browser browser) async {