From 0170005015b68b0dfa872c35669f1e6a8d730869 Mon Sep 17 00:00:00 2001 From: Steve Gravrock Date: Sat, 24 Jul 2021 09:30:39 -0700 Subject: [PATCH] Treat any argument to the done callback as an error This reduces the risk of incorrectly passing a spec due to not correctly detecting that an argument is an `Error` instance. Detecting Error instances in a way that's reliable and portable across different browsers, TrustedTypes, and frames is difficult. [Finishes #178267587] --- lib/jasmine-core/jasmine.js | 8 +-- spec/core/QueueRunnerSpec.js | 120 ++++++----------------------------- src/core/QueueRunner.js | 8 +-- 3 files changed, 23 insertions(+), 113 deletions(-) diff --git a/lib/jasmine-core/jasmine.js b/lib/jasmine-core/jasmine.js index 8dca5bf2..c74a5805 100644 --- a/lib/jasmine-core/jasmine.js +++ b/lib/jasmine-core/jasmine.js @@ -7491,17 +7491,11 @@ getJasmineRequireObj().QueueRunner = function(j$) { next = once(function next(err) { cleanup(); - if (j$.isError_(err)) { + if (typeof err !== 'undefined') { if (!(err instanceof StopExecutionError) && !err.jasmineMessage) { 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." - ); } function runNext() { diff --git a/spec/core/QueueRunnerSpec.js b/spec/core/QueueRunnerSpec.js index 134cfc01..955f3837 100644 --- a/spec/core/QueueRunnerSpec.js +++ b/spec/core/QueueRunnerSpec.js @@ -150,109 +150,31 @@ describe('QueueRunner', function() { }); describe('When next is called with an argument', function() { - describe('that is an Error', function() { - it('explicitly fails and moves to the next function', function() { - var err = new Error('foo'), - queueableFn1 = { - fn: function(done) { - setTimeout(function() { - done(err); - }, 100); - } - }, - queueableFn2 = { fn: jasmine.createSpy('fn2') }, - failFn = jasmine.createSpy('fail'), - queueRunner = new jasmineUnderTest.QueueRunner({ - queueableFns: [queueableFn1, queueableFn2], - fail: failFn - }); + it('explicitly fails and moves to the next function', function() { + var err = 'anything except undefined', + queueableFn1 = { + fn: function(done) { + setTimeout(function() { + done(err); + }, 100); + } + }, + queueableFn2 = { fn: jasmine.createSpy('fn2') }, + failFn = jasmine.createSpy('fail'), + queueRunner = new jasmineUnderTest.QueueRunner({ + queueableFns: [queueableFn1, queueableFn2], + fail: failFn + }); - queueRunner.execute(); + queueRunner.execute(); - expect(failFn).not.toHaveBeenCalled(); - expect(queueableFn2.fn).not.toHaveBeenCalled(); + expect(failFn).not.toHaveBeenCalled(); + expect(queueableFn2.fn).not.toHaveBeenCalled(); - jasmine.clock().tick(100); + jasmine.clock().tick(100); - expect(failFn).toHaveBeenCalledWith(err); - expect(queueableFn2.fn).toHaveBeenCalled(); - }); - - it('does not log a deprecation', function() { - var err = new Error('foo'), - queueableFn1 = { - fn: function(done) { - setTimeout(function() { - done(err); - }, 100); - } - }, - deprecated = jasmine.createSpy('deprecated'), - queueRunner = new jasmineUnderTest.QueueRunner({ - queueableFns: [queueableFn1], - deprecated: deprecated - }); - - queueRunner.execute(); - - jasmine.clock().tick(100); - - expect(deprecated).not.toHaveBeenCalled(); - }); - }); - - describe('that is not an Error', function() { - it('logs a deprecation', function() { - var queueableFn1 = { - fn: function(done) { - setTimeout(function() { - done('not an Error'); - }, 100); - } - }, - deprecated = jasmine.createSpy('deprecated'), - queueRunner = new jasmineUnderTest.QueueRunner({ - queueableFns: [queueableFn1], - deprecated: deprecated - }); - - queueRunner.execute(); - - jasmine.clock().tick(100); - - expect(deprecated).toHaveBeenCalledWith( - '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." - ); - }); - - it('moves to the next function without failing', function() { - var queueableFn1 = { - fn: function(done) { - setTimeout(function() { - done('not an Error'); - }, 100); - } - }, - queueableFn2 = { fn: jasmine.createSpy('fn2') }, - failFn = jasmine.createSpy('fail'), - queueRunner = new jasmineUnderTest.QueueRunner({ - queueableFns: [queueableFn1, queueableFn2], - fail: failFn, - deprecated: function() {} - }); - - queueRunner.execute(); - - expect(failFn).not.toHaveBeenCalled(); - expect(queueableFn2.fn).not.toHaveBeenCalled(); - - jasmine.clock().tick(100); - - expect(failFn).not.toHaveBeenCalled(); - expect(queueableFn2.fn).toHaveBeenCalled(); - }); + expect(failFn).toHaveBeenCalledWith(err); + expect(queueableFn2.fn).toHaveBeenCalled(); }); }); diff --git a/src/core/QueueRunner.js b/src/core/QueueRunner.js index ce227108..57b70901 100644 --- a/src/core/QueueRunner.js +++ b/src/core/QueueRunner.js @@ -100,17 +100,11 @@ getJasmineRequireObj().QueueRunner = function(j$) { next = once(function next(err) { cleanup(); - if (j$.isError_(err)) { + if (typeof err !== 'undefined') { if (!(err instanceof StopExecutionError) && !err.jasmineMessage) { 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." - ); } function runNext() {