Improved error messages from unittest.

pkg/unittest/lib/src/core_matchers.dart

 // Copyright (c) 2012, 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.
 
 part of matcher;
 
 /**
  * Returns a matcher that matches empty strings, maps or iterables
  * (including collections).
  */
@@ skipping 92 matching lines @@
     }
     var expectedIterator = expected.iterator;
     var actualIterator = actual.iterator;
     var position = 0;
     String reason = null;
     while (reason == null) {
       if (expectedIterator.moveNext()) {
         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;
         } else {
           reason = 'shorter than expected';
         }
       } else if (actualIterator.moveNext()) {
         reason = 'longer than expected';
       } else {
         return null;
@@ skipping 13 matching lines @@
     } else {
       if (expected is Iterable && canRecurse) {
         String r = _compareIterables(expected, actual,
             _recursiveMatch, depth+1);
         if (r != null) reason = new StringDescription(r);
       } else if (expected is Map && canRecurse) {
         if (actual is !Map) {
           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;
             }
           }
           if (reason == null) {
             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;
               }
             }
             if (reason == null) {
               for (var key in expected.keys) {
                 reason = _recursiveMatch(expected[key], actual[key],
                     'with key <${key}> ${location}', depth+1);
                 if (reason != null) {
                   break;
                 }
               }
             }
           }
         }
       } 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;
   }
 
   String _match(expected, actual) {
     Description reason = _recursiveMatch(expected, actual, '', 0);
     return reason == null ? null : reason.toString();
   }
 
   // TODO(gram) - see if we can make use of matchState here to avoid
   // recursing again in describeMismatch.
   bool matches(item, MatchState matchState) => _match(_expected, item) == null;
 
   Description describe(Description description) =>
     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. */
 class _StringEqualsMatcher extends BaseMatcher {
   final String _value;
 
   _StringEqualsMatcher(this._value);
 
   bool get showActualValue => true;
 
   bool matches(item, MatchState mismatchState) => _value == item;
 
   Description describe(Description description) =>
       description.addDescriptionOf(_value);
 
   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 ?
           escapedItem.length : escapedValue.length;
       int start;
       for (start = 0; start < minLength; start++) {
         if (escapedValue.codeUnitAt(start) != escapedItem.codeUnitAt(start)) {
           break;
         }
       }
@@ skipping 172 matching lines @@
             'exception' :e,
             'stack': s
         };
         return false;
       }
     }
   }
 
   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);
     }
   }
 
   Description describeMismatch(item, Description mismatchDescription,
                                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;
     }
   }
 }
 
 class _ReturnsNormally extends BaseMatcher {
   const _ReturnsNormally();
 
   bool matches(f, MatchState matchState) {
     try {
       f();
       return true;
     } catch (e, s) {
       matchState.state = {
           'exception' : e,
           'stack': s
       };
       return false;
     }
   }
 
   Description describe(Description description) =>
       description.add("return normally");
 
   Description describeMismatch(item, Description mismatchDescription,
                                MatchState matchState,
                                bool verbose) {
-      mismatchDescription.add(' threw ').
+      mismatchDescription.add('threw ').
           addDescriptionOf(matchState.state['exception']);
       if (verbose) {
         mismatchDescription.add(' at ').
         add(matchState.state['stack'].toString());
       }
       return mismatchDescription;
   }
 }
 
 /*
@@ skipping 141 matching lines @@
  * that matches [matcher].
  */
 Matcher hasLength(matcher) =>
     new _HasLength(wrapMatcher(matcher));
 
 class _HasLength extends BaseMatcher {
   final Matcher _matcher;
   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) =>
     description.add('an object with length of ').
         addDescriptionOf(_matcher);
 
   Description describeMismatch(item, Description mismatchDescription,
                                MatchState matchState, bool verbose) {
     try {
       // We want to generate a different description if there is no length
       // 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');
     }
   }
 }
 
 /**
  * Returns a matcher that matches if the match argument contains
  * the expected value. For [String]s this means substring matching;
  * for [Map]s it means the map has the key, and for [Iterable]s
  * (including [Iterable]s) it means the iterable has a matching
  * element. In the case of iterables, [expected] can itself be a
@@ skipping 111 matching lines @@
     var f = featureValueOf(item);
     if (_matcher.matches(f, matchState)) return true;
     matchState.state = { 'innerState': matchState.state, 'feature': f };
     return false;
   }
 
   Description describe(Description description) =>
       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']);
 }