From bbb1b69b2e9def375e73c6eb397f8ad148a7c62b Mon Sep 17 00:00:00 2001 From: Steve Gravrock Date: Sun, 8 May 2022 15:29:58 -0700 Subject: [PATCH] More reliably report errors that occur late in the suite/spec lifecycle Previously, an error that occurred after Jasmine started to report the suiteDone or specDone event for the current runable would not be reliably reported. Now such an error is reported on the nearest ancestor suite whose suiteDone event has not yet been reported. --- lib/jasmine-core/jasmine.js | 107 ++++++++--- spec/core/SpecSpec.js | 107 ++++++++++- spec/core/SuiteSpec.js | 100 ++++++++++- spec/core/integration/EnvSpec.js | 296 ++++++++++++++++++++++++++----- src/core/Env.js | 66 +++++-- src/core/Spec.js | 15 +- src/core/Suite.js | 22 ++- src/core/TreeProcessor.js | 4 +- 8 files changed, 610 insertions(+), 107 deletions(-) diff --git a/lib/jasmine-core/jasmine.js b/lib/jasmine-core/jasmine.js index 7983656a..fd3442ca 100644 --- a/lib/jasmine-core/jasmine.js +++ b/lib/jasmine-core/jasmine.js @@ -813,14 +813,20 @@ getJasmineRequireObj().Spec = function(j$) { properties: null, debugLogs: null }; + + this.reportedDone = false; } Spec.prototype.addExpectationResult = function(passed, data, isError) { - var expectationResult = this.expectationResultFactory(data); + const expectationResult = this.expectationResultFactory(data); if (passed) { this.result.passedExpectations.push(expectationResult); } else { - this.result.failedExpectations.push(expectationResult); + if (this.reportedDone) { + this.onLateError(expectationResult); + } else { + this.result.failedExpectations.push(expectationResult); + } if (this.throwOnExpectationFailure && !isError) { throw new j$.errors.ExpectationFailed(); @@ -874,7 +880,7 @@ getJasmineRequireObj().Spec = function(j$) { isLeaf: true, queueableFns: [...fns.befores, this.queueableFn, ...fns.afters], onException: function() { - self.onException.apply(self, arguments); + self.handleException.apply(self, arguments); }, onMultipleDone: function() { // Issue a deprecation. Include the context ourselves and pass @@ -924,9 +930,10 @@ getJasmineRequireObj().Spec = function(j$) { debugLogs: null }; this.markedPending = this.markedExcluding; + this.reportedDone = false; }; - Spec.prototype.onException = function onException(e) { + Spec.prototype.handleException = function handleException(e) { if (Spec.isPendingSpecException(e)) { this.pend(extractCustomPendingMessage(e)); return; @@ -1487,16 +1494,21 @@ getJasmineRequireObj().Env = function(j$) { } }; + // TODO: Unify recordLateError with recordLateExpectation? The extra + // diagnostic info added by the latter is probably useful in most cases. function recordLateError(error) { - const result = expectationResultFactory({ - error, - passed: false, - matcherName: '', - expected: '', - actual: '' - }); - result.globalErrorType = 'lateError'; - topSuite.result.failedExpectations.push(result); + const isExpectationResult = + error.matcherName !== undefined && error.passed !== undefined; + const result = isExpectationResult + ? error + : expectationResultFactory({ + error, + passed: false, + matcherName: '', + expected: '', + actual: '' + }); + routeLateFailure(result); } function recordLateExpectation(runable, runableType, result) { @@ -1527,6 +1539,26 @@ getJasmineRequireObj().Env = function(j$) { topSuite.result.failedExpectations.push(delayedExpectationResult); } + function routeLateFailure(expectationResult) { + // Report the result on the nearest ancestor suite that hasn't already + // been reported done. + for (let r = currentRunnable(); r; r = r.parentSuite) { + if (!r.reportedDone) { + if (r === topSuite) { + expectationResult.globalErrorType = 'lateError'; + } + + r.result.failedExpectations.push(expectationResult); + return; + } + } + + // If we get here, all results have been reported and there's nothing we + // can do except log the result and hope the user sees it. + console.error('Jasmine received a result after the suite finished:'); + console.error(expectationResult); + } + var asyncExpectationFactory = function(actual, spec, runableType) { return j$.Expectation.asyncFactory({ matchersUtil: makeMatchersUtil(), @@ -1682,7 +1714,7 @@ getJasmineRequireObj().Env = function(j$) { options.onException = options.onException || function(e) { - (currentRunnable() || topSuite).onException(e); + (currentRunnable() || topSuite).handleException(e); }; options.deprecated = self.deprecated; @@ -1867,10 +1899,10 @@ getJasmineRequireObj().Env = function(j$) { if (suite.hadBeforeAllFailure) { reportChildrenOfBeforeAllFailure(suite).then(function() { - reporter.suiteDone(result, next); + reportSuiteDone(suite, result, next); }); } else { - reporter.suiteDone(result, next); + reportSuiteDone(suite, result, next); } }, orderChildren: function(node) { @@ -1960,6 +1992,7 @@ getJasmineRequireObj().Env = function(j$) { failedExpectations: topSuite.result.failedExpectations, deprecationWarnings: topSuite.result.deprecationWarnings }; + topSuite.reportedDone = true; reporter.jasmineDone(jasmineDoneInfo, function() { done(jasmineDoneInfo); }); @@ -2004,7 +2037,7 @@ getJasmineRequireObj().Env = function(j$) { child.result.status = 'failed'; await new Promise(function(resolve) { - reporter.specDone(child.result, resolve); + reportSpecDone(child, child.result, resolve); }); } } @@ -2218,7 +2251,7 @@ getJasmineRequireObj().Env = function(j$) { } if (declarationError) { - suite.onException(declarationError); + suite.handleException(declarationError); } currentDeclarationSuite = parentSuite; @@ -2284,7 +2317,7 @@ getJasmineRequireObj().Env = function(j$) { hasFailures = true; } - reporter.specDone(result, next); + reportSpecDone(spec, result, next); } function specStarted(spec, next) { @@ -2294,6 +2327,16 @@ getJasmineRequireObj().Env = function(j$) { } }; + function reportSpecDone(spec, result, next) { + spec.reportedDone = true; + reporter.specDone(result, next); + } + + function reportSuiteDone(suite, result, next) { + suite.reportedDone = true; + reporter.suiteDone(result, next); + } + this.it_ = function(description, fn, timeout) { ensureIsNotNested('it'); // it() sometimes doesn't have a fn argument, so only check the type if @@ -9645,7 +9688,7 @@ getJasmineRequireObj().Suite = function(j$) { this.throwOnExpectationFailure = !!attrs.throwOnExpectationFailure; this.autoCleanClosures = attrs.autoCleanClosures === undefined ? true : !!attrs.autoCleanClosures; - this.onLateError = attrs.onLateError; + this.onLateError = attrs.onLateError || function() {}; this.beforeFns = []; this.afterFns = []; @@ -9779,6 +9822,7 @@ getJasmineRequireObj().Suite = function(j$) { this.children.forEach(function(child) { child.reset(); }); + this.reportedDone = false; }; Suite.prototype.addChild = function(child) { @@ -9820,7 +9864,7 @@ getJasmineRequireObj().Suite = function(j$) { return j$.UserContext.fromExisting(this.sharedUserContext()); }; - Suite.prototype.onException = function() { + Suite.prototype.handleException = function() { if (arguments[0] instanceof j$.errors.ExpectationFailed) { return; } @@ -9838,7 +9882,11 @@ getJasmineRequireObj().Suite = function(j$) { failedExpectation.globalErrorType = 'afterAll'; } - this.result.failedExpectations.push(failedExpectation); + if (this.reportedDone) { + this.onLateError(failedExpectation); + } else { + this.result.failedExpectations.push(failedExpectation); + } }; Suite.prototype.onMultipleDone = function() { @@ -9865,8 +9913,15 @@ getJasmineRequireObj().Suite = function(j$) { Suite.prototype.addExpectationResult = function() { if (isFailure(arguments)) { - var data = arguments[1]; - this.result.failedExpectations.push(this.expectationResultFactory(data)); + const data = arguments[1]; + const expectationResult = this.expectationResultFactory(data); + + if (this.reportedDone) { + this.onLateError(expectationResult); + } else { + this.result.failedExpectations.push(expectationResult); + } + if (this.throwOnExpectationFailure) { throw new j$.errors.ExpectationFailed(); } @@ -10020,7 +10075,7 @@ getJasmineRequireObj().TreeProcessor = function() { queueableFns: childFns, userContext: tree.sharedUserContext(), onException: function() { - tree.onException.apply(tree, arguments); + tree.handleException.apply(tree, arguments); }, onComplete: done, onMultipleDone: tree.onMultipleDone @@ -10196,7 +10251,7 @@ getJasmineRequireObj().TreeProcessor = function() { queueableFns: [onStart].concat(wrapChildren(node, segmentNumber)), userContext: node.sharedUserContext(), onException: function() { - node.onException.apply(node, arguments); + node.handleException.apply(node, arguments); }, onMultipleDone: node.onMultipleDone ? node.onMultipleDone.bind(node) diff --git a/spec/core/SpecSpec.js b/spec/core/SpecSpec.js index 2d93aec6..31ba52a8 100644 --- a/spec/core/SpecSpec.js +++ b/spec/core/SpecSpec.js @@ -404,6 +404,105 @@ describe('Spec', function() { ]); }); + it('forwards late expectation failures to onLateError', function() { + const onLateError = jasmine.createSpy('onLateError'); + const expectationResultFactory = jasmine + .createSpy('expectationResultFactory') + .and.returnValue('built expectation result'); + const spec = new jasmineUnderTest.Spec({ + expectationResultFactory, + onLateError, + queueableFn: { fn: function() {} } + }); + const data = { + matcherName: '', + passed: false, + expected: '', + actual: '', + error: new Error('nope') + }; + + spec.reportedDone = true; + spec.addExpectationResult(false, data, true); + + expect(expectationResultFactory).toHaveBeenCalledWith(data); + expect(onLateError).toHaveBeenCalledWith('built expectation result'); + expect(spec.result.failedExpectations).toEqual([]); + }); + + it('does not forward non-late expectation failures to onLateError', function() { + const onLateError = jasmine.createSpy('onLateError'); + const spec = new jasmineUnderTest.Spec({ + expectationResultFactory: r => r, + onLateError, + queueableFn: { fn: function() {} } + }); + const data = { + matcherName: '', + passed: false, + expected: '', + actual: '', + error: new Error('nope') + }; + + spec.addExpectationResult(false, data, true); + + expect(onLateError).not.toHaveBeenCalled(); + }); + + it('forwards late handleException calls to onLateError', function() { + const onLateError = jasmine.createSpy('onLateError'); + const expectationResultFactory = jasmine + .createSpy('expectationResultFactory') + .and.returnValue('built expectation result'); + const spec = new jasmineUnderTest.Spec({ + expectationResultFactory, + onLateError, + queueableFn: { fn: function() {} } + }); + const error = new Error('oops'); + + spec.reportedDone = true; + spec.handleException(error); + + expect(expectationResultFactory).toHaveBeenCalledWith({ + matcherName: '', + passed: false, + expected: '', + actual: '', + error + }); + expect(onLateError).toHaveBeenCalledWith('built expectation result'); + expect(spec.result.failedExpectations).toEqual([]); + }); + + it('does not forward non-late handleException calls to onLateError', function() { + const onLateError = jasmine.createSpy('onLateError'); + const spec = new jasmineUnderTest.Spec({ + expectationResultFactory: r => r, + onLateError, + queueableFn: { fn: function() {} } + }); + const error = new Error('oops'); + + spec.handleException(error); + + expect(onLateError).not.toHaveBeenCalled(); + expect(spec.result.failedExpectations.length).toEqual(1); + }); + + it('clears the reportedDone flag when reset', function() { + const spec = new jasmineUnderTest.Spec({ + expectationResultFactory: r => r, + queueableFn: { fn: function() {} } + }); + spec.reportedDone = true; + + spec.reset(); + + expect(spec.reportedDone).toBeFalse(); + }); + it('does not throw an ExpectationFailed error when handling an error', function() { const resultCallback = jasmine.createSpy('resultCallback'), spec = new jasmineUnderTest.Spec({ @@ -418,7 +517,7 @@ describe('Spec', function() { throwOnExpectationFailure: true }); - spec.onException('failing exception'); + spec.handleException('failing exception'); }); it('can return its full name', function() { @@ -490,7 +589,7 @@ describe('Spec', function() { resultCallback: resultCallback }); - spec.onException('foo'); + spec.handleException('foo'); spec.execute(); const args = fakeQueueRunner.calls.mostRecent().args[0]; @@ -518,7 +617,7 @@ describe('Spec', function() { resultCallback: resultCallback }); - spec.onException(new jasmineUnderTest.errors.ExpectationFailed()); + spec.handleException(new jasmineUnderTest.errors.ExpectationFailed()); spec.execute(); const args = fakeQueueRunner.calls.mostRecent().args[0]; @@ -636,7 +735,7 @@ describe('Spec', function() { resultCallback: resultCallback, queueRunnerFactory: function(config) { spec.debugLog('msg'); - spec.onException(new Error('nope')); + spec.handleException(new Error('nope')); for (const fn of config.queueableFns) { fn.fn(); } diff --git a/spec/core/SuiteSpec.js b/spec/core/SuiteSpec.js index 6cac4540..5a634177 100644 --- a/spec/core/SuiteSpec.js +++ b/spec/core/SuiteSpec.js @@ -154,11 +154,107 @@ describe('Suite', function() { it('does not add an additional failure when an expectation fails', function() { const suite = new jasmineUnderTest.Suite({}); - suite.onException(new jasmineUnderTest.errors.ExpectationFailed()); + suite.handleException(new jasmineUnderTest.errors.ExpectationFailed()); expect(suite.getResult().failedExpectations).toEqual([]); }); + it('forwards late expectation failures to onLateError', function() { + const onLateError = jasmine.createSpy('onLateError'); + const expectationResultFactory = jasmine + .createSpy('expectationResultFactory') + .and.returnValue('built expectation result'); + const suite = new jasmineUnderTest.Suite({ + expectationResultFactory, + onLateError + }); + const data = { + matcherName: '', + passed: false, + expected: '', + actual: '', + error: new Error('nope') + }; + + suite.reportedDone = true; + suite.addExpectationResult(false, data, true); + + expect(expectationResultFactory).toHaveBeenCalledWith(data); + expect(onLateError).toHaveBeenCalledWith('built expectation result'); + expect(suite.result.failedExpectations).toEqual([]); + }); + + it('does not forward non-late expectation failures to onLateError', function() { + const onLateError = jasmine.createSpy('onLateError'); + const suite = new jasmineUnderTest.Suite({ + expectationResultFactory: r => r, + onLateError + }); + const data = { + matcherName: '', + passed: false, + expected: '', + actual: '', + error: new Error('nope') + }; + + suite.addExpectationResult(false, data, true); + + expect(onLateError).not.toHaveBeenCalled(); + expect(suite.result.failedExpectations.length).toEqual(1); + }); + + it('forwards late handleException calls to onLateError', function() { + const onLateError = jasmine.createSpy('onLateError'); + const expectationResultFactory = jasmine + .createSpy('expectationResultFactory') + .and.returnValue('built expectation result'); + const suite = new jasmineUnderTest.Suite({ + expectationResultFactory, + onLateError + }); + const error = new Error('oops'); + + suite.reportedDone = true; + suite.handleException(error); + + expect(expectationResultFactory).toHaveBeenCalledWith({ + matcherName: '', + passed: false, + expected: '', + actual: '', + error + }); + expect(onLateError).toHaveBeenCalledWith('built expectation result'); + expect(suite.result.failedExpectations).toEqual([]); + }); + + it('does not forward non-late handleException calls to onLateError', function() { + const onLateError = jasmine.createSpy('onLateError'); + const suite = new jasmineUnderTest.Suite({ + expectationResultFactory: r => r, + onLateError + }); + const error = new Error('oops'); + + suite.handleException(error); + + expect(onLateError).not.toHaveBeenCalled(); + expect(suite.result.failedExpectations.length).toEqual(1); + }); + + it('clears the reportedDone flag when reset', function() { + const suite = new jasmineUnderTest.Suite({ + expectationResultFactory: r => r, + queueableFn: { fn: function() {} } + }); + suite.reportedDone = true; + + suite.reset(); + + expect(suite.reportedDone).toBeFalse(); + }); + it('calls timer to compute duration', function() { const suite = new jasmineUnderTest.Suite({ env: env, @@ -255,7 +351,7 @@ describe('Suite', function() { return error; } }); - suite.onException(new Error()); + suite.handleException(new Error()); suite.reset(); diff --git a/spec/core/integration/EnvSpec.js b/spec/core/integration/EnvSpec.js index 53e5cd1b..45a72f2b 100644 --- a/spec/core/integration/EnvSpec.js +++ b/spec/core/integration/EnvSpec.js @@ -491,6 +491,63 @@ describe('Env integration', function() { ); }); + describe('When the running spec has reported specDone', function() { + it('routes async errors to an ancestor suite', async function() { + const global = { + setTimeout: function(fn, delay) { + return setTimeout(fn, delay); + }, + clearTimeout: function(fn) { + clearTimeout(fn); + } + }; + spyOn(jasmineUnderTest, 'getGlobal').and.returnValue(global); + + const realClearStack = jasmineUnderTest.getClearStack(global); + const clearStackCallbacks = {}; + let clearStackCallCount = 0; + spyOn(jasmineUnderTest, 'getClearStack').and.returnValue(function(fn) { + clearStackCallCount++; + + if (clearStackCallbacks[clearStackCallCount]) { + clearStackCallbacks[clearStackCallCount](); + } + + realClearStack(fn); + }); + + env.cleanup_(); + env = new jasmineUnderTest.Env(); + + let suiteErrors = []; + env.addReporter({ + suiteDone: function(result) { + const messages = result.failedExpectations.map(e => e.message); + suiteErrors = suiteErrors.concat(messages); + }, + specDone: function() { + clearStackCallbacks[clearStackCallCount + 1] = function() { + global.onerror('fail at the end of the reporter queue'); + }; + clearStackCallbacks[clearStackCallCount + 2] = function() { + global.onerror('fail at the end of the spec queue'); + }; + } + }); + + env.describe('A suite', function() { + env.it('is finishing when the failure occurs', function() {}); + }); + + await env.execute(); + + expect(suiteErrors).toEqual([ + 'fail at the end of the reporter queue thrown', + 'fail at the end of the spec queue thrown' + ]); + }); + }); + it('routes async errors to a running suite', function(done) { const global = { setTimeout: function(fn, delay) { @@ -543,7 +600,7 @@ describe('Env integration', function() { }); describe('When the running suite has reported suiteDone', function() { - it('routes async errors to the parent suite', async function() { + it('routes async errors to an ancestor suite', async function() { const global = { setTimeout: function(fn, delay) { return setTimeout(fn, delay); @@ -570,10 +627,12 @@ describe('Env integration', function() { env.cleanup_(); env = new jasmineUnderTest.Env(); - const reporter = jasmine.createSpyObj('fakeReporter', ['suiteDone']); - env.addReporter(reporter); + let suiteErrors = []; env.addReporter({ suiteDone: function(result) { + const messages = result.failedExpectations.map(e => e.message); + suiteErrors = suiteErrors.concat(messages); + if (result.description === 'A nested suite') { clearStackCallbacks[clearStackCallCount + 1] = function() { global.onerror('fail at the end of the reporter queue'); @@ -593,18 +652,120 @@ describe('Env integration', function() { await env.execute(); - expect(reporter.suiteDone).toHaveFailedExpectationsForRunnable( - 'A suite', - [ - 'fail at the end of the reporter queue thrown', - 'fail at the end of the suite queue thrown' - ] - ); + expect(suiteErrors).toEqual([ + 'fail at the end of the reporter queue thrown', + 'fail at the end of the suite queue thrown' + ]); }); }); describe('When the env has started reporting jasmineDone', function() { - it('does something reasonable'); + it('logs the error to the console', async function() { + const global = { + setTimeout: function(fn, delay) { + return setTimeout(fn, delay); + }, + clearTimeout: function(fn, delay) { + clearTimeout(fn, delay); + } + }; + spyOn(jasmineUnderTest, 'getGlobal').and.returnValue(global); + env.cleanup_(); + env = new jasmineUnderTest.Env(); + + spyOn(console, 'error'); + + env.addReporter({ + jasmineDone: function() { + global.onerror('a very late error'); + } + }); + + env.it('a spec', function() {}); + + await env.execute(); + + /* eslint-disable-next-line no-console */ + expect(console.error).toHaveBeenCalledWith( + 'Jasmine received a result after the suite finished:' + ); + /* eslint-disable-next-line no-console */ + expect(console.error).toHaveBeenCalledWith( + jasmine.objectContaining({ + message: 'a very late error thrown', + globalErrorType: 'afterAll' + }) + ); + }); + }); + + it('routes all errors that occur during stack clearing somewhere', async function() { + const global = { + setTimeout: function(fn, delay) { + return setTimeout(fn, delay); + }, + clearTimeout: function(fn) { + clearTimeout(fn); + } + }; + spyOn(jasmineUnderTest, 'getGlobal').and.returnValue(global); + + const realClearStack = jasmineUnderTest.getClearStack(global); + let clearStackCallCount = 0; + let jasmineDone = false; + const expectedErrors = []; + const expectedErrorsAfterJasmineDone = []; + spyOn(jasmineUnderTest, 'getClearStack').and.returnValue(function(fn) { + clearStackCallCount++; + const msg = `Error in clearStack #${clearStackCallCount}`; + + if (jasmineDone) { + expectedErrorsAfterJasmineDone.push(`${msg} thrown`); + } else { + expectedErrors.push(`${msg} thrown`); + } + + global.onerror(msg); + realClearStack(fn); + }); + spyOn(console, 'error'); + + env.cleanup_(); + env = new jasmineUnderTest.Env(); + + const receivedErrors = []; + function logErrors(event) { + for (const failure of event.failedExpectations) { + receivedErrors.push(failure.message); + } + } + env.addReporter({ + specDone: logErrors, + suiteDone: logErrors, + jasmineDone: function(event) { + jasmineDone = true; + logErrors(event); + } + }); + + env.describe('A suite', function() { + env.it('is finishing when the failure occurs', function() {}); + }); + + await env.execute(); + + expect(receivedErrors.length).toEqual(expectedErrors.length); + + for (const e of expectedErrors) { + expect(receivedErrors).toContain(e); + } + + for (const message of expectedErrorsAfterJasmineDone) { + /* eslint-disable-next-line no-console */ + expect(console.error).toHaveBeenCalledWith( + jasmine.objectContaining({ message }) + ); + } }); }); @@ -642,11 +803,18 @@ describe('Env integration', function() { }); }); - it('reports multiple calls to done in a non-top suite as errors', function(done) { - const reporter = jasmine.createSpyObj('fakeReporter', ['jasmineDone']); + it('reports multiple calls to done in a non-top suite as errors', async function() { + const reporter = jasmine.createSpyObj('fakeReporter', [ + 'jasmineDone', + 'suiteDone' + ]); const message = "An asynchronous beforeAll or afterAll function called its 'done' " + 'callback more than once.\n(in suite: a suite)'; + let lateDone; + reporter.suiteDone.and.callFake(function() { + lateDone(); + }); env.addReporter(reporter); env.describe('a suite', function() { @@ -658,31 +826,50 @@ describe('Env integration', function() { env.afterAll(function(innerDone) { innerDone(); innerDone(); + lateDone = innerDone; }); }); - env.execute(null, function() { - expect(reporter.jasmineDone).toHaveBeenCalled(); - const errors = reporter.jasmineDone.calls.argsFor(0)[0] - .failedExpectations; - expect(errors.length).toEqual(2); - expect(errors[0].message) - .withContext('suite beforeAll') - .toContain(message); - expect(errors[0].globalErrorType).toEqual('lateError'); - expect(errors[1].message) - .withContext('suite afterAll') - .toContain(message); - expect(errors[1].globalErrorType).toEqual('lateError'); - done(); - }); + await env.execute(); + + expect(reporter.suiteDone).toHaveBeenCalled(); + const suiteErrors = reporter.suiteDone.calls.argsFor(0)[0] + .failedExpectations; + expect(suiteErrors.length).toEqual(2); + expect(suiteErrors[0].message) + .withContext('suite beforeAll') + .toContain(message); + expect(suiteErrors[1].message) + .withContext('suite afterAll') + .toContain(message); + + expect(reporter.jasmineDone).toHaveBeenCalled(); + const topErrors = reporter.jasmineDone.calls.argsFor(0)[0] + .failedExpectations; + expect(topErrors.length).toEqual(1); + expect(topErrors[0].message) + .withContext('late suite afterAll') + .toContain(message); + expect(topErrors[0].globalErrorType).toEqual('lateError'); + expect(topErrors[0].globalErrorType).toEqual('lateError'); }); - it('reports multiple calls to done in a spec as errors', function(done) { - const reporter = jasmine.createSpyObj('fakeReporter', ['jasmineDone']); + it('reports multiple calls to done in a spec as errors', async function() { + const reporter = jasmine.createSpyObj('fakeReporter', [ + 'specDone', + 'suiteDone', + 'jasmineDone' + ]); const message = 'An asynchronous spec, beforeEach, or afterEach function called its ' + "'done' callback more than once.\n(in spec: a suite a spec)"; + let lateDone; + reporter.specDone.and.callFake(function() { + lateDone(); + }); + reporter.suiteDone.and.callFake(function() { + lateDone(); + }); env.addReporter(reporter); env.describe('a suite', function() { @@ -697,28 +884,39 @@ describe('Env integration', function() { env.afterEach(function(innerDone) { innerDone(); innerDone(); + lateDone = innerDone; }); }); - env.execute(null, function() { - expect(reporter.jasmineDone).toHaveBeenCalled(); - const errors = reporter.jasmineDone.calls.argsFor(0)[0] - .failedExpectations; - expect(errors.length).toEqual(3); - expect(errors[0].message) - .withContext('error caused by beforeEach') - .toContain(message); - expect(errors[0].globalErrorType).toEqual('lateError'); - expect(errors[1].message) - .withContext('error caused by it') - .toContain(message); - expect(errors[1].globalErrorType).toEqual('lateError'); - expect(errors[2].message) - .withContext('error caused by afterEach') - .toContain(message); - expect(errors[2].globalErrorType).toEqual('lateError'); - done(); - }); + await env.execute(); + + expect(reporter.specDone).toHaveBeenCalled(); + const specErrors = reporter.specDone.calls.argsFor(0)[0].failedExpectations; + expect(specErrors.length).toEqual(3); + expect(specErrors[0].message) + .withContext('error caused by beforeEach') + .toContain(message); + expect(specErrors[1].message) + .withContext('error caused by it') + .toContain(message); + expect(specErrors[2].message) + .withContext('error caused by afterEach') + .toContain(message); + + const suiteErrors = reporter.suiteDone.calls.argsFor(0)[0] + .failedExpectations; + expect(suiteErrors.length).toEqual(1); + expect(suiteErrors[0].message) + .withContext('late error caused by afterEach') + .toContain(message); + + const topErrors = reporter.jasmineDone.calls.argsFor(0)[0] + .failedExpectations; + expect(topErrors.length).toEqual(1); + expect(topErrors[0].message) + .withContext('really late error caused by afterEach') + .toContain(message); + expect(topErrors[0].globalErrorType).toEqual('lateError'); }); it('reports multiple calls to done in reporters as errors', function(done) { diff --git a/src/core/Env.js b/src/core/Env.js index 6b5f78b0..80070801 100644 --- a/src/core/Env.js +++ b/src/core/Env.js @@ -342,16 +342,21 @@ getJasmineRequireObj().Env = function(j$) { } }; + // TODO: Unify recordLateError with recordLateExpectation? The extra + // diagnostic info added by the latter is probably useful in most cases. function recordLateError(error) { - const result = expectationResultFactory({ - error, - passed: false, - matcherName: '', - expected: '', - actual: '' - }); - result.globalErrorType = 'lateError'; - topSuite.result.failedExpectations.push(result); + const isExpectationResult = + error.matcherName !== undefined && error.passed !== undefined; + const result = isExpectationResult + ? error + : expectationResultFactory({ + error, + passed: false, + matcherName: '', + expected: '', + actual: '' + }); + routeLateFailure(result); } function recordLateExpectation(runable, runableType, result) { @@ -382,6 +387,26 @@ getJasmineRequireObj().Env = function(j$) { topSuite.result.failedExpectations.push(delayedExpectationResult); } + function routeLateFailure(expectationResult) { + // Report the result on the nearest ancestor suite that hasn't already + // been reported done. + for (let r = currentRunnable(); r; r = r.parentSuite) { + if (!r.reportedDone) { + if (r === topSuite) { + expectationResult.globalErrorType = 'lateError'; + } + + r.result.failedExpectations.push(expectationResult); + return; + } + } + + // If we get here, all results have been reported and there's nothing we + // can do except log the result and hope the user sees it. + console.error('Jasmine received a result after the suite finished:'); + console.error(expectationResult); + } + var asyncExpectationFactory = function(actual, spec, runableType) { return j$.Expectation.asyncFactory({ matchersUtil: makeMatchersUtil(), @@ -537,7 +562,7 @@ getJasmineRequireObj().Env = function(j$) { options.onException = options.onException || function(e) { - (currentRunnable() || topSuite).onException(e); + (currentRunnable() || topSuite).handleException(e); }; options.deprecated = self.deprecated; @@ -722,10 +747,10 @@ getJasmineRequireObj().Env = function(j$) { if (suite.hadBeforeAllFailure) { reportChildrenOfBeforeAllFailure(suite).then(function() { - reporter.suiteDone(result, next); + reportSuiteDone(suite, result, next); }); } else { - reporter.suiteDone(result, next); + reportSuiteDone(suite, result, next); } }, orderChildren: function(node) { @@ -815,6 +840,7 @@ getJasmineRequireObj().Env = function(j$) { failedExpectations: topSuite.result.failedExpectations, deprecationWarnings: topSuite.result.deprecationWarnings }; + topSuite.reportedDone = true; reporter.jasmineDone(jasmineDoneInfo, function() { done(jasmineDoneInfo); }); @@ -859,7 +885,7 @@ getJasmineRequireObj().Env = function(j$) { child.result.status = 'failed'; await new Promise(function(resolve) { - reporter.specDone(child.result, resolve); + reportSpecDone(child, child.result, resolve); }); } } @@ -1073,7 +1099,7 @@ getJasmineRequireObj().Env = function(j$) { } if (declarationError) { - suite.onException(declarationError); + suite.handleException(declarationError); } currentDeclarationSuite = parentSuite; @@ -1139,7 +1165,7 @@ getJasmineRequireObj().Env = function(j$) { hasFailures = true; } - reporter.specDone(result, next); + reportSpecDone(spec, result, next); } function specStarted(spec, next) { @@ -1149,6 +1175,16 @@ getJasmineRequireObj().Env = function(j$) { } }; + function reportSpecDone(spec, result, next) { + spec.reportedDone = true; + reporter.specDone(result, next); + } + + function reportSuiteDone(suite, result, next) { + suite.reportedDone = true; + reporter.suiteDone(result, next); + } + this.it_ = function(description, fn, timeout) { ensureIsNotNested('it'); // it() sometimes doesn't have a fn argument, so only check the type if diff --git a/src/core/Spec.js b/src/core/Spec.js index f4bc6b54..c4c7101c 100644 --- a/src/core/Spec.js +++ b/src/core/Spec.js @@ -86,14 +86,20 @@ getJasmineRequireObj().Spec = function(j$) { properties: null, debugLogs: null }; + + this.reportedDone = false; } Spec.prototype.addExpectationResult = function(passed, data, isError) { - var expectationResult = this.expectationResultFactory(data); + const expectationResult = this.expectationResultFactory(data); if (passed) { this.result.passedExpectations.push(expectationResult); } else { - this.result.failedExpectations.push(expectationResult); + if (this.reportedDone) { + this.onLateError(expectationResult); + } else { + this.result.failedExpectations.push(expectationResult); + } if (this.throwOnExpectationFailure && !isError) { throw new j$.errors.ExpectationFailed(); @@ -147,7 +153,7 @@ getJasmineRequireObj().Spec = function(j$) { isLeaf: true, queueableFns: [...fns.befores, this.queueableFn, ...fns.afters], onException: function() { - self.onException.apply(self, arguments); + self.handleException.apply(self, arguments); }, onMultipleDone: function() { // Issue a deprecation. Include the context ourselves and pass @@ -197,9 +203,10 @@ getJasmineRequireObj().Spec = function(j$) { debugLogs: null }; this.markedPending = this.markedExcluding; + this.reportedDone = false; }; - Spec.prototype.onException = function onException(e) { + Spec.prototype.handleException = function handleException(e) { if (Spec.isPendingSpecException(e)) { this.pend(extractCustomPendingMessage(e)); return; diff --git a/src/core/Suite.js b/src/core/Suite.js index 43e3ce22..65c2579d 100644 --- a/src/core/Suite.js +++ b/src/core/Suite.js @@ -29,7 +29,7 @@ getJasmineRequireObj().Suite = function(j$) { this.throwOnExpectationFailure = !!attrs.throwOnExpectationFailure; this.autoCleanClosures = attrs.autoCleanClosures === undefined ? true : !!attrs.autoCleanClosures; - this.onLateError = attrs.onLateError; + this.onLateError = attrs.onLateError || function() {}; this.beforeFns = []; this.afterFns = []; @@ -163,6 +163,7 @@ getJasmineRequireObj().Suite = function(j$) { this.children.forEach(function(child) { child.reset(); }); + this.reportedDone = false; }; Suite.prototype.addChild = function(child) { @@ -204,7 +205,7 @@ getJasmineRequireObj().Suite = function(j$) { return j$.UserContext.fromExisting(this.sharedUserContext()); }; - Suite.prototype.onException = function() { + Suite.prototype.handleException = function() { if (arguments[0] instanceof j$.errors.ExpectationFailed) { return; } @@ -222,7 +223,11 @@ getJasmineRequireObj().Suite = function(j$) { failedExpectation.globalErrorType = 'afterAll'; } - this.result.failedExpectations.push(failedExpectation); + if (this.reportedDone) { + this.onLateError(failedExpectation); + } else { + this.result.failedExpectations.push(failedExpectation); + } }; Suite.prototype.onMultipleDone = function() { @@ -249,8 +254,15 @@ getJasmineRequireObj().Suite = function(j$) { Suite.prototype.addExpectationResult = function() { if (isFailure(arguments)) { - var data = arguments[1]; - this.result.failedExpectations.push(this.expectationResultFactory(data)); + const data = arguments[1]; + const expectationResult = this.expectationResultFactory(data); + + if (this.reportedDone) { + this.onLateError(expectationResult); + } else { + this.result.failedExpectations.push(expectationResult); + } + if (this.throwOnExpectationFailure) { throw new j$.errors.ExpectationFailed(); } diff --git a/src/core/TreeProcessor.js b/src/core/TreeProcessor.js index f93fc044..b81ce357 100644 --- a/src/core/TreeProcessor.js +++ b/src/core/TreeProcessor.js @@ -42,7 +42,7 @@ getJasmineRequireObj().TreeProcessor = function() { queueableFns: childFns, userContext: tree.sharedUserContext(), onException: function() { - tree.onException.apply(tree, arguments); + tree.handleException.apply(tree, arguments); }, onComplete: done, onMultipleDone: tree.onMultipleDone @@ -218,7 +218,7 @@ getJasmineRequireObj().TreeProcessor = function() { queueableFns: [onStart].concat(wrapChildren(node, segmentNumber)), userContext: node.sharedUserContext(), onException: function() { - node.onException.apply(node, arguments); + node.handleException.apply(node, arguments); }, onMultipleDone: node.onMultipleDone ? node.onMultipleDone.bind(node)