Add test_runner.dart to Dart version of test scripts. Include TestRunnerTest unit test.

tools/testing/dart/test_runner.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("test_runner");
+
+
+#import("status_file_parser.dart");
+// #import("test_configuration.dart");

[Søren Gjesse, 2011/11/04 09:05:34]

"Code" in comments.

[Mads Ager (google), 2011/11/04 09:06:13]

Remove commented out code.

[Bill Hesse, 2011/11/04 14:49:22]

Done.

+
+/**
+ * Classes and methods for executing tests.
+ *
+ * This module includes:
+ * - Managing parallel execution of tests, including timeout checks.
+ * - Evaluating the output of each test as pass/fail/crash/timeout.
+ */
+
+// Possible outcomes of running a test.
+final CRASH = "Crash";
+final TIMEOUT = "Timeout";
+final FAIL = "Fail";
+final PASS = "Pass";
+// An indication to skip the test.  The caller is responsible for skipping it.
+final SKIP = "Skip";
+
+class TestCase {
+  String executablePath;
+  List<String> arguments;
+  String commandLine;
+  TestOutput output;
+  Set<String> expectedOutcomes;
+  Function completedHandler;
+
+  TestCase(this.executablePath, this.arguments,
+           this.completedHandler, this.expectedOutcomes) {
+    commandLine = executablePath;
+    for (var arg in arguments) {
+      commandLine += " " + arg;
+    }
+  }
+
+  bool get isNegative() => false;
+
+  void completed() { completedHandler(this); }    
+}
+

[Søren Gjesse, 2011/11/04 09:05:34]

Add empty line.

[Bill Hesse, 2011/11/04 14:49:22]

Done.

+class TestOutput {
+  TestCase testCase;  // The TestCase this is the output for.

[Mads Ager (google), 2011/11/04 09:06:13]

I would add the comment above the line instead.

[Bill Hesse, 2011/11/04 14:49:22]

Done.

+  int exitCode;
+  bool timedOut;
+  bool failed = false;
+  List<String> stdout;
+  List<String> stderr;
+  Duration time;
+  
+  TestOutput(this.testCase, this.exitCode, this.timedOut, this.stdout,
+             this.stderr, this.time) {
+    testCase.output = this;
+  }
+

[Mads Ager (google), 2011/11/04 09:06:13]

Remove extra blank?

[Bill Hesse, 2011/11/04 14:49:22]

Done.

+
+  String get result() =>
+      hasCrashed ? CRASH : (hasTimedOut ? TIMEOUT : (hasFailed ? FAIL : PASS));
+
+  bool get unexpectedOutput() => !testCase.expectedOutcomes.contains(result);
+  
+  bool get hasCrashed() =>

[Mads Ager (google), 2011/11/04 09:06:13]

This will fit on one line I think?

[Bill Hesse, 2011/11/04 14:49:22]

Done.

+      !timedOut && exitCode != 255 && exitCode != 0;
+
+  bool get hasTimedOut() => timedOut;
+
+  bool get didFail() => exitCode != 0 && !hasCrashed;
+
+  bool get hasFailed() =>  // Reverse result of a negative test.

[Mads Ager (google), 2011/11/04 09:06:13]

Please move this comment to the line above and make hasFailed a one-liner:

// Reverse result of a negative test.
bool get hasFailed() => (testCase.isNegative ? !didFail : didFail);

+      (testCase.isNegative ? !didFail : didFail);
+}
+
+
+class RunningProcess {
+  Process process;
+  TestCase testCase;
+  String executablePath;

[Mads Ager (google), 2011/11/04 09:06:13]

Isn't the executablePath and the arguments also in the testCase? Do you need
them here?

[Bill Hesse, 2011/11/04 14:49:22]

Done.

+  List<String> arguments;
+  int timeout;
+  bool timedOut = false;
+  int timerKey;  // Running processes have a timeout check callback in a map.

[Mads Ager (google), 2011/11/04 09:06:13]

Move comment above the field instead?

[Bill Hesse, 2011/11/04 14:49:22]

Why?
I have gotten rid of this anyway, by creating a timer per process.

On 2011/11/04 09:06:13, Mads Ager wrote:

+  Date startTime;
+  List<String> output;

[Søren Gjesse, 2011/11/04 09:05:34]

Use names stdout and stderr as on TestOutput class above?

[Bill Hesse, 2011/11/04 14:49:22]

Done.

+  List<String> errorOutput;
+  List<Function> handlers;
+
+  static final int NO_TIMEOUT = 0;
+
+  RunningProcess(this.testCase,

[Mads Ager (google), 2011/11/04 09:06:13]

One line?

[Bill Hesse, 2011/11/04 14:49:22]

Done.

+                 [this.timeout = NO_TIMEOUT]);
+
+  void exitHandler(int exitCode) {

[Søren Gjesse, 2011/11/04 09:05:34]

When we get the stream close handling working correctly (hopefully
http://codereview.chromium.org/8437090/ will be a step in that direction). We
should wait for exit and then end of both streams.

+    new TestOutput(testCase, exitCode, timedOut, output,
+                   errorOutput, new Date.now().difference(startTime));
+    process.close();
+    TimerChecks.remove(timerKey);
+    testCase.completed();
+  }
+

[Mads Ager (google), 2011/11/04 09:06:13]

Either consistently use two new lines or one.

+

[Søren Gjesse, 2011/11/04 09:05:34]

Only one empty line between methods for consistency.

[Bill Hesse, 2011/11/04 14:49:22]

Done.

+  void makeReadHandler(StringInputStream source, List<String> destination) {
+    return () {
+      var line = source.readLine();
+      while (null != line) {
+        destination.add(line);
+        line = source.readLine();
+      }
+    };
+  }
+
+

[Søren Gjesse, 2011/11/04 09:05:34]

Ditto.

+  void start() {
+    Expect.isFalse(testCase.expectedOutcomes.contains(SKIP));
+    process = new Process(testCase.executablePath, testCase.arguments);
+    process.exitHandler = exitHandler;
+    startTime = new Date.now();
+    process.start();
+    
+    InputStream stdout = process.stdout;
+    InputStream stderr = process.stderr;
+    output = new List<String>();
+    errorOutput = new List<String>();
+    StringInputStream stdoutStringStream = new StringInputStream(stdout);
+    StringInputStream stderrStringStream = new StringInputStream(stderr);
+    stdoutStringStream.dataHandler =
+        makeReadHandler(stdoutStringStream, output);
+    stderrStringStream.dataHandler =
+        makeReadHandler(stderrStringStream, errorOutput);
+    timerKey = TimerChecks.add(checkTimeout);
+  }
+
+  void checkTimeout(Date currentTime) {
+    if (timeout != NO_TIMEOUT &&
+        currentTime.difference(startTime).inSeconds >= timeout) {
+      timedOut = true;
+      process.kill();
+    }
+  }
+}
+
+
+class TimerChecks {

[Mads Ager (google), 2011/11/04 09:06:13]

I'm not sure I like this approach. This means that we spent time testing each
test for timeout each second. Why not just start a new Timer per test and cancel
it when the test completes?

+  static int key = 1;
+  static Map<int, Function> timers;
+  static Timer timer;
+
+  static int add(Function callback) {
+    if (timers == null) initialize();
+    timers[key] = callback;
+    return key++;
+  }
+
+  static void remove(int key) => timers.remove(key);
+
+  static void initialize() {
+    timers = new Map<int, Function>();
+    timer = new Timer(callback, 1000, true);
+  }
+
+  static void callback(Timer unused) {
+    Date current = new Date.now();
+    timers.forEach((key, callback_value) => callback_value(current));
+  }
+}