Ensure that the test state transitions to complete before the test is considered complete. (#1028)

In some environments the state change events and the "test complete" event are sent over a stream that does not guarantee ordering (e.g. as independent HTTP requests when using package:sse).
Without this change, such tests will fail intermittently, when the "test complete" event happens to be delivered before the state change event.

pkgs/test_api/CHANGELOG.md

@@ -2,6 +2,8 @@
 
 * Don't swallow exceptions from callbacks in `expectAsync*`.
 * Internal cleanup - fix lints.
+* Fixed a race condition that caused tests to occasionally fail during
+  `tearDownAll` with the message `(tearDownAll) - did not complete [E]`.
 
 ## 0.2.5
 

pkgs/test_api/lib/src/backend/live_test_controller.dart

@@ -35,11 +35,11 @@ class _LiveTest extends LiveTest {
 
   Stream<Message> get onMessage => _controller._onMessageController.stream;
 
-  Future get onComplete => _controller.completer.future;
+  Future<void> get onComplete => _controller.onComplete;
 
-  Future run() => _controller._run();
+  Future<void> run() => _controller._run();
 
-  Future close() => _controller._close();
+  Future<void> close() => _controller._close();
 
   _LiveTest(this._controller);
 }
@@ -101,7 +101,7 @@ class LiveTestController {
   final _onMessageController = StreamController<Message>.broadcast(sync: true);
 
   /// The completer for [LiveTest.onComplete];
-  final completer = Completer();
+  final completer = Completer<void>();
 
   /// Whether [run] has been called.
   var _runCalled = false;
@@ -109,6 +109,8 @@ class LiveTestController {
   /// Whether [close] has been called.
   bool get _isClosed => _onErrorController.isClosed;
 
+  final _stateChangedToComplete = Completer<void>();
+
   /// Creates a new controller for a [LiveTest].
   ///
   /// [test] is the test being run; [suite] is the suite that contains it.
@@ -158,6 +160,11 @@ class LiveTestController {
 
     _state = newState;
     _onStateChangeController.add(newState);
+
+    if (newState.status == Status.complete &&
+        !_stateChangedToComplete.isCompleted) {
+      _stateChangedToComplete.complete();
+    }
   }
 
   /// Emits message over [LiveTest.onMessage].
@@ -173,7 +180,7 @@ class LiveTestController {
 
   /// A wrapper for [_onRun] that ensures that it follows the guarantees for
   /// [LiveTest.run].
-  Future _run() {
+  Future<void> _run() {
     if (_runCalled) {
       throw StateError("LiveTest.run() may not be called more than once.");
     } else if (_isClosed) {
@@ -186,9 +193,15 @@ class LiveTestController {
     return liveTest.onComplete;
   }
 
+  /// Returns a future that completes when the test is complete.
+  ///
+  /// We also wait for the state to transition to Status.complete.
+  Future<void> get onComplete =>
+      Future.wait([completer.future, _stateChangedToComplete.future]);
+
   /// A wrapper for [_onClose] that ensures that all controllers are closed.
-  Future _close() {
-    if (_isClosed) return completer.future;
+  Future<void> _close() {
+    if (_isClosed) return onComplete;
 
     _onStateChangeController.close();
     _onErrorController.close();
@@ -199,6 +212,10 @@ class LiveTestController {
       completer.complete();
     }
 
-    return completer.future;
+    if (!_stateChangedToComplete.isCompleted) {
+      _stateChangedToComplete.complete();
+    }
+
+    return onComplete;
   }
 }