diff --git a/Gruntfile.js b/Gruntfile.js index 6d65e5e3..5891fd61 100644 --- a/Gruntfile.js +++ b/Gruntfile.js @@ -34,11 +34,17 @@ module.exports = function(grunt) { jasmine = new Jasmine({jasmineCore: jasmineCore}); jasmine.loadConfigFile('./spec/support/jasmine.json'); - jasmine.onComplete(function(passed) { - done(passed); - }); - jasmine.execute(); + jasmine.exitOnCompletion = false; + jasmine.execute().then( + result => { + done(result.overallStatus === 'passed'); + }, + err => { + console.error(err); + exit(1); + } + ); } ); diff --git a/lib/jasmine-core/jasmine.js b/lib/jasmine-core/jasmine.js index b0fda0da..4b9c456d 100644 --- a/lib/jasmine-core/jasmine.js +++ b/lib/jasmine-core/jasmine.js @@ -684,6 +684,7 @@ getJasmineRequireObj().Spec = function(j$) { }; this.expectationResultFactory = attrs.expectationResultFactory || function() {}; + this.deprecated = attrs.deprecated || function() {}; this.queueRunnerFactory = attrs.queueRunnerFactory || function() {}; this.catchingExceptions = attrs.catchingExceptions || @@ -778,6 +779,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')); @@ -785,7 +801,8 @@ getJasmineRequireObj().Spec = function(j$) { onComplete(); } }, - userContext: this.userContext() + userContext: this.userContext(), + runnableName: this.getFullName.bind(this) }; if (this.markedPending || excluded === true) { @@ -1586,7 +1603,8 @@ getJasmineRequireObj().Env = function(j$) { */ 'specDone' ], - queueRunnerFactory + queueRunnerFactory, + self.deprecated ); /** @@ -1984,6 +2002,7 @@ getJasmineRequireObj().Env = function(j$) { beforeAndAfterFns: beforeAndAfterFns(suite), expectationFactory: expectationFactory, asyncExpectationFactory: specAsyncExpectationFactory, + deprecated: self.deprecated, resultCallback: specResultCallback, getSpecName: function(spec) { return getSpecName(spec, suite); @@ -7423,10 +7442,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); @@ -7435,6 +7458,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) { @@ -7449,6 +7482,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, @@ -7506,6 +7540,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); }, @@ -7515,31 +7551,46 @@ getJasmineRequireObj().QueueRunner = function(j$) { } self.globalErrors.popListener(handleError); }), - next = once(function next(err) { - cleanup(); + next = once( + function next(err) { + cleanup(); - if (typeof err !== 'undefined') { - if (!(err instanceof StopExecutionError) && !err.jasmineMessage) { - self.fail(err); + if (typeof err !== 'undefined') { + if (!(err instanceof StopExecutionError) && !err.jasmineMessage) { + self.fail(err); + } + self.errored = errored = true; } - self.errored = errored = true; - } - function runNext() { - if (self.completeOnFirstError && errored) { - self.skipToCleanup(iterativeIndex); + function runNext() { + if (self.completeOnFirstError && errored) { + 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; @@ -7555,6 +7606,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 + @@ -7563,6 +7615,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); @@ -7668,7 +7723,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++) { @@ -7712,7 +7767,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 } + ); + } }); } @@ -9194,6 +9257,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]; @@ -9353,7 +9442,10 @@ getJasmineRequireObj().TreeProcessor = function() { onException: function() { tree.onException.apply(tree, arguments); }, - onComplete: done + onComplete: done, + onMultipleDone: tree.onMultipleDone + ? tree.onMultipleDone.bind(tree) + : null }); }; @@ -9525,7 +9617,10 @@ getJasmineRequireObj().TreeProcessor = function() { userContext: node.sharedUserContext(), onException: function() { node.onException.apply(node, arguments); - } + }, + onMultipleDone: node.onMultipleDone + ? node.onMultipleDone.bind(node) + : null }); } }; diff --git a/package.json b/package.json index 9edf722e..49566a8a 100644 --- a/package.json +++ b/package.json @@ -44,7 +44,7 @@ "grunt-contrib-concat": "^1.0.1", "grunt-css-url-embed": "^1.11.1", "grunt-sass": "^3.0.2", - "jasmine": "^3.4.0", + "jasmine": "github:jasmine/jasmine-npm#main", "jasmine-browser-runner": "github:jasmine/jasmine-browser#main", "jsdom": "^15.0.0", "load-grunt-tasks": "^4.0.0", diff --git a/spec/core/QueueRunnerSpec.js b/spec/core/QueueRunnerSpec.js index 955f3837..d40fbf2e 100644 --- a/spec/core/QueueRunnerSpec.js +++ b/spec/core/QueueRunnerSpec.js @@ -227,6 +227,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') }, @@ -302,13 +328,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() { @@ -319,13 +348,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() { diff --git a/spec/core/SpecSpec.js b/spec/core/SpecSpec.js index ef2e1b22..dabc698e 100644 --- a/spec/core/SpecSpec.js +++ b/spec/core/SpecSpec.js @@ -512,4 +512,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 } + ); + }); }); diff --git a/spec/core/SuiteSpec.js b/spec/core/SuiteSpec.js index 2a1cd757..60b96285 100644 --- a/spec/core/SuiteSpec.js +++ b/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 } + ); + }); + }); }); diff --git a/spec/core/TreeProcessorSpec.js b/spec/core/TreeProcessorSpec.js index 5c31f100..b494d1ac 100644 --- a/spec/core/TreeProcessorSpec.js +++ b/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'); diff --git a/spec/core/integration/EnvSpec.js b/spec/core/integration/EnvSpec.js index 5766f2bf..cd084b6b 100644 --- a/spec/core/integration/EnvSpec.js +++ b/spec/core/integration/EnvSpec.js @@ -461,7 +461,7 @@ describe('Env integration', function() { var global = { setTimeout: function(fn, delay) { - setTimeout(fn, delay); + return setTimeout(fn, delay); }, clearTimeout: function(fn, delay) { clearTimeout(fn, delay); @@ -509,6 +509,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']); @@ -1014,7 +1199,7 @@ describe('Env integration', function() { var globalSetTimeout = jasmine .createSpy('globalSetTimeout') .and.callFake(function(cb, t) { - setTimeout(cb, t); + return setTimeout(cb, t); }), delayedFunctionForGlobalClock = jasmine.createSpy( 'delayedFunctionForGlobalClock' @@ -1029,7 +1214,7 @@ describe('Env integration', function() { setTimeout: globalSetTimeout, clearTimeout: clearTimeout, setImmediate: function(cb) { - setTimeout(cb, 0); + return setTimeout(cb, 0); } } }); @@ -1104,7 +1289,7 @@ describe('Env integration', function() { setInterval: setInterval, clearInterval: clearInterval, setImmediate: function(cb) { - realSetTimeout(cb, 0); + return realSetTimeout(cb, 0); } } }); @@ -1193,10 +1378,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', @@ -2255,7 +2436,7 @@ describe('Env integration', function() { it('reports errors that occur during loading', function(done) { var global = { setTimeout: function(fn, delay) { - setTimeout(fn, delay); + return setTimeout(fn, delay); }, clearTimeout: function(fn, delay) { clearTimeout(fn, delay); @@ -2312,7 +2493,7 @@ describe('Env integration', function() { var originalOnerror = jasmine.createSpy('original onerror'); var global = { setTimeout: function(fn, delay) { - setTimeout(fn, delay); + return setTimeout(fn, delay); }, clearTimeout: function(fn, delay) { clearTimeout(fn, delay); @@ -2513,10 +2694,10 @@ describe('Env integration', function() { it('is "failed"', function(done) { var global = { setTimeout: function(fn, delay) { - setTimeout(fn, delay); + return setTimeout(fn, delay); }, clearTimeout: function(fn, delay) { - clearTimeout(fn, delay); + return clearTimeout(fn, delay); } }; spyOn(jasmineUnderTest, 'getGlobal').and.returnValue(global); diff --git a/src/core/Env.js b/src/core/Env.js index 8defeec5..2897bedc 100644 --- a/src/core/Env.js +++ b/src/core/Env.js @@ -609,7 +609,8 @@ getJasmineRequireObj().Env = function(j$) { */ 'specDone' ], - queueRunnerFactory + queueRunnerFactory, + self.deprecated ); /** @@ -1007,6 +1008,7 @@ getJasmineRequireObj().Env = function(j$) { beforeAndAfterFns: beforeAndAfterFns(suite), expectationFactory: expectationFactory, asyncExpectationFactory: specAsyncExpectationFactory, + deprecated: self.deprecated, resultCallback: specResultCallback, getSpecName: function(spec) { return getSpecName(spec, suite); diff --git a/src/core/QueueRunner.js b/src/core/QueueRunner.js index 57b70901..f131ec2d 100644 --- a/src/core/QueueRunner.js +++ b/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,31 +114,46 @@ getJasmineRequireObj().QueueRunner = function(j$) { } self.globalErrors.popListener(handleError); }), - next = once(function next(err) { - cleanup(); + next = once( + function next(err) { + cleanup(); - if (typeof err !== 'undefined') { - if (!(err instanceof StopExecutionError) && !err.jasmineMessage) { - self.fail(err); + if (typeof err !== 'undefined') { + if (!(err instanceof StopExecutionError) && !err.jasmineMessage) { + self.fail(err); + } + self.errored = errored = true; } - self.errored = errored = true; - } - function runNext() { - if (self.completeOnFirstError && errored) { - self.skipToCleanup(iterativeIndex); + function runNext() { + if (self.completeOnFirstError && errored) { + 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; @@ -137,6 +169,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 + @@ -145,6 +178,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); diff --git a/src/core/ReportDispatcher.js b/src/core/ReportDispatcher.js index d254fb59..6c3759ba 100644 --- a/src/core/ReportDispatcher.js +++ b/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 } + ); + } }); } diff --git a/src/core/Spec.js b/src/core/Spec.js index 82d7e235..4ab881a8 100644 --- a/src/core/Spec.js +++ b/src/core/Spec.js @@ -24,6 +24,7 @@ getJasmineRequireObj().Spec = function(j$) { }; this.expectationResultFactory = attrs.expectationResultFactory || function() {}; + this.deprecated = attrs.deprecated || function() {}; this.queueRunnerFactory = attrs.queueRunnerFactory || function() {}; this.catchingExceptions = attrs.catchingExceptions || @@ -118,6 +119,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')); @@ -125,7 +141,8 @@ getJasmineRequireObj().Spec = function(j$) { onComplete(); } }, - userContext: this.userContext() + userContext: this.userContext(), + runnableName: this.getFullName.bind(this) }; if (this.markedPending || excluded === true) { diff --git a/src/core/Suite.js b/src/core/Suite.js index a15be857..5f46561d 100644 --- a/src/core/Suite.js +++ b/src/core/Suite.js @@ -166,6 +166,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]; diff --git a/src/core/TreeProcessor.js b/src/core/TreeProcessor.js index 14798d77..f93fc044 100644 --- a/src/core/TreeProcessor.js +++ b/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 }); } }; diff --git a/src/html/_HTMLReporter.scss b/src/html/_HTMLReporter.scss index d2d58b0d..dc06ff95 100644 --- a/src/html/_HTMLReporter.scss +++ b/src/html/_HTMLReporter.scss @@ -1,3 +1,5 @@ +@use "sass:math"; + $line-height: 14px; $margin-unit: 14px; @@ -117,7 +119,7 @@ body { li { display: inline-block; - height: ($line-height / 2) + 3; + height: math.div($line-height, 2) + 3; width: $line-height; font-size: 16px; @@ -132,7 +134,7 @@ body { } &.jasmine-failed { - line-height: ($line-height / 2) + 2; + line-height: math.div($line-height, 2) + 2; &:before { color: $failing-color; @@ -284,8 +286,8 @@ body { padding-left: 0; &.jasmine-suite { - margin-top: $margin-unit/2; - margin-bottom: $margin-unit/2 + margin-top: math.div($margin-unit, 2); + margin-bottom: math.div($margin-unit, 2) } }