Add support for multiple full paths on macos (#2276)

We cannot look up multiple basename commands in the system path and the
current `macOsExecutable` configuration may have existing uses in
`dart_test.yaml` files so it isn't safe to require full paths. Add a
separate `macOsAbsolutePaths` configuration to enable internal
definitions that check multiple full paths and execute the first one
that exists.

Add the basename `firefox` as a fallback. Adding that command to the
path is one workaround for users with firefox installed in an unexpected
location. This is also the only approach that is compatible with mac on
GitHub actions using the `setup-firefox` action.

There is no support here for `dart_test.yaml`. Users that are
configuring an executable for macOS will continue to have support for
only a single value, even if they are specifying an absolute path.

Tests remain skipped because our mono repo setup does not have a way to
include the `setup-firefox` action.

---------

Co-authored-by: Jacob MacDonald <[email protected]>

Author: natebosch

pkgs/test/CHANGELOG.md

@@ -3,6 +3,7 @@
 * Fix dart2wasm tests on windows.
 * Increase SDK constraint to ^3.5.0-311.0.dev.
 * Support running Node.js tests compiled with dart2wasm.
+* Allow `firefox` or `firefox-bin` executable name on macOS.
 
 ## 1.25.8
 

pkgs/test/lib/src/runner/browser/default_settings.dart

@@ -24,7 +24,11 @@ final defaultSettings = UnmodifiableMapView({
   ),
   Runtime.firefox: ExecutableSettings(
       linuxExecutable: 'firefox',
-      macOSExecutable: '/Applications/Firefox.app/Contents/MacOS/firefox-bin',
+      macOSExecutables: [
+        '/Applications/Firefox.app/Contents/MacOS/firefox-bin',
+        '/Applications/Firefox.app/Contents/MacOS/firefox',
+        'firefox',
+      ],
       windowsExecutable: r'Mozilla Firefox\firefox.exe',
       environmentOverride: 'FIREFOX_EXECUTABLE'),
   Runtime.safari: ExecutableSettings(

pkgs/test/lib/src/runner/executable_settings.dart

@@ -20,11 +20,14 @@ class ExecutableSettings {
   /// looked up on the system path. It may not be relative.
   final String? _linuxExecutable;
 
-  /// The path to the executable on Mac OS.
+  /// Potential commands to execute on Mac OS to launch this executable.
   ///
-  /// This may be an absolute path or a basename, in which case it will be
-  /// looked up on the system path. It may not be relative.
-  final String? _macOSExecutable;
+  /// The values may be an absolute path, or a basename.
+  /// The chosen command will be the first value from this list which either is
+  /// a full path that exists, or is a basename. When a basename is included it
+  /// should always be at the end of the list because it will always terminate
+  /// the search through the list. Relative paths are not allowed.
+  final List<String>? _macOSExectuables;
 
   /// The path to the executable on Windows.
   ///
@@ -45,7 +48,22 @@ class ExecutableSettings {
       if (envVariable != null) return envVariable;
     }
 
-    if (Platform.isMacOS) return _macOSExecutable!;
+    if (Platform.isMacOS) {
+      if (_macOSExectuables != null) {
+        for (final path in _macOSExectuables) {
+          if (p.basename(path) == path) return path;
+          if (p.isAbsolute(path)) {
+            if (File(path).existsSync()) return path;
+          } else {
+            throw ArgumentError(
+                'Mac OS executable must be a basename or an absolute path.'
+                ' Got relative path: $path');
+          }
+        }
+      }
+      throw ArgumentError('Could not find a command basename or an existing '
+          'path in $_macOSExectuables');
+    }
     if (!Platform.isWindows) return _linuxExecutable!;
     final windowsExecutable = _windowsExecutable!;
     if (p.isAbsolute(windowsExecutable)) return windowsExecutable;
@@ -177,12 +195,14 @@ class ExecutableSettings {
       {Iterable<String>? arguments,
       String? linuxExecutable,
       String? macOSExecutable,
+      List<String>? macOSExecutables,
       String? windowsExecutable,
       String? environmentOverride,
       bool? headless})
       : arguments = arguments == null ? const [] : List.unmodifiable(arguments),
         _linuxExecutable = linuxExecutable,
-        _macOSExecutable = macOSExecutable,
+        _macOSExectuables =
+            _normalizeMacExecutable(macOSExecutable, macOSExecutables),
         _windowsExecutable = windowsExecutable,
         _environmentOverride = environmentOverride,
         _headless = headless;
@@ -192,6 +212,13 @@ class ExecutableSettings {
       arguments: arguments.toList()..addAll(other.arguments),
       headless: other._headless ?? _headless,
       linuxExecutable: other._linuxExecutable ?? _linuxExecutable,
-      macOSExecutable: other._macOSExecutable ?? _macOSExecutable,
+      macOSExecutables: other._macOSExectuables ?? _macOSExectuables,
       windowsExecutable: other._windowsExecutable ?? _windowsExecutable);
 }
+
+List<String>? _normalizeMacExecutable(
+    String? singleArgument, List<String>? listArgument) {
+  if (listArgument != null) return listArgument;
+  if (singleArgument != null) return [singleArgument];
+  return null;
+}