Improved error messages from unittest.

pkg/unittest/lib/src/core_matchers.dart

Index: pkg/unittest/lib/src/core_matchers.dart
===================================================================
--- pkg/unittest/lib/src/core_matchers.dart	(revision 23884)
+++ pkg/unittest/lib/src/core_matchers.dart	(working copy)
@@ -110,7 +110,7 @@
         if (actualIterator.moveNext()) {
           Description r = matcher(expectedIterator.current,
                            actualIterator.current,
-                           'mismatch at position ${position}',
+                           'has mismatch at position ${position}: ',
                            depth);
           if (r != null) reason = r.toString();
           ++position;
@@ -144,11 +144,14 @@
           reason = new StringDescription('expected a map');
         } else {
           var err = (expected.length == actual.length) ? '' :
-                    'different map lengths; ';
+                    'has different length and ';
           for (var key in expected.keys) {
             if (!actual.containsKey(key)) {
               reason = new StringDescription(err);
-              reason.add('missing map key ');
+              if (location.length > 0) {
+                reason.add(' ').add(location);
+              }
+              reason.add('is missing map key ');
               reason.addDescriptionOf(key);
               break;
             }
@@ -157,7 +160,10 @@
             for (var key in actual.keys) {
               if (!expected.containsKey(key)) {
                 reason = new StringDescription(err);
-                reason.add('extra map key ');
+                if (location.length > 0) {
+                  reason.add(' ').add(location);
+                }
+                reason.add('has extra map key ');
                 reason.addDescriptionOf(key);
                 break;
               }
@@ -175,19 +181,23 @@
         }
       } else {
         reason = new StringDescription();
+        if (location.length > 0) {
+          reason.add(' ').add(location).add(' ');
+        }
         // If we have recursed, show the expected value too; if not,
         // expect() will show it for us.
         if (depth > 0) {
           reason.add('expected ');
           reason.addDescriptionOf(expected).add(' but ');
         }
-        reason.add('was ');
-        reason.addDescriptionOf(actual);
+        if (reason.length > 0) {
+          // Don't do a 'is <value> reason with no other useful
+          // context; the Actual value will provide that info.
+          reason.add('is ');
+          reason.addDescriptionOf(actual);
+        }
       }
     }
-    if (reason != null && location.length > 0) {
-      reason.add(' ').add(location);
-    }
     return reason;
   }
 
@@ -204,8 +214,19 @@
     description.addDescriptionOf(_expected);
 
   Description describeMismatch(item, Description mismatchDescription,
-                               MatchState matchState, bool verbose) =>
-    mismatchDescription.add(_match(_expected, item));
+                               MatchState matchState, bool verbose) {
+    var reason = _match(_expected, item);
+    // If we didn't get a good reason, that would normally be a 
+    // simple 'is <value>' message. We only add that if the mismatch
+    // description is non empty (so we are supplementing the mismatch
+    // description).
+    if (reason.length == 0 && mismatchDescription.length > 0) {
+      mismatchDescription.add('is ').addDescriptionOf(item);
+    } else {
+      mismatchDescription.add(reason);
+    }
+    return mismatchDescription;
+  }
 }
 
 /** A special equality matcher for strings. */
@@ -224,10 +245,10 @@
   Description describeMismatch(item, Description mismatchDescription,
       MatchState matchState, bool verbose) {
     if (item is! String) {
-      return mismatchDescription.addDescriptionOf(item).add(' not a string');
+      return mismatchDescription.addDescriptionOf(item).add('is not a string');
     } else {
       var buff = new StringBuffer();
-      buff.write('Strings are not equal.');
+      buff.write('is different.');
       var escapedItem = _escape(item);
       var escapedValue = _escape(_value);
       int minLength = escapedItem.length < escapedValue.length ?
@@ -420,9 +441,9 @@
 
   Description describe(Description description) {
     if (_matcher == null) {
-      return description.add("throws an exception");
+      return description.add("throws");
     } else {
-      return description.add('throws an exception which matches ').
+      return description.add('throws ').
           addDescriptionOf(_matcher);
     }
   }
@@ -431,18 +452,17 @@
                                MatchState matchState,
                                bool verbose) {
     if (item is! Function && item is! Future) {
-      return mismatchDescription.add(' not a Function or Future');
+      return mismatchDescription.add('is not a Function or Future');
     } else if (_matcher == null ||  matchState.state == null) {
-      return mismatchDescription.add(' no exception');
+      return mismatchDescription.add('did not throw');
     } else {
       mismatchDescription.
-          add(' exception ').addDescriptionOf(matchState.state['exception']);
+          add('threw ').addDescriptionOf(matchState.state['exception']);
       if (verbose) {
           mismatchDescription.add(' at ').
           add(matchState.state['stack'].toString());
       }
-       mismatchDescription.add(' does not match ').addDescriptionOf(_matcher);
-       return mismatchDescription;
+      return mismatchDescription;
     }
   }
 }
@@ -469,7 +489,7 @@
   Description describeMismatch(item, Description mismatchDescription,
                                MatchState matchState,
                                bool verbose) {
-      mismatchDescription.add(' threw ').
+      mismatchDescription.add('threw ').
           addDescriptionOf(matchState.state['exception']);
       if (verbose) {
         mismatchDescription.add(' at ').
@@ -631,7 +651,16 @@
   const _HasLength([Matcher matcher = null]) : this._matcher = matcher;
 
   bool matches(item, MatchState matchState) {
-    return _matcher.matches(item.length, matchState);
+    try {
+      // We want to generate a different description if there is no length

[Siggi Cherem (dart-lang), 2013/06/12 00:34:45]

adapt the comment? (made sense in describeMismatch, but might need to be
reworded in this context)

[gram, 2013/06/13 19:06:15]

Done.

+      // property. This is harmless code that will throw if no length property
+      // but subtle enough that an optimizer shouldn't strip it out.
+      if (item.length * item.length >= 0) {
+        return _matcher.matches(item.length, matchState);
+      }
+    } catch (e) {
+      return false;
+    }
   }
 
   Description describe(Description description) =>
@@ -645,11 +674,11 @@
       // property. This is harmless code that will throw if no length property
       // but subtle enough that an optimizer shouldn't strip it out.
       if (item.length * item.length >= 0) {
-        return mismatchDescription.add('had length of ').
+        return mismatchDescription.add('has length of ').
             addDescriptionOf(item.length);
       }
     } catch (e) {
-      return mismatchDescription.add('had no length property');
+      return mismatchDescription.add('has no length property');
     }
   }
 }
@@ -781,10 +810,7 @@
       description.add(_featureDescription).add(' ').addDescriptionOf(_matcher);
 
   Description describeMismatch(item, Description mismatchDescription,
-                               MatchState matchState, bool verbose) {
-    mismatchDescription.add(_featureName).add(' ');
-    _matcher.describeMismatch(matchState.state['feature'], mismatchDescription,
-        matchState.state['innerState'], verbose);
-    return mismatchDescription;
-  }
+                               MatchState matchState, bool verbose) =>
+    mismatchDescription.add('has ').add(_featureName).add(' with value ').
+      addDescriptionOf(matchState.state['feature']);
 }