Inline some zone interactions to a single function (#1481)
Combine the behavior of `Invoker.unclosable`, and `errorsDontStopTest`
into a single method since they were previously used in two places to
wrap the same intent.
Add a `runTearDowns` method on `Invoker`. Use it from `Declarer` where
it is running `tearDownAll` callbacks, and from `Invoker` where it is
running a test's `tearDown` callbacks. This is the only place (outside of
tests internal to this repo) that the `waitForOutstandignCallbacks` zone
will be nested and the only place where an `Invoker` is not closable. The
implementation is intended to provide the following properties:
- The outer "test", which may be either a `test` or a `tearDownAll`,
will not be considered complete until all outstanding callbacks from
all tear down callbacks are also complete. This is the reason for a
call to `addOutstandingCallback` in `runTearDowns` which doesn't get
removed until all outstanding callbacks from the tear down are also
complete.
- Tear downs do not need to wait for all outstanding callbacks from
previous tear downs before they can start. They do wait for the
`Future` returned from prior tear downs. This is the reason that the
loop waits for the `Completer` which is completed following the tear
down callback, and the remainder of the `waitForOutstandingCallbacks`
work is `unawaited`.
- If an error is thrown within a tear down callback, only that tear
down's outstanding callbacks will be ignored - other tear down
callbacks will continue to hold the test open as normal and the test
will not be considered complete the way it would be if the error had come
from the test body. This is the reason for the zone nesting, an error
will make it seem as though outstanding callbacks completed in only
the zone where the error is raised.
- Give a verbose name and a longer doc comment to the `closable`
fields. Documents clearly the single intended purpose it has today, and
should prevent it from being accidentally given an extra purpose without
updating the doc.
- Remove the future that never completes part. As far as I can tell, it
isn't an exercised code path today, and if a usage somehow cropped up
it's very unlikely that a non-completing future would be the right
behavior.
Make `waitForOutstandingCallbacks` private and rewrite the tests to call
`runTearDowns`.
diff --git a/pkgs/test_api/lib/src/backend/declarer.dart b/pkgs/test_api/lib/src/backend/declarer.dart
index 9211776..d327699 100644
--- a/pkgs/test_api/lib/src/backend/declarer.dart
+++ b/pkgs/test_api/lib/src/backend/declarer.dart
@@ -8,7 +8,6 @@
import 'package:stack_trace/stack_trace.dart';
import '../frontend/timeout.dart';
-import '../util/test.dart';
import 'group.dart';
import 'group_entry.dart';
import 'invoker.dart';
@@ -352,13 +351,7 @@
return LocalTest(
_prefix('(tearDownAll)'), _metadata.change(timeout: _timeout), () {
- return runZoned(() {
- return Invoker.current!.unclosable(() async {
- while (_tearDownAlls.isNotEmpty) {
- await errorsDontStopTest(_tearDownAlls.removeLast());
- }
- });
- },
+ return runZoned(() => Invoker.current!.runTearDowns(_tearDownAlls),
// Make the declarer visible to running scaffolds so they can add to
// the declarer's `tearDownAll()` list.
zoneValues: {#test.declarer: this});
diff --git a/pkgs/test_api/lib/src/backend/invoker.dart b/pkgs/test_api/lib/src/backend/invoker.dart
index a1c64b6..784e160 100644
--- a/pkgs/test_api/lib/src/backend/invoker.dart
+++ b/pkgs/test_api/lib/src/backend/invoker.dart
@@ -7,7 +7,6 @@
import 'package:stack_trace/stack_trace.dart';
import '../frontend/expect.dart';
-import '../util/test.dart';
import '../util/pretty_print.dart';
import 'closed_exception.dart';
import 'declarer.dart';
@@ -84,29 +83,34 @@
/// Whether to run this test in its own error zone.
final bool _guarded;
- /// Whether the test can be closed in the current zone.
- bool get _closable => Zone.current[_closableKey] as bool;
+ /// Whether the user code is allowed to interact with the invoker despite it
+ /// being closed.
+ ///
+ /// A test is generally closed because the runner is shutting down (in
+ /// response to a signal) or because the test's suite is finished.
+ /// Typically calls to [addTearDown] and [addOutstandingCallback] are only
+ /// allowed before the test is closed. Tear down callbacks, however, are
+ /// allowed to perform these interactions to facilitate resource cleanup on a
+ /// best-effort basis, so the invoker is made to appear open only within the
+ /// zones running the teardown callbacks.
+ bool get _forceOpen => Zone.current[_forceOpenForTearDownKey] as bool;
/// An opaque object used as a key in the zone value map to identify
- /// [_closable].
+ /// [_forceOpen].
///
/// This is an instance variable to ensure that multiple invokers don't step
/// on one anothers' toes.
- final _closableKey = Object();
+ final _forceOpenForTearDownKey = Object();
/// Whether the test has been closed.
///
/// Once the test is closed, [expect] and [expectAsync] will throw
/// [ClosedException]s whenever accessed to help the test stop executing as
/// soon as possible.
- bool get closed => _closable && _onCloseCompleter.isCompleted;
+ bool get closed => !_forceOpen && _onCloseCompleter.isCompleted;
/// A future that completes once the test has been closed.
- Future<void> get onClose => _closable
- ? _onCloseCompleter.future
- // If we're in an unclosable block, return a future that will never
- // complete.
- : Completer<void>().future;
+ Future<void> get onClose => _onCloseCompleter.future;
final _onCloseCompleter = Completer<void>();
/// The test being run.
@@ -120,7 +124,7 @@
'of a test body.');
}
- /// All the zones created by [waitForOutstandingCallbacks], in the order they
+ /// All the zones created by [_waitForOutstandingCallbacks], in the order they
/// were created.
///
/// This is used to throw timeout errors in the most recent zone.
@@ -215,12 +219,34 @@
_outstandingCallbacks.decrement();
}
+ /// Run [tearDowns] in reverse order.
+ ///
+ /// An exception thrown in a tearDown callback will cause the test to fail, if
+ /// it isn't already failing, but it won't prevent the remaining callbacks
+ /// from running. This invoker will not be closeable within the zone that the
+ /// teardowns are running in.
+ Future<void> runTearDowns(List<FutureOr<void> Function()> tearDowns) {
+ heartbeat();
+ return runZoned(() async {
+ while (tearDowns.isNotEmpty) {
+ var completer = Completer();
+
+ addOutstandingCallback();
+ _waitForOutstandingCallbacks(() {
+ Future.sync(tearDowns.removeLast()).whenComplete(completer.complete);
+ }).then((_) => removeOutstandingCallback()).unawaited;
+
+ await completer.future;
+ }
+ }, zoneValues: {_forceOpenForTearDownKey: true});
+ }
+
/// Runs [fn] and completes once [fn] and all outstanding callbacks registered
/// within [fn] have completed.
///
/// Outstanding callbacks registered within [fn] will *not* be registered as
/// outstanding callback outside of [fn].
- Future<void> waitForOutstandingCallbacks(FutureOr<void> Function() fn) {
+ Future<void> _waitForOutstandingCallbacks(FutureOr<void> Function() fn) {
heartbeat();
Zone? zone;
@@ -237,17 +263,6 @@
});
}
- /// Runs [fn] in a zone where [closed] is always `false`.
- ///
- /// This is useful for running code that should be able to register callbacks
- /// and interact with the test framework normally even when the invoker is
- /// closed, for example cleanup code.
- T unclosable<T>(T Function() fn) {
- heartbeat();
-
- return runZoned(fn, zoneValues: {_closableKey: false});
- }
-
/// Notifies the invoker that progress is being made.
///
/// Each heartbeat resets the timeout timer. This helps ensure that
@@ -379,8 +394,8 @@
// Use the event loop over the microtask queue to avoid starvation.
await Future(() {});
- await waitForOutstandingCallbacks(_test._body);
- await waitForOutstandingCallbacks(() => unclosable(_runTearDowns));
+ await _waitForOutstandingCallbacks(_test._body);
+ await _waitForOutstandingCallbacks(() => runTearDowns(_tearDowns));
if (_timeoutTimer != null) _timeoutTimer!.cancel();
@@ -397,7 +412,7 @@
},
zoneValues: {
#test.invoker: this,
- _closableKey: true,
+ _forceOpenForTearDownKey: false,
#runCount: _runCount,
},
zoneSpecification:
@@ -417,13 +432,6 @@
/// Prints [text] as a message to [_controller].
void _print(String text) => _controller.message(Message.print(text));
-
- /// Run [_tearDowns] in reverse order.
- Future<void> _runTearDowns() async {
- while (_tearDowns.isNotEmpty) {
- await errorsDontStopTest(_tearDowns.removeLast());
- }
- }
}
/// A manually incremented/decremented counter that completes a [Future] the
@@ -453,3 +461,7 @@
if (!_completer.isCompleted) _completer.complete();
}
}
+
+extension<T> on Future<T> {
+ void get unawaited {}
+}
diff --git a/pkgs/test_api/lib/src/util/test.dart b/pkgs/test_api/lib/src/util/test.dart
deleted file mode 100644
index 8e051d6..0000000
--- a/pkgs/test_api/lib/src/util/test.dart
+++ /dev/null
@@ -1,25 +0,0 @@
-// Copyright (c) 2018, 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.
-
-import 'dart:async';
-
-import '../backend/invoker.dart';
-
-/// Runs [body] with special error-handling behavior.
-///
-/// Errors emitted [body] will still cause the current test to fail, but they
-/// won't cause it to *stop*. In particular, they won't remove any outstanding
-/// callbacks registered outside of [body].
-///
-/// This may only be called within a test.
-Future errorsDontStopTest(dynamic Function() body) {
- var completer = Completer();
-
- Invoker.current!.addOutstandingCallback();
- Invoker.current!.waitForOutstandingCallbacks(() {
- Future.sync(body).whenComplete(completer.complete);
- }).then((_) => Invoker.current!.removeOutstandingCallback());
-
- return completer.future;
-}
diff --git a/pkgs/test_api/test/backend/invoker_test.dart b/pkgs/test_api/test/backend/invoker_test.dart
index cf645e2..d4175dc 100644
--- a/pkgs/test_api/test/backend/invoker_test.dart
+++ b/pkgs/test_api/test/backend/invoker_test.dart
@@ -449,49 +449,67 @@
});
});
- group('waitForOutstandingCallbacks:', () {
- test('waits for the wrapped function to complete', () async {
- var functionCompleted = false;
- await Invoker.current!.waitForOutstandingCallbacks(() async {
- await pumpEventQueue();
- functionCompleted = true;
- });
-
- expect(functionCompleted, isTrue);
+ group('runTearDowns', () {
+ test('runs multiple tear downs', () async {
+ var firstTearDownStarted = false;
+ var secondTearDownStarted = false;
+ await Invoker.current!.runTearDowns([
+ () {
+ firstTearDownStarted = true;
+ },
+ () {
+ secondTearDownStarted = true;
+ }
+ ]);
+ expect(secondTearDownStarted, isTrue);
+ expect(firstTearDownStarted, isTrue);
});
- test('waits for registered callbacks in the wrapped function to run',
+ test('waits for the future returned tear downs to complete', () async {
+ var firstTearDownWork = Completer<void>();
+ var secondTearDownStarted = false;
+ var result = Invoker.current!.runTearDowns([
+ () {
+ secondTearDownStarted = true;
+ },
() async {
- var callbackRun = false;
- await Invoker.current!.waitForOutstandingCallbacks(() {
- pumpEventQueue().then(expectAsync1((_) {
- callbackRun = true;
- }));
- });
-
- expect(callbackRun, isTrue);
+ await firstTearDownWork.future;
+ },
+ ]);
+ await pumpEventQueue();
+ expect(secondTearDownStarted, isFalse);
+ firstTearDownWork.complete();
+ await result;
+ expect(secondTearDownStarted, isTrue);
});
- test("doesn't automatically block the enclosing context", () async {
- var innerFunctionCompleted = false;
- await Invoker.current!.waitForOutstandingCallbacks(() {
- Invoker.current!.waitForOutstandingCallbacks(() async {
- await pumpEventQueue();
- innerFunctionCompleted = true;
- });
- });
-
- expect(innerFunctionCompleted, isFalse);
+ test('allows next tear down to run while there are still prior callbacks',
+ () async {
+ var firstTearDownAsyncWork = Completer<void>();
+ var secondTearDownStarted = false;
+ unawaited(Invoker.current!.runTearDowns([
+ () {
+ secondTearDownStarted = true;
+ },
+ () {
+ Invoker.current!.addOutstandingCallback();
+ firstTearDownAsyncWork.future
+ .whenComplete(Invoker.current!.removeOutstandingCallback);
+ },
+ ]));
+ await pumpEventQueue();
+ expect(secondTearDownStarted, isTrue);
+ firstTearDownAsyncWork.complete();
});
- test(
- "forwards errors to the enclosing test but doesn't remove its "
- 'outstanding callbacks', () async {
+ test('forwards errors to the enclosing test but does not end it', () async {
var liveTest = _localTest(() async {
Invoker.current!.addOutstandingCallback();
- await Invoker.current!.waitForOutstandingCallbacks(() {
- throw 'oh no';
- });
+ await Invoker.current!.runTearDowns([
+ () {
+ throw 'oh no';
+ }
+ ]);
}).load(suite);
expectStates(liveTest, [
