Changed the syntax some. All the handling of variable argument lists is

Changed the syntax some. All the handling of variable argument lists is
moved to a function callsTo(), and when() is renamed whenThereAre(), so we
write m.whenThereAre(callsTo(...)).

Verification is moved out of Mock; Mock just returns the matching log, and 
LogEntrySet verify() is used instead.

Added the ability to proxy calls, so we can create spys.

Added the ability to verify return values.

Added some tests.
Committed: https://code.google.com/p/dart/source/detail?r=9212

[gram, 2012-06-27 23:11:42]

Some cool stuff.

[Siggi Cherem (dart-lang), 2012-06-28 00:44:40]

https://chromiumcodereview.appspot.com/10718002/diff/1/lib/unittest/mock.dart
File lib/unittest/mock.dart (right):

https://chromiumcodereview.appspot.com/10718002/diff/1/lib/unittest/mock.dart...
lib/unittest/mock.dart:204: Behavior thenCall(value, [count = 1]) {
what is [value]. Looking a the code in the test, seems like this is a closure
that takes the same arguments as the call matcher?

https://chromiumcodereview.appspot.com/10718002/diff/1/lib/unittest/mock.dart...
lib/unittest/mock.dart:329: * of method calls.
=> of a sequence of method calls? (since this is applied on a log?)

https://chromiumcodereview.appspot.com/10718002/diff/1/lib/unittest/mock.dart...
lib/unittest/mock.dart:334: final int count; // -1 for all, 0 for none, 1 for
some.
please change to something more self explanatory (e.g. enums or booleans).

I think using booleans like:
final bool matchAlways;
final bool matchSome;
final bool matchNone;

will make the code in matches easier to understand:
...
if (eaction == action && value.matches(entry.value)) {
  if (matchSome) return true;
  if (matchNone) return false;
} else {
  // Mismatch.
  if (matchAlways) return false;
}
return matchAlways || matchNone;

https://chromiumcodereview.appspot.com/10718002/diff/1/lib/unittest/mock.dart...
lib/unittest/mock.dart:359: }
+ line separation between methods (here and below )

https://chromiumcodereview.appspot.com/10718002/diff/1/lib/unittest/mock.dart...
lib/unittest/mock.dart:442: * [whenThereAre] (to select the method and
parameters) and [thenReturn],
whenThereAre feels really verbose, see some suggestions in the test file...

https://chromiumcodereview.appspot.com/10718002/diff/1/lib/unittest/mock.dart...
lib/unittest/mock.dart:449: *         
thenReturn(0).thenReturn(1).thenReturn(2);
you can also use cascades here :)
m.whenThereAre(callsTo('foo'))..thenReturn(0)
                              ..thenReturn(1)
                              ..thenReturn(2);

https://chromiumcodereview.appspot.com/10718002/diff/1/lib/unittest/mock.dart...
lib/unittest/mock.dart:452: * to some other implementation. This provides a way
to implement 'spies'.
It took me a while to get what are proxies and spies. An example would help.

I'd also emphasize that the value of this is that you can write your own
implementation of the method using a closure and passing it to [thenCall] or
[alwaysCall].

We should also mention where the return value comes from on this case.

This might be an interesting example:
  m.whenThereAre(callsTo('foo')).thenReturn(0)
is equivalent to:
  m.whenThereAre(callsTo('foo')).thenCall(() => 0)

https://chromiumcodereview.appspot.com/10718002/diff/1/lib/unittest/mock.dart...
lib/unittest/mock.dart:455: * behavior, use [forThe] to extract a relevant
subset of method call
:-( I liked verify better... more suggestions on the test file...

https://chromiumcodereview.appspot.com/10718002/diff/1/tests/lib/unittest/uni...
File tests/lib/unittest/unittest_test.dart (right):

https://chromiumcodereview.appspot.com/10718002/diff/1/tests/lib/unittest/uni...
tests/lib/unittest/unittest_test.dart:146: m.whenThereAre(callsTo('Sum', 1,
2)).thenReturn('A').thenReturn('B');
I'm playing around with other ideas to see if there is any that you like:
- m.whenThereIs(aCall('sum', 1, 2)).thenReturn('A')
- m.mock(callTo('sum', 1, 2)).returns('A')
- m.mock(aCallTo('sum', 1, 2)).returns('A')
- aCall(m, 'Sum', 1, 2).returns('A')
- mockCall(m, 'Sum', 1, 2).toReturn('A')

Also, we could overload the noSuchMethod so we can use it to record
expectations? (e.g. when we are done setting up, we can say m.readyToUse(); or
something):
- m.sum(1,2).returns('a')

https://chromiumcodereview.appspot.com/10718002/diff/1/tests/lib/unittest/uni...
tests/lib/unittest/unittest_test.dart:149: m.whenThereAre(callsTo('Sub',
anything, anything)).thenReturn('E');
could we change the example to use lowercase method names instead :)?

https://chromiumcodereview.appspot.com/10718002/diff/1/tests/lib/unittest/uni...
tests/lib/unittest/unittest_test.dart:153: m.forThe(callsTo('Sum', anything,
anything)).verify(calledExactly(4));
Ideas instead of 'm.forThe(_).verify(_)'
- m.verify(_).that(_)
- m.veryfy(_).itWas(_)

[gram, 2012-06-28 17:32:54]

PTAL

https://chromiumcodereview.appspot.com/10718002/diff/1/lib/unittest/mock.dart
File lib/unittest/mock.dart (right):

https://chromiumcodereview.appspot.com/10718002/diff/1/lib/unittest/mock.dart...
lib/unittest/mock.dart:204: Behavior thenCall(value, [count = 1]) {
On 2012/06/28 00:44:40, sigmund wrote:
> what is [value]. Looking a the code in the test, seems like this is a closure
> that takes the same arguments as the call matcher?

Added a comment.

https://chromiumcodereview.appspot.com/10718002/diff/1/lib/unittest/mock.dart...
lib/unittest/mock.dart:329: * of method calls.
On 2012/06/28 00:44:40, sigmund wrote:
> => of a sequence of method calls? (since this is applied on a log?)

I don't like 'sequence', even if the logs are temporally ordered, because we are
filtering to a subset that may not be related temporally. I have adjusted the
comment to make things clearer I hope.

https://chromiumcodereview.appspot.com/10718002/diff/1/lib/unittest/mock.dart...
lib/unittest/mock.dart:334: final int count; // -1 for all, 0 for none, 1 for
some.
On 2012/06/28 00:44:40, sigmund wrote:
> please change to something more self explanatory (e.g. enums or booleans).
> 
> I think using booleans like:
> final bool matchAlways;
> final bool matchSome;
> final bool matchNone;
> 
> will make the code in matches easier to understand:
> ...
> if (eaction == action && value.matches(entry.value)) {
>   if (matchSome) return true;
>   if (matchNone) return false;
> } else {
>   // Mismatch.
>   if (matchAlways) return false;
> }
> return matchAlways || matchNone;

I used a single value as an enum; it scales better than boolean flags (e.g. you
could add something like "not all" easily).

https://chromiumcodereview.appspot.com/10718002/diff/1/lib/unittest/mock.dart...
lib/unittest/mock.dart:359: }
On 2012/06/28 00:44:40, sigmund wrote:
> + line separation between methods (here and below )

Done.

https://chromiumcodereview.appspot.com/10718002/diff/1/lib/unittest/mock.dart...
lib/unittest/mock.dart:442: * [whenThereAre] (to select the method and
parameters) and [thenReturn],
On 2012/06/28 00:44:40, sigmund wrote:
> whenThereAre feels really verbose, see some suggestions in the test file...

Done.

https://chromiumcodereview.appspot.com/10718002/diff/1/lib/unittest/mock.dart...
lib/unittest/mock.dart:449: *         
thenReturn(0).thenReturn(1).thenReturn(2);
On 2012/06/28 00:44:40, sigmund wrote:
> you can also use cascades here :)
> m.whenThereAre(callsTo('foo'))..thenReturn(0)
>                               ..thenReturn(1)
>                               ..thenReturn(2);

Yeah, but most people are never going to know they exist! I only found out
yesterday :-)

https://chromiumcodereview.appspot.com/10718002/diff/1/lib/unittest/mock.dart...
lib/unittest/mock.dart:452: * to some other implementation. This provides a way
to implement 'spies'.
On 2012/06/28 00:44:40, sigmund wrote:
> It took me a while to get what are proxies and spies. An example would help.
> 
> I'd also emphasize that the value of this is that you can write your own
> implementation of the method using a closure and passing it to [thenCall] or
> [alwaysCall].
> 
> We should also mention where the return value comes from on this case.
> 
> This might be an interesting example:
>   m.whenThereAre(callsTo('foo')).thenReturn(0)
> is equivalent to:
>   m.whenThereAre(callsTo('foo')).thenCall(() => 0)

I've changed comments in a few places that hopefully make this clearer,
including your example too.

https://chromiumcodereview.appspot.com/10718002/diff/1/tests/lib/unittest/uni...
File tests/lib/unittest/unittest_test.dart (right):

https://chromiumcodereview.appspot.com/10718002/diff/1/tests/lib/unittest/uni...
tests/lib/unittest/unittest_test.dart:149: m.whenThereAre(callsTo('Sub',
anything, anything)).thenReturn('E');
On 2012/06/28 00:44:40, sigmund wrote:
> could we change the example to use lowercase method names instead :)?

Done, and I changed the names to foo/bar which is more nebulous, as they
certainly don't do addition, etc.

[Siggi Cherem (dart-lang), 2012-06-28 17:51:43]

LGTM! Thanks Gram! (minor comments below...)

https://chromiumcodereview.appspot.com/10718002/diff/3001/lib/unittest/mock.dart
File lib/unittest/mock.dart (right):

https://chromiumcodereview.appspot.com/10718002/diff/3001/lib/unittest/mock.d...
lib/unittest/mock.dart:636: *     mock.forThe(callsTo(...)).verify(...);
should we keep this example below? 

getLogs(mock, callsTo(...)).verify(...);

https://chromiumcodereview.appspot.com/10718002/diff/9001/lib/unittest/mock.dart
File lib/unittest/mock.dart (right):

https://chromiumcodereview.appspot.com/10718002/diff/9001/lib/unittest/mock.d...
lib/unittest/mock.dart:228: /** toString() just returns the matcher. */
nits:
- no need to mention the method name here
- that sounds like if you were returning a matcher, not a string.

=> Returns the matcher's string representation?

https://chromiumcodereview.appspot.com/10718002/diff/9001/lib/unittest/mock.d...
lib/unittest/mock.dart:349: /**
+1 line here too :)

https://chromiumcodereview.appspot.com/10718002/diff/9001/lib/unittest/mock.d...
lib/unittest/mock.dart:352: * [forThe], a [LogEntrySet] of matching call logs is
returned;
:1,$s/forThe/getLogs/g

https://chromiumcodereview.appspot.com/10718002/diff/9001/lib/unittest/mock.d...
lib/unittest/mock.dart:667: return mock.log.getMatches(logFilter, destructive);
<< (only +2 indent)