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

tools/testing/dart/multitest.dart

+// Copyright (c) 2011, 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("multitest");
+
+//#import("status_file_parser.dart");

[Mads Ager (google), 2011/11/29 08:12:04]

Code in comments. Remove?

[Søren Gjesse, 2011/11/29 08:35:30]

Remove these.

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

Done.

+//#import("test_runner.dart");
+
+

[Søren Gjesse, 2011/11/29 08:35:30]

A short comment here showing what is actually happening will be helpful, e.g.

aaa
xxx /// 1
yyy /// 2
bbb

turns into:

<outcome here>

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

Done.

+List ExtractTestsFromMultitest(String filename) {
+    // Read the entire file into a byte buffer and transform it to a
+    // String. This will treat the file as ascii but the only parts
+    // we are interested in will be ascii in any case.
+    File file = new File(filename);
+    file.openSync();
+    List chars = new List(file.lengthSync());
+    int offset = 0;
+    while (offset != chars.length) {
+      offset += file.readListSync(chars, offset, chars.length - offset);
+    }
+    file.closeSync();
+    String contents = new String.fromCharCodes(chars);
+    chars = null;
+    int first_newline = contents.indexOf('\n');
+    final String line_separator =
+        (first_newline == 0 || contents[first_newline - 1] != '\r')
+        ? '\n'
+        : '\r\n';
+    List<String> lines = contents.split(line_separator);
+    if (lines.last() == '') lines.removeLast();

[Mads Ager (google), 2011/11/29 08:12:04]

Do you need to do this? If you do, don't you need to do it in a loop to get rid
of all the blank lines at the end of the file?

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

This should just get rid of a spurious empty line that is
not in the original file, but is introduced by "split".  If the file ends in \n,
then the split will introduce an empty string after it.

+    contents = null;
+    Set<String> validMultitestOutcomes = new Set<String>.from(
+        ['compile-time error', 'runtime error',
+         'static type error', 'dynamic type error', '']);
+
+    List<String> testTemplate = new List<String>();
+    testTemplate.add('// Test created from multitest named $filename.');
+    // Create the set of multitests, which will have a new test added each
+    // time we see a multitest line with a new key.
+    Map<String, List<String>> tests = new Map<String, List<String>>();
+    Map<String, String> outcomes = new Map<String, String>();
+
+    for (String line in lines) {
+      if (line.contains('///')) {
+        var parts = line.split('///')[1].split(':');
+        var key = parts[0].trim();
+        var rest = parts[1].trim();
+        if (tests.containsKey(key)) {
+          Expect.equals('continued', rest);
+          tests[key].add(line);
+        } else {
+          (tests[key] = new List<String>.from(testTemplate)).add(line);
+          outcomes[key] = rest;
+          Expect.isTrue(validMultitestOutcomes.contains(rest));
+        }
+      } else {
+        testTemplate.add(line);
+        for (var test in tests.getValues()) test.add(line);
+      }
+    }
+    tests[''] = testTemplate;

[Mads Ager (google), 2011/11/29 08:12:04]

What is this used for. Can you add a comment about the test with the empty
string as key?

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

The key is now "none".  There is always a "none" multitest created, with none of
the multitest lines in it.
On 2011/11/29 08:12:04, Mads Ager wrote:

+    outcomes[''] = '';
+
+    for (String key in tests.getKeys()) {
+      tests[key] = [Strings.join(tests[key], line_separator) + line_separator,
+                    outcomes[key]];
+
+      print('');

[Mads Ager (google), 2011/11/29 08:12:04]

Remove printing.

[Søren Gjesse, 2011/11/29 08:35:30]

Remove print.

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

Done.

+      print('Test $key, outcomes ${outcomes[key]}');
+      for (var line in tests[key]) {
+        print(line);
+      }
+    }
+    return tests;
+}
+
+void DoMultitest(String filename,
+                 Function doTest(List<String> args, bool isNegative)) {
+  // Convert multitest into a map: key -> [String test, String outcome]
+  Map tests = ExtractTestsFromMultitest(filename);

[Mads Ager (google), 2011/11/29 08:12:04]

You could pass in the maps to be filled out here as well. So you get a test map
and an outcomes map.

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

Done.

+
+  String pathSeparator = new Platform().pathSeparator();
+  int start = filename.lastIndexOf(pathSeparator) + 1;
+  int end = filename.indexOf('.dart', start);
+  String baseFilename = filename.substring(start, end);
+  Directory dir = new Directory("");
+  AddErrorHandler(dir, "Error creating temp directory");

[Mads Ager (google), 2011/11/29 08:12:04]

I would just add the error handler. Abstracting that out makes it harder for me
to understand.

+
+  Async((){dir.createTemp();}).thenHandler((){

[Mads Ager (google), 2011/11/29 08:12:04]

Urgh. I don't like this at all. I find it very hard to read and I don't see what
it adds. Please just use the APIs directly.

dir.createTempHandler = () {

};
dir.createTemp();

[Søren Gjesse, 2011/11/29 08:35:30]

See comment below.

+  dir.createTempHandler = (){
+    String path = dir.path + new Platform().pathSeparator();
+    for (String key in tests.getKeys()) {
+      WriteMultitestToFileAndQueueIt(tests[key], '$path$baseFilename$key.dart',
+                                     doTest);
+      //RegisterFileForDeletion('$path$baseFilename$key.dart');

[Mads Ager (google), 2011/11/29 08:12:04]

Code in comments. And below as well.

[Søren Gjesse, 2011/11/29 08:35:30]

Code in comments.

+    }
+    //RegisterDirForDeletion(dir.path);

[Søren Gjesse, 2011/11/29 08:35:30]

Ditto.

+  };});
+}
+
+WriteMultitestToFileAndQueueIt(List test, String filename, doTest) {
+  FileThen file = new FileThen(new File(filename));

[Mads Ager (google), 2011/11/29 08:12:04]

Why are you inventing new stuff? We have everything we need for async file
operations already.

+  AddErrorHandler(file, "Error creating temp file");

[Mads Ager (google), 2011/11/29 08:12:04]

Just add the error handler?

+  file.createThen((){

[Mads Ager (google), 2011/11/29 08:12:04]

Please use our APIs for this. Don't create extra layers here.

[Søren Gjesse, 2011/11/29 08:35:30]

I think it would me much more readable to have a number of small functions that
"tail calls" one after the other through the handler associated with the
operation performed.

File file ...

void step1() {
  ...
  file.xxxHandler = step2;
  file.xxx
}

void step2() {
  ...
  file.yyyHandler = step3;
  file.yyy
}

void step3 {
  ...
}

Maybe tou need some kind of state object holding the File object together with
other tracking stuff.

+    file.openThen(file.WRITABLE, (){

[Mads Ager (google), 2011/11/29 08:12:04]

file.openHandler = () { ... };
file.open(writable: true);

+      var bytes = test[0].charCodes();
+      var outcome = test[1];
+      file.writeListThen(bytes, 0, bytes.length, (){
+        file.closeThen((){
+          bool isNegative = outcome.contains('compile-time error') ||
+              outcome.contains('runtime error');
+          bool isNegativeIfChecked = outcome.contains('type error');
+
+          doTest(filename, isNegative, isNegativeIfChecked);
+        });
+      });
+    });
+  });
+}
+
+
+void AddErrorHandler(object, String error_string) {

[Mads Ager (google), 2011/11/29 08:12:04]

Please get rid of all of this stuff. There is no need for it.

+  object.errorHandler = (error) { Expect.fail(error_string + ": $error"); };
+}
+
+class DelayedFunction {
+  Function later;
+  DelayedFunction(this.later);
+
+  void after(Function f) {
+    f();
+    later();
+  }
+
+  void thenHandler(Function f) => after(f);
+}
+
+DelayedFunction Async(Function delayed) => new DelayedFunction(delayed);
+
+
+class FileThen {
+  File file;
+  FileThen(this.file);
+  final bool WRITABLE = true;
+
+  void existsThen(void handler(bool exists)) {
+    file.existsHandler = handler;
+    file.exists();
+  }
+  void createThen(void handler()) {
+    file.createHandler = handler;
+    file.create();
+  }
+  void deleteThen(void handler()) {
+    file.deleteHandler = handler;
+    file.delete();
+  }
+  void openThen(bool writeable, void handler()) {
+    file.openHandler = handler;
+    file.open(writeable);
+  }
+  void closeThen(void handler()) {
+    file.closeHandler = handler;
+    file.close();
+  }
+
+  void writeListThen(List<int> buffer, int offset, int bytes, void handler()) {
+    file.noPendingWriteHandler = handler;
+    file.writeList(buffer, offset, bytes);
+  }
+
+  String get name() => file.name;
+
+  void set errorHandler(void handler(String error)) {
+    file.errorHandler = handler;
+  }
+}
+
+// Alternatively, we could write

[Søren Gjesse, 2011/11/29 08:35:30]

Code in comments.

+// file.create().then(handler)
+// instead of 
+// file.createThen(handler)
+//
+// by having a class Setter {
+//   Function setter;
+//   Setter(this.setter);
+//   void then(Function foo) { setter(foo); }
+// }
+// then FileThen.create() would be
+// create() => new Setter((handler) { 
+//   file.createHandler = handler;
+//   file.create();
+// }