Support future matchers and expectAsync in tearDowns.

R=kevmoo@google.com

Review URL: https://codereview.chromium.org//1116443002

CHANGELOG.md

@@ -1,3 +1,8 @@
+### 0.12.0-rc.2
+
+* Allow Future matchers and `expectAsync` to prevent tests'
+  `tearDown`s from completing.
+
 ### 0.12.0-rc.1
 
 * Remove `handleExternalError`. This was never used in practice and its function

lib/src/backend/invoker.dart

@@ -14,6 +14,7 @@ import 'closed_exception.dart';
 import 'live_test.dart';
 import 'live_test_controller.dart';
 import 'metadata.dart';
+import 'outstanding_callback_counter.dart';
 import 'state.dart';
 import 'suite.dart';
 import 'test.dart';
@@ -76,14 +77,13 @@ class Invoker {
   /// The test metadata merged with the suite metadata.
   final Metadata metadata;
 
-  /// Note that this is meaningless once [_onCompleteCompleter] is complete.
-  var _outstandingCallbacks = 0;
-
-  /// The completer to complete once the test body finishes.
-  ///
-  /// This is distinct from [_controller.completer] because a tear-down may need
-  /// to run before the test is truly finished.
-  final _completer = new Completer();
+  /// The outstanding callback counter for the current zone.
+  OutstandingCallbackCounter get _outstandingCallbacks {
+    var counter = Zone.current[this];
+    if (counter != null) return counter;
+    throw new StateError("Can't add or remove outstanding callbacks outside "
+        "of a test body.");
+  }
 
   /// The current invoker, or `null` if none is defined.
   ///
@@ -112,22 +112,13 @@ class Invoker {
   /// Throws a [ClosedException] if this test has been closed.
   void addOutstandingCallback() {
     if (closed) throw new ClosedException();
-    _outstandingCallbacks++;
+    _outstandingCallbacks.addOutstandingCallback();
   }
 
   /// Tells the invoker that a callback declared with [addOutstandingCallback]
   /// is no longer running.
-  void removeOutstandingCallback() {
-    _outstandingCallbacks--;
-
-    if (_outstandingCallbacks != 0) return;
-    if (_completer.isCompleted) return;
-
-    // The test must be passing if we get here, because if there were an error
-    // the completer would already be completed.
-    assert(liveTest.state.result == Result.success);
-    _completer.complete();
-  }
+  void removeOutstandingCallback() =>
+      _outstandingCallbacks.removeOutstandingCallback();
 
   /// Notifies the invoker of an asynchronous error.
   ///
@@ -146,8 +137,7 @@ class Invoker {
     }
 
     _controller.addError(error, stackTrace);
-
-    if (!_completer.isCompleted) _completer.complete();
+    _outstandingCallbacks.removeAllOutstandingCallbacks();
 
     // If a test was marked as success but then had an error, that indicates
     // that it was poorly-written and could be flaky.
@@ -163,11 +153,12 @@ class Invoker {
   void _onRun() {
     _controller.setState(const State(Status.running, Result.success));
 
+    var outstandingCallbacksForBody = new OutstandingCallbackCounter();
+
     Chain.capture(() {
-      runZoned(() {
-        // TODO(nweiz): Make the timeout configurable.
-        // TODO(nweiz): Reset this timer whenever the user's code interacts with
-        // the library.
+      runZonedWithValues(() {
+        // TODO(nweiz): Reset this timer whenever the user's code interacts
+        // with the library.
         var timeout = metadata.timeout.apply(new Duration(seconds: 30));
         var timer = new Timer(timeout, () {
           if (liveTest.isComplete) return;
@@ -176,20 +167,30 @@ class Invoker {
                   "Test timed out after ${niceDuration(timeout)}.", timeout));
         });
 
-        addOutstandingCallback();
-
-        // Run the test asynchronously so that the "running" state change has a
-        // chance to hit its event handler(s) before the test produces an error.
-        // If an error is emitted before the first state change is handled, we
-        // can end up with [onError] callbacks firing before the corresponding
-        // [onStateChange], which violates the timing guarantees.
+        // Run the test asynchronously so that the "running" state change has
+        // a chance to hit its event handler(s) before the test produces an
+        // error. If an error is emitted before the first state change is
+        // handled, we can end up with [onError] callbacks firing before the
+        // corresponding [onStateChange], which violates the timing
+        // guarantees.
         new Future(_test._body)
             .then((_) => removeOutstandingCallback());
 
-        _completer.future.then((_) {
+        _outstandingCallbacks.noOutstandingCallbacks.then((_) {
           if (_test._tearDown == null) return null;
-          return new Future.sync(_test._tearDown);
-        }).catchError(Zone.current.handleUncaughtError).then((_) {
+
+          // Reset the outstanding callback counter to wait for callbacks from
+          // the test's `tearDown` to complete.
+          var outstandingCallbacksForTearDown = new OutstandingCallbackCounter();
+          runZonedWithValues(() {
+            new Future.sync(_test._tearDown)
+              .then((_) => removeOutstandingCallback());
+          }, onError: handleError, zoneValues: {
+            this: outstandingCallbacksForTearDown
+          });
+
+          return outstandingCallbacksForTearDown.noOutstandingCallbacks;
+        }).then((_) {
           timer.cancel();
           _controller.setState(
               new State(Status.complete, liveTest.state.result));
@@ -198,10 +199,14 @@ class Invoker {
           // non-microtask events.
           Timer.run(_controller.completer.complete);
         });
+      }, zoneValues: {
+        #test.invoker: this,
+        // Use the invoker as a key so that multiple invokers can have different
+        // outstanding callback counters at once.
+        this: outstandingCallbacksForBody
       },
           zoneSpecification: new ZoneSpecification(
               print: (self, parent, zone, line) => _controller.print(line)),
-          zoneValues: {#test.invoker: this},
           onError: handleError);
     });
   }

lib/src/backend/outstanding_callback_counter.dart

@@ -0,0 +1,42 @@
+// Copyright (c) 2015, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+library test.backend.outstanding_callback_counter;
+
+import 'dart:async';
+
+/// A class that counts outstanding callbacks for a test and fires a future once
+/// they reach zero.
+///
+/// The outstanding callback count automatically starts at 1.
+class OutstandingCallbackCounter {
+  /// The number of outstanding callbacks.
+  var _count = 1;
+
+  /// A future that fires when the oustanding callback count reaches 0.
+  Future get noOutstandingCallbacks => _completer.future;
+  final _completer = new Completer();
+
+  /// Adds an outstanding callback.
+  void addOutstandingCallback() {
+    _count++;
+  }
+
+  /// Removes an outstanding callback.
+  void removeOutstandingCallback() {
+    _count--;
+    if (_count != 0) return;
+    if (_completer.isCompleted) return;
+    _completer.complete();
+  }
+
+  /// Removes all outstanding callbacks, forcing [noOutstandingCallbacks] to
+  /// fire.
+  ///
+  /// Future calls to [addOutstandingCallback] and [removeOutstandingCallback]
+  /// will be ignored.
+  void removeAllOutstandingCallbacks() {
+    if (!_completer.isCompleted) _completer.complete();
+  }
+}

lib/src/utils.dart

@@ -139,6 +139,16 @@ StreamSink mapSink(StreamSink original, fn(event)) {
   return controller.sink;
 }
 
+/// Like [runZoned], but [zoneValues] are set for the callbacks in
+/// [zoneSpecification] and [onError].
+runZonedWithValues(Function body(), {Map zoneValues,
+    ZoneSpecification zoneSpecification, Function onError}) {
+  return runZoned(() {
+    return runZoned(body,
+        zoneSpecification: zoneSpecification, onError: onError);
+  }, zoneValues: zoneValues);
+}
+
 /// Truncates [text] to fit within [maxLength].
 ///
 /// This will try to truncate along word boundaries and preserve words both at

pubspec.yaml

@@ -1,5 +1,5 @@
 name: test
-version: 0.12.0-rc.1
+version: 0.12.0-dev
 author: Dart Team <misc@dartlang.org>
 description: A library for writing dart unit tests.
 homepage: https://github.com/dart-lang/test

test/backend/invoker_test.dart

@@ -429,6 +429,55 @@ void main() {
             equals(const State(Status.complete, Result.error)));
       });
     });
+
+    test("an asynchronous error in tearDown causes the test to error", () {
+      var liveTest = _localTest(() {}, tearDown: () {
+        Invoker.current.addOutstandingCallback();
+        new Future(() => throw "oh no");
+      }).load(suite);
+
+      expectSingleError(liveTest);
+      return liveTest.run();
+    });
+
+    test("an error reported in the test body after tearDown begins running "
+        "doesn't stop tearDown", () {
+      var tearDownComplete = false;;
+      var completer = new Completer();
+
+      var liveTest;
+      liveTest = _localTest(() {
+        completer.future.then((_) => throw "not again");
+        throw "oh no";
+      }, tearDown: () {
+        completer.complete();
+
+        // Pump the event queue so that we will run the following code after the
+        // test body has thrown a second error.
+        Invoker.current.addOutstandingCallback();
+        pumpEventQueue().then((_) {
+          Invoker.current.removeOutstandingCallback();
+          tearDownComplete = true;
+        });
+      }).load(suite);
+
+      expectStates(liveTest, [
+        const State(Status.running, Result.success),
+        const State(Status.complete, Result.error)
+      ]);
+
+      expectErrors(liveTest, [
+        (error) {
+          expect(lastState.status, equals(Status.complete));
+          expect(error, equals("oh no"));
+        },
+        (error) => expect(error, equals("not again"))
+      ]);
+
+      return liveTest.run().then((_) {
+        expect(tearDownComplete, isTrue);
+      });
+    });
   });
 
   test("a test doesn't complete until there are no outstanding callbacks",
@@ -473,6 +522,56 @@ void main() {
     });
   });
 
+  test("a test's tearDown doesn't complete until there are no outstanding "
+      "callbacks", () {
+    var outstandingCallbackRemoved = false;
+    var liveTest = _localTest(() {}, tearDown: () {
+      Invoker.current.addOutstandingCallback();
+
+      // Pump the event queue to make sure the test isn't coincidentally
+      // completing after the outstanding callback is removed.
+      pumpEventQueue().then((_) {
+        outstandingCallbackRemoved = true;
+        Invoker.current.removeOutstandingCallback();
+      });
+    }).load(suite);
+
+    liveTest.onError.listen(expectAsync((_) {}, count: 0));
+
+    return liveTest.run().then((_) {
+      expect(outstandingCallbackRemoved, isTrue);
+    });
+  });
+
+  test("a test body's outstanding callbacks can't complete its tearDown", () {
+    var outstandingCallbackRemoved = false;
+    var completer = new Completer();
+    var liveTest = _localTest(() {
+      // Once the tearDown runs, remove an outstanding callback to see if it
+      // causes the tearDown to complete.
+      completer.future.then((_) {
+        Invoker.current.removeOutstandingCallback();
+      });
+    }, tearDown: () {
+      Invoker.current.addOutstandingCallback();
+
+      // This will cause the test BODY to remove an outstanding callback, which
+      // shouldn't cause the test to complete.
+      completer.complete();
+
+      pumpEventQueue().then((_) {
+        outstandingCallbackRemoved = true;
+        Invoker.current.removeOutstandingCallback();
+      });
+    }).load(suite);
+
+    liveTest.onError.listen(expectAsync((_) {}, count: 0));
+
+    return liveTest.run().then((_) {
+      expect(outstandingCallbackRemoved, isTrue);
+    });
+  });
+
   test("a test's prints are captured and reported", () {
     expect(() {
       var liveTest = _localTest(() {