Replace the configuration map with a typed object.

tools/testing/dart/runtime_configuration.dart

 // Copyright (c) 2014, the Dart project authors.  Please see the AUTHORS file
 // 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.
 
-library runtime_configuration;
+import 'dart:io';
 
-import 'dart:io' show Directory, File;
-
-import 'compiler_configuration.dart' show CommandArtifact;
-
+import 'compiler_configuration.dart';
+import 'configuration.dart';
 // TODO(ahe): Remove this import, we can precompute all the values required
 // from TestSuite once the refactoring is complete.
-import 'test_suite.dart' show TestSuite, TestUtils;
-
-import 'test_runner.dart' show Command, CommandBuilder;
+import 'test_suite.dart';
+import 'test_runner.dart';
 
 /// Describes the commands to run a given test case or its compiled output.
 ///
 /// A single runtime configuration object exists per test suite, and is thus
 /// shared between multiple test cases, it should not be mutated after
 /// construction.
-//
-// TODO(ahe): I expect this class will become abstract very soon.
-class RuntimeConfiguration {
-  // TODO(ahe): Remove this constructor and move the switch to

[Bill Hesse, 2017/05/29 13:08:28]

I agree this could be made a static method, not a constructor, but I don't see
the advantage to moving it to test_options.dart.

[Bob Nystrom, 2017/05/30 23:29:31]

Moved it back.

-  // test_options.dart.  We probably want to store an instance of
-  // [RuntimeConfiguration] in [configuration] there.
-  factory RuntimeConfiguration(Map<String, dynamic> configuration) {
-    var runtime = configuration['runtime'] as String;
-    var useBlobs = configuration['use_blobs'] as bool;
-
-    switch (runtime) {
-      case 'ContentShellOnAndroid':
-      case 'DartiumOnAndroid':
-      case 'chrome':
-      case 'chromeOnAndroid':
-      case 'dartium':
-      case 'ff':
-      case 'firefox':
-      case 'ie11':
-      case 'ie10':
-      case 'ie9':
-      case 'opera':
-      case 'safari':
-      case 'safarimobilesim':
-        // TODO(ahe): Replace this with one or more browser runtimes.
-        return new DummyRuntimeConfiguration();
-
-      case 'jsshell':
-        return new JsshellRuntimeConfiguration();
-
-      case 'd8':
-        return new D8RuntimeConfiguration();
-
-      case 'none':
-        return new NoneRuntimeConfiguration();
-
-      case 'vm':
-        return new StandaloneDartRuntimeConfiguration();
-
-      case 'flutter':
-        return new StandaloneFlutterEngineConfiguration();
-
-      case 'dart_precompiled':
-        if (configuration['system'] == 'android') {
-          return new DartPrecompiledAdbRuntimeConfiguration(useBlobs: useBlobs);
-        }
-        return new DartPrecompiledRuntimeConfiguration(useBlobs: useBlobs);
-
-      case 'drt':
-        return new DrtRuntimeConfiguration();
-
-      case 'self_check':
-        return new SelfCheckRuntimeConfiguration();
-
-      default:
-        throw "Unknown runtime '$runtime'";
-    }
-  }
-
+abstract class RuntimeConfiguration {
   RuntimeConfiguration._subclass();
 
-  int computeTimeoutMultiplier(
-      {String mode, bool isChecked: false, bool isReload: false, String arch}) {
+  int timeoutMultiplier(
+      {Mode mode,
+      bool isChecked: false,
+      bool isReload: false,
+      Architecture arch}) {
     return 1;
   }
 
   List<Command> computeRuntimeCommands(
       TestSuite suite,
       CommandBuilder commandBuilder,
       CommandArtifact artifact,
       List<String> arguments,
       Map<String, String> environmentOverrides) {
     // TODO(ahe): Make this method abstract.
@@ skipping 74 matching lines @@
 
   List<String> dart2jsPreambles(Uri preambleDir) {
     return ['-f', preambleDir.resolve('jsshell.js').toFilePath(), '-f'];
   }
 }
 
 /// Common runtime configuration for runtimes based on the Dart VM.
 class DartVmRuntimeConfiguration extends RuntimeConfiguration {
   DartVmRuntimeConfiguration() : super._subclass();
 
-  int computeTimeoutMultiplier(
-      {String mode, bool isChecked: false, bool isReload: false, String arch}) {
-    int multiplier = 1;
+  int timeoutMultiplier(
+      {Mode mode,
+      bool isChecked: false,
+      bool isReload: false,
+      Architecture arch}) {
+    var multiplier = 1;
+
     switch (arch) {
-      case 'simarm':
-      case 'arm':
-      case 'simarmv6':
-      case 'armv6':
-      case ' simarmv5te':
-      case 'armv5te':
-      case 'simmips':
-      case 'mips':
-      case 'simarm64':
-      case 'simdbc':
-      case 'simdbc64':
+      case Architecture.simarm:
+      case Architecture.arm:
+      case Architecture.simarmv6:
+      case Architecture.armv6:
+      case Architecture.simarmv5te:
+      case Architecture.armv5te:
+      case Architecture.simmips:
+      case Architecture.mips:
+      case Architecture.simarm64:
+      case Architecture.simdbc:
+      case Architecture.simdbc64:
         multiplier *= 4;
         break;
     }
-    if (mode == 'debug') {
+
+    if (mode.isDebug) {
       multiplier *= 2;
       if (isReload) {
         multiplier *= 2;
       }
     }
     return multiplier;
   }
 }
 
 /// Runtime configuration for Content Shell.  We previously used a similar
 /// program named Dump Render Tree, hence the name.
 class DrtRuntimeConfiguration extends DartVmRuntimeConfiguration {
-  int computeTimeoutMultiplier(
-      {String mode, bool isChecked: false, bool isReload: false, String arch}) {
+  int timeoutMultiplier(
+      {Mode mode,
+      bool isChecked: false,
+      bool isReload: false,
+      Architecture arch}) {
     return 4 // Allow additional time for browser testing to run.
         // TODO(ahe): We might need to distinguish between DRT for running
         // JavaScript and Dart code.  I'm not convinced the inherited timeout
         // multiplier is relevant for JavaScript.
         *
-        super.computeTimeoutMultiplier(
+        super.timeoutMultiplier(
             mode: mode, isChecked: isChecked, isReload: isReload);
   }
 }
 
 /// The standalone Dart VM binary, "dart" or "dart.exe".
 class StandaloneDartRuntimeConfiguration extends DartVmRuntimeConfiguration {
   List<Command> computeRuntimeCommands(
       TestSuite suite,
       CommandBuilder commandBuilder,
       CommandArtifact artifact,
@@ skipping 108 matching lines @@
   List<Command> computeRuntimeCommands(
       TestSuite suite,
       CommandBuilder commandBuilder,
       CommandArtifact artifact,
       List<String> arguments,
       Map<String, String> environmentOverrides) {
     String executable = suite.dartVmBinaryFileName;
     return selfCheckers
         .map((String tester) => commandBuilder.getVmBatchCommand(
             executable, tester, arguments, environmentOverrides,
-            checked: suite.configuration['checked'] as bool))
+            checked: suite.configuration.isChecked))
         .toList();
   }
 
   @override
   bool get shouldSkipNegativeTests => true;
 }
 
 /// Temporary runtime configuration for browser runtimes that haven't been
 /// migrated yet.
 // TODO(ahe): Remove this class.
 class DummyRuntimeConfiguration extends DartVmRuntimeConfiguration {
   List<Command> computeRuntimeCommands(
       TestSuite suite,
       CommandBuilder commandBuilder,
       CommandArtifact artifact,
       List<String> arguments,
       Map<String, String> environmentOverrides) {
     throw "Unimplemented runtime '$runtimeType'";
   }
 }