setUp/tearDown functions can now be asynchronous.

pkg/unittest/lib/src/test_case.dart

Index: pkg/unittest/lib/src/test_case.dart
===================================================================
--- pkg/unittest/lib/src/test_case.dart	(revision 18238)
+++ pkg/unittest/lib/src/test_case.dart	(working copy)
@@ -68,18 +68,39 @@
 
   bool get isComplete => !enabled || result != null;
 
+  void _prepTest() {
+    _config.onTestStart(this);
+    startTime = new DateTime.now();
+    runningTime = null;
+  }
+
   void run() {
     if (enabled) {
       result = stackTrace = null;
       message = '';
       _doneTeardown = false;
       if (_setUp != null) {
-        _setUp();
+        var rtn = _setUp();
+        if (rtn is Future) {
+          rtn.then(expectAsync1((_) {

[Siggi Cherem (dart-lang), 2013/02/08 18:02:23]

I thought you were going to use expect async inside setup (let the user handle
this correctly).

Alternatively, we can get rid of all expectAsync, and instead tell users that
they need to use futures for asynchronous test? (if test returns a future, then
unittest will wait until it's done?)

[gram, 2013/02/11 22:44:22]

I think using Futures for everything is a great idea, but that should be done
separately from this change.

+                _prepTest();

[Siggi Cherem (dart-lang), 2013/02/08 18:02:23]

style nit: unindent this a bit. The style guide is to use only 2 spaces:

rtn.then(expectAsync((_) {
  _prepTest();
  test();
}, id: '...'))
.catchError((e) {
  body...
});

[gram, 2013/02/11 22:44:22]

Done.

+                test();
+              }, id: '[Async setUp completion handler]'))
+             .catchError((e) {
+               _prepTest();
+               // Calling error() will result in the tearDown being done.
+               // One could debate whether tearDown should be done after
+               // a failed setUp. There is no right answer, but doing it
+               // seems to be the more conservative approach.

[Siggi Cherem (dart-lang), 2013/02/08 18:02:23]

might be worth mentioning the rationale here. Unittest will not stop at a test
failure, it will try to run all tests independently of each other.

That got me thinking. When a single test fails, it makes sense to continue
running the next test, but setup/teardown will be repeated over and over again,
is it worth continuing?

+               error("$description: Test setup failed: ${e.error}");
+             });
+          return;
+        }
       }
-      _config.onTestStart(this);
-      startTime = new DateTime.now();
-      runningTime = null;
+      _prepTest();
       test();
+    } else {
+      _nextTestCase();

[Siggi Cherem (dart-lang), 2013/02/08 18:02:23]

I'd like to explore is whether we can make TestCase standalone, without
knowledge about the unittest library.

To make that work, what I'm thinking is that every TestCase would expose a
Future. The future completes when the test is done (we will use a normal
complete for a passing test, and a completeError for failing tests).

Then unittest can simply use the future API to schedule the next test:

testcase.run();
testcase.future
  .then(_testPassed) // mark test as passing
  .catchError(_testFailed) // mark test as failing
  .whenComplete(_nextTestCase); // either way, schedule the next test

     }
   }
 
@@ -90,13 +111,42 @@
       //    runningTime = new DateTime.now().difference(startTime);
       runningTime = new Duration(milliseconds: 0);
     }
+    bool mustRunNextTest = false;
     if (!_doneTeardown) {
+      _doneTeardown = true;
+      mustRunNextTest = true;
       if (_tearDown != null) {
-        _tearDown();
+        var rtn = _tearDown();
+        if (rtn is Future) {
+          rtn.then(expectAsync1((_) {
+              if (result == null) {

[Siggi Cherem (dart-lang), 2013/02/08 18:02:23]

style nit: only 2 spaces indent

[gram, 2013/02/11 22:44:22]

Done.

+                // The test passed. In some cases we will already
+                // have set this result (e.g. if the test was async
+                // and all callbacks completed). If not, we do it here.
+                pass();
+              } else {
+                // The test has already been marked as pass/fail.
+                // Just report the updated result.
+                _config.onTestResult(this);
+              }
+              _nextTestCase();
+             }, id: '[Async tearDown completion handler]'))
+            .catchError((e) {
+              // We don't call fail() as that will potentially result in
+              // spurious messages like 'test failed more than once'.
+              result = ERROR;
+              message = "$description: Test teardown failed: ${e.error}";
+              _config.onTestResult(this);
+              _nextTestCase();
+             });
+          return;
+        }
       }
-      _doneTeardown = true;
     }
     _config.onTestResult(this);
+    if (mustRunNextTest) {
+      _nextTestCase();
+    }
   }
 
   void pass() {