Deprecate multiple calls to done callbacks

lib/jasmine-core/jasmine.js

@@ -772,6 +772,7 @@ getJasmineRequireObj().Spec = function(j$) {
       };
     this.expectationResultFactory =
       attrs.expectationResultFactory || function() {};
+    this.deprecated = attrs.deprecated || function() {};
     this.queueRunnerFactory = attrs.queueRunnerFactory || function() {};
     this.catchingExceptions =
       attrs.catchingExceptions ||
@@ -866,6 +867,21 @@ getJasmineRequireObj().Spec = function(j$) {
       onException: function() {
         self.onException.apply(self, arguments);
       },
+      onMultipleDone: function() {
+        // Issue a deprecation. Include the context ourselves and pass
+        // ignoreRunnable: true, since getting here always means that we've already
+        // moved on and the current runnable isn't the one that caused the problem.
+        self.deprecated(
+          "An asynchronous function called its 'done' " +
+            'callback more than once. This is a bug in the spec, beforeAll, ' +
+            'beforeEach, afterAll, or afterEach function in question. This will ' +
+            'be treated as an error in a future version.\n' +
+            '(in spec: ' +
+            self.getFullName() +
+            ')',
+          { ignoreRunnable: true }
+        );
+      },
       onComplete: function() {
         if (self.result.status === 'failed') {
           onComplete(new j$.StopExecutionError('spec failed'));
@@ -873,7 +889,8 @@ getJasmineRequireObj().Spec = function(j$) {
           onComplete();
         }
       },
-      userContext: this.userContext()
+      userContext: this.userContext(),
+      runnableName: this.getFullName.bind(this)
     };
 
     if (this.markedPending || excluded === true) {
@@ -1929,7 +1946,8 @@ getJasmineRequireObj().Env = function(j$) {
          */
         'specDone'
       ],
-      queueRunnerFactory
+      queueRunnerFactory,
+      self.deprecated
     );
 
     /**
@@ -2344,6 +2362,7 @@ getJasmineRequireObj().Env = function(j$) {
         beforeAndAfterFns: beforeAndAfterFns(suite),
         expectationFactory: expectationFactory,
         asyncExpectationFactory: specAsyncExpectationFactory,
+        deprecated: self.deprecated,
         resultCallback: specResultCallback,
         getSpecName: function(spec) {
           return getSpecName(spec, suite);
@@ -8201,10 +8220,14 @@ getJasmineRequireObj().QueueRunner = function(j$) {
   StopExecutionError.prototype = new Error();
   j$.StopExecutionError = StopExecutionError;
 
-  function once(fn) {
+  function once(fn, onTwice) {
     var called = false;
     return function(arg) {
-      if (!called) {
+      if (called) {
+        if (onTwice) {
+          onTwice();
+        }
+      } else {
         called = true;
         // Direct call using single parameter, because cleanup/next does not need more
         fn(arg);
@@ -8213,6 +8236,16 @@ getJasmineRequireObj().QueueRunner = function(j$) {
     };
   }
 
+  function fallbackOnMultipleDone() {
+    console.error(
+      new Error(
+        "An asynchronous function called its 'done' " +
+          'callback more than once, in a QueueRunner without a onMultipleDone ' +
+          'handler.'
+      )
+    );
+  }
+
   function emptyFn() {}
 
   function QueueRunner(attrs) {
@@ -8227,6 +8260,7 @@ getJasmineRequireObj().QueueRunner = function(j$) {
         fn();
       };
     this.onException = attrs.onException || emptyFn;
+    this.onMultipleDone = attrs.onMultipleDone || fallbackOnMultipleDone;
     this.userContext = attrs.userContext || new j$.UserContext();
     this.timeout = attrs.timeout || {
       setTimeout: setTimeout,
@@ -8284,6 +8318,8 @@ getJasmineRequireObj().QueueRunner = function(j$) {
     var self = this,
       completedSynchronously = true,
       handleError = function handleError(error) {
+        // TODO probably shouldn't next() right away here.
+        // That makes debugging async failures much more confusing.
         onException(error);
         next(error);
       },
@@ -8293,37 +8329,52 @@ getJasmineRequireObj().QueueRunner = function(j$) {
         }
         self.globalErrors.popListener(handleError);
       }),
-      next = once(function next(err) {
+      next = once(
-        cleanup();
+        function next(err) {
+          cleanup();
 
-        if (j$.isError_(err)) {
+          if (j$.isError_(err)) {
-          if (!(err instanceof StopExecutionError) && !err.jasmineMessage) {
+            if (!(err instanceof StopExecutionError) && !err.jasmineMessage) {
-            self.fail(err);
+              self.fail(err);
+            }
+            self.errored = errored = true;
+          } else if (typeof err !== 'undefined' && !self.errored) {
+            self.deprecated(
+              'Any argument passed to a done callback will be treated as an ' +
+                'error in a future release. Call the done callback without ' +
+                "arguments if you don't want to trigger a spec failure."
+            );
           }
-          self.errored = errored = true;
-        } else if (typeof err !== 'undefined' && !self.errored) {
-          self.deprecated(
-            'Any argument passed to a done callback will be treated as an ' +
-              'error in a future release. Call the done callback without ' +
-              "arguments if you don't want to trigger a spec failure."
-          );
-        }
 
-        function runNext() {
+          function runNext() {
-          if (self.completeOnFirstError && errored) {
+            if (self.completeOnFirstError && errored) {
-            self.skipToCleanup(iterativeIndex);
+              self.skipToCleanup(iterativeIndex);
+            } else {
+              self.run(iterativeIndex + 1);
+            }
+          }
+
+          if (completedSynchronously) {
+            self.setTimeout(runNext);
           } else {
-            self.run(iterativeIndex + 1);
+            runNext();
+          }
+        },
+        function() {
+          try {
+            if (!timedOut) {
+              self.onMultipleDone();
+            }
+          } catch (error) {
+            // Any error we catch here is probably due to a bug in Jasmine,
+            // and it's not likely to end up anywhere useful if we let it
+            // propagate. Log it so it can at least show up when debugging.
+            console.error(error);
           }
         }
-
+      ),
-        if (completedSynchronously) {
-          self.setTimeout(runNext);
-        } else {
-          runNext();
-        }
-      }),
       errored = false,
+      timedOut = false,
       queueableFn = self.queueableFns[iterativeIndex],
       timeoutId,
       maybeThenable;
@@ -8339,6 +8390,7 @@ getJasmineRequireObj().QueueRunner = function(j$) {
     if (queueableFn.timeout !== undefined) {
       var timeoutInterval = queueableFn.timeout || j$.DEFAULT_TIMEOUT_INTERVAL;
       timeoutId = self.setTimeout(function() {
+        timedOut = true;
         var error = new Error(
           'Timeout - Async function did not complete within ' +
             timeoutInterval +
@@ -8347,6 +8399,9 @@ getJasmineRequireObj().QueueRunner = function(j$) {
               ? '(custom timeout)'
               : '(set by jasmine.DEFAULT_TIMEOUT_INTERVAL)')
         );
+        // TODO Need to decide what to do about a successful completion after a
+        //   timeout. That should probably not be a deprecation, and maybe not
+        //   an error in 4.0. (But a diagnostic of some sort might be helpful.)
         onException(error);
         next();
       }, timeoutInterval);
@@ -8452,7 +8507,7 @@ getJasmineRequireObj().QueueRunner = function(j$) {
 };
 
 getJasmineRequireObj().ReportDispatcher = function(j$) {
-  function ReportDispatcher(methods, queueRunnerFactory) {
+  function ReportDispatcher(methods, queueRunnerFactory, deprecated) {
     var dispatchedMethods = methods || [];
 
     for (var i = 0; i < dispatchedMethods.length; i++) {
@@ -8496,7 +8551,15 @@ getJasmineRequireObj().ReportDispatcher = function(j$) {
       queueRunnerFactory({
         queueableFns: fns,
         onComplete: onComplete,
-        isReporter: true
+        isReporter: true,
+        onMultipleDone: function() {
+          deprecated(
+            "An asynchronous reporter callback called its 'done' callback " +
+              'more than once. This is a bug in the reporter callback in ' +
+              'question. This will be treated as an error in a future version.',
+            { ignoreRunnable: true }
+          );
+        }
       });
     }
 
@@ -10042,6 +10105,32 @@ getJasmineRequireObj().Suite = function(j$) {
     this.result.failedExpectations.push(failedExpectation);
   };
 
+  Suite.prototype.onMultipleDone = function() {
+    var msg;
+
+    // Issue a deprecation. Include the context ourselves and pass
+    // ignoreRunnable: true, since getting here always means that we've already
+    // moved on and the current runnable isn't the one that caused the problem.
+    if (this.parentSuite) {
+      msg =
+        "An asynchronous function called its 'done' callback more than " +
+        'once. This is a bug in the spec, beforeAll, beforeEach, afterAll, ' +
+        'or afterEach function in question. This will be treated as an error ' +
+        'in a future version.\n' +
+        '(in suite: ' +
+        this.getFullName() +
+        ')';
+    } else {
+      msg =
+        'A top-level beforeAll or afterAll function called its ' +
+        "'done' callback more than once. This is a bug in the beforeAll " +
+        'or afterAll function in question. This will be treated as an ' +
+        'error in a future version.';
+    }
+
+    this.env.deprecated(msg, { ignoreRunnable: true });
+  };
+
   Suite.prototype.addExpectationResult = function() {
     if (isFailure(arguments)) {
       var data = arguments[1];
@@ -10144,7 +10233,10 @@ getJasmineRequireObj().TreeProcessor = function() {
         onException: function() {
           tree.onException.apply(tree, arguments);
         },
-        onComplete: done
+        onComplete: done,
+        onMultipleDone: tree.onMultipleDone
+          ? tree.onMultipleDone.bind(tree)
+          : null
       });
     };
 
@@ -10316,7 +10408,10 @@ getJasmineRequireObj().TreeProcessor = function() {
               userContext: node.sharedUserContext(),
               onException: function() {
                 node.onException.apply(node, arguments);
-              }
+              },
+              onMultipleDone: node.onMultipleDone
+                ? node.onMultipleDone.bind(node)
+                : null
             });
           }
         };

spec/core/QueueRunnerSpec.js

@@ -305,6 +305,32 @@ describe('QueueRunner', function() {
       expect(onComplete).toHaveBeenCalled();
     });
 
+    it('does not call onMultipleDone if an asynchrnous function completes after timing out', function() {
+      var timeout = 3,
+        queueableFn = {
+          fn: function(done) {
+            queueableFnDone = done;
+          },
+          type: 'queueable',
+          timeout: timeout
+        },
+        onComplete = jasmine.createSpy('onComplete'),
+        onMultipleDone = jasmine.createSpy('onMultipleDone'),
+        queueRunner = new jasmineUnderTest.QueueRunner({
+          queueableFns: [queueableFn],
+          onComplete: onComplete,
+          onMultipleDone: onMultipleDone
+        }),
+        queueableFnDone;
+
+      queueRunner.execute();
+      jasmine.clock().tick(timeout);
+      queueableFnDone();
+
+      expect(onComplete).toHaveBeenCalled();
+      expect(onMultipleDone).not.toHaveBeenCalled();
+    });
+
     it('by default does not set a timeout for asynchronous functions', function() {
       var beforeFn = { fn: function(done) {} },
         queueableFn = { fn: jasmine.createSpy('fn') },
@@ -380,13 +406,16 @@ describe('QueueRunner', function() {
           }
         },
         nextQueueableFn = { fn: jasmine.createSpy('nextFn') },
+        onMultipleDone = jasmine.createSpy('onMultipleDone'),
         queueRunner = new jasmineUnderTest.QueueRunner({
-          queueableFns: [queueableFn, nextQueueableFn]
+          queueableFns: [queueableFn, nextQueueableFn],
+          onMultipleDone: onMultipleDone
         });
 
       queueRunner.execute();
       jasmine.clock().tick(1);
       expect(nextQueueableFn.fn.calls.count()).toEqual(1);
+      expect(onMultipleDone).toHaveBeenCalled();
     });
 
     it('does not move to the next spec if done is called after an exception has ended the spec', function() {
@@ -397,13 +426,17 @@ describe('QueueRunner', function() {
           }
         },
         nextQueueableFn = { fn: jasmine.createSpy('nextFn') },
+        deprecated = jasmine.createSpy('deprecated'),
         queueRunner = new jasmineUnderTest.QueueRunner({
+          deprecated: deprecated,
           queueableFns: [queueableFn, nextQueueableFn]
         });
-
       queueRunner.execute();
       jasmine.clock().tick(1);
       expect(nextQueueableFn.fn.calls.count()).toEqual(1);
+      // Don't issue a deprecation. The error already tells the user that
+      // something went wrong.
+      expect(deprecated).not.toHaveBeenCalled();
     });
 
     it('should return a null when you call done', function() {

spec/core/SpecSpec.js

@@ -516,4 +516,31 @@ describe('Spec', function() {
     args.cleanupFns[0].fn();
     expect(resultCallback.calls.first().args[0].failedExpectations).toEqual([]);
   });
+
+  it('passes an onMultipleDone that logs a deprecation', function() {
+    var queueRunnerFactory = jasmine.createSpy('queueRunnerFactory'),
+      deprecated = jasmine.createSpy('depredated'),
+      spec = new jasmineUnderTest.Spec({
+        deprecated: deprecated,
+        queueableFn: { fn: function() {} },
+        queueRunnerFactory: queueRunnerFactory,
+        getSpecName: function() {
+          return 'a spec';
+        }
+      });
+
+    spec.execute();
+
+    expect(queueRunnerFactory).toHaveBeenCalled();
+    queueRunnerFactory.calls.argsFor(0)[0].onMultipleDone();
+
+    expect(deprecated).toHaveBeenCalledWith(
+      "An asynchronous function called its 'done' " +
+        'callback more than once. This is a bug in the spec, beforeAll, ' +
+        'beforeEach, afterAll, or afterEach function in question. This will ' +
+        'be treated as an error in a future version.\n' +
+        '(in spec: a spec)',
+      { ignoreRunnable: true }
+    );
+  });
 });

spec/core/SuiteSpec.js

@@ -142,4 +142,44 @@ describe('Suite', function() {
       );
     });
   });
+
+  describe('#onMultipleDone', function() {
+    it('logs a special deprecation when it is the top suite', function() {
+      var env = jasmine.createSpyObj('env', ['deprecated']);
+      var suite = new jasmineUnderTest.Suite({ env: env, parentSuite: null });
+
+      suite.onMultipleDone();
+
+      expect(env.deprecated).toHaveBeenCalledWith(
+        'A top-level beforeAll or afterAll function called its ' +
+          "'done' callback more than once. This is a bug in the beforeAll " +
+          'or afterAll function in question. This will be treated as an ' +
+          'error in a future version.',
+        { ignoreRunnable: true }
+      );
+    });
+
+    it('logs a deprecation including the suite name when it is a normal suite', function() {
+      var env = jasmine.createSpyObj('env', ['deprecated']);
+      var suite = new jasmineUnderTest.Suite({
+        env: env,
+        description: 'the suite',
+        parentSuite: {
+          description: 'the parent suite',
+          parentSuite: {}
+        }
+      });
+
+      suite.onMultipleDone();
+
+      expect(env.deprecated).toHaveBeenCalledWith(
+        "An asynchronous function called its 'done' callback more than " +
+          'once. This is a bug in the spec, beforeAll, beforeEach, afterAll, ' +
+          'or afterEach function in question. This will be treated as an error ' +
+          'in a future version.\n' +
+          '(in suite: the parent suite the suite)',
+        { ignoreRunnable: true }
+      );
+    });
+  });
 });

spec/core/TreeProcessorSpec.js

@@ -291,7 +291,8 @@ describe('TreeProcessor', function() {
       onComplete: treeComplete,
       onException: jasmine.any(Function),
       userContext: { root: 'context' },
-      queueableFns: [{ fn: jasmine.any(Function) }]
+      queueableFns: [{ fn: jasmine.any(Function) }],
+      onMultipleDone: null
     });
 
     queueRunner.calls.mostRecent().args[0].queueableFns[0].fn('foo');
@@ -321,16 +322,19 @@ describe('TreeProcessor', function() {
       onComplete: treeComplete,
       onException: jasmine.any(Function),
       userContext: { root: 'context' },
-      queueableFns: [{ fn: jasmine.any(Function) }]
+      queueableFns: [{ fn: jasmine.any(Function) }],
+      onMultipleDone: null
     });
 
     queueRunner.calls.mostRecent().args[0].queueableFns[0].fn(nodeDone);
 
     expect(queueRunner).toHaveBeenCalledWith({
       onComplete: jasmine.any(Function),
+      onMultipleDone: null,
       queueableFns: [{ fn: jasmine.any(Function) }],
       userContext: { node: 'context' },
-      onException: jasmine.any(Function)
+      onException: jasmine.any(Function),
+      onMultipleDone: null
     });
 
     queueRunner.calls.mostRecent().args[0].queueableFns[0].fn('foo');

spec/core/integration/EnvSpec.js

@@ -513,6 +513,191 @@ describe('Env integration', function() {
     env.execute(null, assertions);
   });
 
+  it('deprecates multiple calls to done in the top suite', function(done) {
+    var reporter = jasmine.createSpyObj('fakeReporter', ['jasmineDone']);
+    var message =
+      'A top-level beforeAll or afterAll function called its ' +
+      "'done' callback more than once. This is a bug in the beforeAll " +
+      'or afterAll function in question. This will be treated as an ' +
+      'error in a future version.';
+
+    spyOn(console, 'error');
+    env.addReporter(reporter);
+    env.configure({ verboseDeprecations: true });
+    env.beforeAll(function(innerDone) {
+      innerDone();
+      innerDone();
+    });
+    env.it('a spec, so the beforeAll runs', function() {});
+    env.afterAll(function(innerDone) {
+      innerDone();
+      innerDone();
+    });
+
+    env.execute(null, function() {
+      var warnings;
+      expect(reporter.jasmineDone).toHaveBeenCalled();
+      warnings = reporter.jasmineDone.calls.argsFor(0)[0].deprecationWarnings;
+      expect(warnings.length).toEqual(2);
+      expect(warnings[0])
+        .withContext('top beforeAll')
+        .toEqual(jasmine.objectContaining({ message: message }));
+      expect(warnings[1])
+        .withContext('top afterAll')
+        .toEqual(jasmine.objectContaining({ message: message }));
+      done();
+    });
+  });
+
+  it('deprecates multiple calls to done in a non-top suite', function(done) {
+    var reporter = jasmine.createSpyObj('fakeReporter', ['jasmineDone']);
+    var message =
+      "An asynchronous function called its 'done' " +
+      'callback more than once. This is a bug in the spec, beforeAll, ' +
+      'beforeEach, afterAll, or afterEach function in question. This will ' +
+      'be treated as an error in a future version.';
+
+    spyOn(console, 'error');
+    env.addReporter(reporter);
+    env.configure({ verboseDeprecations: true });
+    env.describe('a suite', function() {
+      env.beforeAll(function(innerDone) {
+        innerDone();
+        innerDone();
+      });
+      env.it('a spec, so that before/afters run', function() {});
+      env.afterAll(function(innerDone) {
+        innerDone();
+        innerDone();
+      });
+    });
+
+    env.execute(null, function() {
+      var warnings;
+      expect(reporter.jasmineDone).toHaveBeenCalled();
+      warnings = reporter.jasmineDone.calls.argsFor(0)[0].deprecationWarnings;
+      expect(warnings.length).toEqual(2);
+      expect(warnings[0])
+        .withContext('suite beforeAll')
+        .toEqual(
+          jasmine.objectContaining({
+            message: message + '\n(in suite: a suite)'
+          })
+        );
+      expect(warnings[1])
+        .withContext('suite afterAll')
+        .toEqual(
+          jasmine.objectContaining({
+            message: message + '\n(in suite: a suite)'
+          })
+        );
+      done();
+    });
+  });
+
+  it('deprecates multiple calls to done in a spec', function(done) {
+    var reporter = jasmine.createSpyObj('fakeReporter', ['jasmineDone']);
+    var message =
+      "An asynchronous function called its 'done' " +
+      'callback more than once. This is a bug in the spec, beforeAll, ' +
+      'beforeEach, afterAll, or afterEach function in question. This will ' +
+      'be treated as an error in a future version.\n' +
+      '(in spec: a suite a spec)';
+
+    spyOn(console, 'error');
+    env.addReporter(reporter);
+    env.configure({ verboseDeprecations: true });
+    env.describe('a suite', function() {
+      env.beforeEach(function(innerDone) {
+        innerDone();
+        innerDone();
+      });
+      env.it('a spec', function(innerDone) {
+        innerDone();
+        innerDone();
+      });
+      env.afterEach(function(innerDone) {
+        innerDone();
+        innerDone();
+      });
+    });
+
+    env.execute(null, function() {
+      var warnings;
+      expect(reporter.jasmineDone).toHaveBeenCalled();
+      warnings = reporter.jasmineDone.calls.argsFor(0)[0].deprecationWarnings;
+      expect(warnings.length).toEqual(3);
+      expect(warnings[0])
+        .withContext('warning caused by beforeEach')
+        .toEqual(jasmine.objectContaining({ message: message }));
+      expect(warnings[1])
+        .withContext('warning caused by it')
+        .toEqual(jasmine.objectContaining({ message: message }));
+      expect(warnings[2])
+        .withContext('warning caused by afterEach')
+        .toEqual(jasmine.objectContaining({ message: message }));
+      done();
+    });
+  });
+
+  it('deprecates multiple calls to done in reporters', function(done) {
+    var message =
+      "An asynchronous reporter callback called its 'done' callback more " +
+      'than once. This is a bug in the reporter callback in question. This ' +
+      'will be treated as an error in a future version.\nNote: This message ' +
+      'will be shown only once. Set config.verboseDeprecations to true to ' +
+      'see every occurrence.';
+    var reporter = jasmine.createSpyObj('fakeReport', ['jasmineDone']);
+    reporter.specDone = function(result, done) {
+      done();
+      done();
+    };
+    env.addReporter(reporter);
+
+    env.it('a spec', function() {});
+
+    spyOn(console, 'error');
+    env.execute(null, function() {
+      expect(reporter.jasmineDone).toHaveBeenCalled();
+      warnings = reporter.jasmineDone.calls.argsFor(0)[0].deprecationWarnings;
+      expect(warnings.length).toEqual(1);
+      expect(warnings[0]).toEqual(
+        jasmine.objectContaining({ message: message })
+      );
+      done();
+    });
+  });
+
+  it('does not deprecate a call to done that comes after a timeout', function(done) {
+    var reporter = jasmine.createSpyObj('fakeReporter', ['jasmineDone']),
+      firstSpecDone;
+
+    reporter.specDone = function(result, reporterDone) {
+      setTimeout(function() {
+        firstSpecDone();
+        reporterDone();
+      });
+    };
+    env.addReporter(reporter);
+
+    env.it(
+      'a spec',
+      function(innerDone) {
+        firstSpecDone = innerDone;
+      },
+      1
+    );
+
+    env.execute(null, function() {
+      expect(reporter.jasmineDone).toHaveBeenCalledWith(
+        jasmine.objectContaining({
+          deprecationWarnings: []
+        })
+      );
+      done();
+    });
+  });
+
   describe('suiteDone reporting', function() {
     it('reports when an afterAll fails an expectation', function(done) {
       var reporter = jasmine.createSpyObj('fakeReport', ['suiteDone']);
@@ -1197,10 +1382,6 @@ describe('Env integration', function() {
     });
 
     it('should wait a custom interval before reporting async functions that fail to complete', function(done) {
-      if (jasmine.getEnv().skipBrowserFlake) {
-        jasmine.getEnv().skipBrowserFlake();
-      }
-
       createMockedEnv();
       var reporter = jasmine.createSpyObj('fakeReport', [
         'jasmineDone',
@@ -2519,7 +2700,7 @@ describe('Env integration', function() {
             return setTimeout(fn, delay);
           },
           clearTimeout: function(fn, delay) {
-            clearTimeout(fn, delay);
+            return clearTimeout(fn, delay);
           }
         };
         spyOn(jasmineUnderTest, 'getGlobal').and.returnValue(global);
@@ -2774,6 +2955,10 @@ describe('Env integration', function() {
   });
 
   it('provides custom equality testers to async matchers', function(done) {
+    if (jasmine.getEnv().skipBrowserFlake) {
+      jasmine.getEnv().skipBrowserFlake();
+    }
+
     jasmine.getEnv().requirePromises();
 
     var specDone = jasmine.createSpy('specDone');

src/core/Env.js

@@ -891,7 +891,8 @@ getJasmineRequireObj().Env = function(j$) {
          */
         'specDone'
       ],
-      queueRunnerFactory
+      queueRunnerFactory,
+      self.deprecated
     );
 
     /**
@@ -1306,6 +1307,7 @@ getJasmineRequireObj().Env = function(j$) {
         beforeAndAfterFns: beforeAndAfterFns(suite),
         expectationFactory: expectationFactory,
         asyncExpectationFactory: specAsyncExpectationFactory,
+        deprecated: self.deprecated,
         resultCallback: specResultCallback,
         getSpecName: function(spec) {
           return getSpecName(spec, suite);

src/core/QueueRunner.js

@@ -5,10 +5,14 @@ getJasmineRequireObj().QueueRunner = function(j$) {
   StopExecutionError.prototype = new Error();
   j$.StopExecutionError = StopExecutionError;
 
-  function once(fn) {
+  function once(fn, onTwice) {
     var called = false;
     return function(arg) {
-      if (!called) {
+      if (called) {
+        if (onTwice) {
+          onTwice();
+        }
+      } else {
         called = true;
         // Direct call using single parameter, because cleanup/next does not need more
         fn(arg);
@@ -17,6 +21,16 @@ getJasmineRequireObj().QueueRunner = function(j$) {
     };
   }
 
+  function fallbackOnMultipleDone() {
+    console.error(
+      new Error(
+        "An asynchronous function called its 'done' " +
+          'callback more than once, in a QueueRunner without a onMultipleDone ' +
+          'handler.'
+      )
+    );
+  }
+
   function emptyFn() {}
 
   function QueueRunner(attrs) {
@@ -31,6 +45,7 @@ getJasmineRequireObj().QueueRunner = function(j$) {
         fn();
       };
     this.onException = attrs.onException || emptyFn;
+    this.onMultipleDone = attrs.onMultipleDone || fallbackOnMultipleDone;
     this.userContext = attrs.userContext || new j$.UserContext();
     this.timeout = attrs.timeout || {
       setTimeout: setTimeout,
@@ -88,6 +103,8 @@ getJasmineRequireObj().QueueRunner = function(j$) {
     var self = this,
       completedSynchronously = true,
       handleError = function handleError(error) {
+        // TODO probably shouldn't next() right away here.
+        // That makes debugging async failures much more confusing.
         onException(error);
         next(error);
       },
@@ -97,37 +114,52 @@ getJasmineRequireObj().QueueRunner = function(j$) {
         }
         self.globalErrors.popListener(handleError);
       }),
-      next = once(function next(err) {
+      next = once(
-        cleanup();
+        function next(err) {
+          cleanup();
 
-        if (j$.isError_(err)) {
+          if (j$.isError_(err)) {
-          if (!(err instanceof StopExecutionError) && !err.jasmineMessage) {
+            if (!(err instanceof StopExecutionError) && !err.jasmineMessage) {
-            self.fail(err);
+              self.fail(err);
+            }
+            self.errored = errored = true;
+          } else if (typeof err !== 'undefined' && !self.errored) {
+            self.deprecated(
+              'Any argument passed to a done callback will be treated as an ' +
+                'error in a future release. Call the done callback without ' +
+                "arguments if you don't want to trigger a spec failure."
+            );
           }
-          self.errored = errored = true;
-        } else if (typeof err !== 'undefined' && !self.errored) {
-          self.deprecated(
-            'Any argument passed to a done callback will be treated as an ' +
-              'error in a future release. Call the done callback without ' +
-              "arguments if you don't want to trigger a spec failure."
-          );
-        }
 
-        function runNext() {
+          function runNext() {
-          if (self.completeOnFirstError && errored) {
+            if (self.completeOnFirstError && errored) {
-            self.skipToCleanup(iterativeIndex);
+              self.skipToCleanup(iterativeIndex);
+            } else {
+              self.run(iterativeIndex + 1);
+            }
+          }
+
+          if (completedSynchronously) {
+            self.setTimeout(runNext);
           } else {
-            self.run(iterativeIndex + 1);
+            runNext();
+          }
+        },
+        function() {
+          try {
+            if (!timedOut) {
+              self.onMultipleDone();
+            }
+          } catch (error) {
+            // Any error we catch here is probably due to a bug in Jasmine,
+            // and it's not likely to end up anywhere useful if we let it
+            // propagate. Log it so it can at least show up when debugging.
+            console.error(error);
           }
         }
-
+      ),
-        if (completedSynchronously) {
-          self.setTimeout(runNext);
-        } else {
-          runNext();
-        }
-      }),
       errored = false,
+      timedOut = false,
       queueableFn = self.queueableFns[iterativeIndex],
       timeoutId,
       maybeThenable;
@@ -143,6 +175,7 @@ getJasmineRequireObj().QueueRunner = function(j$) {
     if (queueableFn.timeout !== undefined) {
       var timeoutInterval = queueableFn.timeout || j$.DEFAULT_TIMEOUT_INTERVAL;
       timeoutId = self.setTimeout(function() {
+        timedOut = true;
         var error = new Error(
           'Timeout - Async function did not complete within ' +
             timeoutInterval +
@@ -151,6 +184,9 @@ getJasmineRequireObj().QueueRunner = function(j$) {
               ? '(custom timeout)'
               : '(set by jasmine.DEFAULT_TIMEOUT_INTERVAL)')
         );
+        // TODO Need to decide what to do about a successful completion after a
+        //   timeout. That should probably not be a deprecation, and maybe not
+        //   an error in 4.0. (But a diagnostic of some sort might be helpful.)
         onException(error);
         next();
       }, timeoutInterval);

src/core/ReportDispatcher.js

@@ -1,5 +1,5 @@
 getJasmineRequireObj().ReportDispatcher = function(j$) {
-  function ReportDispatcher(methods, queueRunnerFactory) {
+  function ReportDispatcher(methods, queueRunnerFactory, deprecated) {
     var dispatchedMethods = methods || [];
 
     for (var i = 0; i < dispatchedMethods.length; i++) {
@@ -43,7 +43,15 @@ getJasmineRequireObj().ReportDispatcher = function(j$) {
       queueRunnerFactory({
         queueableFns: fns,
         onComplete: onComplete,
-        isReporter: true
+        isReporter: true,
+        onMultipleDone: function() {
+          deprecated(
+            "An asynchronous reporter callback called its 'done' callback " +
+              'more than once. This is a bug in the reporter callback in ' +
+              'question. This will be treated as an error in a future version.',
+            { ignoreRunnable: true }
+          );
+        }
       });
     }
 

src/core/Spec.js

@@ -40,6 +40,7 @@ getJasmineRequireObj().Spec = function(j$) {
       };
     this.expectationResultFactory =
       attrs.expectationResultFactory || function() {};
+    this.deprecated = attrs.deprecated || function() {};
     this.queueRunnerFactory = attrs.queueRunnerFactory || function() {};
     this.catchingExceptions =
       attrs.catchingExceptions ||
@@ -134,6 +135,21 @@ getJasmineRequireObj().Spec = function(j$) {
       onException: function() {
         self.onException.apply(self, arguments);
       },
+      onMultipleDone: function() {
+        // Issue a deprecation. Include the context ourselves and pass
+        // ignoreRunnable: true, since getting here always means that we've already
+        // moved on and the current runnable isn't the one that caused the problem.
+        self.deprecated(
+          "An asynchronous function called its 'done' " +
+            'callback more than once. This is a bug in the spec, beforeAll, ' +
+            'beforeEach, afterAll, or afterEach function in question. This will ' +
+            'be treated as an error in a future version.\n' +
+            '(in spec: ' +
+            self.getFullName() +
+            ')',
+          { ignoreRunnable: true }
+        );
+      },
       onComplete: function() {
         if (self.result.status === 'failed') {
           onComplete(new j$.StopExecutionError('spec failed'));
@@ -141,7 +157,8 @@ getJasmineRequireObj().Spec = function(j$) {
           onComplete();
         }
       },
-      userContext: this.userContext()
+      userContext: this.userContext(),
+      runnableName: this.getFullName.bind(this)
     };
 
     if (this.markedPending || excluded === true) {

src/core/Suite.js

@@ -201,6 +201,32 @@ getJasmineRequireObj().Suite = function(j$) {
     this.result.failedExpectations.push(failedExpectation);
   };
 
+  Suite.prototype.onMultipleDone = function() {
+    var msg;
+
+    // Issue a deprecation. Include the context ourselves and pass
+    // ignoreRunnable: true, since getting here always means that we've already
+    // moved on and the current runnable isn't the one that caused the problem.
+    if (this.parentSuite) {
+      msg =
+        "An asynchronous function called its 'done' callback more than " +
+        'once. This is a bug in the spec, beforeAll, beforeEach, afterAll, ' +
+        'or afterEach function in question. This will be treated as an error ' +
+        'in a future version.\n' +
+        '(in suite: ' +
+        this.getFullName() +
+        ')';
+    } else {
+      msg =
+        'A top-level beforeAll or afterAll function called its ' +
+        "'done' callback more than once. This is a bug in the beforeAll " +
+        'or afterAll function in question. This will be treated as an ' +
+        'error in a future version.';
+    }
+
+    this.env.deprecated(msg, { ignoreRunnable: true });
+  };
+
   Suite.prototype.addExpectationResult = function() {
     if (isFailure(arguments)) {
       var data = arguments[1];

src/core/TreeProcessor.js

@@ -44,7 +44,10 @@ getJasmineRequireObj().TreeProcessor = function() {
         onException: function() {
           tree.onException.apply(tree, arguments);
         },
-        onComplete: done
+        onComplete: done,
+        onMultipleDone: tree.onMultipleDone
+          ? tree.onMultipleDone.bind(tree)
+          : null
       });
     };
 
@@ -216,7 +219,10 @@ getJasmineRequireObj().TreeProcessor = function() {
               userContext: node.sharedUserContext(),
               onException: function() {
                 node.onException.apply(node, arguments);
-              }
+              },
+              onMultipleDone: node.onMultipleDone
+                ? node.onMultipleDone.bind(node)
+                : null
             });
           }
         };