From 0e5c028d239b8801581852aa31a19ecc9f64f500 Mon Sep 17 00:00:00 2001
From: Nate Bosch <nbosch@google.com>
Date: Fri, 10 Mar 2023 14:47:58 -0800
Subject: [PATCH] Only use environment variable for chrome (#1970)

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.
---
 .../src/runner/browser/default_settings.dart   |  3 ++-
 .../lib/src/runner/executable_settings.dart    | 12 ++++++++++--
 .../test/test/runner/browser/firefox_test.dart | 18 ++++++++++++++++++
 3 files changed, 30 insertions(+), 3 deletions(-)

diff --git a/pkgs/test/lib/src/runner/browser/default_settings.dart b/pkgs/test/lib/src/runner/browser/default_settings.dart
index d60880480..35bb6c987 100644
--- a/pkgs/test/lib/src/runner/browser/default_settings.dart
+++ b/pkgs/test/lib/src/runner/browser/default_settings.dart
@@ -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',
diff --git a/pkgs/test/lib/src/runner/executable_settings.dart b/pkgs/test/lib/src/runner/executable_settings.dart
index cf073de41..cd67b9352 100644
--- a/pkgs/test/lib/src/runner/executable_settings.dart
+++ b/pkgs/test/lib/src/runner/executable_settings.dart
@@ -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!;
@@ -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.
diff --git a/pkgs/test/test/runner/browser/firefox_test.dart b/pkgs/test/test/runner/browser/firefox_test.dart
index a1189d8a3..71e7b4624 100644
--- a/pkgs/test/test/runner/browser/firefox_test.dart
+++ b/pkgs/test/test/runner/browser/firefox_test.dart
@@ -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);
+  });
 }