diff --git a/lib/jasmine-core/jasmine.js b/lib/jasmine-core/jasmine.js index 9e6d02f9..abb59a37 100644 --- a/lib/jasmine-core/jasmine.js +++ b/lib/jasmine-core/jasmine.js @@ -11539,19 +11539,7 @@ getJasmineRequireObj().TreeRunner = function(j$) { fns.unshift(start); if (config.detectLateRejectionHandling) { - // Conditional because the setTimeout imposes a significant performance - // penalty in suites with lots of fast specs. - const globalErrors = this.#globalErrors; - fns.push({ - fn: done => { - // setTimeout is necessary to trigger rejectionhandled events - // TODO: let clearStack know about this so it doesn't do redundant setTimeouts - this.#setTimeout(function() { - globalErrors.reportUnhandledRejections(); - done(); - }); - } - }); + fns.push(this.#lateUnhandledRejectionChecker()); } fns.push(complete); @@ -11560,11 +11548,32 @@ getJasmineRequireObj().TreeRunner = function(j$) { #executeSuiteSegment(suite, segmentNumber, done) { const isTopSuite = suite === this.#executionTree.topSuite; + const isExcluded = this.#executionTree.isExcluded(suite); + let befores = []; + let afters = []; + + if (suite.beforeAllFns.length > 0 && !isExcluded) { + befores = [...suite.beforeAllFns]; + if (this.#getConfig().detectLateRejectionHandling) { + befores.push(this.#lateUnhandledRejectionChecker()); + } + } + + if (suite.afterAllFns.length > 0 && !isExcluded) { + afters = [...suite.afterAllFns]; + if (this.#getConfig().detectLateRejectionHandling) { + afters.push(this.#lateUnhandledRejectionChecker()); + } + } + const children = isTopSuite ? this.#executionTree.childrenOfTopSuite() : this.#executionTree.childrenOfSuiteSegment(suite, segmentNumber); - const wrappedChildren = this.#wrapNodes(children); - const queueableFns = this.#addBeforeAndAfterAlls(suite, wrappedChildren); + const queueableFns = [ + ...befores, + ...this.#wrapNodes(children), + ...afters + ]; if (!isTopSuite) { queueableFns.unshift({ @@ -11595,6 +11604,25 @@ getJasmineRequireObj().TreeRunner = function(j$) { }); } + // Returns a queueable fn that reports any still-unhandled rejections in + // cases where detectLateRejectionHandling is enabled. Should only be called + // when detectLateRejectionHandling is enabled, because the setTimeout + // imposes a significant performance penalty in suites with lots of fast + // specs. + #lateUnhandledRejectionChecker() { + const globalErrors = this.#globalErrors; + return { + fn: done => { + // setTimeout is necessary to trigger rejectionhandled events + // TODO: let clearStack know about this so it doesn't do redundant setTimeouts + this.#setTimeout(function() { + globalErrors.reportUnhandledRejections(); + done(); + }); + } + }; + } + #suiteSegmentStart(suite, next) { this.#currentRunableTracker.pushSuite(suite); this.#runableResources.initForRunable(suite.id, suite.parentSuite.id); @@ -11684,16 +11712,6 @@ getJasmineRequireObj().TreeRunner = function(j$) { } } - #addBeforeAndAfterAlls(suite, wrappedChildren) { - if (this.#executionTree.isExcluded(suite)) { - return wrappedChildren; - } - - return suite.beforeAllFns - .concat(wrappedChildren) - .concat(suite.afterAllFns); - } - #suiteSkipPolicy() { if (this.#getConfig().stopOnSpecFailure) { return j$.CompleteOnFirstErrorSkipPolicy; diff --git a/spec/core/TreeRunnerSpec.js b/spec/core/TreeRunnerSpec.js index c08ba8f8..0e2eea1a 100644 --- a/spec/core/TreeRunnerSpec.js +++ b/spec/core/TreeRunnerSpec.js @@ -161,111 +161,6 @@ describe('TreeRunner', function() { expect(spec.executionFinished).toHaveBeenCalledWith(false, true); await expectAsync(executePromise).toBePending(); }); - - describe('Late promise rejection handling', function() { - it('is enabled when the detectLateRejectionHandling param is true', function() { - const before = jasmine.createSpy('before'); - const after = jasmine.createSpy('after'); - const queueableFn = { - fn: jasmine.createSpy('test body').and.callFake(function() { - expect(before).toHaveBeenCalled(); - expect(after).not.toHaveBeenCalled(); - }) - }; - const spec = new jasmineUnderTest.Spec({ - queueableFn, - beforeAndAfterFns: function() { - return { befores: [before], afters: [after] }; - } - }); - - const { - runQueue, - setTimeout, - suiteRunQueueArgs, - globalErrors - } = runSingleSpecSuite(spec, { detectLateRejectionHandling: true }); - - suiteRunQueueArgs.queueableFns[0].fn(); - expect(runQueue).toHaveBeenCalledTimes(1); - const specRunQueueOpts = runQueue.calls.mostRecent().args[0]; - - expect(specRunQueueOpts.queueableFns).toEqual([ - { fn: jasmine.any(Function) }, - before, - queueableFn, - after, - { fn: jasmine.any(Function) }, - { - fn: jasmine.any(Function), - type: 'specCleanup' - } - ]); - - const done = jasmine.createSpy('done'); - specRunQueueOpts.queueableFns[4].fn(done); - expect(globalErrors.reportUnhandledRejections).not.toHaveBeenCalled(); - expect(done).not.toHaveBeenCalled(); - - expect(setTimeout).toHaveBeenCalledOnceWith(jasmine.any(Function)); - setTimeout.calls.argsFor(0)[0](); - expect(globalErrors.reportUnhandledRejections).toHaveBeenCalled(); - expect(globalErrors.reportUnhandledRejections).toHaveBeenCalledBefore( - done - ); - }); - }); - - function runSingleSpecSuite(spec, optionalConfig) { - const topSuiteId = 'suite1'; - spec.parentSuiteId = topSuiteId; - const topSuite = new jasmineUnderTest.Suite({ id: topSuiteId }); - topSuite.addChild(spec); - const executionTree = { - topSuite, - childrenOfTopSuite() { - return [{ spec }]; - }, - isExcluded() { - return false; - } - }; - const runQueue = jasmine.createSpy('runQueue'); - const reportDispatcher = mockReportDispatcher(); - const runableResources = mockRunableResources(); - const globalErrors = mockGlobalErrors(); - const setTimeout = jasmine.createSpy('setTimeout'); - const currentRunableTracker = new jasmineUnderTest.CurrentRunableTracker(); - const subject = new jasmineUnderTest.TreeRunner({ - executionTree, - runQueue, - globalErrors, - setTimeout, - runableResources, - reportDispatcher, - currentRunableTracker, - getConfig() { - return optionalConfig || {}; - }, - reportChildrenOfBeforeAllFailure() {} - }); - - const executePromise = subject.execute(); - expect(runQueue).toHaveBeenCalledTimes(1); - const suiteRunQueueArgs = runQueue.calls.mostRecent().args[0]; - runQueue.calls.reset(); - - return { - runQueue, - globalErrors, - setTimeout, - currentRunableTracker, - runableResources, - reportDispatcher, - suiteRunQueueArgs, - executePromise - }; - } }); describe('Suite execution', function() { @@ -514,6 +409,249 @@ describe('TreeRunner', function() { }); }); + describe('Late promise rejection handling', function() { + it('works for specs when the detectLateRejectionHandling param is true', function() { + const before = jasmine.createSpy('before'); + const after = jasmine.createSpy('after'); + const queueableFn = { + fn: jasmine.createSpy('test body').and.callFake(function() { + expect(before).toHaveBeenCalled(); + expect(after).not.toHaveBeenCalled(); + }) + }; + const spec = new jasmineUnderTest.Spec({ + queueableFn, + beforeAndAfterFns: function() { + return { befores: [before], afters: [after] }; + } + }); + + const { + runQueue, + setTimeout, + suiteRunQueueArgs, + globalErrors + } = runSingleSpecSuite(spec, { detectLateRejectionHandling: true }); + + suiteRunQueueArgs.queueableFns[0].fn(); + expect(runQueue).toHaveBeenCalledTimes(1); + const specRunQueueOpts = runQueue.calls.mostRecent().args[0]; + + expect(specRunQueueOpts.queueableFns).toEqual([ + { fn: jasmine.any(Function) }, + before, + queueableFn, + after, + { fn: jasmine.any(Function) }, + { + fn: jasmine.any(Function), + type: 'specCleanup' + } + ]); + + const done = jasmine.createSpy('done'); + specRunQueueOpts.queueableFns[4].fn(done); + expect(globalErrors.reportUnhandledRejections).not.toHaveBeenCalled(); + expect(done).not.toHaveBeenCalled(); + + expect(setTimeout).toHaveBeenCalledOnceWith(jasmine.any(Function)); + setTimeout.calls.argsFor(0)[0](); + expect(globalErrors.reportUnhandledRejections).toHaveBeenCalled(); + expect(globalErrors.reportUnhandledRejections).toHaveBeenCalledBefore( + done + ); + }); + + it('works for beforeAll when the detectLateRejectionHandling param is true', async function() { + const topSuite = new jasmineUnderTest.Suite({ id: 'topSuite' }); + const suite = new jasmineUnderTest.Suite({ + id: 'suite', + parentSuite: topSuite + }); + suite.beforeAll(function() {}); + const spec = new jasmineUnderTest.Spec({ + queueableFn: { fn() {} }, + parentSuite: suite + }); + const executionTree = { + topSuite, + childrenOfTopSuite() { + return [{ suite }]; + }, + childrenOfSuiteSegment() { + return [{ spec }]; + }, + isExcluded() { + return false; + } + }; + const runQueue = jasmine.createSpy('runQueue'); + const reportDispatcher = mockReportDispatcher(); + const globalErrors = mockGlobalErrors(); + const setTimeout = jasmine.createSpy('setTimeout'); + const subject = new jasmineUnderTest.TreeRunner({ + executionTree, + runQueue, + globalErrors, + runableResources: mockRunableResources(), + reportDispatcher, + setTimeout, + currentRunableTracker: new jasmineUnderTest.CurrentRunableTracker(), + getConfig() { + return { detectLateRejectionHandling: true }; + }, + reportChildrenOfBeforeAllFailure() {} + }); + + const executePromise = subject.execute(); + expect(runQueue).toHaveBeenCalledTimes(1); + const topSuiteRunQueueOpts = runQueue.calls.mostRecent().args[0]; + runQueue.calls.reset(); + topSuiteRunQueueOpts.queueableFns[0].fn(function() {}); + + expect(runQueue).toHaveBeenCalledTimes(1); + const suiteRunQueueOpts = runQueue.calls.mostRecent().args[0]; + expect(suiteRunQueueOpts.queueableFns).toEqual([ + { fn: jasmine.any(Function) }, // onStart + jasmine.objectContaining({ type: 'beforeAll' }), + { fn: jasmine.any(Function) }, // detect late rejection handling + { fn: jasmine.any(Function) } // spec + ]); + suiteRunQueueOpts.queueableFns[0].fn(); + const done = jasmine.createSpy('done'); + suiteRunQueueOpts.queueableFns[2].fn(done); + expect(globalErrors.reportUnhandledRejections).not.toHaveBeenCalled(); + + expect(setTimeout).toHaveBeenCalledOnceWith(jasmine.any(Function)); + setTimeout.calls.argsFor(0)[0](); + expect(globalErrors.reportUnhandledRejections).toHaveBeenCalledBefore( + done + ); + + await expectAsync(executePromise).toBePending(); + }); + + it('works for afterAll when the detectLateRejectionHandling param is true', async function() { + const topSuite = new jasmineUnderTest.Suite({ id: 'topSuite' }); + const suite = new jasmineUnderTest.Suite({ + id: 'suite', + parentSuite: topSuite + }); + suite.afterAll(function() {}); + const spec = new jasmineUnderTest.Spec({ + queueableFn: { fn() {} }, + parentSuite: suite + }); + const executionTree = { + topSuite, + childrenOfTopSuite() { + return [{ suite }]; + }, + childrenOfSuiteSegment() { + return [{ spec }]; + }, + isExcluded() { + return false; + } + }; + const runQueue = jasmine.createSpy('runQueue'); + const reportDispatcher = mockReportDispatcher(); + const globalErrors = mockGlobalErrors(); + const setTimeout = jasmine.createSpy('setTimeout'); + const subject = new jasmineUnderTest.TreeRunner({ + executionTree, + runQueue, + globalErrors, + runableResources: mockRunableResources(), + reportDispatcher, + setTimeout, + currentRunableTracker: new jasmineUnderTest.CurrentRunableTracker(), + getConfig() { + return { detectLateRejectionHandling: true }; + }, + reportChildrenOfBeforeAllFailure() {} + }); + + const executePromise = subject.execute(); + expect(runQueue).toHaveBeenCalledTimes(1); + const topSuiteRunQueueOpts = runQueue.calls.mostRecent().args[0]; + runQueue.calls.reset(); + topSuiteRunQueueOpts.queueableFns[0].fn(function() {}); + + expect(runQueue).toHaveBeenCalledTimes(1); + const suiteRunQueueOpts = runQueue.calls.mostRecent().args[0]; + expect(suiteRunQueueOpts.queueableFns).toEqual([ + { fn: jasmine.any(Function) }, // onStart + { fn: jasmine.any(Function) }, // spec + jasmine.objectContaining({ type: 'afterAll' }), + { fn: jasmine.any(Function) } // detect late rejection handling + ]); + suiteRunQueueOpts.queueableFns[0].fn(); + const done = jasmine.createSpy('done'); + suiteRunQueueOpts.queueableFns[3].fn(done); + expect(globalErrors.reportUnhandledRejections).not.toHaveBeenCalled(); + + expect(setTimeout).toHaveBeenCalledOnceWith(jasmine.any(Function)); + setTimeout.calls.argsFor(0)[0](); + expect(globalErrors.reportUnhandledRejections).toHaveBeenCalledBefore( + done + ); + + await expectAsync(executePromise).toBePending(); + }); + }); + + function runSingleSpecSuite(spec, optionalConfig) { + const topSuiteId = 'suite1'; + spec.parentSuiteId = topSuiteId; + const topSuite = new jasmineUnderTest.Suite({ id: topSuiteId }); + topSuite.addChild(spec); + const executionTree = { + topSuite, + childrenOfTopSuite() { + return [{ spec }]; + }, + isExcluded() { + return false; + } + }; + const runQueue = jasmine.createSpy('runQueue'); + const reportDispatcher = mockReportDispatcher(); + const runableResources = mockRunableResources(); + const globalErrors = mockGlobalErrors(); + const setTimeout = jasmine.createSpy('setTimeout'); + const currentRunableTracker = new jasmineUnderTest.CurrentRunableTracker(); + const subject = new jasmineUnderTest.TreeRunner({ + executionTree, + runQueue, + globalErrors, + setTimeout, + runableResources, + reportDispatcher, + currentRunableTracker, + getConfig() { + return optionalConfig || {}; + }, + reportChildrenOfBeforeAllFailure() {} + }); + + const executePromise = subject.execute(); + expect(runQueue).toHaveBeenCalledTimes(1); + const suiteRunQueueArgs = runQueue.calls.mostRecent().args[0]; + runQueue.calls.reset(); + + return { + runQueue, + globalErrors, + setTimeout, + currentRunableTracker, + runableResources, + reportDispatcher, + suiteRunQueueArgs, + executePromise + }; + } + function mockReportDispatcher() { const reportDispatcher = jasmine.createSpyObj( 'reportDispatcher', diff --git a/spec/core/integration/GlobalErrorHandlingSpec.js b/spec/core/integration/GlobalErrorHandlingSpec.js index 814d5ec4..c7367d5c 100644 --- a/spec/core/integration/GlobalErrorHandlingSpec.js +++ b/spec/core/integration/GlobalErrorHandlingSpec.js @@ -809,15 +809,17 @@ describe('Global error handling (integration)', function() { describe('When the detectLateRejectionHandling config option is set', function() { describe('When the unhandled rejection event has a promise', function() { - it('reports the rejection unless a corresponding rejection handled event occurs', async function() { - function makeEvent(suffix) { - const reason = `rejection ${suffix}`; - const promise = Promise.reject(reason); - promise.catch(() => {}); - return { reason, promise }; - } + function makeEvent(suffix) { + const reason = `rejection ${suffix}`; + const promise = Promise.reject(reason); + promise.catch(() => {}); + return { reason, promise }; + } - const global = { + let global, reporter; + + beforeEach(function() { + global = { ...browserEventMethods(), setTimeout: function(fn, delay) { return setTimeout(fn, delay); @@ -833,43 +835,132 @@ describe('Global error handling (integration)', function() { env.cleanup_(); env = new jasmineUnderTest.Env(); env.configure({ detectLateRejectionHandling: true }); - const reporter = jasmine.createSpyObj('fakeReporter', [ + + reporter = jasmine.createSpyObj('fakeReporter', [ 'specDone', 'suiteDone' ]); env.addReporter(reporter); + }); - env.describe('A suite', function() { - env.it('fails', function(specDone) { - setTimeout(function() { - const events = ['spec 1', 'spec 2'].map(makeEvent); + describe('During spec execution', function() { + it('reports the rejection unless a corresponding rejection handled event occurs', async function() { + env.describe('A suite', function() { + env.it('fails', function(specDone) { + setTimeout(function() { + const events = ['spec 1', 'spec 2'].map(makeEvent); - for (const e of events) { - dispatchErrorEvent(global, 'unhandledrejection', e); - } + for (const e of events) { + dispatchErrorEvent(global, 'unhandledrejection', e); + } - dispatchErrorEvent(global, 'rejectionhandled', events[0]); - specDone(); + dispatchErrorEvent(global, 'rejectionhandled', events[0]); + specDone(); + }); }); }); + + await env.execute(); + + expect(reporter.specDone).toHaveBeenCalledWith( + jasmine.objectContaining({ + fullName: 'A suite fails', + failedExpectations: [ + // Only the second rejection should be reported, since the first + // one was eventually handled. + jasmine.objectContaining({ + message: + 'Unhandled promise rejection: rejection spec 2 thrown' + }) + ] + }) + ); }); + }); - await env.execute(); + describe('During beforeAll execution', function() { + it('reports the rejection unless a corresponding rejection handled event occurs by the end of the beforeAll', async function() { + env.describe('A suite', function() { + let events; - expect(reporter.specDone).toHaveBeenCalledWith( - jasmine.objectContaining({ - fullName: 'A suite fails', - failedExpectations: [ - // Only the second rejection should be reported, since the first - // one was eventually handled. - jasmine.objectContaining({ - message: - 'Unhandled promise rejection: rejection spec 2 thrown' - }) - ] - }) - ); + env.beforeAll(function(beforeAllDone) { + setTimeout(function() { + events = ['suite 1', 'suite 2'].map(makeEvent); + + for (const e of events) { + dispatchErrorEvent(global, 'unhandledrejection', e); + } + + dispatchErrorEvent(global, 'rejectionhandled', events[0]); + beforeAllDone(); + }); + }); + + env.it('is a spec', function(specDone) { + setTimeout(function() { + // Should not prevent the second rejection from being reported + dispatchErrorEvent(global, 'rejectionhandled', events[1]); + specDone(); + }); + }); + }); + + await env.execute(); + + expect(reporter.suiteDone).toHaveBeenCalledWith( + jasmine.objectContaining({ + fullName: 'A suite', + failedExpectations: [ + // Only the second rejection should be reported, since the first + // one was eventually handled. + jasmine.objectContaining({ + message: + 'Unhandled promise rejection: rejection suite 2 thrown' + }) + ] + }) + ); + }); + }); + + describe('During afterAll execution', function() { + it('reports the rejection unless a corresponding rejection handled event occurs by the end of the afterAll', async function() { + env.describe('A suite', function() { + let events; + + env.afterAll(function(beforeAllDone) { + setTimeout(function() { + events = ['suite 1', 'suite 2'].map(makeEvent); + + for (const e of events) { + dispatchErrorEvent(global, 'unhandledrejection', e); + } + + dispatchErrorEvent(global, 'rejectionhandled', events[0]); + beforeAllDone(); + }); + }); + + env.it('is a spec', function() {}); + }); + + await env.execute(); + + expect(reporter.suiteDone).toHaveBeenCalledWith( + jasmine.objectContaining({ + fullName: 'A suite', + failedExpectations: [ + // Only the second rejection should be reported, since the first + // one was eventually handled. + jasmine.objectContaining({ + message: + 'Unhandled promise rejection: rejection suite 2 thrown' + }) + ] + }) + ); + }); }); }); diff --git a/src/core/TreeRunner.js b/src/core/TreeRunner.js index c760054d..f6d36c9f 100644 --- a/src/core/TreeRunner.js +++ b/src/core/TreeRunner.js @@ -117,19 +117,7 @@ getJasmineRequireObj().TreeRunner = function(j$) { fns.unshift(start); if (config.detectLateRejectionHandling) { - // Conditional because the setTimeout imposes a significant performance - // penalty in suites with lots of fast specs. - const globalErrors = this.#globalErrors; - fns.push({ - fn: done => { - // setTimeout is necessary to trigger rejectionhandled events - // TODO: let clearStack know about this so it doesn't do redundant setTimeouts - this.#setTimeout(function() { - globalErrors.reportUnhandledRejections(); - done(); - }); - } - }); + fns.push(this.#lateUnhandledRejectionChecker()); } fns.push(complete); @@ -138,11 +126,32 @@ getJasmineRequireObj().TreeRunner = function(j$) { #executeSuiteSegment(suite, segmentNumber, done) { const isTopSuite = suite === this.#executionTree.topSuite; + const isExcluded = this.#executionTree.isExcluded(suite); + let befores = []; + let afters = []; + + if (suite.beforeAllFns.length > 0 && !isExcluded) { + befores = [...suite.beforeAllFns]; + if (this.#getConfig().detectLateRejectionHandling) { + befores.push(this.#lateUnhandledRejectionChecker()); + } + } + + if (suite.afterAllFns.length > 0 && !isExcluded) { + afters = [...suite.afterAllFns]; + if (this.#getConfig().detectLateRejectionHandling) { + afters.push(this.#lateUnhandledRejectionChecker()); + } + } + const children = isTopSuite ? this.#executionTree.childrenOfTopSuite() : this.#executionTree.childrenOfSuiteSegment(suite, segmentNumber); - const wrappedChildren = this.#wrapNodes(children); - const queueableFns = this.#addBeforeAndAfterAlls(suite, wrappedChildren); + const queueableFns = [ + ...befores, + ...this.#wrapNodes(children), + ...afters + ]; if (!isTopSuite) { queueableFns.unshift({ @@ -173,6 +182,25 @@ getJasmineRequireObj().TreeRunner = function(j$) { }); } + // Returns a queueable fn that reports any still-unhandled rejections in + // cases where detectLateRejectionHandling is enabled. Should only be called + // when detectLateRejectionHandling is enabled, because the setTimeout + // imposes a significant performance penalty in suites with lots of fast + // specs. + #lateUnhandledRejectionChecker() { + const globalErrors = this.#globalErrors; + return { + fn: done => { + // setTimeout is necessary to trigger rejectionhandled events + // TODO: let clearStack know about this so it doesn't do redundant setTimeouts + this.#setTimeout(function() { + globalErrors.reportUnhandledRejections(); + done(); + }); + } + }; + } + #suiteSegmentStart(suite, next) { this.#currentRunableTracker.pushSuite(suite); this.#runableResources.initForRunable(suite.id, suite.parentSuite.id); @@ -262,16 +290,6 @@ getJasmineRequireObj().TreeRunner = function(j$) { } } - #addBeforeAndAfterAlls(suite, wrappedChildren) { - if (this.#executionTree.isExcluded(suite)) { - return wrappedChildren; - } - - return suite.beforeAllFns - .concat(wrappedChildren) - .concat(suite.afterAllFns); - } - #suiteSkipPolicy() { if (this.#getConfig().stopOnSpecFailure) { return j$.CompleteOnFirstErrorSkipPolicy;