diff --git a/lib/jasmine-core/jasmine.js b/lib/jasmine-core/jasmine.js index fb91318e..71194eeb 100644 --- a/lib/jasmine-core/jasmine.js +++ b/lib/jasmine-core/jasmine.js @@ -7056,7 +7056,8 @@ getJasmineRequireObj().QueueRunner = function(j$) { }), errored = false, queueableFn = self.queueableFns[iterativeIndex], - timeoutId; + timeoutId, + maybeThenable; next.fail = function nextFail() { self.fail.apply(null, arguments); @@ -7084,7 +7085,7 @@ getJasmineRequireObj().QueueRunner = function(j$) { try { if (queueableFn.fn.length === 0) { - var maybeThenable = queueableFn.fn.call(self.userContext); + maybeThenable = queueableFn.fn.call(self.userContext); if (maybeThenable && j$.isFunction_(maybeThenable.then)) { maybeThenable.then(next, onPromiseRejection); @@ -7092,7 +7093,8 @@ getJasmineRequireObj().QueueRunner = function(j$) { return { completedSynchronously: false }; } } else { - queueableFn.fn.call(self.userContext, next); + maybeThenable = queueableFn.fn.call(self.userContext, next); + this.diagnoseConflictingAsync_(queueableFn.fn, maybeThenable); completedSynchronously = false; return { completedSynchronously: false }; } @@ -7145,6 +7147,25 @@ getJasmineRequireObj().QueueRunner = function(j$) { }); }; + QueueRunner.prototype.diagnoseConflictingAsync_ = function(fn, retval) { + if (retval && j$.isFunction_(retval.then)) { + // Issue a warning that matches the user's code + if (j$.isAsyncFunction_(fn)) { + this.deprecated('An asynchronous before/it/after ' + + 'function was defined with the async keyword but also took a ' + + 'done callback. This is not supported and will stop working in' + + ' the future. Either remove the done callback (recommended) or ' + + 'remove the async keyword.'); + } else { + this.deprecated('An asynchronous before/it/after ' + + 'function took a done callback but also returned a promise. ' + + 'This is not supported and will stop working in the future. ' + + 'Either remove the done callback (recommended) or change the ' + + 'function to not return a promise.'); + } + } + }; + return QueueRunner; }; diff --git a/spec/core/QueueRunnerSpec.js b/spec/core/QueueRunnerSpec.js index ac18b81d..c722aaff 100644 --- a/spec/core/QueueRunnerSpec.js +++ b/spec/core/QueueRunnerSpec.js @@ -512,6 +512,48 @@ describe('QueueRunner', function() { expect(onExceptionCallback).toHaveBeenCalledWith('foo'); expect(queueableFn2.fn).toHaveBeenCalled(); }); + + it('issues a deprecation if the function also takes a parameter', function() { + var queueableFn = { + fn: function(done) { + return new StubPromise(); + } + }, + deprecated = jasmine.createSpy('deprecated'), + queueRunner = new jasmineUnderTest.QueueRunner({ + queueableFns: [queueableFn], + deprecated: deprecated + }), + env = jasmineUnderTest.getEnv(); + + queueRunner.execute(); + + expect(deprecated).toHaveBeenCalledWith('An asynchronous ' + + 'before/it/after function took a done callback but also returned a '+ + 'promise. This is not supported and will stop working in the future. ' + + 'Either remove the done callback (recommended) or change the function ' + + 'to not return a promise.' + ); + }); + + it('issues a more specific deprecation if the function is `async`', function() { + jasmine.getEnv().requireAsyncAwait(); + eval('var fn = async function(done){};'); + var deprecated = jasmine.createSpy('deprecated'), + queueRunner = new jasmineUnderTest.QueueRunner({ + queueableFns: [{fn: fn}], + deprecated: deprecated + }); + + queueRunner.execute(); + + expect(deprecated).toHaveBeenCalledWith('An asynchronous ' + + 'before/it/after function was defined with the async keyword but ' + + 'also took a done callback. This is not supported and will stop ' + + 'working in the future. Either remove the done callback ' + + '(recommended) or remove the async keyword.' + ); + }); }); it('passes the error instance to exception handlers in HTML browsers', function() { diff --git a/src/core/QueueRunner.js b/src/core/QueueRunner.js index df383632..f009c3b5 100644 --- a/src/core/QueueRunner.js +++ b/src/core/QueueRunner.js @@ -123,7 +123,8 @@ getJasmineRequireObj().QueueRunner = function(j$) { }), errored = false, queueableFn = self.queueableFns[iterativeIndex], - timeoutId; + timeoutId, + maybeThenable; next.fail = function nextFail() { self.fail.apply(null, arguments); @@ -151,7 +152,7 @@ getJasmineRequireObj().QueueRunner = function(j$) { try { if (queueableFn.fn.length === 0) { - var maybeThenable = queueableFn.fn.call(self.userContext); + maybeThenable = queueableFn.fn.call(self.userContext); if (maybeThenable && j$.isFunction_(maybeThenable.then)) { maybeThenable.then(next, onPromiseRejection); @@ -159,7 +160,8 @@ getJasmineRequireObj().QueueRunner = function(j$) { return { completedSynchronously: false }; } } else { - queueableFn.fn.call(self.userContext, next); + maybeThenable = queueableFn.fn.call(self.userContext, next); + this.diagnoseConflictingAsync_(queueableFn.fn, maybeThenable); completedSynchronously = false; return { completedSynchronously: false }; } @@ -212,5 +214,24 @@ getJasmineRequireObj().QueueRunner = function(j$) { }); }; + QueueRunner.prototype.diagnoseConflictingAsync_ = function(fn, retval) { + if (retval && j$.isFunction_(retval.then)) { + // Issue a warning that matches the user's code + if (j$.isAsyncFunction_(fn)) { + this.deprecated('An asynchronous before/it/after ' + + 'function was defined with the async keyword but also took a ' + + 'done callback. This is not supported and will stop working in' + + ' the future. Either remove the done callback (recommended) or ' + + 'remove the async keyword.'); + } else { + this.deprecated('An asynchronous before/it/after ' + + 'function took a done callback but also returned a promise. ' + + 'This is not supported and will stop working in the future. ' + + 'Either remove the done callback (recommended) or change the ' + + 'function to not return a promise.'); + } + } + }; + return QueueRunner; };