diff --git a/CHANGELOG.md b/CHANGELOG.md index 14c532be5..f57697dd5 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/lib/src/backend/invoker.dart b/lib/src/backend/invoker.dart index 81a931e4e..a0151c385 100644 --- a/lib/src/backend/invoker.dart +++ b/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); }); } diff --git a/lib/src/backend/outstanding_callback_counter.dart b/lib/src/backend/outstanding_callback_counter.dart new file mode 100644 index 000000000..ba4ba42c7 --- /dev/null +++ b/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(); + } +} diff --git a/lib/src/utils.dart b/lib/src/utils.dart index f236814aa..9b6fd099d 100644 --- a/lib/src/utils.dart +++ b/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 diff --git a/pubspec.yaml b/pubspec.yaml index 3e7ab7963..6c960024c 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -1,5 +1,5 @@ name: test -version: 0.12.0-rc.1 +version: 0.12.0-dev author: Dart Team description: A library for writing dart unit tests. homepage: https://github.com/dart-lang/test diff --git a/test/backend/invoker_test.dart b/test/backend/invoker_test.dart index be27391e8..58fdfa09b 100644 --- a/test/backend/invoker_test.dart +++ b/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(() {