From 1bad048c15aa973ee179832c0e13dbc74f99135e Mon Sep 17 00:00:00 2001 From: slackersoft Date: Fri, 20 Jun 2014 08:16:42 -0700 Subject: [PATCH] Extract afterAll checking from queueRunner --- spec/core/QueueRunnerSpec.js | 31 ++++++++++++++++++++++++------- src/core/Env.js | 16 ++++++++++------ src/core/QueueRunner.js | 24 ++++++++++-------------- 3 files changed, 44 insertions(+), 27 deletions(-) diff --git a/spec/core/QueueRunnerSpec.js b/spec/core/QueueRunnerSpec.js index e677104b..d715b7cb 100644 --- a/spec/core/QueueRunnerSpec.js +++ b/spec/core/QueueRunnerSpec.js @@ -95,13 +95,15 @@ describe("QueueRunner", function() { it("sets a timeout if requested for asynchronous functions so they don't go on forever", function() { var timeout = 3, - beforeFn = { fn: function(done) { }, timeout: function() { return timeout; } }, - queueableFn = { fn: jasmine.createSpy('fn') }, + beforeFn = { fn: function(done) { }, type: 'before', timeout: function() { return timeout; } }, + queueableFn = { fn: jasmine.createSpy('fn'), type: 'queueable' }, onComplete = jasmine.createSpy('onComplete'), + reportException = jasmine.createSpy('reportException'), onException = jasmine.createSpy('onException'), queueRunner = new j$.QueueRunner({ queueableFns: [beforeFn, queueableFn], onComplete: onComplete, + reportException: reportException, onException: onException }); @@ -110,6 +112,7 @@ describe("QueueRunner", function() { jasmine.clock().tick(timeout); + expect(reportException).toHaveBeenCalledWith(jasmine.any(Error), 'before'); expect(onException).toHaveBeenCalledWith(jasmine.any(Error)); expect(queueableFn.fn).toHaveBeenCalled(); expect(onComplete).toHaveBeenCalled(); @@ -119,10 +122,12 @@ describe("QueueRunner", function() { var beforeFn = { fn: function(done) { } }, queueableFn = { fn: jasmine.createSpy('fn') }, onComplete = jasmine.createSpy('onComplete'), + reportException = jasmine.createSpy('reportException'), onException = jasmine.createSpy('onException'), queueRunner = new j$.QueueRunner({ queueableFns: [beforeFn, queueableFn], onComplete: onComplete, + reportException: reportException, onException: onException, }); @@ -131,37 +136,44 @@ describe("QueueRunner", function() { jasmine.clock().tick(j$.DEFAULT_TIMEOUT_INTERVAL); + expect(reportException).not.toHaveBeenCalled(); expect(onException).not.toHaveBeenCalled(); expect(queueableFn.fn).not.toHaveBeenCalled(); expect(onComplete).not.toHaveBeenCalled(); }); - it("clears the timeout when an async function throws an exception, to prevent additional onException calls", function() { + it("clears the timeout when an async function throws an exception, to prevent additional exception reporting", function() { var queueableFn = { fn: function(done) { throw new Error("error!"); } }, onComplete = jasmine.createSpy('onComplete'), + reportException = jasmine.createSpy('reportException'), onException = jasmine.createSpy('onException'), queueRunner = new j$.QueueRunner({ queueableFns: [queueableFn], onComplete: onComplete, + reportException: reportException, onException: onException }); queueRunner.execute(); expect(onComplete).toHaveBeenCalled(); + expect(reportException).toHaveBeenCalled(); expect(onException).toHaveBeenCalled(); jasmine.clock().tick(j$.DEFAULT_TIMEOUT_INTERVAL); + expect(reportException.calls.count()).toEqual(1); expect(onException.calls.count()).toEqual(1); }); it("clears the timeout when the done callback is called", function() { var queueableFn = { fn: function(done) { done(); } }, onComplete = jasmine.createSpy('onComplete'), + reportException = jasmine.createSpy('reportException'), onException = jasmine.createSpy('onException'), queueRunner = new j$.QueueRunner({ queueableFns: [queueableFn], onComplete: onComplete, + reportException: reportException onException: onException }); @@ -170,6 +182,7 @@ describe("QueueRunner", function() { expect(onComplete).toHaveBeenCalled(); jasmine.clock().tick(j$.DEFAULT_TIMEOUT_INTERVAL); + expect(reportException).not.toHaveBeenCalled(); expect(onException).not.toHaveBeenCalled(); }); @@ -200,19 +213,23 @@ describe("QueueRunner", function() { }); }); - it("calls an exception handler when an exception is thrown in a fn", function() { - var queueableFn = { fn: function() { + it("calls exception handlers when an exception is thrown in a fn", function() { + var queueableFn = { type: 'queueable', + fn: function() { throw new Error('fake error'); } }, exceptionCallback = jasmine.createSpy('exception callback'), + onExceptionCallback = jasmine.createSpy('on exception callback'), queueRunner = new j$.QueueRunner({ queueableFns: [queueableFn], - onException: exceptionCallback + reportException: exceptionCallback, + onException: onExceptionCallback }); queueRunner.execute(); - expect(exceptionCallback).toHaveBeenCalledWith(jasmine.any(Error)); + expect(exceptionCallback).toHaveBeenCalledWith(jasmine.any(Error), 'queueable'); + expect(onExceptionCallback).toHaveBeenCalledWith(jasmine.any(Error)); }); it("rethrows an exception if told to", function() { diff --git a/src/core/Env.js b/src/core/Env.js index e8174694..165241ad 100644 --- a/src/core/Env.js +++ b/src/core/Env.js @@ -166,9 +166,13 @@ getJasmineRequireObj().Env = function(j$) { var queueRunnerFactory = function(options) { options.catchException = catchException; - options.reporter = reporter; options.clearStack = options.clearStack || clearStack; options.timer = {setTimeout: realSetTimeout, clearTimeout: realClearTimeout}; + options.reportException = function(e, type) { + if (type === 'afterAll') { + reporter.afterAllError(e); + } + }; new j$.QueueRunner(options).execute(); }; @@ -296,7 +300,7 @@ getJasmineRequireObj().Env = function(j$) { expectationResultFactory: expectationResultFactory, queueRunnerFactory: queueRunnerFactory, userContext: function() { return suite.clonedSharedUserContext(); }, - queueableFn: { fn: fn, timeout: function() { return j$.DEFAULT_TIMEOUT_INTERVAL; } } + queueableFn: { fn: fn, type: 'it', timeout: function() { return j$.DEFAULT_TIMEOUT_INTERVAL; } } }); runnableLookupTable[spec.id] = spec; @@ -337,19 +341,19 @@ getJasmineRequireObj().Env = function(j$) { }; this.beforeEach = function(beforeEachFunction) { - currentDeclarationSuite.beforeEach({ fn: beforeEachFunction, timeout: function() { return j$.DEFAULT_TIMEOUT_INTERVAL; } }); + currentDeclarationSuite.beforeEach({ fn: beforeEachFunction, type: 'beforeEach', timeout: function() { return j$.DEFAULT_TIMEOUT_INTERVAL; } }); }; this.beforeAll = function(beforeAllFunction) { - currentDeclarationSuite.beforeAll({ fn: beforeAllFunction, timeout: function() { return j$.DEFAULT_TIMEOUT_INTERVAL; } }); + currentDeclarationSuite.beforeAll({ fn: beforeAllFunction, type: 'beforeAll', timeout: function() { return j$.DEFAULT_TIMEOUT_INTERVAL; } }); }; this.afterEach = function(afterEachFunction) { - currentDeclarationSuite.afterEach({ fn: afterEachFunction, timeout: function() { return j$.DEFAULT_TIMEOUT_INTERVAL; } }); + currentDeclarationSuite.afterEach({ fn: afterEachFunction, type: 'afterEach', timeout: function() { return j$.DEFAULT_TIMEOUT_INTERVAL; } }); }; this.afterAll = function(afterAllFunction) { - currentDeclarationSuite.afterAll({ fn: afterAllFunction, isAfterAll: true, timeout: function() { return j$.DEFAULT_TIMEOUT_INTERVAL; } }); + currentDeclarationSuite.afterAll({ fn: afterAllFunction, type: 'afterAll', timeout: function() { return j$.DEFAULT_TIMEOUT_INTERVAL; } }); }; this.pending = function() { diff --git a/src/core/QueueRunner.js b/src/core/QueueRunner.js index beca52fb..a0facf84 100644 --- a/src/core/QueueRunner.js +++ b/src/core/QueueRunner.js @@ -18,7 +18,7 @@ getJasmineRequireObj().QueueRunner = function(j$) { this.catchException = attrs.catchException || function() { return true; }; this.userContext = attrs.userContext || {}; this.timer = attrs.timeout || {setTimeout: setTimeout, clearTimeout: clearTimeout}; - this.reporter = attrs.reporter; + this.reportException = attrs.reportException || function() {}; } QueueRunner.prototype.execute = function() { @@ -50,10 +50,7 @@ getJasmineRequireObj().QueueRunner = function(j$) { try { queueableFn.fn.call(self.userContext); } catch (e) { - if(queueableFn.isAfterAll){ - runner.reporter.afterAllError(e); - } - handleException(e); + handleException(e, queueableFn); } } @@ -70,10 +67,7 @@ getJasmineRequireObj().QueueRunner = function(j$) { if (queueableFn.timeout) { timeoutId = Function.prototype.apply.apply(self.timer.setTimeout, [j$.getGlobal(), [function() { var error = new Error('Timeout - Async callback was not invoked within timeout specified by jasmine.DEFAULT_TIMEOUT_INTERVAL.'); - if (queueableFn.isAfterAll) { - runner.reporter.afterAllError(error); - } - self.onException(error); + onException(error, queueableFn); next(); }, queueableFn.timeout()]]); } @@ -81,16 +75,18 @@ getJasmineRequireObj().QueueRunner = function(j$) { try { queueableFn.fn.call(self.userContext, next); } catch (e) { - if(queueableFn.isAfterAll) { - runner.reporter.afterAllError(e); - } - handleException(e); + handleException(e, queueableFn); next(); } } - function handleException(e) { + function onException(e, queueableFn) { + self.reportException(e, queueableFn.type); self.onException(e); + } + + function handleException(e, queueableFn) { + onException(e, queueableFn); if (!self.catchException(e)) { //TODO: set a var when we catch an exception and //use a finally block to close the loop in a nice way..