tools/test.dart: Add multitest support to Dart implementation of test runner.

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 7d157583e9642d9d06e179a6a29707b3f7b3ff56..b29916323a2c6288b15f325898faabf7b6acf74b 100644
--- a/tools/testing/dart/test_suite.dart
+++ b/tools/testing/dart/test_suite.dart
@@ -6,6 +6,8 @@
 
 #import("status_file_parser.dart");
 #import("test_runner.dart");
+#import("multitest.dart");
+
 
 interface TestSuite {
   void forEachTest(Function onTest, [Function onDone]);
@@ -180,25 +182,47 @@ class StandardTestSuite implements TestSuite {
       return;
     }
 
-    int start = filename.lastIndexOf('src' + new Platform().pathSeparator());
-    String testName = filename.substring(start + 4, filename.length - 5);
-    Set<String> expectations = testExpectations.expectations(testName);
-
-    if (expectations.contains(SKIP)) return;
 
-    var optionsFromFile = optionsFromFile(filename);
-    var isNegative = optionsFromFile['isNegative'];
-    var argumentLists = argumentListsFromFile(filename, optionsFromFile);
     var timeout = configuration['timeout'];
+    var optionsFromFile = optionsFromFile(filename);
 
-    for (var args in argumentLists) {
-      doTest(new TestCase(testName,
-                          shellPath,
-                          args,
-                          timeout,
-                          completeHandler,
-                          expectations,
-                          isNegative));
+    Function doTestBound(filename, isNegative, isNegativeIfChecked) {

[Bill Hesse, 2011/11/29 16:38:56]

Name changed to createTestCase().  Not uploaded yet.

[Mads Ager (google), 2011/11/30 09:11:23]

We could use a named optional argument for isNegativeIfChecked. Alternatively,
we could pass down enough information to determine if the test is negative in
the multitest generation and avoid the extra parameter and logic here.

[Bill Hesse, 2011/11/30 10:01:39]

Done.

+      // Look up expectations in status files using a modified file path.
+      String pathSeparator = new Platform().pathSeparator();
+      String testName;
+      int start = filename.lastIndexOf('src' + pathSeparator);
+      if (start != -1) {
+        testName = filename.substring(start + 4, filename.length - 5);
+      } else {
+        start = filename.lastIndexOf(pathSeparator);
+        int middle = filename.lastIndexOf('_');

[Mads Ager (google), 2011/11/30 09:11:23]

This will end up biting us because it is specific to the multi test case that
you have just added. This will break matching for other simple test files that
contain underscores. We need to find another way of mapping the multi tests to
the expectations. At least we need to make sure that this only happens for the
multi tests. Maybe a named optional argument that is the name used for test
expectation matching. If nothing is supplied we use the filename otherwise we
use the thing supplied?

[Bill Hesse, 2011/11/30 10:01:39]

This only applies to multitests, because all other tests are in a src/
subdirectory.  If this assumption is not true, then yes, we should do something
different.  But otherwise, I've just added a comment that only multitests get to
the else clause.

On 2011/11/30 09:11:23, Mads Ager wrote:

+        testName = filename.substring(start + 1, middle) + pathSeparator +
+            filename.substring(middle + 1, filename.length - 5);
+      }
+      Set<String> expectations = testExpectations.expectations(testName);
+
+      if (expectations.contains(SKIP)) return;
+
+      isNegative = isNegative ||
+          (configuration['checked'] && isNegativeIfChecked);
+      var argumentLists = argumentListsFromFile(filename, optionsFromFile);
+      for (var args in argumentLists) {
+        doTest(new TestCase(testName,
+                            shellPath,
+                            args,
+                            timeout,
+                            completeHandler,
+                            expectations,
+                            isNegative));
+      }
+    }
+
+
+    if (optionsFromFile['isMultitest']) {
+      DoMultitest(filename, doTestBound);
+    } else {
+      var isNegative = optionsFromFile['isNegative'];
+      doTestBound(filename, isNegative, false);
     }
   }
 
@@ -210,15 +234,22 @@ class StandardTestSuite implements TestSuite {
                                            Map optionsFromFile) {
     List args = TestUtils.standardOptions(configuration);
 
+    bool isMultitest = optionsFromFile["isMultitest"];
     List<String> dartOptions = optionsFromFile["dartOptions"];
+    List<List<String>> vmOptionsList = optionsFromFile["vmOptions"];
+    Expect.isTrue(!isMultitest || dartOptions == null);
     args.addAll(dartOptions == null ? [filename] : dartOptions);
 
     var result = new List<List<String>>();
-    List<List<String>> vmOptionsList = optionsFromFile["vmOptions"];
     if (vmOptionsList.isEmpty()) {
+      Expect.fail("empty vmOptionsList");

[Mads Ager (google), 2011/11/30 09:11:23]

Remove this case completely and just replace with an Expect.isFalse or with
always throwing an exception in this case?

       result.add(args);
     } else {
       for (var vmOptions in vmOptionsList) {
+        if (isMultitest) {
+          // Make copy of vmOptions, since we will modify it at each iteration.
+          vmOptions = new List<String>.from(vmOptions);
+        }
         vmOptions.addAll(args);
         result.add(vmOptions);
       }
@@ -254,6 +285,7 @@ class StandardTestSuite implements TestSuite {
     for (var match in matches) {
       result.add(match[1].split(' ').filter((e) => e != ''));
     }
+    if (result.isEmpty()) result.add([]);
 
     matches = dartOptionsRegExp.allMatches(contents);
     for (var match in matches) {
@@ -272,9 +304,12 @@ class StandardTestSuite implements TestSuite {
       isNegative = true;
     }
 
+    bool isMultitest = contents.contains("///");
+
     return { "vmOptions": result,
              "dartOptions": dartOptions,
-             "isNegative" : isNegative };
+             "isNegative": isNegative,
+             "isMultitest": isMultitest};
   }
 }