Update test.dart for detection output of machine formatted errors

tools/testing/dart/test_runner.dart

Index: tools/testing/dart/test_runner.dart
diff --git a/tools/testing/dart/test_runner.dart b/tools/testing/dart/test_runner.dart
index fe5a2ea8933db1b5e19d109e42798f082badf437..e240227a599bd0650de4ff4fc33f488ddc7ff993 100644
--- a/tools/testing/dart/test_runner.dart
+++ b/tools/testing/dart/test_runner.dart
@@ -32,6 +32,8 @@ class Command {
   Command(this.executable, this.arguments) {
     commandLine = "$executable ${Strings.join(arguments, ' ')}";
   }
+  
+  String toString() => commandLine;
 }
 
 /**
@@ -65,13 +67,15 @@ class TestCase {
   bool isNegative;
   Set<String> expectedOutcomes;
   Function completedHandler;
+  TestInformation info;
 
   TestCase(this.displayName,
            this.commands,
            this.configuration,
            this.completedHandler,
            this.expectedOutcomes,
-           [this.isNegative = false]) {
+           [this.isNegative = false,
+            this.info = null]) {
     if (!isNegative) {
       this.isNegative = displayName.contains("NegativeTest");
     }
@@ -165,7 +169,22 @@ class BrowserTestCase extends TestCase {
  * the time the process took to run.  It also contains a pointer to the
  * [TestCase] this is the output of.
  */
-class TestOutput {
+interface TestOutput default TestOutputImpl { 

[zundel, 2012/02/29 08:11:40]

I didn't come up with much better than what was here before, but I did change
this to an interface.  I didn't want to expose the generative constructor of
TestOutputImpl since I want to direct users of the API to the factory method
instead.

+  TestOutput.fromCase(TestCase testCase, int exitCode, bool timedOut,
+    List<String> stdout, List<String> stderr, Duration time);
+  
+  String get result();
+  
+  bool get unexpectedOutput();
+  
+  bool get hasCrashed();
+  
+  bool get hasTimedOut(); 
+  
+  bool get didFail();
+}
+
+class TestOutputImpl implements TestOutput {
   TestCase testCase;
   int exitCode;
   bool timedOut;
@@ -173,18 +192,34 @@ class TestOutput {
   List<String> stdout;
   List<String> stderr;
   Duration time;
+  
   /**
    * Set to true if we encounter a condition in the output that indicates we
    * need to rerun this test.
    */
   bool requestRetry;
 
-  TestOutput(this.testCase, this.exitCode, this.timedOut, this.stdout,
+  // Don't call  this constructor, call TestOutput.fromCase() to 
+  // get anew TestOutput instance.
+  TestOutputImpl(this.testCase, this.exitCode, this.timedOut, this.stdout,
              this.stderr, this.time) {
     testCase.output = this;
     requestRetry = false;
   }
 
+  factory TestOutputImpl.fromCase (testCase, exitCode, timedOut, stdout, stderr, 
+    time) {
+    if (testCase is BrowserTestCase) {
+      return new BrowserTestOutputImpl(testCase, exitCode, timedOut,
+        stdout, stderr, time);
+    } else if (testCase.configuration['component'] == 'dartc') {
+      return new AnalysisTestOutputImpl(testCase, exitCode, timedOut,
+        stdout, stderr, time);
+    }
+    return new TestOutputImpl(testCase, exitCode, timedOut,
+      stdout, stderr, time);
+  }
+  
   String get result() =>
       hasCrashed ? CRASH : (hasTimedOut ? TIMEOUT : (hasFailed ? FAIL : PASS));
 
@@ -207,8 +242,18 @@ class TestOutput {
   bool get hasTimedOut() => timedOut;
 
   bool get didFail() {
-    if (testCase is !BrowserTestCase) return (exitCode != 0 && !hasCrashed);
+    return (exitCode != 0 && !hasCrashed);
+  }
+  
+  // Reverse result of a negative test.
+  bool get hasFailed() => (testCase.isNegative ? !didFail : didFail);  
+}
 
+class BrowserTestOutputImpl extends TestOutputImpl {
+  BrowserTestOutputImpl(testCase, exitCode, timedOut, stdout, stderr, time) : 
+    super(testCase, exitCode, timedOut, stdout, stderr, time);
+    
+  bool get didFail() {
     // Browser case:
     // If the browser test failed, it may have been because DumpRenderTree
     // and the virtual framebuffer X server didn't hook up, or DRT crashed with
@@ -235,12 +280,136 @@ class TestOutput {
       }
       previous_line = line;
     }
-
     return true;
   }
+}
 
-  // Reverse result of a negative test.
-  bool get hasFailed() => (testCase.isNegative ? !didFail : didFail);
+// The static analyzer does not actaully execute code, so 
+// the criteria for success now depend on the text sent
+// to stderr.
+class AnalysisTestOutputImpl extends TestOutputImpl { 
+  AnalysisTestOutputImpl(testCase, exitCode, timedOut, stdout, stderr, time) : 
+    super(testCase, exitCode, timedOut, stdout, stderr, time) {
+  }
+  
+  bool get didFail() {
+    if (hasCrashed) return false;
+
+    List<String> errors = [];
+    List<String> staticWarnings = [];
+    
+    // Read the returned list of errors and stuff them away.
+    for (String line in stderr) {
+      if (line.length == 0) continue;
+      List<String> fields = splitMachineError(line);
+      switch(fields[0]) {
+      case 'ERROR':
+        errors.add(fields);
+        break;
+      case 'WARNING':
+        // We only care about testing Static type warnings
+        // ignore all others
+        if (fields[1] == 'STATIC_TYPE') {
+          staticWarnings.add(fields);
+        } 
+        break;
+      default:
+        // Skip error output that doesn't match the machine format
+      }
+    }
+    if (testCase.info != null 
+        && testCase.info.optionsFromFile['isMultitest']) {
+      return _didMultitestFail(errors, staticWarnings);
+    }
+    return _didStandardTestFail(errors, staticWarnings);
+  }
+  
+  bool _didMultitestFail(List errors, List staticWarnings) {
+    String outcome = testCase.info.multitestOutcome;
+    if ((outcome == '' || outcome == 'compile-time error') && errors.length > 0) {
+      return true;
+    } else if (outcome == 'static type error' && staticWarnings.length > 0) {
+      return true;
+    }
+    return false;
+  }
+  
+  bool _didStandardTestFail(List errors, List staticWarnings) {
+    bool hasFatalTypeErrors = false;
+    int numStaticTypeAnnotations = 0;
+    int numCompileTimeAnnotations = 0;
+    if (testCase.info != null) {
+      var optionsFromFile = testCase.info.optionsFromFile;
+      hasFatalTypeErrors = optionsFromFile['hasFatalTypeErrors'];
+      for (Command c in testCase.commands) {
+        for (String arg in c.arguments) {
+          if (arg == '--fatal-type-errors') {
+            hasFatalTypeErrors = true;
+            break;
+          }
+        }
+      }
+      numStaticTypeAnnotations = optionsFromFile['numStaticTypeAnnotations'];
+      numCompileTimeAnnotations = optionsFromFile['numCompileTimeAnnotations'];
+    }
+    
+    // TODO(zundel): These assertions are catching some sort of issue 
+    // where the output between two test cases is getting crossed.
+    if (errors.length == 0) {
+      if (!hasFatalTypeErrors) {
+        Expect.isTrue(exitCode == 0,
+            "Expected error: exitCode:${exitCode} command[0]:${testCase.commands[0]}");
+      }
+    } else {
+      Expect.isTrue(exitCode != 0,
+        "Unexpected error: exitCode:${exitCode} command[0]:${testCase.commands[0]}  errors[0]:${errors[0]}");
+    }
+    
+    if (numCompileTimeAnnotations > 0 
+        && numCompileTimeAnnotations < errors.length) {
+      // Expected compile-time errors were not returned.  The test did not 'fail' in the way
+      // intended so don't return failed.
+      // TODO(zundel): give a good diagnostic message here
+      return false;
+    }
+    
+    if (numStaticTypeAnnotations > 0 || hasFatalTypeErrors) {
+      // TODO(zundel): match up the annotation line numbers
+      // with the reported error line numbers
+      if (staticWarnings.length < numStaticTypeAnnotations) {
+        // TODO(zundel): How to give a good diagnostic message here?
+        return true;
+      }
+      return false;
+    } else if (errors.length != 0) {
+      return true;
+    } 
+    return false;
+  }
+  
+  // Parse a line delimited by the | character using \ as an escape charager
+  // like:  FOO|BAR|FOO\|BAR|FOO\\BAZ as 4 fields: FOO BAR FOO|BAR FOO\BAZ
+  List<String> splitMachineError(String line) {
+    StringBuffer field = new StringBuffer();
+    List<String> result = [];
+    bool escaped = false;
+    for (var i = 0 ; i < line.length; i++) {
+      var c = line[i];
+      if (!escaped && c == '\\') {
+        escaped = true;
+        continue;
+      }
+      escaped = false;
+      if (c == '|') {
+        result.add(field.toString());
+        field.clear();
+        continue;
+      }
+      field.add(c);
+    }
+    result.add(field.toString());
+    return result;
+  }
 }
 
 /**
@@ -277,8 +446,8 @@ class RunningProcess {
    * command.
    */
   void testComplete(int exitCode) {
-    new TestOutput(testCase, exitCode, timedOut, stdout,
-                   stderr, new Date.now().difference(startTime));
+    new TestOutput.fromCase(testCase, exitCode, timedOut, stdout,
+                            stderr, new Date.now().difference(startTime));
     timeoutTimer.cancel();
     if (testCase.output.unexpectedOutput && testCase.configuration['verbose']) {
       print(testCase.displayName);
@@ -295,8 +464,7 @@ class RunningProcess {
       testCase.output.requestRetry = false;
       this.timedOut = false;
       testCase.dynamic.numRetries--;
-      print("Potential flake. " +
-          "Re-running ${testCase.displayName} " +
+      print("Potential flake. Re-running ${testCase.displayName} " +
           "(${testCase.dynamic.numRetries} attempt(s) remains)");
       this.start();
     } else {
@@ -452,8 +620,8 @@ class BatchRunnerProcess {
 
   void doStartTest(TestCase testCase) {
     _startTime = new Date.now();
-    _testStdout = new List<String>();
-    _testStderr = new List<String>();
+    _testStdout = [];
+    _testStderr = [];
     _stdoutStream.lineHandler = _readOutput(_stdoutStream, _testStdout);
     _stderrStream.lineHandler = _readOutput(_stderrStream, _testStderr);
     _timer = new Timer(_timeoutHandler, testCase.timeout * 1000);
@@ -474,8 +642,10 @@ class BatchRunnerProcess {
     var exitCode = 0;
     if (outcome == "CRASH") exitCode = -10;
     if (outcome == "FAIL" || outcome == "TIMEOUT") exitCode = 1;
-    new TestOutput(test, exitCode, outcome == "TIMEOUT", _testStdout,
-                   _testStderr, new Date.now().difference(_startTime));
+    new TestOutput.fromCase(test, exitCode, outcome == "TIMEOUT", _testStdout,
+                            _testStderr, new Date.now().difference(_startTime));
+    _testStdout = [];
+    _testStderr = [];
     test.completed();
   }
 
@@ -508,18 +678,16 @@ class BatchRunnerProcess {
 
   void _exitHandler(exitCode) {
     if (_timer != null) _timer.cancel();
+    _reportResult(">>> TEST CRASH");

[zundel, 2012/02/29 08:11:40]

I moved this because _startProcess() clears the _testStdout and _testStdin
fields, thus we loose diagnostic output on a crash.

     _process.close();
-    _startProcess(() {
-      _reportResult(">>> TEST CRASH");
-    });
+    _startProcess(() {});
   }
 
   void _timeoutHandler(ignore) {
     _process.exitHandler = (exitCode) {
+      _reportResult(">>> TEST TIMEOUT");
       _process.close();
-      _startProcess(() {
-        _reportResult(">>> TEST TIMEOUT");
-      });
+      _startProcess(() {});
     };
     _process.kill();
   }