From 22835e2ec81169a5c05ed1ceb4fd6d0e8f4e87d6 Mon Sep 17 00:00:00 2001 From: Nate Bosch Date: Fri, 13 Sep 2024 11:13:20 -0700 Subject: [PATCH] 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 --- pkgs/test/CHANGELOG.md | 1 + .../src/runner/browser/default_settings.dart | 6 ++- .../lib/src/runner/executable_settings.dart | 41 +++++++++++++++---- 3 files changed, 40 insertions(+), 8 deletions(-) diff --git a/pkgs/test/CHANGELOG.md b/pkgs/test/CHANGELOG.md index cc228ade6..04ef333c1 100644 --- a/pkgs/test/CHANGELOG.md +++ b/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 diff --git a/pkgs/test/lib/src/runner/browser/default_settings.dart b/pkgs/test/lib/src/runner/browser/default_settings.dart index b2b24d3ce..312fc1130 100644 --- a/pkgs/test/lib/src/runner/browser/default_settings.dart +++ b/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( diff --git a/pkgs/test/lib/src/runner/executable_settings.dart b/pkgs/test/lib/src/runner/executable_settings.dart index cd67b9352..904dc5149 100644 --- a/pkgs/test/lib/src/runner/executable_settings.dart +++ b/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? _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? arguments, String? linuxExecutable, String? macOSExecutable, + List? 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? _normalizeMacExecutable( + String? singleArgument, List? listArgument) { + if (listArgument != null) return listArgument; + if (singleArgument != null) return [singleArgument]; + return null; +}