Test runner should respect `CHROME_EXECUTABLE`

[Zekfad]

I have custom installation of Google Chrome Beta, and using CHROME_EXECUTABLE to get it work with Dart.
It works fine with all tools except for tests, here's the log:

dart test -p chrome

D:\Users\vorob\Desktop\Projects\fetch_client>dart test -p chrome
00:09 +0: compiling test\client_conformance_test.dart
Info: Compiling with sound null safety.
Compiled 11,042,001 characters Dart to 1,611,431 characters JavaScript in 9.22 seconds using 333.664 MB of memory
Dart file /C:/Users/vorob/AppData/Local/Temp/dart_test_5c16f440/runInBrowser.dart compiled to JavaScript: /C:/Users/vorob/AppData/Local/Temp/dart_test_bc500204/test_970b1edf/client_conformance_test.dart.browser_test.dart.js

00:09 +0 -1: compiling test\client_conformance_test.dart [E]
  Failed to load "test\client_conformance_test.dart":
  Failed to run Chrome: The system cannot find the file specified.

    Command: C:\Users\vorob\AppData\Local\Google\Chrome\Application\chrome.exe --user-data-dir=C:\Users\vorob\AppData\Local\Temp\dart_test_d63e0bf1 http://localhost:9848/rgecBooVKJ7rmWD9HqMkt5sOZVYUpeei/packages/test/src/runner/browser/static/index.html?managerUrl=ws%3A%2F%2Flocalhost%3A9848%2FrgecBooVKJ7rmWD9HqMkt5sOZVYUpeei%2F0&debug=false --enable-logging=stdout --v=1 
--disable-extensions --disable-popup-blocking --bwsi --no-first-run --no-default-browser-check --disable-default-apps --disable-translate --disable-dev-shm-usage --headless --disable-gpu 
--remote-debugging-port=0.
  dart:io-patch/process_patch.dart 401:33                      _ProcessImpl._start
  dart:io-patch/process_patch.dart 38:20                       Process.start
  package:test/src/runner/browser/chrome.dart 80:37            new Chrome.<fn>.tryPort
  package:test/src/runner/browser/chrome.dart 97:40            new Chrome.<fn>
  package:test/src/runner/browser/browser.dart 68:39           new Browser.<fn>
  dart:async/zone.dart 1398:13                                 _rootRun
  dart:async/zone.dart 1300:19                                 _CustomZone.run
  dart:async/zone.dart 1803:10                                 _runZoned
  dart:async/zone.dart 1791:12                                 runZonedGuarded
  package:test/src/runner/browser/browser.dart 67:5            new Browser
  package:test/src/runner/browser/chrome.dart 123:9            new Chrome._
  package:test/src/runner/browser/chrome.dart 46:19            new Chrome
  package:test/src/runner/browser/browser_manager.dart 159:16  BrowserManager._newBrowser
  package:test/src/runner/browser/browser_manager.dart 116:19  BrowserManager._start
  package:test/src/runner/browser/browser_manager.dart 106:7   BrowserManager.start
  package:test/src/runner/browser/platform.dart 426:33         BrowserPlatform._browserManagerFor
  package:test/src/runner/browser/platform.dart 271:32         BrowserPlatform.load
  ===== asynchronous gap ===========================
  package:test_core/src/runner/loader.dart 220:25              Loader.loadFile.<fn>
  ===== asynchronous gap ===========================
  package:test_core/src/runner/load_suite.dart 99:19           new LoadSuite.<fn>.<fn>


To run this test again: D:\Android\flutter\bin\cache\dart-sdk\bin\dart.exe test test\client_conformance_test.dart -p chrome --plain-name "compiling test\client_conformance_test.dart"     
00:09 +0 -1: Some tests failed.

[jakemac53]

When you say "all other tools" can you clarify what tools you mean? We could consider supporting that but we already have our own way of configuring which executable to run for any given browser, through the dart_test.yaml configuration.

I don't know that we want to support multiple ways of overriding this. cc @natebosch

You can customize this for all packages on your computer using a global config file.

[Zekfad]

Missed the part with global config, my bad.
For the tools I mean flutter devtools and dart-code for vscode. They use Chrome executable from the environment variable.
But I still think it would be good to use CHROME_EXECUTABLE as a default fallback, if user not provided explicit config, that makes it easy to spin development environment with a single script, without edit of any configs.

[jakemac53]

Is there a similar environment variable for all the other browsers? Or do those tools only support chrome?

[jakemac53]

If we did this we would also have to make a decision about the precedence between local config/global config/environment variable. In particular I am not sure where the environment variable fits in there.

[Zekfad]

From what I see, there's also variable for edge:

https://github.com/flutter/flutter/blob/c6ec84e97803c24f51140f895856e1e1d99b9101/packages/flutter_tools/lib/src/web/chrome.dart#L19-L23

Regarding preference, I do believe that config is the main source, same goes for example for popular package dotenv.

[jakemac53]

Discussed with @natebosch and we decided it should be highest precedence, because it follows the order of how difficult it is to change the config. The most difficult to change is the lowest priority, the easiest to change overrides the rest. Since env vars are easier to change than config, they are higher precedence. If we add command line argument config for this, it would be even higher priority.