Proper support for VMOptions declarations for dart2dart.

tools/testing/dart/test_suite.dart

 // Copyright (c) 2012, 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.
 
 /**
  * Classes and methods for enumerating and preparing tests.
  *
  * This library includes:
  *
  * - Creating tests by listing all the Dart files in certain directories,
@@ skipping 399 matching lines @@
     if (configuration['compiler'] == 'dartc') {
       // dartc can detect static type warnings by the
       // format of the error line
       if (info.hasFatalTypeErrors) {
         isNegative = true;
       } else if (info.hasRuntimeErrors) {
         isNegative = false;
       }
     }
 
-    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,
                           isNegative,
                           info));
     }
   }
 
-  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']));
       }
       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)];
     }
   }
 
   Function makeTestCaseCreator(Map optionsFromFile) {
     return (Path filePath,
             bool isNegative,
             [bool isNegativeIfChecked = false,
              bool hasFatalTypeErrors = false,
              bool hasRuntimeErrors = false,
              Set<String> multitestOutcome = null]) {
@@ skipping 345 matching lines @@
   void directoryListingDone(ignore) {
     listingDone = true;
     if (activeTestGenerators == 0) {
       doDone();
     }
   }
 
   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') {
       args.add('--error_format');
       args.add('machine');
     }
     if ((configuration['compiler'] == 'frog')
         && (configuration['runtime'] == 'none')) {
       args.add('--compile-only');
     }
 
     bool isMultitest = optionsFromFile["isMultitest"];
     List<String> dartOptions = optionsFromFile["dartOptions"];
     List<List<String>> vmOptionsList = getVmOptions(optionsFromFile);
     Expect.isTrue(!isMultitest || dartOptions == null);
     if (dartOptions == null) {
       args.add(filePath.toNativePath());
     } else {
       var executable_name = dartOptions[0];
       // TODO(ager): Get rid of this hack when the runtime checkout goes away.
       var file = new File(executable_name);
       if (!file.existsSync()) {
         executable_name = '../$executable_name';
         Expect.isTrue(new File(executable_name).existsSync());
         dartOptions[0] = executable_name;
       }
       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;
   }
 
   /**
    * Special options for individual tests are currently specified in various
    * ways: with comments directly in test files, by using certain imports, or by
    * creating additional files in the test directories.
    *
    * Here is a list of options that are used by 'test.dart' today:
    *   - Flags can be passed to the vm or dartium process that runs the test by
    *   adding a comment to the test file:
@@ skipping 547 matching lines @@
  * $noCrash tests are expected to be flaky but not crash
  * $pass tests are expected to pass
  * $failOk tests are expected to fail that we won't fix
  * $fail tests are expected to fail that we should fix
  * $crash tests are expected to crash that we should fix
  * $timeout tests are allowed to timeout
 """;
     print(report);
    }
 }