Make sure the queue runner goes async for async specs

- Even if `done` is called synchronously. See #1327 #1334 jasmine/gulp-jasmine-browser#48
2017-06-06 14:29:02 -07:00
parent 3372af1cf1
commit 578f63b9bd
4 changed files with 100 additions and 59 deletions
--- a/lib/jasmine-core/jasmine.js
+++ b/lib/jasmine-core/jasmine.js
@@ -3909,6 +3909,10 @@ getJasmineRequireObj().QueueRunner = function(j$) {
      var clearTimeout = function () {
          Function.prototype.apply.apply(self.timeout.clearTimeout, [j$.getGlobal(), [timeoutId]]);
        },
+        setTimeout = function(delayedFn, delay) {
+          return Function.prototype.apply.apply(self.timeout.setTimeout, [j$.getGlobal(), [delayedFn, delay]]);
+        },
+        completedSynchronously = true,
        handleError = function(error) {
          onException(error);
          next();
@@ -3919,7 +3923,13 @@ getJasmineRequireObj().QueueRunner = function(j$) {
        }),
        next = once(function () {
          cleanup();
-          self.run(queueableFns, iterativeIndex + 1);
+          if (completedSynchronously) {
+            setTimeout(function() {
+              self.run(queueableFns, iterativeIndex + 1);
+            });
+          } else {
+            self.run(queueableFns, iterativeIndex + 1);
+          }
        }),
        timeoutId;

@@ -3931,11 +3941,11 @@ getJasmineRequireObj().QueueRunner = function(j$) {
      self.globalErrors.pushListener(handleError);

      if (queueableFn.timeout) {
-        timeoutId = Function.prototype.apply.apply(self.timeout.setTimeout, [j$.getGlobal(), [function() {
+        timeoutId = setTimeout(function() {
          var error = new Error('Timeout - Async callback was not invoked within timeout specified by jasmine.DEFAULT_TIMEOUT_INTERVAL.');
          onException(error);
          next();
-        }, queueableFn.timeout()]]);
+        }, queueableFn.timeout());
      }

      try {
@@ -3944,10 +3954,12 @@ getJasmineRequireObj().QueueRunner = function(j$) {

          if (maybeThenable && j$.isFunction_(maybeThenable.then)) {
            maybeThenable.then(next, next.fail);
+            completedSynchronously = false;
            return false;
          }
        } else {
          queueableFn.fn.call(self.userContext, next);
+          completedSynchronously = false;
          return false;
        }
      } catch (e) {
--- a/spec/core/QueueRunnerSpec.js
+++ b/spec/core/QueueRunnerSpec.js
@@ -189,6 +189,7 @@ describe("QueueRunner", function() {

      queueRunner.execute();

+      jasmine.clock().tick(1);
      expect(onComplete).toHaveBeenCalled();

      jasmine.clock().tick(jasmineUnderTest.DEFAULT_TIMEOUT_INTERVAL);
@@ -197,12 +198,13 @@ describe("QueueRunner", function() {

    it("only moves to the next spec the first time you call done", function() {
      var queueableFn = { fn: function(done) {done(); done();} },
-        nextQueueableFn = { fn: jasmine.createSpy('nextFn') };
-      queueRunner = new jasmineUnderTest.QueueRunner({
-        queueableFns: [queueableFn, nextQueueableFn]
-      });
+        nextQueueableFn = { fn: jasmine.createSpy('nextFn') },
+        queueRunner = new jasmineUnderTest.QueueRunner({
+          queueableFns: [queueableFn, nextQueueableFn]
+        });

      queueRunner.execute();
+      jasmine.clock().tick(1);
      expect(nextQueueableFn.fn.calls.count()).toEqual(1);
    });

@@ -211,10 +213,10 @@ describe("QueueRunner", function() {
         setTimeout(done, 1);
         throw new Error('error!');
       } },
-        nextQueueableFn = { fn: jasmine.createSpy('nextFn') };
-      queueRunner = new jasmineUnderTest.QueueRunner({
-        queueableFns: [queueableFn, nextQueueableFn]
-      });
+       nextQueueableFn = { fn: jasmine.createSpy('nextFn') },
+       queueRunner = new jasmineUnderTest.QueueRunner({
+         queueableFns: [queueableFn, nextQueueableFn]
+       });

      queueRunner.execute();
      jasmine.clock().tick(1);
@@ -271,6 +273,33 @@ describe("QueueRunner", function() {
      expect(onException).toHaveBeenCalledWith(errorWithMessage(/^foo$/));
      expect(nextQueueableFn.fn).toHaveBeenCalled();
    });
+
+    it("handles exceptions thrown while waiting for the stack to clear", function() {
+      var queueableFn = { fn: function(done) { done() } },
+      global = {},
+      errorListeners = [],
+      globalErrors = {
+        pushListener: function(f) { errorListeners.push(f); },
+        popListener: function() { errorListeners.pop(); }
+      },
+      clearStack = jasmine.createSpy('clearStack'),
+      onException = jasmine.createSpy('onException'),
+      queueRunner = new jasmineUnderTest.QueueRunner({
+        queueableFns: [queueableFn],
+        globalErrors: globalErrors,
+        clearStack: clearStack,
+        onException: onException
+      }),
+      error = new Error('nope');
+
+      queueRunner.execute();
+      jasmine.clock().tick();
+      expect(clearStack).toHaveBeenCalled();
+      expect(errorListeners.length).toEqual(1);
+      errorListeners[0](error);
+      clearStack.calls.argsFor(0)[0]();
+      expect(onException).toHaveBeenCalledWith(error);
+    });
  });

  describe("with a function that returns a promise", function() {
@@ -392,32 +421,6 @@ describe("QueueRunner", function() {
    expect(nextQueueableFn.fn).toHaveBeenCalled();
  });

-  it("handles exceptions thrown while waiting for the stack to clear", function() {
-    var queueableFn = { fn: function(done) { done() } },
-      global = {},
-      errorListeners = [],
-      globalErrors = {
-        pushListener: function(f) { errorListeners.push(f); },
-        popListener: function() { errorListeners.pop(); }
-      },
-      clearStack = jasmine.createSpy('clearStack'),
-      onException = jasmine.createSpy('onException'),
-      queueRunner = new jasmineUnderTest.QueueRunner({
-        queueableFns: [queueableFn],
-        globalErrors: globalErrors,
-        clearStack: clearStack,
-        onException: onException
-      }),
-      error = new Error('nope');
-
-    queueRunner.execute();
-    expect(clearStack).toHaveBeenCalled();
-    expect(errorListeners.length).toEqual(1);
-    errorListeners[0](error);
-    clearStack.calls.argsFor(0)[0]();
-    expect(onException).toHaveBeenCalledWith(error);
-  });
-
  it("calls a provided complete callback when done", function() {
    var queueableFn = { fn: jasmine.createSpy('fn') },
      completeCallback = jasmine.createSpy('completeCallback'),
@@ -431,23 +434,34 @@ describe("QueueRunner", function() {
    expect(completeCallback).toHaveBeenCalled();
  });

-  it("calls a provided stack clearing function when done", function() {
-    var asyncFn = { fn: function(done) { done() } },
-        afterFn = { fn: jasmine.createSpy('afterFn') },
-        completeCallback = jasmine.createSpy('completeCallback'),
-        clearStack = jasmine.createSpy('clearStack'),
-        queueRunner = new jasmineUnderTest.QueueRunner({
-          queueableFns: [asyncFn, afterFn],
-          clearStack: clearStack,
-          onComplete: completeCallback
-        });
+  describe("clearing the stack", function() {
+    beforeEach(function() {
+      jasmine.clock().install();
+    });

-    clearStack.and.callFake(function(fn) { fn(); });
+    afterEach(function() {
+      jasmine.clock().uninstall();
+    });

-    queueRunner.execute();
-    expect(afterFn.fn).toHaveBeenCalled();
-    expect(clearStack).toHaveBeenCalled();
-    clearStack.calls.argsFor(0)[0]();
-    expect(completeCallback).toHaveBeenCalled();
+    it("calls a provided stack clearing function when done", function() {
+      var asyncFn = { fn: function(done) { done() } },
+          afterFn = { fn: jasmine.createSpy('afterFn') },
+          completeCallback = jasmine.createSpy('completeCallback'),
+          clearStack = jasmine.createSpy('clearStack'),
+          queueRunner = new jasmineUnderTest.QueueRunner({
+            queueableFns: [asyncFn, afterFn],
+            clearStack: clearStack,
+            onComplete: completeCallback
+          });
+
+      clearStack.and.callFake(function(fn) { fn(); });
+
+      queueRunner.execute();
+      jasmine.clock().tick();
+      expect(afterFn.fn).toHaveBeenCalled();
+      expect(clearStack).toHaveBeenCalled();
+      clearStack.calls.argsFor(0)[0]();
+      expect(completeCallback).toHaveBeenCalled();
+    });
  });
 });
--- a/spec/core/integration/EnvSpec.js
+++ b/spec/core/integration/EnvSpec.js
@@ -456,7 +456,7 @@ describe("Env integration", function() {
    env.execute();
  });

-  it("copes with late async failures", function(done) {
+  it("copes with async failures after done has been called", function(done) {
    var global = {
      setTimeout: function(fn, delay) { setTimeout(fn, delay) },
      clearTimeout: function(fn, delay) { clearTimeout(fn, delay) },
@@ -466,6 +466,7 @@ describe("Env integration", function() {
      reporter = jasmine.createSpyObj('fakeReporter', [ "specDone", "jasmineDone", "suiteDone" ]);

    reporter.jasmineDone.and.callFake(function() {
+      expect(reporter.specDone).not.toHaveFailedExpecationsForRunnable('A suite fails', ['fail thrown']);
      expect(reporter.suiteDone).toHaveFailedExpecationsForRunnable('A suite', ['fail thrown']);
      done();
    });
@@ -474,9 +475,11 @@ describe("Env integration", function() {

    env.fdescribe('A suite', function() {
      env.it('fails', function(specDone) {
-        specDone();
        setTimeout(function() {
-          global.onerror('fail');
+          specDone();
+          setTimeout(function() {
+            global.onerror('fail');
+          });
        });
      });
    });
--- a/src/core/QueueRunner.js
+++ b/src/core/QueueRunner.js
@@ -56,6 +56,10 @@ getJasmineRequireObj().QueueRunner = function(j$) {
      var clearTimeout = function () {
          Function.prototype.apply.apply(self.timeout.clearTimeout, [j$.getGlobal(), [timeoutId]]);
        },
+        setTimeout = function(delayedFn, delay) {
+          return Function.prototype.apply.apply(self.timeout.setTimeout, [j$.getGlobal(), [delayedFn, delay]]);
+        },
+        completedSynchronously = true,
        handleError = function(error) {
          onException(error);
          next();
@@ -66,7 +70,13 @@ getJasmineRequireObj().QueueRunner = function(j$) {
        }),
        next = once(function () {
          cleanup();
-          self.run(queueableFns, iterativeIndex + 1);
+          if (completedSynchronously) {
+            setTimeout(function() {
+              self.run(queueableFns, iterativeIndex + 1);
+            });
+          } else {
+            self.run(queueableFns, iterativeIndex + 1);
+          }
        }),
        timeoutId;

@@ -78,11 +88,11 @@ getJasmineRequireObj().QueueRunner = function(j$) {
      self.globalErrors.pushListener(handleError);

      if (queueableFn.timeout) {
-        timeoutId = Function.prototype.apply.apply(self.timeout.setTimeout, [j$.getGlobal(), [function() {
+        timeoutId = setTimeout(function() {
          var error = new Error('Timeout - Async callback was not invoked within timeout specified by jasmine.DEFAULT_TIMEOUT_INTERVAL.');
          onException(error);
          next();
-        }, queueableFn.timeout()]]);
+        }, queueableFn.timeout());
      }

      try {
@@ -91,10 +101,12 @@ getJasmineRequireObj().QueueRunner = function(j$) {

          if (maybeThenable && j$.isFunction_(maybeThenable.then)) {
            maybeThenable.then(next, next.fail);
+            completedSynchronously = false;
            return false;
          }
        } else {
          queueableFn.fn.call(self.userContext, next);
+          completedSynchronously = false;
          return false;
        }
      } catch (e) {