Proper support for VMOptions declarations for dart2dart.

tools/testing/dart/test_suite.dart

Index: tools/testing/dart/test_suite.dart
diff --git a/tools/testing/dart/test_suite.dart b/tools/testing/dart/test_suite.dart
index d2ce8543459aaf254b5dce26b5f08716c9c1fafa..92aba4ec793a1db957282ab9116f3c2b0fe42f2e 100644
--- a/tools/testing/dart/test_suite.dart
+++ b/tools/testing/dart/test_suite.dart
@@ -417,12 +417,14 @@ class StandardTestSuite implements TestSuite {
       }
     }
 
-    var argumentLists = argumentListsFromFile(info.filePath,
-                                              info.optionsFromFile);
+    var commonArguments = commonArgumentsFromFile(info.filePath,
+                                                  info.optionsFromFile);
 
-    for (var args in argumentLists) {
+    List<List<String>> vmOptionsList = getVmOptions(info.optionsFromFile);
+    Expect.isFalse(vmOptionsList.isEmpty(), "empty vmOptionsList");
+    for (var vmOptions in vmOptionsList) {
       doTest(new TestCase('$suiteName/$testName',
-                          makeCommands(info, args),
+                          makeCommands(info, vmOptions, commonArguments),
                           configuration,
                           completeHandler,
                           expectations,
@@ -431,13 +433,14 @@ class StandardTestSuite implements TestSuite {
     }
   }
 
-  List<Command> makeCommands(TestInformation info, var args) {
+  List<Command> makeCommands(TestInformation info, var vmOptions, var args) {
     switch (configuration['compiler']) {
     case 'dart2js':
-      args = new List.from(args);
+      var arguments = new List.from(vmOptions);

[Bill Hesse, 2012/08/13 08:13:11]

We do not need to add vmOptions to dart2js.  They are zeroed out in this case
much earlier.  If we eliminate this here, then we can eliminate the zeroing out.

In fact, we can eliminate the entire function getVmOptions at line 1035.  Let's
do that now.

[Anton Muhin, 2012/08/13 09:23:34]

Done.

+      arguments.addAll(args);
       String tempDir = createOutputDirectory(info.filePath, '');
-      args.add('--out=$tempDir/out.js');
-      List<Command> commands = <Command>[new Command(shellPath(), args)];
+      arguments.add('--out=$tempDir/out.js');
+      List<Command> commands = <Command>[new Command(shellPath(), arguments)];
       if (configuration['runtime'] == 'd8') {
         var d8 = TestUtils.d8FileName(configuration);
         commands.add(new Command(d8, ['$tempDir/out.js']));
@@ -445,23 +448,29 @@ class StandardTestSuite implements TestSuite {
       return commands;
 
     case 'dart2dart':
-      args = new List.from(args);
-      args.add('--output-type=dart');
+      var compilerArguments = new List.from(args);
+      compilerArguments.add('--output-type=dart');
       String tempDir = createOutputDirectory(info.filePath, '');
-      args.add('--out=$tempDir/out.dart');
-      List<Command> commands = <Command>[new Command(shellPath(), args)];
+      compilerArguments.add('--out=$tempDir/out.dart');
+      List<Command> commands =
+          <Command>[new Command(shellPath(), compilerArguments)];
       if (configuration['runtime'] == 'vm') {
         // TODO(antonm): support checked.
+        var vmArguments = new List.from(vmOptions);

[Bill Hesse, 2012/08/13 08:13:11]

Let's add runtime == 'vm' before adding vmOptions, in both places.  In that
case, you don't need to split the default case into none and dartc, but I think
it would still be good.

[Anton Muhin, 2012/08/13 09:23:34]

Sorry, I do not quite follow.

On 2012/08/13 08:13:11, Bill Hesse wrote:

+        vmArguments.addAll(
+            ['--enable_checked_mode', '$tempDir/out.dart']);
         commands.add(new Command(
             TestUtils.vmFileName(configuration),
-            ['--enable_checked_mode', '$tempDir/out.dart']));
+            vmArguments));
       } else {
         throw 'Unsupported runtime ${configuration["runtime"]} for dart2dart';
       }
       return commands;
 
     default:

[Bill Hesse, 2012/08/13 08:13:11]

Perhaps split the default case into 'none' and 'dartc', and add vmoptions only
to none.  'frog' should be eliminated.

[Anton Muhin, 2012/08/13 09:23:34]

Done.

-      return <Command>[new Command(shellPath(), args)];
+      var arguments = new List.from(vmOptions);
+      arguments.addAll(args);
+      return <Command>[new Command(shellPath(), arguments)];
     }
   }
 
@@ -827,8 +836,7 @@ class StandardTestSuite implements TestSuite {
   void completeHandler(TestCase testCase) {
   }
 
-  List<List<String>> argumentListsFromFile(Path filePath,
-                                           Map optionsFromFile) {
+  List<String> commonArgumentsFromFile(Path filePath, Map optionsFromFile) {
     List args = TestUtils.standardOptions(configuration);
     args.addAll(additionalOptions(filePath));
     if (configuration['compiler'] == 'dartc') {
@@ -858,15 +866,7 @@ class StandardTestSuite implements TestSuite {
       args.addAll(dartOptions);
     }
 
-    var result = new List<List<String>>();
-    Expect.isFalse(vmOptionsList.isEmpty(), "empty vmOptionsList");
-    for (var vmOptions in vmOptionsList) {
-      var options = new List<String>.from(vmOptions);
-      options.addAll(args);
-      result.add(options);
-    }
-
-    return result;
+    return args;
   }
 
   /**