[web] Running integration tests on Safari on Local (#18488)

* rebase. carry driver functionality to drivermanager

* rebase. nabling integration tests on Safari on MacOS

* addressing reviewer comments, updating web_drivers dependency commit number

* addressing reviewer comments

* addressing reviewer comments

lib/web_ui/dev/driver_manager.dart

+// Copyright 2013 The Flutter Authors. 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:io' as io;
+
+import 'package:meta/meta.dart';
+import 'package:path/path.dart' as pathlib;
+import 'package:web_driver_installer/chrome_driver_installer.dart';
+import 'package:web_driver_installer/safari_driver_runner.dart';
+
+import 'chrome_installer.dart';
+import 'common.dart';
+import 'environment.dart';
+import 'utils.dart';
+
+/// [DriverManager] implementation for Chrome.
+///
+/// This manager can be used for both MacOS and Linux.
+class ChromeDriverManager extends DriverManager {
+  ChromeDriverManager(String browser) : super(browser);
+
+  Future<void> _installDriver() async {
+    if (_browserDriverDir.existsSync()) {
+      _browserDriverDir.deleteSync(recursive: true);
+    }
+
+    _browserDriverDir.createSync(recursive: true);
+    temporaryDirectories.add(_drivers);
+
+    final io.Directory temp = io.Directory.current;
+    io.Directory.current = _browserDriverDir;
+
+    try {
+      // TODO(nurhan): https://github.com/flutter/flutter/issues/53179
+      final String chromeDriverVersion = await queryChromeDriverVersion();
+      ChromeDriverInstaller chromeDriverInstaller =
+          ChromeDriverInstaller.withVersion(chromeDriverVersion);
+      await chromeDriverInstaller.install(alwaysInstall: true);
+    } finally {
+      io.Directory.current = temp;
+    }
+  }
+
+  /// Throw an error if driver directory does not exists.
+  ///
+  /// Driver should already exist on LUCI as a CIPD package.
+  Future<void> _verifyDriverForLUCI() {
+    if (!_browserDriverDir.existsSync()) {
+      throw StateError('Failed to locate Chrome driver on LUCI on path:'
+          '${_browserDriverDir.path}');
+    }
+    return Future<void>.value();
+  }
+
+  Future<void> _startDriver(String driverPath) async {
+    await startProcess('./chromedriver/chromedriver', ['--port=4444'],
+        workingDirectory: driverPath);
+    print('INFO: Driver started');
+  }
+}
+
+/// [DriverManager] implementation for Safari.
+///
+/// This manager is will only be created/used for MacOS.
+class SafariDriverManager extends DriverManager {
+  SafariDriverManager(String browser) : super(browser);
+
+  Future<void> _installDriver() {
+    // No-op.
+    // macOS comes with Safari Driver installed.
+    return new Future<void>.value();
+  }
+
+  Future<void> _verifyDriverForLUCI() {
+    // No-op.
+    // macOS comes with Safari Driver installed.
+    return Future<void>.value();
+  }
+
+  Future<void> _startDriver(String driverPath) async {
+    final SafariDriverRunner safariDriverRunner = SafariDriverRunner();
+
+    final io.Process process =
+        await safariDriverRunner.runDriver(version: 'system');
+
+    processesToCleanUp.add(process);
+  }
+}
+
+/// Abstract class for preparing the browser driver before running the integration
+/// tests.
+abstract class DriverManager {
+  /// Installation directory for browser's driver.
+  @protected
+  final io.Directory _browserDriverDir;
+
+  /// This is the parent directory for all drivers.
+  ///
+  /// This directory is saved to [temporaryDirectories] and deleted before
+  /// tests shutdown.
+  @protected
+  final io.Directory _drivers;
+
+  DriverManager(String browser)
+      : this._browserDriverDir = io.Directory(pathlib.join(
+            environment.webUiDartToolDir.path,
+            'drivers',
+            browser,
+            '${browser}driver-${io.Platform.operatingSystem.toString()}')),
+        this._drivers = io.Directory(
+            pathlib.join(environment.webUiDartToolDir.path, 'drivers'));
+
+  Future<void> prepareDriver() async {
+    if (!isLuci) {
+      // LUCI installs driver from CIPD, so we skip installing it on LUCI.
+      await _installDriver();
+    } else {
+      await _verifyDriverForLUCI();
+    }
+    await _startDriver(_browserDriverDir.path);
+  }
+
+  /// Always re-install since driver can change frequently.
+  /// It usually changes with each the browser version changes.
+  /// A better solution would be installing the browser and the driver at the
+  /// same time.
+  /// TODO(nurhan): https://github.com/flutter/flutter/issues/53179. Partly
+  // solved. Remaining local integration tests using the locked Chrome version.
+  Future<void> _installDriver();
+
+  Future<void> _verifyDriverForLUCI();
+
+  @protected
+  Future<void> _startDriver(String driverPath);
+
+  static DriverManager chooseDriver(String browser) {
+    if (browser == 'chrome') {
+      return ChromeDriverManager(browser);
+    } else if (browser == 'safari' && io.Platform.isMacOS) {
+      return SafariDriverManager(browser);
+    } else {
+      throw StateError('Integration tests are only supported on Chrome or '
+          'on Safari (running on MacOS)');
+    }
+  }
+}

lib/web_ui/dev/integration_tests_manager.dart

@@ -6,58 +6,33 @@
 
 import 'dart:io' as io;
 import 'package:path/path.dart' as pathlib;
-import 'package:web_driver_installer/chrome_driver_installer.dart';
 
 import 'chrome_installer.dart';
+import 'driver_manager.dart';
 import 'environment.dart';
 import 'exceptions.dart';
 import 'common.dart';
 import 'utils.dart';
 
+const String _unsupportedConfigurationWarning = 'WARNING: integration '
+    'tests are only supported on Chrome or on Safari (running on MacOS)';
+
 class IntegrationTestsManager {
   final String _browser;
 
-  /// Installation directory for browser's driver.
-  ///
-  /// Always re-install since driver can change frequently.
-  /// It usually changes with each the browser version changes.
-  /// A better solution would be installing the browser and the driver at the
-  /// same time.
-  // TODO(nurhan): https://github.com/flutter/flutter/issues/53179. Partly
-  // solved. Remaining local integration tests using the locked Chrome version.
-  final io.Directory _browserDriverDir;
-
-  /// This is the parent directory for all drivers.
-  ///
-  /// This directory is saved to [temporaryDirectories] and deleted before
-  /// tests shutdown.
-  final io.Directory _drivers;
-
   final bool _useSystemFlutter;
 
+  final DriverManager _driverManager;
+
   IntegrationTestsManager(this._browser, this._useSystemFlutter)
-      : this._browserDriverDir = io.Directory(pathlib.join(
-            environment.webUiDartToolDir.path,
-            'drivers',
-            _browser,
-            '${_browser}driver-${io.Platform.operatingSystem.toString()}')),
-        this._drivers = io.Directory(
-            pathlib.join(environment.webUiDartToolDir.path, 'drivers'));
+      : _driverManager = DriverManager.chooseDriver(_browser);
 
   Future<bool> runTests() async {
-    if (_browser != 'chrome') {
-      print('WARNING: integration tests are only supported on chrome for now');
-      return false;
-    } else {
-      if (!isLuci) {
-        // LUCI installs driver from CIPD, so we skip installing it on LUCI.
-        await _prepareDriver();
-      } else {
-        _verifyDriverForLUCI();
-      }
-      await _startDriver(_browserDriverDir.path);
-      // TODO(nurhan): https://github.com/flutter/flutter/issues/52987
+    if (validateIfTestsShouldRun()) {
+      await _driverManager.prepareDriver();
       return await _runTests();
+    } else {
+      return false;
     }
   }
 
@@ -101,44 +76,6 @@ class IntegrationTestsManager {
         useSystemFlutter: _useSystemFlutter);
   }
 
-  /// Driver should already exist on LUCI as a CIPD package.
-  ///
-  /// Throw an error if directory does not exists.
-  void _verifyDriverForLUCI() {
-    if (!_browserDriverDir.existsSync()) {
-      throw StateError('Failed to locate Chrome driver on LUCI on path:'
-          '${_browserDriverDir.path}');
-    }
-  }
-
-  Future<void> _startDriver(String workingDirectory) async {
-    await startProcess('./chromedriver/chromedriver', ['--port=4444'],
-        workingDirectory: workingDirectory);
-    print('INFO: Driver started');
-  }
-
-  Future<void> _prepareDriver() async {
-    if (_browserDriverDir.existsSync()) {
-      _browserDriverDir.deleteSync(recursive: true);
-    }
-
-    _browserDriverDir.createSync(recursive: true);
-    temporaryDirectories.add(_drivers);
-
-    io.Directory temp = io.Directory.current;
-    io.Directory.current = _browserDriverDir;
-
-    // TODO(nurhan): https://github.com/flutter/flutter/issues/53179
-    final String chromeDriverVersion = await queryChromeDriverVersion();
-    ChromeDriverInstaller chromeDriverInstaller =
-        ChromeDriverInstaller.withVersion(chromeDriverVersion);
-    // TODO(yjbanov): remove this dynamic hack when chromeDriverInstaller.install returns Future<void>
-    //                https://github.com/flutter/flutter/issues/59376
-    final dynamic installationFuture = chromeDriverInstaller.install(alwaysInstall: true) as dynamic;
-    await installationFuture;
-    io.Directory.current = temp;
-  }
-
   /// Runs all the web tests under e2e_tests/web.
   Future<bool> _runTests() async {
     // Only list the files under e2e_tests/web.
@@ -224,34 +161,19 @@ class IntegrationTestsManager {
       io.Directory directory, String testName) async {
     final String executable =
         _useSystemFlutter ? 'flutter' : environment.flutterCommand.path;
+    final IntegrationArguments arguments =
+        IntegrationArguments.fromBrowser(_browser);
     final int exitCode = await runProcess(
       executable,
-      <String>[
-        'drive',
-        '--target=test_driver/${testName}',
-        '-d',
-        'web-server',
-        '--profile',
-        '--browser-name=$_browser',
-        if (isLuci) '--chrome-binary=${preinstalledChromeExecutable()}',
-        if (isLuci) '--headless',
-        '--local-engine=host_debug_unopt',
-      ],
+      arguments.getTestArguments(testName, 'profile'),
       workingDirectory: directory.path,
     );
 
     if (exitCode != 0) {
-      String statementToRun = 'flutter drive '
-          '--target=test_driver/${testName} -d web-server --profile '
-          '--browser-name=$_browser --local-engine=host_debug_unopt';
-      if (isLuci) {
-        statementToRun = '$statementToRun --chrome-binary='
-            '${preinstalledChromeExecutable()}';
-      }
       io.stderr
           .writeln('ERROR: Failed to run test. Exited with exit code $exitCode'
-              '. Statement to run $testName locally use the following '
-              'command:\n\n$statementToRun');
+              '. To run $testName locally use the following command:'
+              '\n\n${arguments.getCommandToRun(testName, 'profile')}');
       return false;
     } else {
       return true;
@@ -359,9 +281,92 @@ class IntegrationTestsManager {
           'further instructions');
     }
   }
+
+  /// Validate the given `browser`, `platform` combination is suitable for
+  /// integration tests to run.
+  bool validateIfTestsShouldRun() {
+    // Chrome tests should run at all Platforms (Linux, MacOS, Windows).
+    // They can also run successfully on CI and local.
+    if (_browser == 'chrome') {
+      return true;
+    } else if (_browser == 'safari' && io.Platform.isMacOS && !isLuci) {
+      return true;
+    } else {
+      io.stderr.writeln(_unsupportedConfigurationWarning);
+      return false;
+    }
+  }
+}
+
+/// Interface for collecting arguments to give `flutter drive` to run the
+/// integration tests.
+abstract class IntegrationArguments {
+  IntegrationArguments();
+
+  factory IntegrationArguments.fromBrowser(String browser) {
+    if (browser == 'chrome') {
+      return ChromeIntegrationArguments();
+    } else if (browser == 'safari' && io.Platform.isMacOS) {
+      return SafariIntegrationArguments();
+    } else {
+      throw StateError(_unsupportedConfigurationWarning);
+    }
+  }
+
+  List<String> getTestArguments(String testName, String mode);
+
+  String getCommandToRun(String testName, String mode);
+}
+
+/// Arguments to give `flutter drive` to run the integration tests on Chrome.
+class ChromeIntegrationArguments extends IntegrationArguments {
+  List<String> getTestArguments(String testName, String mode) {
+    return <String>[
+      'drive',
+      '--target=test_driver/${testName}',
+      '-d',
+      'web-server',
+      '--$mode',
+      '--browser-name=chrome',
+      if (isLuci) '--chrome-binary=${preinstalledChromeExecutable()}',
+      if (isLuci) '--headless',
+      '--local-engine=host_debug_unopt',
+    ];
+  }
+
+  String getCommandToRun(String testName, String mode) {
+    String statementToRun = 'flutter drive '
+        '--target=test_driver/${testName} -d web-server --profile '
+        '--browser-name=chrome --local-engine=host_debug_unopt';
+    if (isLuci) {
+      statementToRun = '$statementToRun --chrome-binary='
+          '${preinstalledChromeExecutable()}';
+    }
+    return statementToRun;
+  }
 }
 
-/// Prepares a key for the [blockedTests] map.
+/// Arguments to give `flutter drive` to run the integration tests on Safari.
+class SafariIntegrationArguments extends IntegrationArguments {
+  SafariIntegrationArguments();
+
+  List<String> getTestArguments(String testName, String mode) {
+    return <String>[
+      'drive',
+      '--target=test_driver/${testName}',
+      '-d',
+      'web-server',
+      '--$mode',
+      '--browser-name=safari',
+      '--local-engine=host_debug_unopt',
+    ];
+  }
+
+  String getCommandToRun(String testName, String mode) =>
+      'flutter ${getTestArguments(testName, mode).join(' ')}';
+}
+
+/// Prepares a key for the [blackList] map.
 ///
 /// Uses the browser name and the operating system name.
 String getBlockedTestsListMapKey(String browser) =>
@@ -387,4 +392,9 @@ const Map<String, List<String>> blockedTestsListsMap = <String, List<String>>{
     'target_platform_ios_e2e.dart',
     'target_platform_android_e2e.dart',
   ],
+  'safari-macos': [
+    'target_platform_ios_e2e.dart',
+    'target_platform_android_e2e.dart',
+    'image_loading_e2e.dart',
+  ],
 };

lib/web_ui/dev/test_runner.dart

@@ -106,7 +106,7 @@ class TestCommand extends Command<bool> with ArgUtils {
       print('Running the unit tests only');
       return TestTypesRequested.unit;
     } else if (boolArg('integration-tests-only')) {
-      if (!isChrome) {
+      if (!isChrome && !isSafariOnMacOS) {
         throw UnimplementedError(
             'Integration tests are only available on Chrome Desktop for now');
       }
@@ -132,7 +132,7 @@ class TestCommand extends Command<bool> with ArgUtils {
       case TestTypesRequested.all:
         // TODO(nurhan): https://github.com/flutter/flutter/issues/53322
         // TODO(nurhan): Expand browser matrix for felt integration tests.
-        if (runAllTests && isChrome) {
+        if (runAllTests && (isChrome || isSafariOnMacOS)) {
           bool unitTestResult = await runUnitTests();
           bool integrationTestResult = await runIntegrationTests();
           if (integrationTestResult != unitTestResult) {
@@ -263,6 +263,10 @@ class TestCommand extends Command<bool> with ArgUtils {
   /// Whether [browser] is set to "chrome".
   bool get isChrome => browser == 'chrome';
 
+  /// Whether [browser] is set to "safari".
+  bool get isSafariOnMacOS => browser == 'safari'
+      && io.Platform.isMacOS;
+
   /// Use system flutter instead of cloning the repository.
   ///
   /// Read the flag help for more details. Uses PATH to locate flutter.

lib/web_ui/pubspec.yaml

@@ -32,4 +32,4 @@ dev_dependencies:
     git:
       url: git://github.com/flutter/web_installers.git
       path: packages/web_drivers/
-      ref: 90c69a79b2764c93875dc8ed4f4932c27a6f3a86
+      ref: 41f96bb55d2f064dac3c9fc727ebdf4b0cdf79c4