From 0a3f688fcdaa6ae4803ada9190666cb2cf5995ef Mon Sep 17 00:00:00 2001
From: Natalie Weizenbaum <nweiz@google.com>
Date: Tue, 3 Oct 2017 20:08:08 -0700
Subject: [PATCH] Send test platforms to the remote listener (#694)

This ensures that the remote listener has access to any platforms that
are dynamically loaded in the test runner, so they can be used in
platform selectors.

See #99
See #391
---
 lib/src/backend/test_platform.dart          | 54 ++++++++++++++++-----
 lib/src/runner/browser/browser_manager.dart |  7 +--
 lib/src/runner/browser/platform.dart        |  6 +--
 lib/src/runner/loader.dart                  | 17 ++++++-
 lib/src/runner/node/platform.dart           |  6 +--
 lib/src/runner/plugin/platform.dart         | 14 +++---
 lib/src/runner/plugin/platform_helpers.dart | 13 ++---
 lib/src/runner/remote_listener.dart         |  5 +-
 8 files changed, 87 insertions(+), 35 deletions(-)

diff --git a/lib/src/backend/test_platform.dart b/lib/src/backend/test_platform.dart
index d387eba4..69146515 100644
--- a/lib/src/backend/test_platform.dart
+++ b/lib/src/backend/test_platform.dart
@@ -60,6 +60,18 @@ class TestPlatform {
       all.firstWhere((platform) => platform.identifier == identifier,
           orElse: () => null);
 
+  static Set<TestPlatform> _builtIn = new Set.from([
+    TestPlatform.vm,
+    TestPlatform.dartium,
+    TestPlatform.contentShell,
+    TestPlatform.chrome,
+    TestPlatform.phantomJS,
+    TestPlatform.firefox,
+    TestPlatform.safari,
+    TestPlatform.internetExplorer,
+    TestPlatform.nodeJS
+  ]);
+
   /// The human-friendly name of the platform.
   final String name;
 
@@ -88,20 +100,40 @@ class TestPlatform {
       this.isBlink: false,
       this.isHeadless: false});
 
+  /// Converts a JSON-safe representation generated by [serialize] back into a
+  /// [TestPlatform].
+  factory TestPlatform.deserialize(Object serialized) {
+    if (serialized is String) return find(serialized);
+
+    var map = serialized as Map;
+    return new TestPlatform._(map["name"], map["identifier"],
+        isDartVM: map["isDartVM"],
+        isBrowser: map["isBrowser"],
+        isJS: map["isJS"],
+        isBlink: map["isBlink"],
+        isHeadless: map["isHeadless"]);
+  }
+
+  /// Converts [this] into a JSON-safe object that can be converted back to a
+  /// [TestPlatform] using [new TestPlatform.deserialize].
+  Object serialize() {
+    if (_builtIn.contains(this)) return identifier;
+
+    return {
+      "name": name,
+      "identifier": identifier,
+      "isDartVM": isDartVM,
+      "isBrowser": isBrowser,
+      "isJS": isJS,
+      "isBlink": isBlink,
+      "isHeadless": isHeadless
+    };
+  }
+
   String toString() => name;
 }
 
-final List<TestPlatform> _allPlatforms = [
-  TestPlatform.vm,
-  TestPlatform.dartium,
-  TestPlatform.contentShell,
-  TestPlatform.chrome,
-  TestPlatform.phantomJS,
-  TestPlatform.firefox,
-  TestPlatform.safari,
-  TestPlatform.internetExplorer,
-  TestPlatform.nodeJS
-];
+final List<TestPlatform> _allPlatforms = TestPlatform._builtIn.toList();
 
 /// **Do not call this function without express permission from the test package
 /// authors**.
diff --git a/lib/src/runner/browser/browser_manager.dart b/lib/src/runner/browser/browser_manager.dart
index 89667203..38599ea0 100644
--- a/lib/src/runner/browser/browser_manager.dart
+++ b/lib/src/runner/browser/browser_manager.dart
@@ -201,7 +201,8 @@ class BrowserManager {
   ///
   /// If [mapper] is passed, it's used to map stack traces for errors coming
   /// from this test suite.
-  Future<RunnerSuite> load(String path, Uri url, SuiteConfiguration suiteConfig,
+  Future<RunnerSuite> load(
+      String path, Uri url, SuiteConfiguration suiteConfig, Object message,
       {StackTraceMapper mapper}) async {
     url = url.replace(
         fragment: Uri.encodeFull(JSON.encode({
@@ -236,8 +237,8 @@ class BrowserManager {
       });
 
       try {
-        controller = await deserializeSuite(
-            path, _platform, suiteConfig, await _environment, suiteChannel,
+        controller = await deserializeSuite(path, _platform, suiteConfig,
+            await _environment, suiteChannel, message,
             mapper: mapper);
         _controllers.add(controller);
         return controller.suite;
diff --git a/lib/src/runner/browser/platform.dart b/lib/src/runner/browser/platform.dart
index 06c1c41d..31c7cabe 100644
--- a/lib/src/runner/browser/platform.dart
+++ b/lib/src/runner/browser/platform.dart
@@ -192,8 +192,8 @@ class BrowserPlatform extends PlatformPlugin {
   ///
   /// This will start a browser to load the suite if one isn't already running.
   /// Throws an [ArgumentError] if [browser] isn't a browser platform.
-  Future<RunnerSuite> load(
-      String path, TestPlatform browser, SuiteConfiguration suiteConfig) async {
+  Future<RunnerSuite> load(String path, TestPlatform browser,
+      SuiteConfiguration suiteConfig, Object message) async {
     assert(suiteConfig.platforms.contains(browser.identifier));
 
     if (!browser.isBrowser) {
@@ -271,7 +271,7 @@ class BrowserPlatform extends PlatformPlugin {
     var browserManager = await _browserManagerFor(browser);
     if (_closed || browserManager == null) return null;
 
-    var suite = await browserManager.load(path, suiteUrl, suiteConfig,
+    var suite = await browserManager.load(path, suiteUrl, suiteConfig, message,
         mapper: browser.isJS ? _mappers[path] : null);
     if (_closed) return null;
     return suite;
diff --git a/lib/src/runner/loader.dart b/lib/src/runner/loader.dart
index b57121d6..b8166f07 100644
--- a/lib/src/runner/loader.dart
+++ b/lib/src/runner/loader.dart
@@ -47,6 +47,20 @@ class Loader {
   List<TestPlatform> get allPlatforms =>
       new List.unmodifiable(_platformCallbacks.keys);
 
+  List<Map<String, Object>> get _allPlatformsSerialized {
+    if (__allPlatformsSerialized != null &&
+        __allPlatformsSerialized.length == _platformCallbacks.length) {
+      return __allPlatformsSerialized;
+    }
+
+    __allPlatformsSerialized = _platformCallbacks.keys
+        .map((platform) => platform.serialize())
+        .toList();
+    return __allPlatformsSerialized;
+  }
+
+  List<Map<String, Object>> __allPlatformsSerialized;
+
   /// Creates a new loader that loads tests on platforms defined in
   /// [Configuration.current].
   ///
@@ -169,7 +183,8 @@ class Loader {
 
         try {
           var plugin = await memo.runOnce(_platformCallbacks[platform]);
-          var suite = await plugin.load(path, platform, platformConfig);
+          var suite = await plugin.load(path, platform, platformConfig,
+              {"testPlatforms": _allPlatformsSerialized});
           if (suite != null) _suites.add(suite);
           return suite;
         } catch (error, stackTrace) {
diff --git a/lib/src/runner/node/platform.dart b/lib/src/runner/node/platform.dart
index 179edc41..dcb41c0c 100644
--- a/lib/src/runner/node/platform.dart
+++ b/lib/src/runner/node/platform.dart
@@ -51,12 +51,12 @@ class NodePlatform extends PlatformPlugin {
       throw new UnimplementedError();
 
   Future<RunnerSuite> load(String path, TestPlatform platform,
-      SuiteConfiguration suiteConfig) async {
+      SuiteConfiguration suiteConfig, Object message) async {
     assert(platform == TestPlatform.nodeJS);
 
     var pair = await _loadChannel(path, suiteConfig);
-    var controller = await deserializeSuite(
-        path, platform, suiteConfig, new PluginEnvironment(), pair.first,
+    var controller = await deserializeSuite(path, platform, suiteConfig,
+        new PluginEnvironment(), pair.first, message,
         mapper: pair.last);
     return controller.suite;
   }
diff --git a/lib/src/runner/plugin/platform.dart b/lib/src/runner/plugin/platform.dart
index f3e7eec0..744c4634 100644
--- a/lib/src/runner/plugin/platform.dart
+++ b/lib/src/runner/plugin/platform.dart
@@ -23,8 +23,8 @@ import 'platform_helpers.dart';
 /// In order to support interactive debugging, a plugin must override [load] as
 /// well, which returns a [RunnerSuite] that can contain a custom [Environment]
 /// and control debugging metadata such as [RunnerSuite.isDebugging] and
-/// [RunnerSuite.onDebugging]. To make this easier, implementations can call
-/// [deserializeSuite] in `platform_helpers.dart`.
+/// [RunnerSuite.onDebugging]. The plugin must create this suite by calling the
+/// [deserializeSuite] helper function.
 ///
 /// A platform plugin can be registered with [Loader.registerPlatformPlugin].
 abstract class PlatformPlugin {
@@ -52,16 +52,16 @@ abstract class PlatformPlugin {
   /// fine-grained control over the [RunnerSuite], including providing a custom
   /// implementation of [Environment].
   ///
-  /// It's recommended that subclasses overriding this method call
-  /// [deserializeSuite] in `platform_helpers.dart` to obtain a
-  /// [RunnerSuiteController].
+  /// Subclasses overriding this method must call [deserializeSuite] in
+  /// `platform_helpers.dart` to obtain a [RunnerSuiteController]. They must
+  /// pass the opaque [message] parameter to the [deserializeSuite] call.
   Future<RunnerSuite> load(String path, TestPlatform platform,
-      SuiteConfiguration suiteConfig) async {
+      SuiteConfiguration suiteConfig, Object message) async {
     // loadChannel may throw an exception. That's fine; it will cause the
     // LoadSuite to emit an error, which will be presented to the user.
     var channel = loadChannel(path, platform);
     var controller = await deserializeSuite(
-        path, platform, suiteConfig, new PluginEnvironment(), channel);
+        path, platform, suiteConfig, new PluginEnvironment(), channel, message);
     return controller.suite;
   }
 
diff --git a/lib/src/runner/plugin/platform_helpers.dart b/lib/src/runner/plugin/platform_helpers.dart
index 786acd69..c1d2d74d 100644
--- a/lib/src/runner/plugin/platform_helpers.dart
+++ b/lib/src/runner/plugin/platform_helpers.dart
@@ -33,18 +33,19 @@ final _deserializeTimeout = new Duration(minutes: 8);
 ///
 /// If the suite is closed, this will close [channel].
 ///
-/// If [mapTrace] is passed, it will be used to adjust stack traces for any
-/// errors emitted by tests.
+/// The [message] parameter is an opaque object passed from the runner to
+/// [PlatformPlugin.load]. Plugins shouldn't interact with it other than to pass
+/// it on to [deserializeSuite].
 ///
-/// If [asciiSymbols] is passed, it controls whether the `symbol` package is
-/// configured to use plain ASCII or Unicode symbols. It defaults to `true` on
-/// Windows and `false` elsewhere.
+/// If [mapper] is passed, it will be used to adjust stack traces for any errors
+/// emitted by tests.
 Future<RunnerSuiteController> deserializeSuite(
     String path,
     TestPlatform platform,
     SuiteConfiguration suiteConfig,
     Environment environment,
     StreamChannel channel,
+    Object message,
     {StackTraceMapper mapper}) async {
   var disconnector = new Disconnector();
   var suiteChannel = new MultiChannel(channel.transform(disconnector));
@@ -60,7 +61,7 @@ Future<RunnerSuiteController> deserializeSuite(
     'stackTraceMapper': mapper?.serialize(),
     'foldTraceExcept': Configuration.current.foldTraceExcept.toList(),
     'foldTraceOnly': Configuration.current.foldTraceOnly.toList(),
-  });
+  }..addAll(message as Map));
 
   var completer = new Completer();
 
diff --git a/lib/src/runner/remote_listener.dart b/lib/src/runner/remote_listener.dart
index e0fe0fef..be13c59b 100644
--- a/lib/src/runner/remote_listener.dart
+++ b/lib/src/runner/remote_listener.dart
@@ -77,7 +77,10 @@ class RemoteListener {
       if (message['asciiGlyphs'] ?? false) glyph.ascii = true;
       var metadata = new Metadata.deserialize(message['metadata']);
       verboseChain = metadata.verboseTrace;
-      var declarer = new Declarer(TestPlatform.all,
+      var declarer = new Declarer(
+          message['testPlatforms']
+              .map((platform) => new TestPlatform.deserialize(platform))
+              .toList(),
           metadata: metadata,
           collectTraces: message['collectTraces'],
           noRetry: message['noRetry']);
-- 
GitLab