Support configuring the Node platform (flutter#703)

See flutter#391

Author: nex3

doc/configuration.md

@@ -54,7 +54,7 @@ tags:
   * [`on_platform`](#on_platform)
   * [`override_platforms`](#override_platforms)
   * [`define_platforms`](#define_platforms)
-  * [Browser Settings](#browser-settings)
+  * [Browser/Node.js Settings](#browser-and-node-js-settings)
     * [`executable`](#executable)
 * [Configuration Presets](#configuration-presets)
   * [`presets`](#presets)
@@ -587,9 +587,9 @@ override_platforms:
 This tells the test runner to use the `chromium` executable for Chrome tests. It
 calls that executable with the same logic and flags it normally uses for Chrome.
 
-Each platform can define exactly which settings it supports. All browsers
-support [the same settings](#browser-settings), but the VM doesn't support any
-settings and so can't be overridden.
+Each platform can define exactly which settings it supports. All browsers and
+Node.js support [the same settings](#browser-and-node-js-settings), but the VM
+doesn't support any settings and so can't be overridden.
 
 ### `define_platforms`
 
@@ -620,24 +620,25 @@ might pass `testOn: "chromium"` to declare that a test is Chromium-specific.
 User-defined platforms also count as their parents, so Chromium will run tests
 that say `testOn: "chrome"` as well.
 
-Each platform can define exactly which settings it supports. All browsers
-support [the same settings](#browser-settings), but the VM doesn't support any
-settings and so can't be extended.
+Each platform can define exactly which settings it supports. All browsers and
+Node.js support [the same settings](#browser-and-node-js-settings), but the VM
+doesn't support any settings and so can't be extended.
 
 This field is not supported in the
 [global configuration file](#global-configuration).
 
-### Browser Settings
+### Browser and Node.js Settings
 
-All built-in browser platforms provide the same settings that can be set using
-[`define_platforms`](#define_platforms), which control how the browser
-executable is invoked.
+All built-in browser platforms, as well as the built-in Node.js platform,
+provide the same settings that can be set using
+[`define_platforms`](#define_platforms), which control how their executables are
+invoked.
 
 #### `executable`
 
 The `executable` field tells the test runner where to look for the executable to
-use to start the browser. It has three sub-keys, one for each supported operating
-system, which each take a path or an executable name:
+use to start the subprocess. It has three sub-keys, one for each supported
+operating system, which each take a path or an executable name:
 
 ```yaml
 define_platforms:

lib/src/runner/node/platform.dart

@@ -11,22 +11,27 @@ import 'package:node_preamble/preamble.dart' as preamble;
 import 'package:package_resolver/package_resolver.dart';
 import 'package:path/path.dart' as p;
 import 'package:stream_channel/stream_channel.dart';
+import 'package:yaml/yaml.dart';
 
 import '../../backend/test_platform.dart';
 import '../../util/io.dart';
 import '../../util/stack_trace_mapper.dart';
 import '../../utils.dart';
+import '../application_exception.dart';
 import '../compiler_pool.dart';
 import '../configuration.dart';
 import '../configuration/suite.dart';
+import '../executable_settings.dart';
 import '../load_exception.dart';
+import '../plugin/customizable_platform.dart';
 import '../plugin/environment.dart';
 import '../plugin/platform.dart';
 import '../plugin/platform_helpers.dart';
 import '../runner_suite.dart';
 
 /// A platform that loads tests in Node.js processes.
-class NodePlatform extends PlatformPlugin {
+class NodePlatform extends PlatformPlugin
+    implements CustomizablePlatform<ExecutableSettings> {
   /// The test runner configuration.
   final Configuration _config;
 
@@ -39,22 +44,41 @@ class NodePlatform extends PlatformPlugin {
   /// The HTTP client to use when fetching JS files for `pub serve`.
   final HttpClient _http;
 
-  /// The Node executable to use.
-  String get _executable => Platform.isWindows ? "node.exe" : "node";
+  /// Executable settings for [TestPlatform.nodeJS] and platforms that extend
+  /// it.
+  final _settings = {
+    TestPlatform.nodeJS: new ExecutableSettings(
+        linuxExecutable: "node",
+        macOSExecutable: "node",
+        windowsExecutable: "node.exe")
+  };
 
   NodePlatform()
       : _config = Configuration.current,
         _http =
             Configuration.current.pubServeUrl == null ? null : new HttpClient();
 
+  ExecutableSettings parsePlatformSettings(YamlMap settings) =>
+      new ExecutableSettings.parse(settings);
+
+  ExecutableSettings mergePlatformSettings(
+          ExecutableSettings settings1, ExecutableSettings settings2) =>
+      settings1.merge(settings2);
+
+  void customizePlatform(TestPlatform platform, ExecutableSettings settings) {
+    var oldSettings = _settings[platform] ?? _settings[platform.root];
+    if (oldSettings != null) settings = oldSettings.merge(settings);
+    _settings[platform] = settings;
+  }
+
   StreamChannel loadChannel(String path, TestPlatform platform) =>
       throw new UnimplementedError();
 
   Future<RunnerSuite> load(String path, TestPlatform platform,
       SuiteConfiguration suiteConfig, Object message) async {
     assert(platform == TestPlatform.nodeJS);
 
-    var pair = await _loadChannel(path, suiteConfig);
+    var pair = await _loadChannel(path, platform, suiteConfig);
     var controller = await deserializeSuite(path, platform, suiteConfig,
         new PluginEnvironment(), pair.first, message,
         mapper: pair.last);
@@ -65,9 +89,9 @@ class NodePlatform extends PlatformPlugin {
   ///
   /// Returns that channel along with a [StackTraceMapper] representing the
   /// source map for the compiled suite.
-  Future<Pair<StreamChannel, StackTraceMapper>> _loadChannel(
-      String path, SuiteConfiguration suiteConfig) async {
-    var pair = await _spawnProcess(path, suiteConfig);
+  Future<Pair<StreamChannel, StackTraceMapper>> _loadChannel(String path,
+      TestPlatform platform, SuiteConfiguration suiteConfig) async {
+    var pair = await _spawnProcess(path, platform, suiteConfig);
     var process = pair.first;
 
     // Node normally doesn't emit any standard error, but if it does we forward
@@ -91,8 +115,8 @@ class NodePlatform extends PlatformPlugin {
   ///
   /// Returns that channel along with a [StackTraceMapper] representing the
   /// source map for the compiled suite.
-  Future<Pair<Process, StackTraceMapper>> _spawnProcess(
-      String path, SuiteConfiguration suiteConfig) async {
+  Future<Pair<Process, StackTraceMapper>> _spawnProcess(String path,
+      TestPlatform platform, SuiteConfiguration suiteConfig) async {
     var dir = new Directory(_compiledDir).createTempSync('test_').path;
     var jsPath = p.join(dir, p.basename(path) + ".node_test.dart.js");
 
@@ -122,7 +146,7 @@ class NodePlatform extends PlatformPlugin {
             sdkRoot: p.toUri(sdkDir));
       }
 
-      return new Pair(await Process.start(_executable, [jsPath]), mapper);
+      return new Pair(await _startProcess(platform, jsPath), mapper);
     }
 
     var url = _config.pubServeUrl.resolveUri(
@@ -141,7 +165,20 @@ class NodePlatform extends PlatformPlugin {
           sdkRoot: p.toUri('packages/\$sdk'));
     }
 
-    return new Pair(await Process.start(_executable, [jsPath]), mapper);
+    return new Pair(await _startProcess(platform, jsPath), mapper);
+  }
+
+  /// Starts the Node.js process for [platform] with [jsPath].
+  Future<Process> _startProcess(TestPlatform platform, String jsPath) async {
+    try {
+      return await Process.start(_settings[platform].executable, [jsPath]);
+    } catch (error, stackTrace) {
+      await new Future.error(
+          new ApplicationException(
+              "Failed to run ${platform.name}: ${getErrorMessage(error)}"),
+          stackTrace);
+      return null;
+    }
   }
 
   /// Runs an HTTP GET on [url].

test/runner/configuration/custom_platform_test.dart

@@ -98,6 +98,18 @@ void main() {
       }, tags: "chrome");
     });
 
+    test("can override Node.js without any changes", () async {
+      await d.file("dart_test.yaml", """
+        override_platforms:
+          node:
+            settings: {}
+      """).create();
+
+      var test = await runTest(["-p", "node", "test.dart"]);
+      expect(test.stdout, emitsThrough(contains("All tests passed!")));
+      await test.shouldExit(0);
+    }, tags: "node");
+
     group("errors", () {
       test("rejects a non-map value", () async {
         await d.file("dart_test.yaml", "override_platforms: 12").create();
@@ -329,6 +341,22 @@ void main() {
                   contains("Failed to run Chrome: $noSuchFileMessage")));
           await test.shouldExit(1);
         });
+
+        test("executable must exist for Node.js", () async {
+          await d.file("dart_test.yaml", """
+            override_platforms:
+              node:
+                settings:
+                  executable: _does_not_exist
+          """).create();
+
+          var test = await runTest(["-p", "node", "test.dart"]);
+          expect(
+              test.stdout,
+              emitsThrough(
+                  contains("Failed to run Node.js: $noSuchFileMessage")));
+          await test.shouldExit(1);
+        }, tags: "node");
       });
     });
   });