Improved error messages from unittest.

Improved error messages from unittest.

R=sigmund@google.com

Committed: https://code.google.com/p/dart/source/detail?r=24045

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

lgtm!

https://codereview.chromium.org/16408019/diff/2001/pkg/unittest/lib/src/core_...
File pkg/unittest/lib/src/core_matchers.dart (right):

https://codereview.chromium.org/16408019/diff/2001/pkg/unittest/lib/src/core_...
pkg/unittest/lib/src/core_matchers.dart:655: // We want to generate a different
description if there is no length
adapt the comment? (made sense in describeMismatch, but might need to be
reworded in this context)

https://codereview.chromium.org/16408019/diff/2001/pkg/unittest/lib/src/expec...
File pkg/unittest/lib/src/expect.dart (right):

https://codereview.chromium.org/16408019/diff/2001/pkg/unittest/lib/src/expec...
pkg/unittest/lib/src/expect.dart:147: if (info.length > 0) {
how about use mismatchDescription.length here?

https://codereview.chromium.org/16408019/diff/2001/pkg/unittest/lib/src/expec...
pkg/unittest/lib/src/expect.dart:148: description.add('   Which: ${info}\n');
then this can simply be $mispatchDescription

https://codereview.chromium.org/16408019/diff/2001/pkg/unittest/lib/src/expec...
pkg/unittest/lib/src/expect.dart:148: description.add('   Which: ${info}\n');
"   Which:" => " Reason:" ?

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

PTAL. I apologize for sending this back but I realized that by dropping the
description in trivial 'equals' cases, we would break descriptions in some
nested cases. I have fixed at least some of these and in the process changed how
we use matchState to make it cleaner, simpler and more consistent. I'll stop
here and then add some more tests and any necessary fixes in a subsequent CL.

I've changed the wording of some messages too - we used to use 'position' in a
number of places, which I have changed to 'index' for iterables or 'offset' for
string character positions. This can make messages that combine these two more
understandable (see an example in the added matcher test for nested
everyElement).

https://codereview.chromium.org/16408019/diff/2001/pkg/unittest/lib/src/core_...
File pkg/unittest/lib/src/core_matchers.dart (right):

https://codereview.chromium.org/16408019/diff/2001/pkg/unittest/lib/src/core_...
pkg/unittest/lib/src/core_matchers.dart:655: // We want to generate a different
description if there is no length
On 2013/06/12 00:34:45, Siggi Cherem (dart-lang) wrote:
> adapt the comment? (made sense in describeMismatch, but might need to be
> reworded in this context)

Done.

https://codereview.chromium.org/16408019/diff/2001/pkg/unittest/lib/src/expec...
File pkg/unittest/lib/src/expect.dart (right):

https://codereview.chromium.org/16408019/diff/2001/pkg/unittest/lib/src/expec...
pkg/unittest/lib/src/expect.dart:147: if (info.length > 0) {
On 2013/06/12 00:34:45, Siggi Cherem (dart-lang) wrote:
> how about use mismatchDescription.length here?

Done.

https://codereview.chromium.org/16408019/diff/2001/pkg/unittest/lib/src/expec...
pkg/unittest/lib/src/expect.dart:148: description.add('   Which: ${info}\n');
On 2013/06/12 00:34:45, Siggi Cherem (dart-lang) wrote:
> then this can simply be $mispatchDescription

Done.

https://codereview.chromium.org/16408019/diff/2001/pkg/unittest/lib/src/expec...
pkg/unittest/lib/src/expect.dart:148: description.add('   Which: ${info}\n');
On 2013/06/12 00:34:45, Siggi Cherem (dart-lang) wrote:
> "   Which:" => " Reason:" ?

"Which" scans better IMO.

[Siggi Cherem (dart-lang), 2013-06-14 01:20:56]

very very cool. LGTM!

https://codereview.chromium.org/16408019/diff/18001/pkg/unittest/lib/src/core...
File pkg/unittest/lib/src/core_matchers.dart (right):

https://codereview.chromium.org/16408019/diff/18001/pkg/unittest/lib/src/core...
pkg/unittest/lib/src/core_matchers.dart:140: _recursiveMatch, depth+1,
location);
nit: fix the spaces around +

https://codereview.chromium.org/16408019/diff/18001/pkg/unittest/lib/src/expe...
File pkg/unittest/lib/src/expect.dart (right):

https://codereview.chromium.org/16408019/diff/18001/pkg/unittest/lib/src/expe...
pkg/unittest/lib/src/expect.dart:27: for (var key in values.keys) {
I believe now you can do matchState.addAll(values);

https://codereview.chromium.org/16408019/diff/18001/pkg/unittest/lib/src/iter...
File pkg/unittest/lib/src/iterable_matchers.dart (right):

https://codereview.chromium.org/16408019/diff/18001/pkg/unittest/lib/src/iter...
pkg/unittest/lib/src/iterable_matchers.dart:220: addStateInfo(matchState,
{'index': i, 'expected': e, 'actual':
nit: move the last key to the next line, so key/value pairs are together

https://codereview.chromium.org/16408019/diff/18001/pkg/unittest/lib/unittest...
File pkg/unittest/lib/unittest.dart (right):

https://codereview.chromium.org/16408019/diff/18001/pkg/unittest/lib/unittest...
pkg/unittest/lib/unittest.dart:895: if (!formatStacks) {
nit: use fast-exit pattern 
if (!formatStacks) return "$stack";

[gram, 2013-06-14 19:32:29]

Committed patchset #9 manually as r24045 (presubmit successful).