Unit test improvements.

The base config now sets things up so failed expects are routed through it.
A flag controls whether or not an expect failure stops the test (the default)
or if the test keeps pressing on.

Another flag has been added to disable the exception that gets thrown at the 
end of there were errors. The default remains to throw.

Finally, some cleaning up of test failure stacks is done. In particular we 
remove internal unittest frames from the stack.

R=sigmund@google.com

Review URL: https://codereview.chromium.org//14021021

git-svn-id: https://dart.googlecode.com/svn/branches/bleeding_edge/dart@22186 260f80e4-7a28-3924-810f-c04153c831b5
diff --git a/pkg/unittest/lib/src/config.dart b/pkg/unittest/lib/src/config.dart
index dc25771..8b5a60c 100644
--- a/pkg/unittest/lib/src/config.dart
+++ b/pkg/unittest/lib/src/config.dart
@@ -4,6 +4,18 @@
 
 part of unittest;
 
+// A custom failure handler for [expect] that routes expect failures
+// to the config.
+class _ExpectFailureHandler extends DefaultFailureHandler {
+  Configuration _config;
+
+  _ExpectFailureHandler(this._config) : super();
+
+  void fail(String reason) {
+    _config.onExpectFailure(reason);
+  }
+}
+
 /**
  * Hooks to configure the unittest library for different platforms. This class
  * implements the API in a platform-independent way. Tests that want to take
@@ -30,6 +42,29 @@
   final bool autoStart = true;
 
   /**
+   * If true (the default), throw an exception at the end if any tests failed.
+   */
+  bool throwOnTestFailures = true;
+
+  /**
+   * If true (the default), then tests will stop after the first failed
+   * [expect]. If false, failed [expect]s will not cause the test
+   * to stop (other exceptions will still terminate the test).
+   */
+  bool stopTestOnExpectFailure = true;
+
+  // If stopTestOnExpectFailure is false, we need to capture failures, which
+  // we do with this List.
+  List _testLogBuffer = new List();
+  
+  /**
+   * The constructor sets up a failure handler for [expect] that redirects
+   * [expect] failures to [onExpectFailure].
+   */
+  Configuration() {
+    configureExpectFailureHandler(new _ExpectFailureHandler(this));
+  }
+  /**
    * Called as soon as the unittest framework becomes initialized. This is done
    * even before tests are added to the test framework. It might be used to
    * determine/debug errors that occur before the test harness starts executing.
@@ -46,18 +81,48 @@
 
   /**
    * Called when each test starts. Useful to show intermediate progress on
-   * a test suite.
+   * a test suite. Derived classes should call this first before their own
+   * override code.
    */
   void onTestStart(TestCase testCase) {
     assert(testCase != null);
+    _testLogBuffer.clear();
   }
 
   /**
    * Called when each test is first completed. Useful to show intermediate
-   * progress on a test suite.
+   * progress on a test suite. Derived classes should call this first 
+   * before their own override code.
    */
   void onTestResult(TestCase testCase) {
     assert(testCase != null);
+    if (!stopTestOnExpectFailure && _testLogBuffer.length > 0) {
+      // Write the message/stack pairs up to the last pairs.
+      var reason = new StringBuffer();
+      for (var i = 0; i < _testLogBuffer.length - 2; i += 2) {
+        reason.write(_testLogBuffer[i]);
+        reason.write('\n');
+        reason.write(_formatStack(_testLogBuffer[i+1]));
+        reason.write('\n');
+      }
+      // Write the last message.
+      reason.write(_testLogBuffer[_testLogBuffer.length - 2]);
+      if (testCase.result == PASS) {
+        testCase._result = FAIL;
+        testCase._message = reason.toString();
+        // Use the last stack as the overall failure stack.    
+        testCase._stackTrace = 
+            _formatStack(_testLogBuffer[_testLogBuffer.length - 1]);
+      } else {
+        // Add the last stack to the message; we have a further stack
+        // caused by some other failure.
+        reason.write(_formatStack(_testLogBuffer[_testLogBuffer.length - 1]));
+        reason.write('\n');
+        // Add the existing reason to the end of the expect log to 
+        // create the final message.
+        testCase._message = '${reason.toString()}\n${testCase._message}';
+      }
+    }
   }
 
   /**
@@ -78,6 +143,23 @@
   }
 
   /**
+   * Handles failures from expect(). The default in
+   * this base configuration is to throw an exception;
+   */
+  void onExpectFailure(String reason) {
+    if (stopTestOnExpectFailure) {
+      throw new TestFailure(reason);
+    } else {
+      _testLogBuffer.add(reason);
+      try {
+        throw '';
+      } catch (_, stack) {
+        _testLogBuffer.add(stack);
+      }
+    }
+  }
+  
+  /**
    * Called with the result of all test cases. The default implementation prints
    * the result summary using the built-in [print] command. Browser tests
    * commonly override this to reformat the output.
@@ -127,7 +209,9 @@
       _receivePort.close();
     } else {
       _receivePort.close();
-      throw new Exception('Some tests failed.');
+      if (throwOnTestFailures) {
+        throw new Exception('Some tests failed.');
+      }
     }
   }
 
diff --git a/pkg/unittest/lib/src/test_case.dart b/pkg/unittest/lib/src/test_case.dart
index 8e761f6..67b791e 100644
--- a/pkg/unittest/lib/src/test_case.dart
+++ b/pkg/unittest/lib/src/test_case.dart
@@ -122,6 +122,7 @@
           // seems to be the more conservative approach, because
           // unittest will not stop at a test failure.
           var stack = getAttachedStackTrace(e);
+          if (stack == null) stack = '';
           error("$description: Test setup failed: $e", "$stack");
         });
     } else {
@@ -148,7 +149,7 @@
   // is the first time the result is being set.
   void _setResult(String testResult, String messageText, String stack) {
     _message = messageText;
-    _stackTrace = stack;
+    _stackTrace = _formatStack(stack);
     if (result == null) {
       _result = testResult;
       _config.onTestResult(this);
@@ -199,6 +200,7 @@
   }
 
   void fail(String messageText, [String stack = '']) {
+    assert(stack != null);
     if (result != null) {
       String newMessage = (result == PASS)
           ? 'Test failed after initially passing: $messageText'
@@ -211,6 +213,7 @@
   }
 
   void error(String messageText, [String stack = '']) {
+    assert(stack != null);
     _complete(ERROR, messageText, stack);
   }
 
diff --git a/pkg/unittest/lib/unittest.dart b/pkg/unittest/lib/unittest.dart
index 56c0e02..6562509 100644
--- a/pkg/unittest/lib/unittest.dart
+++ b/pkg/unittest/lib/unittest.dart
@@ -167,8 +167,9 @@
 library unittest;
 
 import 'dart:async';
-import 'dart:isolate';
 import 'dart:collection';
+import 'dart:isolate';
+import 'dart:math' show max;
 import 'matcher.dart';
 export 'matcher.dart';
 
@@ -181,8 +182,17 @@
 
 Configuration _config;
 
-/** [Configuration] used by the unittest library. */
-Configuration get unittestConfiguration => _config;
+/**
+ * [Configuration] used by the unittest library. Note that if a 
+ * configuration has not been set, calling this getter will create
+ * a default configuration.
+ */
+Configuration get unittestConfiguration {
+  if (_config == null) {
+    _config = new Configuration();
+  }
+  return _config;
+}
 
 /**
  * Sets the [Configuration] used by the unittest library.
@@ -820,10 +830,7 @@
 
   _uncaughtErrorMessage = null;
 
-  if (_config == null) {
-    unittestConfiguration = new Configuration();
-  }
-  _config.onInit();
+  unittestConfiguration.onInit();
 
   if (configAutoStart && _config.autoStart) {
     // Immediately queue the suite up. It will run after a timeout (i.e. after
@@ -859,3 +866,80 @@
 
 /** Signature for a test function. */
 typedef dynamic TestFunction();
+
+// Stack formatting utility. Strips extraneous content from a stack trace.
+// Stack frame lines are parsed with a regexp, which has been tested
+// in Chrome, Firefox and the VM. If a line fails to be parsed it is 
+// included in the output to be conservative.
+//
+// The output stack consists of everything after the call to TestCase._run.
+// If we see an 'expect' in the frame we will prune everything above that
+// as well.
+final _frameRegExp = new RegExp(
+    r'^\s*' // Skip leading spaces.
+    r'(?:'  // Group of choices for the prefix.
+      r'(?:#\d+\s*)|' // Skip VM's #<frameNumber>.
+      r'(?:at )|'     // Skip Firefox's 'at '.
+      r'(?:))'        // Other environments have nothing here.
+    r'(.+)'           // Extract the function/method.
+    r'\s*[@\(]'       // Skip space and @ or (.
+    r'('              // This group of choices is for the source file.
+      r'(?:.+:\/\/.+\/[^:]*)|' // Handle file:// or http:// URLs.
+      r'(?:dart:[^:]*)|'  // Handle dart:<lib>.
+      r'(?:package:[^:]*)' // Handle package:<path>
+    r'):([:\d]+)[\)]?$'); // Get the line number and optional column number.
+
+String _formatStack(stack) {
+  var lines;
+  if (stack is StackTrace) {
+    lines = stack.toString().split('\n');
+  } else if (stack is String) {
+    lines = stack.split('\n');
+  } else {
+    return stack.toString();
+  }
+ 
+  // Calculate the max width of first column so we can 
+  // pad to align the second columns.
+  int padding = lines.fold(0, (n, line) {
+    var match = _frameRegExp.firstMatch(line);
+    if (match == null) return n;
+    return max(n, match[1].length + 1);
+  });
+
+  // We remove all entries that have a location in unittest.
+  // We strip out anything before _nextBatch too.
+  var sb = new StringBuffer();
+  for (var i = 0; i < lines.length; i++) {
+    var line = lines[i];
+    if (line == '') continue;
+    var match = _frameRegExp.firstMatch(line);
+    if (match == null) {
+      sb.write(line);
+      sb.write('\n');
+    } else {
+      var member = match[1];
+      var location = match[2];
+      var position = match[3];
+      if (member.indexOf('TestCase._runTest') >= 0) {
+        // Don't include anything after this.
+        break;
+      } else if (member.indexOf('expect') >= 0) {
+        // It looks like this was an expect() failure;
+        // drop all the frames up to here.
+        sb.clear();
+      } else {
+        sb.write(member);
+        // Pad second column to a fixed position.
+        for (var j = 0; j <= padding - member.length; j++) {
+          sb.write(' ');
+        }
+        sb.write(location);
+        sb.write(' ');
+        sb.write(position);
+        sb.write('\n');
+      }
+    }
+  }
+  return sb.toString();
+}