Teach the network simulator to wait for Promises.

Tools/GardeningServer/lib/network-simulator.html

 <!--
 Copyright 2014 The Chromium Authors. All rights reserved.
 Use of this source code is governed by a BSD-style license that can be
 found in the LICENSE file.
 -->
 
 <script>
 function NetworkSimulator(assert, done) {
   this._assert = assert;
   this._done = done;
@@ skipping 26 matching lines @@
 NetworkSimulator.prototype.runTest = function(testCase) {
   if (NetworkSimulator._testInProgress) {
     this._assert(false, "runTest calls cannot be nested");
     this._done();
     return;
   }
 
   NetworkSimulator._testInProgress = true;
 
   var self = this;
-  return new Promise(function(resolve, reject) {

[Jeffrey Yasskin, 2014/09/03 22:11:50]

I think it's an antipattern to use the resolve/reject functions unless you have
to deal with an existing callback-taking system. Using them makes the first hop
of a promise asymmetric with the rest of the hops.

-    var realNet = window.net;
+  var realNet = window.net;
 
-    function reset() {
-      window.net = realNet;
-      NetworkSimulator._testInProgress = false;
-    }
+  function reset() {
+    window.net = realNet;
+    NetworkSimulator._testInProgress = false;
+  }
 
+  return Promise.resolve().then(function() {
     // All net.* methods should return promises. This watches all
     // promises generated by test-overridden methods.
     window.net = {};
     ['probe', 'jsonp', 'get', 'post',
      'ajax', 'json', 'xml'].forEach(function(method) {
        if (method in self) {
          net[method] = function() {
            return self.schedulePromise(self[method].apply(self, arguments));
          };
        };
      });
-
-    try {
-      testCase();
-    } catch(e) {
-      // Make sure errors thrown in the test case don't leave window.net in a bad state.
-      reset();
-      self._assert(false, "Test case threw an error:" + e);
-    }
-
-    self.resolvePromises().then(function() {
+  }).then(function() {
+    return testCase();
+  }).catch(function(e) {
+    // Make sure errors thrown in the test case don't leave window.net in a bad state.
+    reset();
+    throw e;
+  }).then(function() {
+    return self.resolvePromises().then(function() {

[Jeffrey Yasskin, 2014/09/03 22:11:50]

I'm not sure we need the resolvePromises() step anymore, since now callers can
explicitly chain from those promises.

       reset();
       self._assert(window.net == realNet);
-      resolve();
     }).catch(function(e) {
       reset();
-      self._assert(false, "Failed to finish test: " + e);
-    });
-  });
+      self._assert(false, "Failed to finish test:" + e);

[Jeffrey Yasskin, 2014/09/03 22:11:50]

I'm not sure if we want to throw a string here or the original error. Throwing
the original error gives a much better stack trace but isn't as clear that it
happened during cleanup.

[ojan, 2014/09/04 18:26:53]

I think this is fine for now. Or, we could just do two _assert calls.

[Jeffrey Yasskin, 2014/09/04 18:48:11]

Two _assert calls doesn't help: Chai uses exceptions to report assertion
failures, so the second _assert would never run. I'll leave it like this for
now.

+    })
+  }).then(this._done, this._done);

[Jeffrey Yasskin, 2014/09/03 22:11:50]

Mocha actually supports returning promises from the it() function
(http://visionmedia.github.io/mocha/#asynchronous-code), so we could omit all
the use of _done here and have callers "return runTest(...)" instead.

[ojan, 2014/09/04 18:26:53]

sgtm

[Jeffrey Yasskin, 2014/09/04 18:48:11]

After experimenting a bit, it looks safer to keep the explicit _done calls. If
we switch to returning a Promise, and a caller forgets to propagate that Promise
out of the it() function, any delayed assertions are simply ignored, which will
be hard to debug. Making people pass in a _done function makes sure Mocha knows
the test will be asynchronous.

 };
 </script>