Some unit test fixes

Some unit test fixes:
Fixed a couple of issues in the ensureInitialized() function.
Added expectAsyncUntil methods to support callbackDone style tests.
Split the matcher tests up so that we can exclude those that use isInstance
from the environments where they don't work.
Eliminated the need for two functions calls in throws/throwsA.
Fixed some tests.
Changed some uses of Expect to expect.
Fixed a bug when using expectAsync with a count of zero.
Fixed a bug where we weren't updating the call count in invoke0/1/2.
Made expect behave sensibly in cases where the user doesn't supply a 
matcher (e.g. expect(foo, 0)). USing the predicate form with a 
reason now requires a named parameter.
Committed: https://code.google.com/p/dart/source/detail?r=8753

[gram, 2012-06-15 00:16:50]

PTAL again. Changes from before:

- the invoke0, invoke1 and invoke2 methods in SpreadArgsHelper were mssing the
++_calls line which caused some tests to time out.
- I moved the expect() call after port.close() in v2_spawn_uri_vm_test and
v2_spawn_uri_vm_negative_test as the port will not be closed if expect throws.
- the transferables test should be excluded if we use dart2js as it uses
isInstance which only works in the VM
- html/fileapi_test was using asyncTest/callbackDone; I fixed that.
- the rest of the new stuff is almost all just changing Expect to expect in a
number of tests

[gram, 2012-06-15 00:17:55]

Sorry, I was unclear - otherwise this is the same as the change you reviewed
yesterday

On 2012/06/15 00:16:50, gram wrote:
> PTAL again. Changes from before:
> 
> - the invoke0, invoke1 and invoke2 methods in SpreadArgsHelper were mssing the
> ++_calls line which caused some tests to time out.
> - I moved the expect() call after port.close() in v2_spawn_uri_vm_test and
> v2_spawn_uri_vm_negative_test as the port will not be closed if expect throws.
> - the transferables test should be excluded if we use dart2js as it uses
> isInstance which only works in the VM
> - html/fileapi_test was using asyncTest/callbackDone; I fixed that.
> - the rest of the new stuff is almost all just changing Expect to expect in a
> number of tests

[gram, 2012-06-15 19:33:35]

Adding Eli as Bob is out.

[eub, 2012-06-15 21:02:49]

Looks good, just a couple of suggestions.  Thanks!

http://codereview.chromium.org/10544167/diff/7001/lib/unittest/expect.dart
File lib/unittest/expect.dart (right):

http://codereview.chromium.org/10544167/diff/7001/lib/unittest/expect.dart#ne...
lib/unittest/expect.dart:35: void expect(actual, [matcherOrReason = null, String
reason = '']) {
BTW, I imagine you had prior review discussion of splitting this into
    assert(predicate_expression, [reason]
and
    expect(actual, matcher, [reason])
?

http://codereview.chromium.org/10544167/diff/7001/lib/unittest/unittest.dart
File lib/unittest/unittest.dart (right):

http://codereview.chromium.org/10544167/diff/7001/lib/unittest/unittest.dart#...
lib/unittest/unittest.dart:297: // arguments inside [_expectAsync], once bug 282
is fixed or frog is replaced by
"now that frog is replaced by dart2js" :)

http://codereview.chromium.org/10544167/diff/7001/tests/lib/unittest/matchers...
File tests/lib/unittest/matchers_test.dart (right):

http://codereview.chromium.org/10544167/diff/7001/tests/lib/unittest/matchers...
tests/lib/unittest/matchers_test.dart:64: shouldFail(a, equals(b), "Expected:
<{}> but: was <{}>");
Today I learned something about Map's equality operator.

http://codereview.chromium.org/10544167/diff/7001/tests/lib/unittest/matchers...
tests/lib/unittest/matchers_test.dart:68: shouldPass(a, anything);
Might exercise some other cases like (null, anything).

http://codereview.chromium.org/10544167/diff/7001/tests/lib/unittest/matchers...
tests/lib/unittest/matchers_test.dart:92: c.add(0);
I have a religious objection to mutating shared test state.

[gram, 2012-06-15 21:25:55]

PTAL

http://codereview.chromium.org/10544167/diff/7001/lib/unittest/expect.dart
File lib/unittest/expect.dart (right):

http://codereview.chromium.org/10544167/diff/7001/lib/unittest/expect.dart#ne...
lib/unittest/expect.dart:35: void expect(actual, [matcherOrReason = null, String
reason = '']) {
On 2012/06/15 21:02:49, eub wrote:
> BTW, I imagine you had prior review discussion of splitting this into
>     assert(predicate_expression, [reason]
> and
>     expect(actual, matcher, [reason])
> ?

Yes - the aim is to have just one expect(), but to have it support shorthand
cases, such as:

expect(foo);

expect(foo, 10);

as well as:

expect(foo, isTrue);

expect(foo, equals(10));

The only controversy comes from the ambiguous:

expect(foo, "bar");

which we currently treat as an assert(foo) reason bar.

Given that we have named arguments, we could change this to not overload the
second argument, while still asserting in there is no matcher, so:

expect(foo, "bar") means expect(foo, equals("bar"))

and the previous case would need to be:

expect(foo, reason:"bar");

I'm quite open to that change if you think it is an improvement but I'd like to
make that a separate CL.

http://codereview.chromium.org/10544167/diff/7001/lib/unittest/unittest.dart
File lib/unittest/unittest.dart (right):

http://codereview.chromium.org/10544167/diff/7001/lib/unittest/unittest.dart#...
lib/unittest/unittest.dart:297: // arguments inside [_expectAsync], once bug 282
is fixed or frog is replaced by
On 2012/06/15 21:02:49, eub wrote:
> "now that frog is replaced by dart2js" :)

I tweaked the comment a bit. I'm not sure it makes sense any more to remove the
class.

http://codereview.chromium.org/10544167/diff/7001/tests/lib/unittest/matchers...
File tests/lib/unittest/matchers_test.dart (right):

http://codereview.chromium.org/10544167/diff/7001/tests/lib/unittest/matchers...
tests/lib/unittest/matchers_test.dart:64: shouldFail(a, equals(b), "Expected:
<{}> but: was <{}>");
On 2012/06/15 21:02:49, eub wrote:
> Today I learned something about Map's equality operator.

Done.

http://codereview.chromium.org/10544167/diff/7001/tests/lib/unittest/matchers...
tests/lib/unittest/matchers_test.dart:68: shouldPass(a, anything);
On 2012/06/15 21:02:49, eub wrote:
> Might exercise some other cases like (null, anything).

Done.

http://codereview.chromium.org/10544167/diff/7001/tests/lib/unittest/matchers...
tests/lib/unittest/matchers_test.dart:92: c.add(0);
On 2012/06/15 21:02:49, eub wrote:
> I have a religious objection to mutating shared test state.

Point taken; I replicated these so each test has its own copy.

[eub, 2012-06-15 21:34:16]

lgtm

[gram, 2012-06-15 22:12:38]

PTAL. When using predicate form of expect() reason is now required to be a named
argument, which eliminates the ugly "matcherOrReason" overloaded second
parameter stuff.

[eub, 2012-06-15 22:30:43]

LGTM, minor nits.

http://codereview.chromium.org/10544167/diff/9002/lib/unittest/expect.dart
File lib/unittest/expect.dart (right):

http://codereview.chromium.org/10544167/diff/9002/lib/unittest/expect.dart#ne...
lib/unittest/expect.dart:5: /**
Document that "matcher" may be a value to match for equality, assuming that is a
documented part of the interface.

http://codereview.chromium.org/10544167/diff/9002/lib/unittest/expect.dart#ne...
lib/unittest/expect.dart:29: *     expect(foo <= bar, reason: "foo not less than
bar");
Utter nitpick: "foo not less than or equal to bar" :-P

http://codereview.chromium.org/10544167/diff/9002/lib/unittest/expect.dart#ne...
lib/unittest/expect.dart:39: void expect(actual, [matcher = null, String reason
= '']) {
How do you like null instead of ''?

http://codereview.chromium.org/10544167/diff/9002/lib/unittest/expect.dart#ne...
lib/unittest/expect.dart:41: // Treat this as an assert(predicate, reason).
[reason]

http://codereview.chromium.org/10544167/diff/9002/lib/unittest/expect.dart#ne...
lib/unittest/expect.dart:52: if (matcher is! Matcher) {
This is a duplicate "is" check w/ the one in wrapMatcher.

http://codereview.chromium.org/10544167/diff/9002/lib/unittest/expect.dart#ne...
lib/unittest/expect.dart:61: reason = '${(e is String) ? e : e.toString()} at
$trace';
It would be nice if String had a no-op toString, eh.

[gram, 2012-06-15 22:40:42]

PTAL

http://codereview.chromium.org/10544167/diff/9002/lib/unittest/expect.dart
File lib/unittest/expect.dart (right):

http://codereview.chromium.org/10544167/diff/9002/lib/unittest/expect.dart#ne...
lib/unittest/expect.dart:29: *     expect(foo <= bar, reason: "foo not less than
bar");
On 2012/06/15 22:30:43, eub wrote:
> Utter nitpick: "foo not less than or equal to bar" :-P

Done.

http://codereview.chromium.org/10544167/diff/9002/lib/unittest/expect.dart#ne...
lib/unittest/expect.dart:39: void expect(actual, [matcher = null, String reason
= '']) {
On 2012/06/15 22:30:43, eub wrote:
> How do you like null instead of ''?

Done.

http://codereview.chromium.org/10544167/diff/9002/lib/unittest/expect.dart#ne...
lib/unittest/expect.dart:39: void expect(actual, [matcher = null, String reason
= '']) {
On 2012/06/15 22:30:43, eub wrote:
> How do you like null instead of ''?

Done.

http://codereview.chromium.org/10544167/diff/9002/lib/unittest/expect.dart#ne...
lib/unittest/expect.dart:41: // Treat this as an assert(predicate, reason).
On 2012/06/15 22:30:43, eub wrote:
> [reason]

Done.

http://codereview.chromium.org/10544167/diff/9002/lib/unittest/expect.dart#ne...
lib/unittest/expect.dart:52: if (matcher is! Matcher) {
On 2012/06/15 22:30:43, eub wrote:
> This is a duplicate "is" check w/ the one in wrapMatcher.

Done.

http://codereview.chromium.org/10544167/diff/9002/lib/unittest/expect.dart#ne...
lib/unittest/expect.dart:61: reason = '${(e is String) ? e : e.toString()} at
$trace';
On 2012/06/15 22:30:43, eub wrote:
> It would be nice if String had a no-op toString, eh.

:-)

[eub, 2012-06-15 23:14:51]

lgtm