Temporal assertion support.

Temporal assertion support.
Committed: https://code.google.com/p/dart/source/detail?r=9769

[Siggi Cherem (dart-lang), 2012-07-19 18:05:00]

http://codereview.chromium.org/10804014/diff/1/lib/unittest/mock.dart
File lib/unittest/mock.dart (right):

http://codereview.chromium.org/10804014/diff/1/lib/unittest/mock.dart#newcode353
lib/unittest/mock.dart:353: get first() {
we might fit this in a one liner :)
get first() => (logs == null || logs.length == 0) ? null : logs[0];

http://codereview.chromium.org/10804014/diff/1/lib/unittest/mock.dart#newcode361
lib/unittest/mock.dart:361: get last() {
ditto

http://codereview.chromium.org/10804014/diff/1/lib/unittest/mock.dart#newcode365
lib/unittest/mock.dart:365: return logs[logs.length-1];
use logs.last() instead?

http://codereview.chromium.org/10804014/diff/1/lib/unittest/mock.dart#newcode627
lib/unittest/mock.dart:627: LogEntryList fromLast(LogEntryList segment, [bool
inPlace = false]) =>
we should clarify on these comments the edge conditions. E.g. does this include
segment.last? Can we select the behaviour (e.g. a boolean argument to specify
whether to include it or not?)

http://codereview.chromium.org/10804014/diff/1/lib/unittest/mock.dart#newcode665
lib/unittest/mock.dart:665: * entries that match [mocknameFilter] and
[logFilter] will be included in
mockname -> mockName (capital N)

http://codereview.chromium.org/10804014/diff/1/lib/unittest/mock.dart#newcode711
lib/unittest/mock.dart:711: LogEntry logEntry = logIterator.next();
I don't see why you need to use the iterator explicitly for logs. Change to:
for (var logEntry in logs) {
 ...
?

http://codereview.chromium.org/10804014/diff/1/lib/unittest/mock.dart#newcode760
lib/unittest/mock.dart:760: LogEntryList preceding(LogEntryList keys,
I find these methods quite strange. Maybe we need more examples to illustrate
what they are for?
Currently it seems that the output is a flattened list of values for each key? 

preceding([a, b], dispance: 3) ==> [a1,a2,a3,b1,b2,b3]

It might make more sense to me to have this for a single LogEntry, or to return
a list of lists, one per key.

http://codereview.chromium.org/10804014/diff/1/tests/lib/unittest/unittest_te...
File tests/lib/unittest/unittest_test.dart (right):

http://codereview.chromium.org/10804014/diff/1/tests/lib/unittest/unittest_te...
tests/lib/unittest/unittest_test.dart:417: } else if (testName.startsWith('mock
test 15 ')) {
(as discussed) we should eventually move out of this test file any test that
doesn't require this special testing technique. Probably all of the mock tests
should live in a separate file.

http://codereview.chromium.org/10804014/diff/1/tests/lib/unittest/unittest_te...
tests/lib/unittest/unittest_test.dart:593: keyList.logs.clear();
BTW - whenever we move the tests into their own test file (so that we can use
unittest in the normal way), I think we should split these tests in multiple
smalller tests. Each one of these 'clear' to me indicates you are starting a new
kind of test with the same setup (same group?)

Can we use groups/setup/teardown in this current way of writing tests? or should
we wait until we factor this out?

[gram, 2012-07-19 18:52:19]

PTAL.

http://codereview.chromium.org/10804014/diff/1/lib/unittest/mock.dart
File lib/unittest/mock.dart (right):

http://codereview.chromium.org/10804014/diff/1/lib/unittest/mock.dart#newcode353
lib/unittest/mock.dart:353: get first() {
On 2012/07/19 18:05:00, sigmund wrote:
> we might fit this in a one liner :)
> get first() => (logs == null || logs.length == 0) ? null : logs[0];

Done.

http://codereview.chromium.org/10804014/diff/1/lib/unittest/mock.dart#newcode361
lib/unittest/mock.dart:361: get last() {
On 2012/07/19 18:05:00, sigmund wrote:
> ditto

Done.

http://codereview.chromium.org/10804014/diff/1/lib/unittest/mock.dart#newcode365
lib/unittest/mock.dart:365: return logs[logs.length-1];
On 2012/07/19 18:05:00, sigmund wrote:
> use logs.last() instead?

Done.

http://codereview.chromium.org/10804014/diff/1/lib/unittest/mock.dart#newcode627
lib/unittest/mock.dart:627: LogEntryList fromLast(LogEntryList segment, [bool
inPlace = false]) =>
On 2012/07/19 18:05:00, sigmund wrote:
> we should clarify on these comments the edge conditions. E.g. does this
include
> segment.last? Can we select the behaviour (e.g. a boolean argument to specify
> whether to include it or not?)

segment here need not have any elements in common with the logs being inspected;
we are using it just to get a timestamp. So it doesn't make sense to include
that element in the output as it may or may not exist.

The comment is unambiguous, I think:

Returns log events that happened from the TIME of the last event in [segment]
onwards.

These are just versions of before/after/until/from (which all take Dates) but we
are getting the date from a log which was most likely the result of some prior
filtering step.

A typical use case might be finding the calls to some error function in the logs
from one mock, and then finding the calls to some related method that preceded
these in a different mock (where each mock has its own log).

I do plan on expanding the mock article next week with some info on these with
use cases so that should make things more clear.

http://codereview.chromium.org/10804014/diff/1/lib/unittest/mock.dart#newcode665
lib/unittest/mock.dart:665: * entries that match [mocknameFilter] and
[logFilter] will be included in
On 2012/07/19 18:05:00, sigmund wrote:
> mockname -> mockName (capital N)

Done.

http://codereview.chromium.org/10804014/diff/1/lib/unittest/mock.dart#newcode711
lib/unittest/mock.dart:711: LogEntry logEntry = logIterator.next();
On 2012/07/19 18:05:00, sigmund wrote:
> I don't see why you need to use the iterator explicitly for logs. Change to:
> for (var logEntry in logs) {
>  ...
> ?

You're right - I iterated on a few versions of this code and it made sense in an
earlier version where I buffered both preceders and followers in scratch, but it
is unncessary now. Fixed.

http://codereview.chromium.org/10804014/diff/1/lib/unittest/mock.dart#newcode760
lib/unittest/mock.dart:760: LogEntryList preceding(LogEntryList keys,
On 2012/07/19 18:05:00, sigmund wrote:
> I find these methods quite strange. Maybe we need more examples to illustrate
> what they are for?
> Currently it seems that the output is a flattened list of values for each key?

> 
> preceding([a, b], dispance: 3) ==> [a1,a2,a3,b1,b2,b3]
> 
> It might make more sense to me to have this for a single LogEntry, or to
return
> a list of lists, one per key.

I respectfully disagree, other than providing more examples, which I will do.

http://codereview.chromium.org/10804014/diff/1/tests/lib/unittest/unittest_te...
File tests/lib/unittest/unittest_test.dart (right):

http://codereview.chromium.org/10804014/diff/1/tests/lib/unittest/unittest_te...
tests/lib/unittest/unittest_test.dart:417: } else if (testName.startsWith('mock
test 15 ')) {
On 2012/07/19 18:05:00, sigmund wrote:
> (as discussed) we should eventually move out of this test file any test that
> doesn't require this special testing technique. Probably all of the mock tests
> should live in a separate file.

Agreed; I'll move these out in a separate CL.

http://codereview.chromium.org/10804014/diff/1/tests/lib/unittest/unittest_te...
tests/lib/unittest/unittest_test.dart:593: keyList.logs.clear();
On 2012/07/19 18:05:00, sigmund wrote:
> BTW - whenever we move the tests into their own test file (so that we can use
> unittest in the normal way), I think we should split these tests in multiple
> smalller tests. Each one of these 'clear' to me indicates you are starting a
new
> kind of test with the same setup (same group?)
> 
> Can we use groups/setup/teardown in this current way of writing tests? or
should
> we wait until we factor this out?

Let me do it when it gets factored out.

[Siggi Cherem (dart-lang), 2012-07-19 20:02:25]

lgtm!

http://codereview.chromium.org/10804014/diff/1/lib/unittest/mock.dart
File lib/unittest/mock.dart (right):

http://codereview.chromium.org/10804014/diff/1/lib/unittest/mock.dart#newcode627
lib/unittest/mock.dart:627: LogEntryList fromLast(LogEntryList segment, [bool
inPlace = false]) =>
On 2012/07/19 18:52:19, gram wrote:
> On 2012/07/19 18:05:00, sigmund wrote:
> > we should clarify on these comments the edge conditions. E.g. does this
> include
> > segment.last? Can we select the behaviour (e.g. a boolean argument to
specify
> > whether to include it or not?)
> 
> segment here need not have any elements in common with the logs being
inspected;
> we are using it just to get a timestamp. So it doesn't make sense to include
> that element in the output as it may or may not exist.
> 
> The comment is unambiguous, I think:
> 
> Returns log events that happened from the TIME of the last event in [segment]
> onwards.
> 
> These are just versions of before/after/until/from (which all take Dates) but
we
> are getting the date from a log which was most likely the result of some prior
> filtering step.
> 
> A typical use case might be finding the calls to some error function in the
logs
> from one mock, and then finding the calls to some related method that preceded
> these in a different mock (where each mock has its own log).
> 
> I do plan on expanding the mock article next week with some info on these with
> use cases so that should make things more clear. 

Thanks for the clarification, it makes a lot of sense

http://codereview.chromium.org/10804014/diff/2002/lib/unittest/mock.dart
File lib/unittest/mock.dart (right):

http://codereview.chromium.org/10804014/diff/2002/lib/unittest/mock.dart#newc...
lib/unittest/mock.dart:754: *          logFilter:
callsTo('process')).toString());
Awesome example!
The print/toString was slighly distracting for me here, maybe we can trim the
example? For instance,

failingProcess = log.preceding(
    log.getLogs(callsTo('commit'), returning(-1)),
    logFilter: callsTo('process'));

http://codereview.chromium.org/10804014/diff/2002/lib/unittest/mock.dart#newc...
lib/unittest/mock.dart:764: *         distance: 3).toString());
ditto