Communicate with Node over sockets (#729)

We were previously communicating over standard in/out, but this was
fragile: if raw JS code printed anything to stdout, it would break our
connection.

CHANGELOG.md

@@ -3,9 +3,11 @@
 * Node.js tests can now import modules from a top-level `node_modules`
   directory, if one exists.
 
+* Raw `console.log()` calls no longer crash Node.js tests.
+
 * When a browser crashes, include its standard output in the error message.
 
-## 0.12.28
+## 0.12.28+1
 
 * Add a `pumpEventQueue()` function to make it easy to wait until all
   asynchronous tasks are complete.

lib/src/bootstrap/node.dart

@@ -3,13 +3,13 @@
 // BSD-style license that can be found in the LICENSE file.
 
 import "../runner/plugin/remote_platform_helpers.dart";
-import "../runner/node/stdio_channel.dart";
+import "../runner/node/socket_channel.dart";
 
 /// Bootstraps a browser test to communicate with the test runner.
 ///
 /// This should NOT be used directly, instead use the `test/pub_serve`
 /// transformer which will bootstrap your test and call this method.
 void internalBootstrapNodeTest(Function getMain()) {
   var channel = serializeSuite(getMain);
-  stdioChannel().pipe(channel);
+  socketChannel().pipe(channel);
 }

lib/src/runner/node/platform.dart

@@ -7,6 +7,7 @@ import 'dart:io';
 import 'dart:convert';
 
 import 'package:async/async.dart';
+import 'package:multi_server_socket/multi_server_socket.dart';
 import 'package:node_preamble/preamble.dart' as preamble;
 import 'package:package_resolver/package_resolver.dart';
 import 'package:path/path.dart' as p;
@@ -91,15 +92,22 @@ class NodePlatform extends PlatformPlugin
   /// source map for the compiled suite.
   Future<Pair<StreamChannel, StackTraceMapper>> _loadChannel(String path,
       TestPlatform platform, SuiteConfiguration suiteConfig) async {
-    var pair = await _spawnProcess(path, platform, suiteConfig);
+    var server = await MultiServerSocket.loopback(0);
+
+    var pair = await _spawnProcess(path, platform, suiteConfig, server.port);
     var process = pair.first;
 
-    // Node normally doesn't emit any standard error, but if it does we forward
-    // it to the print handler so it's associated with the load test.
+    // Forward Node's standard IO to the print handler so it's associated with
+    // the load test.
+    //
+    // TODO(nweiz): Associate this with the current test being run, if any.
+    process.stdout.transform(lineSplitter).listen(print);
     process.stderr.transform(lineSplitter).listen(print);
 
-    var channel = new StreamChannel.withGuarantees(
-            process.stdout, process.stdin)
+    var socket = await server.first;
+    // TODO(nweiz): Remove the DelegatingStreamSink wrapper when sdk#31504 is
+    // fixed.
+    var channel = new StreamChannel(socket, new DelegatingStreamSink(socket))
         .transform(new StreamChannelTransformer.fromCodec(UTF8))
         .transform(chunksToLines)
         .transform(jsonDocument)
@@ -115,8 +123,11 @@ 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,
-      TestPlatform platform, SuiteConfiguration suiteConfig) async {
+  Future<Pair<Process, StackTraceMapper>> _spawnProcess(
+      String path,
+      TestPlatform platform,
+      SuiteConfiguration suiteConfig,
+      int socketPort) async {
     var dir = new Directory(_compiledDir).createTempSync('test_').path;
     var jsPath = p.join(dir, p.basename(path) + ".node_test.dart.js");
 
@@ -146,7 +157,8 @@ class NodePlatform extends PlatformPlugin
             sdkRoot: p.toUri(sdkDir));
       }
 
-      return new Pair(await _startProcess(platform, jsPath), mapper);
+      return new Pair(
+          await _startProcess(platform, jsPath, socketPort), mapper);
     }
 
     var url = _config.pubServeUrl.resolveUri(
@@ -165,20 +177,21 @@ class NodePlatform extends PlatformPlugin
           sdkRoot: p.toUri('packages/\$sdk'));
     }
 
-    return new Pair(await _startProcess(platform, jsPath), mapper);
+    return new Pair(await _startProcess(platform, jsPath, socketPort), mapper);
   }
 
   /// Starts the Node.js process for [platform] with [jsPath].
-  Future<Process> _startProcess(TestPlatform platform, String jsPath) async {
+  Future<Process> _startProcess(
+      TestPlatform platform, String jsPath, int socketPort) async {
     var settings = _settings[platform];
 
     var nodeModules = p.absolute('node_modules');
     var nodePath = Platform.environment["NODE_PATH"];
     nodePath = nodePath == null ? nodeModules : "$nodePath:$nodeModules";
 
     try {
-      return await Process.start(
-          settings.executable, settings.arguments.toList()..add(jsPath),
+      return await Process.start(settings.executable,
+          settings.arguments.toList()..add(jsPath)..add(socketPort.toString()),
           environment: {'NODE_PATH': nodePath});
     } catch (error, stackTrace) {
       await new Future.error(

lib/src/runner/node/stdio_channel.dart → lib/src/runner/node/socket_channel.dart

@@ -11,37 +11,34 @@ import 'package:stream_channel/stream_channel.dart';
 import '../../utils.dart';
 
 @JS("require")
-external _Process _require(String module);
+external _Net _require(String module);
+
+@JS("process.argv")
+external List<String> get _args;
 
 @JS()
-class _Process {
-  external _Stdin get stdin;
-  external _Stdout get stdout;
+class _Net {
+  external _Socket connect(int port);
 }
 
 @JS()
-class _Stdin {
+class _Socket {
   external setEncoding(String encoding);
   external on(String event, void callback(String chunk));
+  external write(String data);
 }
 
-@JS()
-class _Stdout {
-  external setDefaultEncoding(String encoding);
-  external write(String chunk);
-}
-
-/// Returns a [StreamChannel] of JSON-encodable objects that communicates over
-/// the current process's stdout and stdin streams.
-StreamChannel stdioChannel() {
+/// Returns a [StreamChannel] of JSON-encodable objects that communicates over a
+/// socket whose port is given by `process.argv[2]`.
+StreamChannel socketChannel() {
   var controller = new StreamChannelController<String>(
       allowForeignErrors: false, sync: true);
-  var process = _require("process");
-  process.stdin.setEncoding("utf8");
-  process.stdout.setDefaultEncoding("utf8");
+  var net = _require("net");
+  var socket = net.connect(int.parse(_args[2]));
+  socket.setEncoding("utf8");
 
-  controller.local.stream.listen((chunk) => process.stdout.write(chunk));
-  process.stdin.on("data", allowInterop(controller.local.sink.add));
+  controller.local.stream.listen((chunk) => socket.write(chunk));
+  socket.on("data", allowInterop(controller.local.sink.add));
 
   return controller.foreign.transform(chunksToLines).transform(jsonDocument);
 }

lib/src/utils.dart

@@ -33,8 +33,10 @@ final lineSplitter = new StreamTransformer<List<int>, String>(
         .listen(null, cancelOnError: cancelOnError));
 
 /// A [StreamChannelTransformer] that converts a chunked string channel to a
-/// line-by-line channel. Note that this is only safe for channels whose
-/// messages are guaranteed not to contain newlines.
+/// line-by-line channel.
+///
+/// Note that this is only safe for channels whose messages are guaranteed not
+/// to contain newlines.
 final chunksToLines = new StreamChannelTransformer(
     const LineSplitter(),
     new StreamSinkTransformer.fromHandlers(

pubspec.yaml

@@ -1,5 +1,5 @@
 name: test
-version: 0.12.29-dev
+version: 0.12.29
 author: Dart Team <misc@dartlang.org>
 description: A library for writing dart unit tests.
 homepage: https://github.com/dart-lang/test
@@ -17,6 +17,7 @@ dependencies:
   io: '^0.3.0'
   js: '^0.6.0'
   meta: '^1.0.0'
+  multi_server_socket: '^1.0.0'
   node_preamble: '^1.3.0'
   package_resolver: '^1.0.0'
   path: '^1.2.0'

test/runner/node/runner_test.dart

@@ -169,6 +169,29 @@ void main() {
     await test.shouldExit(0);
   });
 
+  test("forwards raw JS prints from the Node test", () async {
+    await d.file("test.dart", """
+      import 'dart:async';
+
+      import 'package:js/js.dart';
+      import 'package:test/test.dart';
+
+      @JS("console.log")
+      external void log(value);
+
+      void main() {
+        test("test", () {
+          log("Hello,");
+          return new Future(() => log("world!"));
+        });
+      }
+    """).create();
+
+    var test = await runTest(["-p", "node", "test.dart"]);
+    expect(test.stdout, emitsInOrder([emitsThrough("Hello,"), "world!"]));
+    await test.shouldExit(0);
+  });
+
   test("dartifies stack traces for JS-compiled tests by default", () async {
     await d.file("test.dart", _failure).create();