From 9549376032671864acaf3ec04bcd0ce8c623333d Mon Sep 17 00:00:00 2001 From: Koji Wakamiya Date: Sat, 2 Mar 2024 16:22:43 +0900 Subject: [PATCH 1/2] Fix path to launch firefox on macOS --- pkgs/test/CHANGELOG.md | 1 + pkgs/test/lib/src/runner/browser/default_settings.dart | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/pkgs/test/CHANGELOG.md b/pkgs/test/CHANGELOG.md index 43a233ed1..5af649801 100644 --- a/pkgs/test/CHANGELOG.md +++ b/pkgs/test/CHANGELOG.md @@ -1,6 +1,7 @@ ## 1.25.3-wip * Remove outdated StreamMatcher link from README table of contents. +* Fix path to launch firefox on macOS. ## 1.25.2 diff --git a/pkgs/test/lib/src/runner/browser/default_settings.dart b/pkgs/test/lib/src/runner/browser/default_settings.dart index b2b24d3ce..816e8daab 100644 --- a/pkgs/test/lib/src/runner/browser/default_settings.dart +++ b/pkgs/test/lib/src/runner/browser/default_settings.dart @@ -24,7 +24,7 @@ final defaultSettings = UnmodifiableMapView({ ), Runtime.firefox: ExecutableSettings( linuxExecutable: 'firefox', - macOSExecutable: '/Applications/Firefox.app/Contents/MacOS/firefox-bin', + macOSExecutable: '/Applications/Firefox.app/Contents/MacOS/firefox', windowsExecutable: r'Mozilla Firefox\firefox.exe', environmentOverride: 'FIREFOX_EXECUTABLE'), Runtime.safari: ExecutableSettings( From 004c0405bf89f6dfbb02f65dab04ce0f63228d9d Mon Sep 17 00:00:00 2001 From: Jake Macdonald Date: Tue, 5 Mar 2024 19:06:37 +0000 Subject: [PATCH 2/2] add support for multiple search locations for browser executables --- pkgs/test/CHANGELOG.md | 2 +- .../src/runner/browser/default_settings.dart | 37 +++--- .../lib/src/runner/executable_settings.dart | 105 ++++++++++++------ pkgs/test/lib/src/runner/node/platform.dart | 6 +- .../test/test/runner/browser/chrome_test.dart | 6 +- .../test/runner/browser/firefox_test.dart | 6 +- .../runner/browser/microsoft_edge_test.dart | 6 +- .../test/test/runner/browser/safari_test.dart | 6 +- 8 files changed, 106 insertions(+), 68 deletions(-) diff --git a/pkgs/test/CHANGELOG.md b/pkgs/test/CHANGELOG.md index 5af649801..e4fe680f4 100644 --- a/pkgs/test/CHANGELOG.md +++ b/pkgs/test/CHANGELOG.md @@ -1,7 +1,7 @@ ## 1.25.3-wip * Remove outdated StreamMatcher link from README table of contents. -* Fix path to launch firefox on macOS. +* Add support for the new firefox binary location on macOS. ## 1.25.2 diff --git a/pkgs/test/lib/src/runner/browser/default_settings.dart b/pkgs/test/lib/src/runner/browser/default_settings.dart index 816e8daab..d1f1846e1 100644 --- a/pkgs/test/lib/src/runner/browser/default_settings.dart +++ b/pkgs/test/lib/src/runner/browser/default_settings.dart @@ -9,25 +9,30 @@ import '../executable_settings.dart'; /// Default settings for starting browser executables. final defaultSettings = UnmodifiableMapView({ - Runtime.chrome: ExecutableSettings( - linuxExecutable: 'google-chrome', - macOSExecutable: - '/Applications/Google Chrome.app/Contents/MacOS/Google Chrome', - windowsExecutable: r'Google\Chrome\Application\chrome.exe', - environmentOverride: 'CHROME_EXECUTABLE'), + Runtime.chrome: ExecutableSettings(linuxExecutables: [ + 'google-chrome' + ], macOSExecutables: [ + '/Applications/Google Chrome.app/Contents/MacOS/Google Chrome' + ], windowsExecutables: [ + r'Google\Chrome\Application\chrome.exe' + ], environmentOverride: 'CHROME_EXECUTABLE'), Runtime.edge: ExecutableSettings( - linuxExecutable: 'microsoft-edge-stable', - windowsExecutable: r'Microsoft\Edge\Application\msedge.exe', - macOSExecutable: - '/Applications/Microsoft Edge.app/Contents/MacOS/Microsoft Edge', + linuxExecutables: ['microsoft-edge-stable'], + windowsExecutables: [r'Microsoft\Edge\Application\msedge.exe'], + macOSExecutables: [ + '/Applications/Microsoft Edge.app/Contents/MacOS/Microsoft Edge' + ], environmentOverride: 'MS_EDGE_EXECUTABLE', ), - Runtime.firefox: ExecutableSettings( - linuxExecutable: 'firefox', - macOSExecutable: '/Applications/Firefox.app/Contents/MacOS/firefox', - windowsExecutable: r'Mozilla Firefox\firefox.exe', - environmentOverride: 'FIREFOX_EXECUTABLE'), + Runtime.firefox: ExecutableSettings(linuxExecutables: [ + 'firefox' + ], macOSExecutables: [ + '/Applications/Firefox.app/Contents/MacOS/firefox', + '/Applications/Firefox.app/Contents/MacOS/firefox-bin' + ], windowsExecutables: [ + r'Mozilla Firefox\firefox.exe' + ], environmentOverride: 'FIREFOX_EXECUTABLE'), Runtime.safari: ExecutableSettings( - macOSExecutable: '/Applications/Safari.app/Contents/MacOS/Safari', + macOSExecutables: ['/Applications/Safari.app/Contents/MacOS/Safari'], environmentOverride: 'SAFARI_EXECUTABLE'), }); diff --git a/pkgs/test/lib/src/runner/executable_settings.dart b/pkgs/test/lib/src/runner/executable_settings.dart index cd67b9352..93f2ad246 100644 --- a/pkgs/test/lib/src/runner/executable_settings.dart +++ b/pkgs/test/lib/src/runner/executable_settings.dart @@ -18,13 +18,13 @@ class ExecutableSettings { /// /// 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? _linuxExecutable; + final List _linuxExecutables; /// The path to the executable on Mac OS. /// /// 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; + final List _macOSExecutables; /// The path to the executable on Windows. /// @@ -32,7 +32,7 @@ class ExecutableSettings { /// up on the system path; or a relative path, in which case it will be looked /// up relative to the paths in the `LOCALAPPDATA`, `PROGRAMFILES`, and /// `PROGRAMFILES(X64)` environment variables. - final String? _windowsExecutable; + final List _windowsExecutables; /// The environment variable, if any, to use as an override for the default /// path. @@ -45,12 +45,17 @@ class ExecutableSettings { if (envVariable != null) return envVariable; } - if (Platform.isMacOS) return _macOSExecutable!; - if (!Platform.isWindows) return _linuxExecutable!; - final windowsExecutable = _windowsExecutable!; - if (p.isAbsolute(windowsExecutable)) return windowsExecutable; - if (p.basename(windowsExecutable) == windowsExecutable) { - return windowsExecutable; + if (Platform.isMacOS) { + return _macOSExecutables.firstWhere((path) => File(path).existsSync()); + } + if (!Platform.isWindows) { + return _linuxExecutables.firstWhere((path) => File(path).existsSync()); + } + for (var windowsExecutable in _windowsExecutables) { + if (p.isAbsolute(windowsExecutable)) return windowsExecutable; + if (p.basename(windowsExecutable) == windowsExecutable) { + return windowsExecutable; + } } var prefixes = [ @@ -62,15 +67,17 @@ class ExecutableSettings { for (var prefix in prefixes) { if (prefix == null) continue; - var path = p.join(prefix, windowsExecutable); - if (File(path).existsSync()) return path; + for (var windowsExecutable in _windowsExecutables) { + var path = p.join(prefix, windowsExecutable); + if (File(path).existsSync()) return path; + } } // If we can't find a path that works, return one that doesn't. This will // cause an "executable not found" error to surface. return p.join( prefixes.firstWhere((prefix) => prefix != null, orElse: () => '.')!, - _windowsExecutable); + _windowsExecutables.first); } /// Whether to invoke the browser in headless mode. @@ -97,9 +104,9 @@ class ExecutableSettings { } } - String? linuxExecutable; - String? macOSExecutable; - String? windowsExecutable; + final linuxExecutables = []; + final macOSExecutables = []; + final windowsExecutables = []; var executableNode = settings.nodes['executable']; if (executableNode != null) { var value = executableNode.value; @@ -110,14 +117,22 @@ class ExecutableSettings { _assertNotRelative(executableNode as YamlScalar); } - linuxExecutable = value; - macOSExecutable = value; - windowsExecutable = value; + linuxExecutables.add(value); + macOSExecutables.add(value); + windowsExecutables.add(value); } else if (executableNode is YamlMap) { - linuxExecutable = _getExecutable(executableNode.nodes['linux']); - macOSExecutable = _getExecutable(executableNode.nodes['mac_os']); - windowsExecutable = _getExecutable(executableNode.nodes['windows'], - allowRelative: true); + if (_getExecutable(executableNode.nodes['linux']) + case final linuxExecutable?) { + linuxExecutables.add(linuxExecutable); + } + if (_getExecutable(executableNode.nodes['mac_os']) + case final macOSExecutable?) { + macOSExecutables.add(macOSExecutable); + } + if (_getExecutable(executableNode.nodes['windows'], allowRelative: true) + case final windowsExecutable?) { + windowsExecutables.add(windowsExecutable); + } } else { throw SourceSpanFormatException( 'Must be a map or a string.', executableNode.span); @@ -138,9 +153,9 @@ class ExecutableSettings { return ExecutableSettings( arguments: arguments, - linuxExecutable: linuxExecutable, - macOSExecutable: macOSExecutable, - windowsExecutable: windowsExecutable, + linuxExecutables: linuxExecutables, + macOSExecutables: macOSExecutables, + windowsExecutables: windowsExecutables, headless: headless); } @@ -175,23 +190,41 @@ class ExecutableSettings { ExecutableSettings( {Iterable? arguments, - String? linuxExecutable, - String? macOSExecutable, - String? windowsExecutable, + List linuxExecutables = const [], + List macOSExecutables = const [], + List windowsExecutables = const [], String? environmentOverride, bool? headless}) : arguments = arguments == null ? const [] : List.unmodifiable(arguments), - _linuxExecutable = linuxExecutable, - _macOSExecutable = macOSExecutable, - _windowsExecutable = windowsExecutable, + _linuxExecutables = linuxExecutables, + _macOSExecutables = macOSExecutables, + _windowsExecutables = windowsExecutables, _environmentOverride = environmentOverride, _headless = headless; /// Merges [this] with [other], with [other]'s settings taking priority. + /// + /// Executable paths are not merged, but overridden by [other] unless empty. ExecutableSettings merge(ExecutableSettings other) => ExecutableSettings( - arguments: arguments.toList()..addAll(other.arguments), - headless: other._headless ?? _headless, - linuxExecutable: other._linuxExecutable ?? _linuxExecutable, - macOSExecutable: other._macOSExecutable ?? _macOSExecutable, - windowsExecutable: other._windowsExecutable ?? _windowsExecutable); + arguments: arguments.toList()..addAll(other.arguments), + headless: other._headless ?? _headless, + linuxExecutables: [ + if (other._linuxExecutables.isEmpty) + ..._linuxExecutables + else + ...other._linuxExecutables, + ], + macOSExecutables: [ + if (other._macOSExecutables.isEmpty) + ..._macOSExecutables + else + ...other._macOSExecutables, + ], + windowsExecutables: [ + if (other._windowsExecutables.isEmpty) + ..._windowsExecutables + else + ...other._windowsExecutables, + ], + ); } diff --git a/pkgs/test/lib/src/runner/node/platform.dart b/pkgs/test/lib/src/runner/node/platform.dart index b27009e07..b71064d26 100644 --- a/pkgs/test/lib/src/runner/node/platform.dart +++ b/pkgs/test/lib/src/runner/node/platform.dart @@ -49,9 +49,9 @@ class NodePlatform extends PlatformPlugin /// it. final _settings = { Runtime.nodeJS: ExecutableSettings( - linuxExecutable: 'node', - macOSExecutable: 'node', - windowsExecutable: 'node.exe') + linuxExecutables: ['node'], + macOSExecutables: ['node'], + windowsExecutables: ['node.exe']) }; NodePlatform() : _config = Configuration.current; diff --git a/pkgs/test/test/runner/browser/chrome_test.dart b/pkgs/test/test/runner/browser/chrome_test.dart index ffec2279e..1763d119a 100644 --- a/pkgs/test/test/runner/browser/chrome_test.dart +++ b/pkgs/test/test/runner/browser/chrome_test.dart @@ -47,9 +47,9 @@ webSocket.addEventListener("open", function() { test('reports an error in onExit', () { var chrome = Chrome(Uri.https('dart.dev'), configuration(), settings: ExecutableSettings( - linuxExecutable: '_does_not_exist', - macOSExecutable: '_does_not_exist', - windowsExecutable: '_does_not_exist')); + linuxExecutables: ['_does_not_exist'], + macOSExecutables: ['_does_not_exist'], + windowsExecutables: ['_does_not_exist'])); expect( chrome.onExit, throwsA(isApplicationException( diff --git a/pkgs/test/test/runner/browser/firefox_test.dart b/pkgs/test/test/runner/browser/firefox_test.dart index 864f49c49..def812d8a 100644 --- a/pkgs/test/test/runner/browser/firefox_test.dart +++ b/pkgs/test/test/runner/browser/firefox_test.dart @@ -44,9 +44,9 @@ webSocket.addEventListener("open", function() { test('reports an error in onExit', () { var firefox = Firefox(Uri.https('dart.dev'), settings: ExecutableSettings( - linuxExecutable: '_does_not_exist', - macOSExecutable: '_does_not_exist', - windowsExecutable: '_does_not_exist')); + linuxExecutables: ['_does_not_exist'], + macOSExecutables: ['_does_not_exist'], + windowsExecutables: ['_does_not_exist'])); expect( firefox.onExit, throwsA(isApplicationException( diff --git a/pkgs/test/test/runner/browser/microsoft_edge_test.dart b/pkgs/test/test/runner/browser/microsoft_edge_test.dart index a48bfd2d4..018d29f93 100644 --- a/pkgs/test/test/runner/browser/microsoft_edge_test.dart +++ b/pkgs/test/test/runner/browser/microsoft_edge_test.dart @@ -38,9 +38,9 @@ webSocket.addEventListener("open", function() { test('reports an error in onExit', () { var edge = MicrosoftEdge(Uri.parse('https://dart.dev'), configuration(), settings: ExecutableSettings( - linuxExecutable: '_does_not_exist', - macOSExecutable: '_does_not_exist', - windowsExecutable: '_does_not_exist')); + linuxExecutables: ['_does_not_exist'], + macOSExecutables: ['_does_not_exist'], + windowsExecutables: ['_does_not_exist'])); expect( edge.onExit, throwsA(isApplicationException( diff --git a/pkgs/test/test/runner/browser/safari_test.dart b/pkgs/test/test/runner/browser/safari_test.dart index b5d8d2fec..13d3cfb0f 100644 --- a/pkgs/test/test/runner/browser/safari_test.dart +++ b/pkgs/test/test/runner/browser/safari_test.dart @@ -45,9 +45,9 @@ webSocket.addEventListener("open", function() { test('reports an error in onExit', () { var safari = Safari(Uri.https('dart.dev'), settings: ExecutableSettings( - linuxExecutable: '_does_not_exist', - macOSExecutable: '_does_not_exist', - windowsExecutable: '_does_not_exist')); + linuxExecutables: ['_does_not_exist'], + macOSExecutables: ['_does_not_exist'], + windowsExecutables: ['_does_not_exist'])); expect( safari.onExit, throwsA(isApplicationException(