Support nested matchers in deep equality matching.

Support nested matchers in deep equality matching.

R=kevmoo@google.com, sigmund@google.com

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

[Bob Nystrom, 2014-02-28 01:59:47]

Equals supports deep matching, so you can do stuff like:

expect(['a', {'b': 'c'}], equals(['a', {'b': 'c'}]);

What it doesn't support is *matchers* inside there. It only does equality
checks. This extends that so that if the expectation is a matcher, it validates
using that, to allow stuff like:

expect(['a', {'b': 'c'}], equals([contains('a'), {'b': startsWith('c')}]);

[kevmoo, 2014-02-28 02:39:56]

But what if I want to test a Map that actually contains matchers? ;-)

LGTM

[Siggi Cherem (dart-lang), 2014-02-28 02:41:03]

lgtm - nice cleanup

https://codereview.chromium.org/184103002/diff/1/pkg/unittest/lib/src/core_ma...
File pkg/unittest/lib/src/core_matchers.dart (right):

https://codereview.chromium.org/184103002/diff/1/pkg/unittest/lib/src/core_ma...
pkg/unittest/lib/src/core_matchers.dart:122: }
since you are already fixing style :), consider changing the 5 lines above to:

if (actualIterator.moveNext()) return ['longer than expected', newL];
return null;

https://codereview.chromium.org/184103002/diff/1/pkg/unittest/lib/src/core_ma...
pkg/unittest/lib/src/core_matchers.dart:159: return ['expected a map',
location];
nit: this might fit the 1-line if:
   if(...) return ...;

https://codereview.chromium.org/184103002/diff/1/pkg/unittest/lib/src/pretty_...
File pkg/unittest/lib/src/pretty_print.dart (right):

https://codereview.chromium.org/184103002/diff/1/pkg/unittest/lib/src/pretty_...
pkg/unittest/lib/src/pretty_print.dart:7: import '../matcher.dart';
use package: ?

https://codereview.chromium.org/184103002/diff/1/pkg/unittest/test/matchers_t...
File pkg/unittest/test/matchers_test.dart (right):

https://codereview.chromium.org/184103002/diff/1/pkg/unittest/test/matchers_t...
pkg/unittest/test/matchers_test.dart:612: "Which: does not match a string ending
with 'ba' at location ['foo']");
80 col

[kevmoo, 2014-02-28 16:24:52]

Also: please update the changelog. :-)

[Bob Nystrom, 2014-02-28 17:31:13]

Committed patchset #1 manually as r33164 (presubmit successful).

[Bob Nystrom, 2014-02-28 17:31:24]

Thanks!

https://codereview.chromium.org/184103002/diff/1/pkg/unittest/lib/src/core_ma...
File pkg/unittest/lib/src/core_matchers.dart (right):

https://codereview.chromium.org/184103002/diff/1/pkg/unittest/lib/src/core_ma...
pkg/unittest/lib/src/core_matchers.dart:122: }
On 2014/02/28 02:41:04, Siggi Cherem (dart-lang) wrote:
> since you are already fixing style :), consider changing the 5 lines above to:
> 
> if (actualIterator.moveNext()) return ['longer than expected', newL];
> return null;

Done, and then some.

https://codereview.chromium.org/184103002/diff/1/pkg/unittest/lib/src/core_ma...
pkg/unittest/lib/src/core_matchers.dart:159: return ['expected a map',
location];
On 2014/02/28 02:41:04, Siggi Cherem (dart-lang) wrote:
> nit: this might fit the 1-line if:
>    if(...) return ...;

Done.

https://codereview.chromium.org/184103002/diff/1/pkg/unittest/lib/src/pretty_...
File pkg/unittest/lib/src/pretty_print.dart (right):

https://codereview.chromium.org/184103002/diff/1/pkg/unittest/lib/src/pretty_...
pkg/unittest/lib/src/pretty_print.dart:7: import '../matcher.dart';
On 2014/02/28 02:41:04, Siggi Cherem (dart-lang) wrote:
> use package: ?

Do you think that's better? It seems a bit weird to do that when the next line
doesn't.

https://codereview.chromium.org/184103002/diff/1/pkg/unittest/test/matchers_t...
File pkg/unittest/test/matchers_test.dart (right):

https://codereview.chromium.org/184103002/diff/1/pkg/unittest/test/matchers_t...
pkg/unittest/test/matchers_test.dart:612: "Which: does not match a string ending
with 'ba' at location ['foo']");
On 2014/02/28 02:41:04, Siggi Cherem (dart-lang) wrote:
> 80 col

Done.

[Bob Nystrom, 2014-02-28 17:32:04]

On 2014/02/28 16:24:52, kevmoo wrote:
> But what if I want to test a Map that actually contains matchers? ;-)

I did actually consider that. It punishes the meta case of using unittest to
test unittest, but I'm OK with sacrificing that.

> Also: please update the changelog. :-)

Done!

- bob

[Siggi Cherem (dart-lang), 2014-02-28 17:37:18]

https://codereview.chromium.org/184103002/diff/1/pkg/unittest/lib/src/pretty_...
File pkg/unittest/lib/src/pretty_print.dart (right):

https://codereview.chromium.org/184103002/diff/1/pkg/unittest/lib/src/pretty_...
pkg/unittest/lib/src/pretty_print.dart:7: import '../matcher.dart';
On 2014/02/28 17:31:24, Bob Nystrom wrote:
> On 2014/02/28 02:41:04, Siggi Cherem (dart-lang) wrote:
> > use package: ?
> 
> Do you think that's better? It seems a bit weird to do that when the next line
> doesn't.

Not really.

I think in general is a bad idea to use '../'. I feel that generally people have
a good understanding of what's deeper inside a package, but not so much of
what's above it.