Remove registerTestPlatform() (#695)

Now that the Loader is the canonical place for determining which
TestPlatforms are valid, we no longer need a separate mechanism for
registering platforms. Adding a plugin to the loader is sufficient.

See #391

Author: nex3

lib/src/backend/test_platform.dart

@@ -2,65 +2,52 @@
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
-import 'dart:collection';
-
 // TODO(nweiz): support pluggable platforms.
 /// An enum of all platforms on which tests can run.
 class TestPlatform {
   // When adding new platforms, be sure to update the baseline and derived
   // variable tests in test/backend/platform_selector/evaluate_test.
 
   /// The command-line Dart VM.
-  static const TestPlatform vm =
-      const TestPlatform._("VM", "vm", isDartVM: true);
+  static const TestPlatform vm = const TestPlatform("VM", "vm", isDartVM: true);
 
   /// Dartium.
-  static const TestPlatform dartium = const TestPlatform._("Dartium", "dartium",
+  static const TestPlatform dartium = const TestPlatform("Dartium", "dartium",
       isBrowser: true, isBlink: true, isDartVM: true);
 
   /// Dartium content shell.
-  static const TestPlatform contentShell = const TestPlatform._(
+  static const TestPlatform contentShell = const TestPlatform(
       "Dartium Content Shell", "content-shell",
       isBrowser: true, isBlink: true, isDartVM: true, isHeadless: true);
 
   /// Google Chrome.
-  static const TestPlatform chrome = const TestPlatform._("Chrome", "chrome",
+  static const TestPlatform chrome = const TestPlatform("Chrome", "chrome",
       isBrowser: true, isJS: true, isBlink: true);
 
   /// PhantomJS.
-  static const TestPlatform phantomJS = const TestPlatform._(
+  static const TestPlatform phantomJS = const TestPlatform(
       "PhantomJS", "phantomjs",
       isBrowser: true, isJS: true, isBlink: true, isHeadless: true);
 
   /// Mozilla Firefox.
   static const TestPlatform firefox =
-      const TestPlatform._("Firefox", "firefox", isBrowser: true, isJS: true);
+      const TestPlatform("Firefox", "firefox", isBrowser: true, isJS: true);
 
   /// Apple Safari.
   static const TestPlatform safari =
-      const TestPlatform._("Safari", "safari", isBrowser: true, isJS: true);
+      const TestPlatform("Safari", "safari", isBrowser: true, isJS: true);
 
   /// Microsoft Internet Explorer.
-  static const TestPlatform internetExplorer = const TestPlatform._(
+  static const TestPlatform internetExplorer = const TestPlatform(
       "Internet Explorer", "ie",
       isBrowser: true, isJS: true);
 
   /// The command-line Node.js VM.
   static const TestPlatform nodeJS =
-      const TestPlatform._("Node.js", "node", isJS: true);
-
-  /// A list of all instances of [TestPlatform].
-  static final UnmodifiableListView<TestPlatform> all =
-      new UnmodifiableListView<TestPlatform>(_allPlatforms);
-
-  /// Finds a platform by its identifier string.
-  ///
-  /// If no platform is found, returns `null`.
-  static TestPlatform find(String identifier) =>
-      all.firstWhere((platform) => platform.identifier == identifier,
-          orElse: () => null);
+      const TestPlatform("Node.js", "node", isJS: true);
 
-  static Set<TestPlatform> _builtIn = new Set.from([
+  /// The platforms that are supported by the test runner by default.
+  static const List<TestPlatform> builtIn = const [
     TestPlatform.vm,
     TestPlatform.dartium,
     TestPlatform.contentShell,
@@ -70,7 +57,7 @@ class TestPlatform {
     TestPlatform.safari,
     TestPlatform.internetExplorer,
     TestPlatform.nodeJS
-  ]);
+  ];
 
   /// The human-friendly name of the platform.
   final String name;
@@ -93,7 +80,7 @@ class TestPlatform {
   /// Whether this platform has no visible window.
   final bool isHeadless;
 
-  const TestPlatform._(this.name, this.identifier,
+  const TestPlatform(this.name, this.identifier,
       {this.isDartVM: false,
       this.isBrowser: false,
       this.isJS: false,
@@ -103,10 +90,13 @@ class TestPlatform {
   /// Converts a JSON-safe representation generated by [serialize] back into a
   /// [TestPlatform].
   factory TestPlatform.deserialize(Object serialized) {
-    if (serialized is String) return find(serialized);
+    if (serialized is String) {
+      return builtIn
+          .firstWhere((platform) => platform.identifier == serialized);
+    }
 
     var map = serialized as Map;
-    return new TestPlatform._(map["name"], map["identifier"],
+    return new TestPlatform(map["name"], map["identifier"],
         isDartVM: map["isDartVM"],
         isBrowser: map["isBrowser"],
         isJS: map["isJS"],
@@ -117,7 +107,7 @@ class TestPlatform {
   /// 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;
+    if (builtIn.contains(this)) return identifier;
 
     return {
       "name": name,
@@ -132,26 +122,3 @@ class TestPlatform {
 
   String toString() => name;
 }
-
-final List<TestPlatform> _allPlatforms = TestPlatform._builtIn.toList();
-
-/// **Do not call this function without express permission from the test package
-/// authors**.
-///
-/// This constructs and globally registers a new TestPlatform with the provided
-/// details.
-TestPlatform registerTestPlatform(String name, String identifier,
-    {bool isDartVM: false,
-    bool isBrowser: false,
-    bool isJS: false,
-    bool isBlink: false,
-    bool isHeadless: false}) {
-  var platform = new TestPlatform._(name, identifier,
-      isDartVM: isDartVM,
-      isBrowser: isBrowser,
-      isJS: isJS,
-      isBlink: isBlink,
-      isHeadless: isHeadless);
-  _allPlatforms.add(platform);
-  return platform;
-}

lib/src/runner.dart

@@ -130,7 +130,7 @@ class Runner {
     if (testOn == PlatformSelector.all) return;
 
     var unsupportedPlatforms = _config.suiteDefaults.platforms
-        .map(TestPlatform.find)
+        .map(_loader.findTestPlatform)
         .where((platform) =>
             platform != null && !testOn.evaluate(platform, os: currentOS))
         .toList();
@@ -145,7 +145,7 @@ class Runner {
     var unsupportedBrowsers =
         unsupportedPlatforms.where((platform) => platform.isBrowser);
     if (unsupportedBrowsers.isNotEmpty) {
-      var supportsAnyBrowser = TestPlatform.all
+      var supportsAnyBrowser = _loader.allPlatforms
           .where((platform) => platform.isBrowser)
           .any((platform) => testOn.evaluate(platform));
 

lib/src/runner/configuration/args.dart

@@ -18,7 +18,7 @@ import 'values.dart';
 final ArgParser _parser = (() {
   var parser = new ArgParser(allowTrailingOptions: true);
 
-  var allPlatforms = TestPlatform.all.toList()..remove(TestPlatform.vm);
+  var allPlatforms = TestPlatform.builtIn.toList()..remove(TestPlatform.vm);
   if (!Platform.isMacOS) allPlatforms.remove(TestPlatform.safari);
   if (!Platform.isWindows) allPlatforms.remove(TestPlatform.internetExplorer);
 

lib/src/runner/loader.dart

@@ -43,6 +43,10 @@ class Loader {
   /// These are passed to the plugins' async memoizers when a plugin is needed.
   final _platformCallbacks = <TestPlatform, AsyncFunction>{};
 
+  /// A map of all platforms registered in [_platformCallbacks], indexed by
+  /// their string identifiers.
+  final _platformsByIdentifier = <String, TestPlatform>{};
+
   /// All plaforms supported by this [Loader].
   List<TestPlatform> get allPlatforms =>
       new List.unmodifiable(_platformCallbacks.keys);
@@ -98,9 +102,15 @@ class Loader {
     for (var platform in platforms) {
       _platformPlugins[platform] = memoizer;
       _platformCallbacks[platform] = getPlugin;
+      _platformsByIdentifier[platform.identifier] = platform;
     }
   }
 
+  /// Returns the [TestPlatform] registered with this loader that's identified
+  /// by [identifier], or `null` if none can be found.
+  TestPlatform findTestPlatform(String identifier) =>
+      _platformsByIdentifier[identifier];
+
   /// Loads all test suites in [dir] according to [suiteConfig].
   ///
   /// This will load tests from files that match the global configuration's
@@ -156,7 +166,8 @@ class Loader {
     }
 
     for (var platformName in suiteConfig.platforms) {
-      var platform = TestPlatform.find(platformName);
+      var platform = findTestPlatform(platformName);
+      assert(platform != null, 'Unknown platform "$platformName".');
 
       if (!suiteConfig.metadata.testOn.evaluate(platform, os: currentOS)) {
         continue;

lib/src/runner/remote_listener.dart

@@ -73,14 +73,14 @@ class RemoteListener {
       }
 
       var message = await channel.stream.first;
+      var platforms = message['testPlatforms']
+          .map((platform) => new TestPlatform.deserialize(platform))
+          .toList();
 
       if (message['asciiGlyphs'] ?? false) glyph.ascii = true;
       var metadata = new Metadata.deserialize(message['metadata']);
       verboseChain = metadata.verboseTrace;
-      var declarer = new Declarer(
-          message['testPlatforms']
-              .map((platform) => new TestPlatform.deserialize(platform))
-              .toList(),
+      var declarer = new Declarer(platforms,
           metadata: metadata,
           collectTraces: message['collectTraces'],
           noRetry: message['noRetry']);
@@ -94,7 +94,8 @@ class RemoteListener {
 
       var os =
           message['os'] == null ? null : OperatingSystem.find(message['os']);
-      var platform = TestPlatform.find(message['platform']);
+      var platform = platforms
+          .firstWhere((platform) => platform.identifier == message['platform']);
       var suite = new Suite(declarer.build(),
           platform: platform, os: os, path: message['path']);
       new RemoteListener._(suite, printZone)._listen(channel);

lib/test.dart

@@ -53,7 +53,7 @@ Declarer get _declarer {
   // In order to run the tests, we set up our own Declarer via
   // [_globalDeclarer], and schedule a microtask to run the tests once they're
   // finished being defined.
-  _globalDeclarer = new Declarer(TestPlatform.all);
+  _globalDeclarer = new Declarer(TestPlatform.builtIn);
   scheduleMicrotask(() async {
     var suite = new RunnerSuite(const PluginEnvironment(),
         SuiteConfiguration.empty, _globalDeclarer.build(),