Fix timer tests: timer tests were not using expectAsync to wait for

Fix timer tests: timer tests were not using expectAsync to wait for
callbacks to be executed.

BUG=http:///dartbug/5730

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

[Siggi Cherem (dart-lang), 2012-10-08 21:50:07]

Graham - can you take a look at the change in unittest below? Thanks!

https://codereview.chromium.org/11028076/diff/1/pkg/unittest/unittest.dart
File pkg/unittest/unittest.dart (right):

https://codereview.chromium.org/11028076/diff/1/pkg/unittest/unittest.dart#ne...
pkg/unittest/unittest.dart:325: print("No valid test, did you forget to run your
test inside a call "
I'm surprised you need this... I'm cc'ing Graham for input (he is currently
maintaining this code).

https://codereview.chromium.org/11028076/diff/1/tests/isolate/timer_cancel1_t...
File tests/isolate/timer_cancel1_test.dart (right):

https://codereview.chromium.org/11028076/diff/1/tests/isolate/timer_cancel1_t...
tests/isolate/timer_cancel1_test.dart:25: canceleeTimer = new Timer(1000,
expectAsync1(timeoutHandlerUnreachable, count: 0));
80 col

https://codereview.chromium.org/11028076/diff/1/tests/isolate/timer_cancel_te...
File tests/isolate/timer_cancel_test.dart (right):

https://codereview.chromium.org/11028076/diff/1/tests/isolate/timer_cancel_te...
tests/isolate/timer_cancel_test.dart:32: cancelTimer = new Timer(1000,
expectAsync1(timeoutHandlerUnreachable, count: 0));
80 col, the name for the callbacks are quite verbose, it might be worth to
rename them:

timeoutHandlerUnreachable -> unreachableHandler
timeoutHandler -> handler
timeoutHandlerRepeate -> repeatHandler

https://codereview.chromium.org/11028076/diff/1/tests/isolate/timer_not_avail...
File tests/isolate/timer_not_available_test.dart (right):

https://codereview.chromium.org/11028076/diff/1/tests/isolate/timer_not_avail...
tests/isolate/timer_not_available_test.dart:14: print(e);
is this needed? or just debugging code?

https://codereview.chromium.org/11028076/diff/1/tests/isolate/timer_repeat_te...
File tests/isolate/timer_repeat_test.dart (right):

https://codereview.chromium.org/11028076/diff/1/tests/isolate/timer_repeat_te...
tests/isolate/timer_repeat_test.dart:32: timer = new Timer.repeating(_TIMEOUT,
expectAsync1(timeoutHandler, count: _ITERATIONS));
80 col

[gram, 2012-10-08 22:03:30]

https://codereview.chromium.org/11028076/diff/1/pkg/unittest/unittest.dart
File pkg/unittest/unittest.dart (right):

https://codereview.chromium.org/11028076/diff/1/pkg/unittest/unittest.dart#ne...
pkg/unittest/unittest.dart:325: print("No valid test, did you forget to run your
test inside a call "
On 2012/10/08 21:50:07, sigmund wrote:
> I'm surprised you need this... I'm cc'ing Graham for input (he is currently
> maintaining this code).

We have an assert on the next line for the converse of this. As such, having a
print here should be harmless. But I agree, it shouldn't be necessary.

[justinfagnani, 2012-10-08 22:12:48]

ptal

https://codereview.chromium.org/11028076/diff/1/pkg/unittest/unittest.dart
File pkg/unittest/unittest.dart (right):

https://codereview.chromium.org/11028076/diff/1/pkg/unittest/unittest.dart#ne...
pkg/unittest/unittest.dart:325: print("No valid test, did you forget to run your
test inside a call "
On 2012/10/08 21:50:07, sigmund wrote:
> I'm surprised you need this... I'm cc'ing Graham for input (he is currently
> maintaining this code).

I added an expectAsync to a test before wrapping it in test() and it took a
while for me to figure out what was going on. This is for the next person (or
me) who does that.

https://codereview.chromium.org/11028076/diff/1/pkg/unittest/unittest.dart#ne...
pkg/unittest/unittest.dart:325: print("No valid test, did you forget to run your
test inside a call "
On 2012/10/08 22:03:30, gram wrote:
> On 2012/10/08 21:50:07, sigmund wrote:
> > I'm surprised you need this... I'm cc'ing Graham for input (he is currently
> > maintaining this code).
> 
> We have an assert on the next line for the converse of this. As such, having a
> print here should be harmless. But I agree, it shouldn't be necessary.

The problem was that I wasn't getting the source of the failure or any useful
message. Should we expect a stacktrace to be printed for failed asserts?

https://codereview.chromium.org/11028076/diff/1/tests/isolate/timer_cancel1_t...
File tests/isolate/timer_cancel1_test.dart (right):

https://codereview.chromium.org/11028076/diff/1/tests/isolate/timer_cancel1_t...
tests/isolate/timer_cancel1_test.dart:25: canceleeTimer = new Timer(1000,
expectAsync1(timeoutHandlerUnreachable, count: 0));
On 2012/10/08 21:50:07, sigmund wrote:
> 80 col

Done.

https://codereview.chromium.org/11028076/diff/1/tests/isolate/timer_cancel_te...
File tests/isolate/timer_cancel_test.dart (right):

https://codereview.chromium.org/11028076/diff/1/tests/isolate/timer_cancel_te...
tests/isolate/timer_cancel_test.dart:32: cancelTimer = new Timer(1000,
expectAsync1(timeoutHandlerUnreachable, count: 0));
On 2012/10/08 21:50:07, sigmund wrote:
> 80 col, the name for the callbacks are quite verbose, it might be worth to
> rename them:
> 
> timeoutHandlerUnreachable -> unreachableHandler
> timeoutHandler -> handler
> timeoutHandlerRepeate -> repeatHandler

Done.

https://codereview.chromium.org/11028076/diff/1/tests/isolate/timer_not_avail...
File tests/isolate/timer_not_available_test.dart (right):

https://codereview.chromium.org/11028076/diff/1/tests/isolate/timer_not_avail...
tests/isolate/timer_not_available_test.dart:14: print(e);
On 2012/10/08 21:50:07, sigmund wrote:
> is this needed? or just debugging code?

removed

https://codereview.chromium.org/11028076/diff/1/tests/isolate/timer_repeat_te...
File tests/isolate/timer_repeat_test.dart (right):

https://codereview.chromium.org/11028076/diff/1/tests/isolate/timer_repeat_te...
tests/isolate/timer_repeat_test.dart:32: timer = new Timer.repeating(_TIMEOUT,
expectAsync1(timeoutHandler, count: _ITERATIONS));
On 2012/10/08 21:50:07, sigmund wrote:
> 80 col

Done.

[Siggi Cherem (dart-lang), 2012-10-08 22:29:15]

lgtm

https://codereview.chromium.org/11028076/diff/1/pkg/unittest/unittest.dart
File pkg/unittest/unittest.dart (right):

https://codereview.chromium.org/11028076/diff/1/pkg/unittest/unittest.dart#ne...
pkg/unittest/unittest.dart:325: print("No valid test, did you forget to run your
test inside a call "
On 2012/10/08 22:12:48, justinfagnani wrote:
> On 2012/10/08 22:03:30, gram wrote:
> > On 2012/10/08 21:50:07, sigmund wrote:
> > > I'm surprised you need this... I'm cc'ing Graham for input (he is
currently
> > > maintaining this code).
> > 
> > We have an assert on the next line for the converse of this. As such, having
a
> > print here should be harmless. But I agree, it shouldn't be necessary.
> 
> The problem was that I wasn't getting the source of the failure or any useful
> message. Should we expect a stacktrace to be printed for failed asserts?

I see - I hope so. We can also turn the assert into a throw inside this
condition instead. That way it is thrown also if you don't have assertions
enabled in the vm

[Siggi Cherem (dart-lang), 2012-10-08 23:12:37]

lgtm