From e4c22477809ab93828834c1967c624f98c2dd707 Mon Sep 17 00:00:00 2001
From: Natalie Weizenbaum <nweiz@google.com>
Date: Tue, 20 Dec 2016 15:34:24 -0800
Subject: [PATCH] Add an addTearDown() function. (#510)

See #109
Closes #93
---
 CHANGELOG.md                          |   3 +
 lib/src/backend/declarer.dart         |  52 ++---
 lib/src/backend/invoker.dart          |  22 +-
 lib/src/utils.dart                    |  19 ++
 lib/test.dart                         |  19 ++
 test/frontend/add_tear_down_test.dart | 325 ++++++++++++++++++++++++++
 test/utils.dart                       |   3 +
 7 files changed, 402 insertions(+), 41 deletions(-)
 create mode 100644 test/frontend/add_tear_down_test.dart

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 043cb0b2..5e9ebe0a 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,5 +1,8 @@
 ## 0.12.18
 
+* Add an `addTearDown()` function, which allows tests to register additional
+  tear-down callbacks as they're running.
+
 * Add the `spawnHybridUri()` and `spawnHybridCode()` functions, which allow
   browser tests to run code on the VM.
 
diff --git a/lib/src/backend/declarer.dart b/lib/src/backend/declarer.dart
index 5377c51e..7d61edfe 100644
--- a/lib/src/backend/declarer.dart
+++ b/lib/src/backend/declarer.dart
@@ -105,15 +105,24 @@ class Declarer {
         tags: tags));
 
     _entries.add(new LocalTest(_prefix(name), metadata, () async {
-      // TODO(nweiz): It might be useful to throw an error here if a test starts
-      // running while other tests from the same declarer are also running,
-      // since they might share closurized state.
+      var parents = <Declarer>[];
+      for (var declarer = this; declarer != null; declarer = declarer._parent) {
+        parents.add(declarer);
+      }
+
+      // Register all tear-down functions in all declarers. Iterate through
+      // parents outside-in so that the Invoker gets the functions in the order
+      // they were declared in source.
+      for (var declarer in parents.reversed) {
+        for (var tearDown in declarer._tearDowns) {
+          Invoker.current.addTearDown(tearDown);
+        }
+      }
 
       await Invoker.current.waitForOutstandingCallbacks(() async {
         await _runSetUps();
         await body();
       });
-      await _runTearDowns();
     }, trace: _collectTraces ? new Trace.current(2) : null));
   }
 
@@ -197,23 +206,6 @@ class Declarer {
     await Future.forEach(_setUps, (setUp) => setUp());
   }
 
-  /// Run the tear-up functions for this and any parent groups.
-  ///
-  /// If no set-up functions are declared, this returns a [Future] that
-  /// completes immediately.
-  ///
-  /// This should only be called within a test.
-  Future _runTearDowns() {
-    return Invoker.current.unclosable(() {
-      var tearDowns = [];
-      for (var declarer = this; declarer != null; declarer = declarer._parent) {
-        tearDowns.addAll(declarer._tearDowns.reversed);
-      }
-
-      return Future.forEach(tearDowns, _errorsDontStopTest);
-    });
-  }
-
   /// Returns a [Test] that runs the callbacks in [_setUpAll].
   Test get _setUpAll {
     if (_setUpAlls.isEmpty) return null;
@@ -229,24 +221,8 @@ class Declarer {
 
     return new LocalTest(_prefix("(tearDownAll)"), _metadata, () {
       return Invoker.current.unclosable(() {
-        return Future.forEach(_tearDownAlls.reversed, _errorsDontStopTest);
+        return Future.forEach(_tearDownAlls.reversed, errorsDontStopTest);
       });
     }, trace: _tearDownAllTrace);
   }
-
-  /// 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].
-  Future _errorsDontStopTest(body()) {
-    var completer = new Completer();
-
-    Invoker.current.addOutstandingCallback();
-    Invoker.current.waitForOutstandingCallbacks(() {
-      new Future.sync(body).whenComplete(completer.complete);
-    }).then((_) => Invoker.current.removeOutstandingCallback());
-
-    return completer.future;
-  }
 }
diff --git a/lib/src/backend/invoker.dart b/lib/src/backend/invoker.dart
index 9d2205e9..c5fcee67 100644
--- a/lib/src/backend/invoker.dart
+++ b/lib/src/backend/invoker.dart
@@ -126,11 +126,23 @@ class Invoker {
   /// This will be `null` until the test starts running.
   Timer _timeoutTimer;
 
+  /// The tear-down functions to run when this test finishes.
+  final _tearDowns = new List<AsyncFunction>();
+
   Invoker._(Suite suite, LocalTest test, {Iterable<Group> groups}) {
     _controller = new LiveTestController(
         suite, test, _onRun, _onCloseCompleter.complete, groups: groups);
   }
 
+  /// Runs [callback] after this test completes.
+  ///
+  /// The [callback] may return a [Future]. Like all tear-downs, callbacks are
+  /// run in the reverse of the order they're declared.
+  void addTearDown(callback()) {
+    if (closed) throw new ClosedException();
+    _tearDowns.add(callback);
+  }
+
   /// Tells the invoker that there's a callback running that it should wait for
   /// before considering the test successful.
   ///
@@ -301,13 +313,17 @@ class Invoker {
         // 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
+        // corresponding [onStateChkange], which violates the timing
         // guarantees.
         //
         // Using [new Future] also avoids starving the DOM or other
         // microtask-level events.
-        new Future(_test._body)
-            .then((_) => removeOutstandingCallback());
+        new Future(() async {
+          await _test._body();
+          await unclosable(() =>
+              Future.forEach(_tearDowns.reversed, errorsDontStopTest));
+          removeOutstandingCallback();
+        });
 
         await _outstandingCallbacks.noOutstandingCallbacks;
         if (_timeoutTimer != null) _timeoutTimer.cancel();
diff --git a/lib/src/utils.dart b/lib/src/utils.dart
index dbf9e635..eb67a9c9 100644
--- a/lib/src/utils.dart
+++ b/lib/src/utils.dart
@@ -11,6 +11,7 @@ import 'package:async/async.dart' hide StreamQueue;
 import 'package:path/path.dart' as p;
 import 'package:stack_trace/stack_trace.dart';
 
+import 'backend/invoker.dart';
 import 'backend/operating_system.dart';
 import 'util/stream_queue.dart';
 
@@ -347,6 +348,24 @@ void invoke(fn()) {
   fn();
 }
 
+/// 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(body()) {
+  var completer = new Completer();
+
+  Invoker.current.addOutstandingCallback();
+  Invoker.current.waitForOutstandingCallbacks(() {
+    new Future.sync(body).whenComplete(completer.complete);
+  }).then((_) => Invoker.current.removeOutstandingCallback());
+
+  return completer.future;
+}
+
 /// Returns a random base64 string containing [bytes] bytes of data.
 ///
 /// [seed] is passed to [math.Random].
diff --git a/lib/test.dart b/lib/test.dart
index c2040bd9..5985f7b8 100644
--- a/lib/test.dart
+++ b/lib/test.dart
@@ -7,6 +7,7 @@ import 'dart:async';
 import 'package:path/path.dart' as p;
 
 import 'src/backend/declarer.dart';
+import 'src/backend/invoker.dart';
 import 'src/backend/test_platform.dart';
 import 'src/frontend/timeout.dart';
 import 'src/runner/configuration/suite.dart';
@@ -228,8 +229,26 @@ void setUp(callback()) => _declarer.setUp(callback);
 ///
 /// Each callback at the top level or in a given group will be run in the
 /// reverse of the order they were declared.
+///
+/// See also [addTearDown], which adds tear-downs to a running test.
 void tearDown(callback()) => _declarer.tearDown(callback);
 
+/// Registers a function to be run after the current test.
+///
+/// This is called within a running test, and adds a tear-down only for the
+/// current test. It allows testing libraries to add cleanup logic as soon as
+/// there's something to clean up.
+///
+/// The [callback] is run before any callbacks registered with [tearDown]. Like
+/// [tearDown], the most recently registered callback is run first.
+void addTearDown(callback()) {
+  if (Invoker.current == null) {
+    throw new StateError("addTearDown() may only be called within a test.");
+  }
+
+  Invoker.current.addTearDown(callback);
+}
+
 /// Registers a function to be run once before all tests.
 ///
 /// [callback] may be asynchronous; if so, it must return a [Future].
diff --git a/test/frontend/add_tear_down_test.dart b/test/frontend/add_tear_down_test.dart
new file mode 100644
index 00000000..83b2e9ce
--- /dev/null
+++ b/test/frontend/add_tear_down_test.dart
@@ -0,0 +1,325 @@
+// Copyright (c) 2016, 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 'package:async/async.dart';
+import 'package:test/test.dart';
+
+import '../utils.dart';
+
+void main() {
+  test("runs after the test body", () {
+    return expectTestsPass(() {
+      var test1Run = false;
+      var tearDownRun = false;
+      test("test 1", () {
+        addTearDown(() {
+          expect(test1Run, isTrue);
+          expect(tearDownRun, isFalse);
+          tearDownRun = true;
+        });
+
+        expect(tearDownRun, isFalse);
+        test1Run = true;
+      });
+
+      test("test 2", () {
+        expect(tearDownRun, isTrue);
+      });
+    });
+  });
+
+  test("multiples run in reverse order", () {
+    return expectTestsPass(() {
+      var tearDown1Run = false;
+      var tearDown2Run = false;
+      var tearDown3Run = false;
+
+      test("test 1", () {
+        addTearDown(() {
+          expect(tearDown1Run, isFalse);
+          expect(tearDown2Run, isTrue);
+          expect(tearDown3Run, isTrue);
+          tearDown1Run = true;
+        });
+
+        addTearDown(() {
+          expect(tearDown1Run, isFalse);
+          expect(tearDown2Run, isFalse);
+          expect(tearDown3Run, isTrue);
+          tearDown2Run = true;
+        });
+
+        addTearDown(() {
+          expect(tearDown1Run, isFalse);
+          expect(tearDown2Run, isFalse);
+          expect(tearDown3Run, isFalse);
+          tearDown3Run = true;
+        });
+
+        expect(tearDown1Run, isFalse);
+        expect(tearDown2Run, isFalse);
+        expect(tearDown3Run, isFalse);
+      });
+
+      test("test 2", () {
+        expect(tearDown1Run, isTrue);
+        expect(tearDown2Run, isTrue);
+        expect(tearDown3Run, isTrue);
+      });
+    });
+  });
+
+  test("runs before a normal tearDown", () {
+    return expectTestsPass(() {
+      var groupTearDownRun = false;
+      var testTearDownRun = false;
+      group("group", () {
+        tearDown(() {
+          expect(testTearDownRun, isTrue);
+          expect(groupTearDownRun, isFalse);
+          groupTearDownRun = true;
+        });
+
+        test("test 1", () {
+          addTearDown(() {
+            expect(groupTearDownRun, isFalse);
+            expect(testTearDownRun, isFalse);
+            testTearDownRun = true;
+          });
+
+          expect(groupTearDownRun, isFalse);
+          expect(testTearDownRun, isFalse);
+        });
+      });
+
+      test("test 2", () {
+        expect(groupTearDownRun, isTrue);
+        expect(testTearDownRun, isTrue);
+      });
+    });
+  });
+
+  group("asynchronously", () {
+    test("blocks additional test tearDowns on in-band async", () {
+      return expectTestsPass(() {
+        var tearDown1Run = false;
+        var tearDown2Run = false;
+        var tearDown3Run = false;
+        test("test", () {
+          addTearDown(() async {
+            expect(tearDown1Run, isFalse);
+            expect(tearDown2Run, isTrue);
+            expect(tearDown3Run, isTrue);
+            await pumpEventQueue();
+            tearDown1Run = true;
+          });
+
+          addTearDown(() async {
+            expect(tearDown1Run, isFalse);
+            expect(tearDown2Run, isFalse);
+            expect(tearDown3Run, isTrue);
+            await pumpEventQueue();
+            tearDown2Run = true;
+          });
+
+          addTearDown(() async {
+            expect(tearDown1Run, isFalse);
+            expect(tearDown2Run, isFalse);
+            expect(tearDown3Run, isFalse);
+            await pumpEventQueue();
+            tearDown3Run = true;
+          });
+
+          expect(tearDown1Run, isFalse);
+          expect(tearDown2Run, isFalse);
+          expect(tearDown3Run, isFalse);
+        });
+      });
+    });
+
+    test("doesn't block additional test tearDowns on out-of-band async", () {
+      return expectTestsPass(() {
+        var tearDown1Run = false;
+        var tearDown2Run = false;
+        var tearDown3Run = false;
+        test("test", () {
+          addTearDown(() {
+            expect(tearDown1Run, isFalse);
+            expect(tearDown2Run, isFalse);
+            expect(tearDown3Run, isFalse);
+
+            expect(new Future(() {
+              tearDown1Run = true;
+            }), completes);
+          });
+
+          addTearDown(() {
+            expect(tearDown1Run, isFalse);
+            expect(tearDown2Run, isFalse);
+            expect(tearDown3Run, isFalse);
+
+            expect(new Future(() {
+              tearDown2Run = true;
+            }), completes);
+          });
+
+          addTearDown(() {
+            expect(tearDown1Run, isFalse);
+            expect(tearDown2Run, isFalse);
+            expect(tearDown3Run, isFalse);
+
+            expect(new Future(() {
+              tearDown3Run = true;
+            }), completes);
+          });
+
+          expect(tearDown1Run, isFalse);
+          expect(tearDown2Run, isFalse);
+          expect(tearDown3Run, isFalse);
+        });
+      });
+    });
+
+    test("blocks additional group tearDowns on in-band async", () {
+      return expectTestsPass(() {
+        var groupTearDownRun = false;
+        var testTearDownRun = false;
+        tearDown(() async {
+          expect(groupTearDownRun, isFalse);
+          expect(testTearDownRun, isTrue);
+          await pumpEventQueue();
+          groupTearDownRun = true;
+        });
+
+        test("test", () {
+          addTearDown(() async {
+            expect(groupTearDownRun, isFalse);
+            expect(testTearDownRun, isFalse);
+            await pumpEventQueue();
+            testTearDownRun = true;
+          });
+
+          expect(groupTearDownRun, isFalse);
+          expect(testTearDownRun, isFalse);
+        });
+      });
+    });
+
+    test("doesn't block additional group tearDowns on out-of-band async", () {
+      return expectTestsPass(() {
+        var groupTearDownRun = false;
+        var testTearDownRun = false;
+        tearDown(() {
+          expect(groupTearDownRun, isFalse);
+          expect(testTearDownRun, isFalse);
+
+          expect(new Future(() {
+            groupTearDownRun = true;
+          }), completes);
+        });
+
+        test("test", () {
+          addTearDown(() {
+            expect(groupTearDownRun, isFalse);
+            expect(testTearDownRun, isFalse);
+
+            expect(new Future(() {
+              testTearDownRun = true;
+            }), completes);
+          });
+
+          expect(groupTearDownRun, isFalse);
+          expect(testTearDownRun, isFalse);
+        });
+      });
+    });
+
+    test("blocks further tests on in-band async", () {
+      return expectTestsPass(() {
+        var tearDownRun = false;
+        test("test 1", () {
+          addTearDown(() async {
+            expect(tearDownRun, isFalse);
+            await pumpEventQueue();
+            tearDownRun = true;
+          });
+        });
+
+        test("test 2", () {
+          expect(tearDownRun, isTrue);
+        });
+      });
+    });
+
+    test("blocks further tests on out-of-band async", () {
+      return expectTestsPass(() {
+        var tearDownRun = false;
+        test("test 1", () {
+          addTearDown(() async {
+            expect(tearDownRun, isFalse);
+            expect(pumpEventQueue().then((_) {
+              tearDownRun = true;
+            }), completes);
+          });
+        });
+
+        test("after", () {
+          expect(tearDownRun, isTrue);
+        });
+      });
+    });
+  });
+
+  group("with an error", () {
+    test("reports the error", () async {
+      var engine = declareEngine(() {
+        test("test", () {
+          addTearDown(() => throw new TestFailure("fail"));
+        });
+      });
+
+      var queue = new StreamQueue(engine.onTestStarted);
+      var liveTestFuture = queue.next;
+
+      expect(await engine.run(), isFalse);
+
+      var liveTest = await liveTestFuture;
+      expect(liveTest.test.name, equals("test"));
+      expectTestFailed(liveTest, "fail");
+    });
+
+    test("runs further test tearDowns", () async {
+      // Declare this in the outer test so if it doesn't run, the outer test
+      // will fail.
+      var shouldRun = expectAsync0(() {});
+
+      var engine = declareEngine(() {
+        test("test", () {
+          addTearDown(() => throw "error");
+          addTearDown(shouldRun);
+        });
+      });
+
+      expect(await engine.run(), isFalse);
+    });
+
+    test("runs further group tearDowns", () async {
+      // Declare this in the outer test so if it doesn't run, the outer test
+      // will fail.
+      var shouldRun = expectAsync0(() {});
+
+      var engine = declareEngine(() {
+        tearDown(shouldRun);
+
+        test("test", () {
+          addTearDown(() => throw "error");
+        });
+      });
+
+      expect(await engine.run(), isFalse);
+    });
+  });
+}
diff --git a/test/utils.dart b/test/utils.dart
index 74833a4f..5beb0fd8 100644
--- a/test/utils.dart
+++ b/test/utils.dart
@@ -288,6 +288,9 @@ Future expectTestBlocks(test(), stopBlocking(value)) async {
 
 /// Runs [body] with a declarer, runs all the declared tests, and asserts that
 /// they pass.
+///
+/// This is typically used to run multiple tests where later tests make
+/// assertions about the results of previous ones.
 Future expectTestsPass(void body()) async {
   var engine = declareEngine(body);
   var success = await engine.run();
-- 
GitLab