Skip to content

Commit

Permalink
Only use environment variable for chrome (#1970)
Browse files Browse the repository at this point in the history
Add a new argument to the `ExectuableSettings` constructor to add an
environment variable that can override the specific executable, instead
of overriding for all uses of `ExectuableSettings`. Pass this variable
from the chrome browser.

Add a test that the override is ignored when running a firefox test.
  • Loading branch information
natebosch committed Mar 10, 2023
1 parent 0b08d70 commit 0e5c028
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 3 deletions.
3 changes: 2 additions & 1 deletion pkgs/test/lib/src/runner/browser/default_settings.dart
Expand Up @@ -13,7 +13,8 @@ final defaultSettings = UnmodifiableMapView({
linuxExecutable: 'google-chrome',
macOSExecutable:
'/Applications/Google Chrome.app/Contents/MacOS/Google Chrome',
windowsExecutable: r'Google\Chrome\Application\chrome.exe'),
windowsExecutable: r'Google\Chrome\Application\chrome.exe',
environmentOverride: 'CHROME_EXECUTABLE'),
Runtime.firefox: ExecutableSettings(
linuxExecutable: 'firefox',
macOSExecutable: '/Applications/Firefox.app/Contents/MacOS/firefox-bin',
Expand Down
12 changes: 10 additions & 2 deletions pkgs/test/lib/src/runner/executable_settings.dart
Expand Up @@ -34,10 +34,16 @@ class ExecutableSettings {
/// `PROGRAMFILES(X64)` environment variables.
final String? _windowsExecutable;

/// The environment variable, if any, to use as an override for the default
/// path.
final String? _environmentOverride;

/// The path to the executable for the current operating system.
String get executable {
final envVariable = Platform.environment['CHROME_EXECUTABLE'];
if (envVariable != null) return envVariable;
if (_environmentOverride != null) {
final envVariable = Platform.environment[_environmentOverride];
if (envVariable != null) return envVariable;
}

if (Platform.isMacOS) return _macOSExecutable!;
if (!Platform.isWindows) return _linuxExecutable!;
Expand Down Expand Up @@ -172,11 +178,13 @@ class ExecutableSettings {
String? linuxExecutable,
String? macOSExecutable,
String? windowsExecutable,
String? environmentOverride,
bool? headless})
: arguments = arguments == null ? const [] : List.unmodifiable(arguments),
_linuxExecutable = linuxExecutable,
_macOSExecutable = macOSExecutable,
_windowsExecutable = windowsExecutable,
_environmentOverride = environmentOverride,
_headless = headless;

/// Merges [this] with [other], with [other]'s settings taking priority.
Expand Down
18 changes: 18 additions & 0 deletions pkgs/test/test/runner/browser/firefox_test.dart
Expand Up @@ -79,4 +79,22 @@ void main() {
expect(test.stdout, emitsThrough(contains('-1: Some tests failed.')));
await test.shouldExit(1);
});

test('not impacted by CHROME_EXECUTABLE var', () async {
await d.file('test.dart', '''
import 'dart:html';
import 'package:test/test.dart';
void main() {
test("success", () {
assert(window.navigator.vendor != 'Google Inc.');
});
}
''').create();
var test = await runTest(['-p', 'firefox', 'test.dart'],
environment: {'CHROME_EXECUTABLE': '/some/bad/path'});
expect(test.stdout, emitsThrough(contains('+1: All tests passed!')));
await test.shouldExit(0);
});
}

0 comments on commit 0e5c028

Please sign in to comment.