Abort unit tests in the event of async failure while awaiting a future.

Abort unit tests in the event of async failure while awaiting a future.

Previously, if a unit test returned a future, then while the unit test
framework was awaiting that future, asynchronous failures occurring
during callback functions would not abort the test.

In addition to fixing the bug, fix a flaw in the _defer() function
which prevented async_exception_test from actually testing
asynchronous exceptions.

BUG= http://dartbug.com/20153
R=kevmoo@google.com

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

[kevmoo, 2014-07-29 19:59:02]

A few questions. Thanks for helping w/ this

https://codereview.chromium.org/416133002/diff/1/pkg/unittest/lib/src/test_ca...
File pkg/unittest/lib/src/test_case.dart (right):

https://codereview.chromium.org/416133002/diff/1/pkg/unittest/lib/src/test_ca...
pkg/unittest/lib/src/test_case.dart:105:
testReturn.catchError(_errorHandler('Test')).whenComplete(_markCallbackComplete);
long line

Also, would you add a note explaining the behavior here? The control flow in
unittest is generally complex. It'd be good to explain a bit more that we have
historically.

https://codereview.chromium.org/416133002/diff/1/pkg/unittest/test/async_exce...
File pkg/unittest/test/async_exception_with_future_test.dart (right):

https://codereview.chromium.org/416133002/diff/1/pkg/unittest/test/async_exce...
pkg/unittest/test/async_exception_with_future_test.dart:19: return new
Completer().future;
Add a note that the goal is to terminate the test as soon as an error occurs,
even when waiting for an async operation.

https://codereview.chromium.org/416133002/diff/1/pkg/unittest/test/async_exce...
pkg/unittest/test/async_exception_with_future_test.dart:21: };
Could you also test that teardown is still called?

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

https://codereview.chromium.org/416133002/diff/1/pkg/unittest/test/utils.dart...
pkg/unittest/test/utils.dart:8: return (new Future.value()).then((_) => fn());
Why not 
  return new Future(() => fn());
?

Could you add a note here explaining the behavior?

[Paul Berry, 2014-07-29 21:54:38]

PTAL

https://codereview.chromium.org/416133002/diff/1/pkg/unittest/lib/src/test_ca...
File pkg/unittest/lib/src/test_case.dart (right):

https://codereview.chromium.org/416133002/diff/1/pkg/unittest/lib/src/test_ca...
pkg/unittest/lib/src/test_case.dart:105:
testReturn.catchError(_errorHandler('Test')).whenComplete(_markCallbackComplete);
On 2014/07/29 19:59:01, kevmoo wrote:
> long line
> 
> Also, would you add a note explaining the behavior here? The control flow in
> unittest is generally complex. It'd be good to explain a bit more that we have
> historically. 

Done.

https://codereview.chromium.org/416133002/diff/1/pkg/unittest/test/async_exce...
File pkg/unittest/test/async_exception_with_future_test.dart (right):

https://codereview.chromium.org/416133002/diff/1/pkg/unittest/test/async_exce...
pkg/unittest/test/async_exception_with_future_test.dart:19: return new
Completer().future;
On 2014/07/29 19:59:02, kevmoo wrote:
> Add a note that the goal is to terminate the test as soon as an error occurs,
> even when waiting for an async operation.

Done.

https://codereview.chromium.org/416133002/diff/1/pkg/unittest/test/async_exce...
pkg/unittest/test/async_exception_with_future_test.dart:21: };
On 2014/07/29 19:59:01, kevmoo wrote:
> Could you also test that teardown is still called?

Done.

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

https://codereview.chromium.org/416133002/diff/1/pkg/unittest/test/utils.dart...
pkg/unittest/test/utils.dart:8: return (new Future.value()).then((_) => fn());
On 2014/07/29 19:59:02, kevmoo wrote:
> Why not 
>   return new Future(() => fn());
> ?
> 
> Could you add a note here explaining the behavior?

In fact, this works too and is even shorter:

  return new Future(fn);

I'm not sure exactly what behavior you wanted explained.  Let me know if what
I've added isn't what you're looking for.

[kevmoo, 2014-07-31 01:19:05]

Thanks much!

Two last things.

1) Update the pubspec to version +4
2) Update the changelog w/ a reference to the fix and the bug. See Polymer for
inspiration - http://pub.dartlang.org/packages/polymer#changelog

[Paul Berry, 2014-07-31 20:41:18]

On 2014/07/31 01:19:05, kevmoo wrote:
> Thanks much!
> 
> Two last things.
> 
> 1) Update the pubspec to version +4
> 2) Update the changelog w/ a reference to the fix and the bug. See Polymer for
> inspiration - http://pub.dartlang.org/packages/polymer#changelog

Ok, I've updated the pubspec and changelog, rebased the CL, and verified that
tests still pass.

[kevmoo, 2014-08-07 22:10:24]

This is great. Thanks, much.

LGTM

[Paul Berry, 2014-08-07 23:29:35]

Committed patchset #3 manually as r39013 (presubmit successful).