From 585287b9d6e03dc546be7269e1ea569f4bd9ca0d Mon Sep 17 00:00:00 2001 From: Steve Gravrock Date: Thu, 15 Jun 2017 17:30:00 -0700 Subject: [PATCH] When stop on failure is enabled, skip subsequent it() and beforeEach(). Note: afterEach() functions are still run, because skipping them is highly likely to pollute specs that run after the failure. [Finishes #92252330] - Fixes #577 - Fixes #807 --- lib/jasmine-core/jasmine.js | 89 +++++++++++----- spec/core/QueueRunnerSpec.js | 101 +++++++++++++++++++ spec/core/SpecSpec.js | 22 +++- spec/core/integration/SpecRunningSpec.js | 123 +++++++++++++++++++++++ src/core/Env.js | 6 ++ src/core/QueueRunner.js | 77 +++++++++----- src/core/Spec.js | 6 +- 7 files changed, 368 insertions(+), 56 deletions(-) diff --git a/lib/jasmine-core/jasmine.js b/lib/jasmine-core/jasmine.js index 6d67727e..fbbc212d 100644 --- a/lib/jasmine-core/jasmine.js +++ b/lib/jasmine-core/jasmine.js @@ -473,10 +473,12 @@ getJasmineRequireObj().Spec = function(j$) { } var fns = this.beforeAndAfterFns(); - var allFns = fns.befores.concat(this.queueableFn).concat(fns.afters); + var regularFns = fns.befores.concat(this.queueableFn); this.queueRunnerFactory({ - queueableFns: allFns, + isLeaf: true, + queueableFns: regularFns, + cleanupFns: fns.afters, onException: function() { self.onException.apply(self, arguments); }, onComplete: complete, userContext: this.userContext() @@ -817,6 +819,7 @@ getJasmineRequireObj().Env = function(j$) { options.timeout = {setTimeout: realSetTimeout, clearTimeout: realClearTimeout}; options.fail = self.fail; options.globalErrors = globalErrors; + options.completeOnFirstError = throwOnExpectationFailure && options.isLeaf; new j$.QueueRunner(options).execute(); }; @@ -1141,6 +1144,7 @@ getJasmineRequireObj().Env = function(j$) { this.afterEach = function(afterEachFunction, timeout) { ensureIsFunctionOrAsync(afterEachFunction, 'afterEach'); + afterEachFunction.isCleanup = true; currentDeclarationSuite.afterEach({ fn: afterEachFunction, timeout: function() { return timeout || j$.DEFAULT_TIMEOUT_INTERVAL; } @@ -1189,6 +1193,10 @@ getJasmineRequireObj().Env = function(j$) { message: message, error: error && error.message ? error : null }); + + if (self.throwingExpectationFailures()) { + throw new Error(message); + } }; } @@ -3934,7 +3942,9 @@ getJasmineRequireObj().QueueRunner = function(j$) { } function QueueRunner(attrs) { - this.queueableFns = attrs.queueableFns || []; + var queueableFns = attrs.queueableFns || []; + this.queueableFns = queueableFns.concat(attrs.cleanupFns || []); + this.firstCleanupIx = queueableFns.length; this.onComplete = attrs.onComplete || function() {}; this.clearStack = attrs.clearStack || function(fn) {fn();}; this.onException = attrs.onException || function() {}; @@ -3943,6 +3953,7 @@ getJasmineRequireObj().QueueRunner = function(j$) { this.timeout = attrs.timeout || {setTimeout: setTimeout, clearTimeout: clearTimeout}; this.fail = attrs.fail || function() {}; this.globalErrors = attrs.globalErrors || { pushListener: function() {}, popListener: function() {} }; + this.completeOnFirstError = !!attrs.completeOnFirstError; } QueueRunner.prototype.execute = function() { @@ -3951,20 +3962,32 @@ getJasmineRequireObj().QueueRunner = function(j$) { self.onException(error); }; this.globalErrors.pushListener(this.handleFinalError); - this.run(this.queueableFns, 0); + this.run(0); }; - QueueRunner.prototype.run = function(queueableFns, recursiveIndex) { - var length = queueableFns.length, + QueueRunner.prototype.skipToCleanup = function(lastRanIndex) { + if (lastRanIndex < this.firstCleanupIx) { + this.run(this.firstCleanupIx); + } else { + this.run(lastRanIndex + 1); + } + }; + + QueueRunner.prototype.run = function(recursiveIndex) { + var length = this.queueableFns.length, self = this, iterativeIndex; for(iterativeIndex = recursiveIndex; iterativeIndex < length; iterativeIndex++) { - var queueableFn = queueableFns[iterativeIndex]; - var completedSynchronously = attempt(queueableFn); + var result = attempt(iterativeIndex); - if (!completedSynchronously) { + if (!result.completedSynchronously) { + return; + } + + if (this.completeOnFirstError && result.errored) { + this.skipToCleanup(iterativeIndex); return; } } @@ -3974,7 +3997,7 @@ getJasmineRequireObj().QueueRunner = function(j$) { self.onComplete(); }); - function attempt(queueableFn) { + function attempt() { var clearTimeout = function () { Function.prototype.apply.apply(self.timeout.clearTimeout, [j$.getGlobal(), [timeoutId]]); }, @@ -3992,18 +4015,28 @@ getJasmineRequireObj().QueueRunner = function(j$) { }), next = once(function () { cleanup(); + + function runNext() { + if (self.completeOnFirstError && errored) { + self.skipToCleanup(iterativeIndex); + } else { + self.run(iterativeIndex + 1); + } + } + if (completedSynchronously) { - setTimeout(function() { - self.run(queueableFns, iterativeIndex + 1); - }); + setTimeout(runNext); } else { - self.run(queueableFns, iterativeIndex + 1); + runNext(); } }), + errored = false, + queueableFn = self.queueableFns[iterativeIndex], timeoutId; next.fail = function() { self.fail.apply(null, arguments); + errored = true; next(); }; @@ -4024,31 +4057,33 @@ getJasmineRequireObj().QueueRunner = function(j$) { if (maybeThenable && j$.isFunction_(maybeThenable.then)) { maybeThenable.then(next, next.fail); completedSynchronously = false; - return false; + return { completedSynchronously: false }; } } else { queueableFn.fn.call(self.userContext, next); completedSynchronously = false; - return false; + return { completedSynchronously: false }; } } catch (e) { handleException(e, queueableFn); + errored = true; } cleanup(); - return true; - } + return { completedSynchronously: true, errored: errored }; - function onException(e) { - self.onException(e); - } + function onException(e) { + self.onException(e); + errored = true; + } - function handleException(e, queueableFn) { - onException(e); - 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.. - throw e; + function handleException(e, queueableFn) { + onException(e); + 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.. + throw e; + } } } }; diff --git a/spec/core/QueueRunnerSpec.js b/spec/core/QueueRunnerSpec.js index 27412e35..3d6a4aa3 100644 --- a/spec/core/QueueRunnerSpec.js +++ b/spec/core/QueueRunnerSpec.js @@ -19,6 +19,26 @@ describe("QueueRunner", function() { expect(calls).toEqual(['fn1', 'fn2']); }); + it("runs cleanup functions after the others", function() { + var calls = [], + queueableFn1 = { fn: jasmine.createSpy('fn1') }, + queueableFn2 = { fn: jasmine.createSpy('fn2') }, + queueRunner = new jasmineUnderTest.QueueRunner({ + queueableFns: [queueableFn1], + cleanupFns: [queueableFn2] + }); + queueableFn1.fn.and.callFake(function() { + calls.push('fn1'); + }); + queueableFn2.fn.and.callFake(function() { + calls.push('fn2'); + }); + + queueRunner.execute(); + + expect(calls).toEqual(['fn1', 'fn2']); + }); + it("calls each function with a consistent 'this'-- an empty object", function() { var queueableFn1 = { fn: jasmine.createSpy('fn1') }, queueableFn2 = { fn: jasmine.createSpy('fn2') }, @@ -421,6 +441,87 @@ describe("QueueRunner", function() { expect(nextQueueableFn.fn).toHaveBeenCalled(); }); + describe("When configured to complete on first error", function() { + it("skips to cleanup functions on the first exception", function() { + var queueableFn = { fn: function() { throw new Error("error"); } }, + nextQueueableFn = { fn: jasmine.createSpy("nextFunction") }, + cleanupFn = { fn: jasmine.createSpy("cleanup") }, + queueRunner = new jasmineUnderTest.QueueRunner({ + queueableFns: [queueableFn, nextQueueableFn], + cleanupFns: [cleanupFn], + completeOnFirstError: true + }); + + queueRunner.execute(); + expect(nextQueueableFn.fn).not.toHaveBeenCalled(); + expect(cleanupFn.fn).toHaveBeenCalled(); + }); + + it("does not skip when a cleanup function throws", function() { + var queueableFn = { fn: function() { } }, + cleanupFn1 = { fn: function() { throw new Error("error"); } }, + cleanupFn2 = { fn: jasmine.createSpy("cleanupFn2") }, + queueRunner = new jasmineUnderTest.QueueRunner({ + queueableFns: [queueableFn], + cleanupFns: [cleanupFn1, cleanupFn2], + completeOnFirstError: true + }); + + queueRunner.execute(); + expect(cleanupFn2.fn).toHaveBeenCalled(); + }); + + describe("with an asynchronous function", function() { + beforeEach(function() { + jasmine.clock().install(); + }); + + afterEach(function() { + jasmine.clock().uninstall(); + }); + + + it("skips to cleanup functions on the first exception", function() { + var errorListeners = [], + queueableFn = { fn: function(done) {} }, + nextQueueableFn = { fn: jasmine.createSpy('nextFunction') }, + cleanupFn = { fn: jasmine.createSpy('cleanup') }, + queueRunner = new jasmineUnderTest.QueueRunner({ + globalErrors: { + pushListener: function(f) { errorListeners.push(f); }, + popListener: function() { errorListeners.pop(); }, + }, + queueableFns: [queueableFn, nextQueueableFn], + cleanupFns: [cleanupFn], + completeOnFirstError: true, + }); + + queueRunner.execute(); + errorListeners[errorListeners.length - 1](new Error('error')); + expect(nextQueueableFn.fn).not.toHaveBeenCalled(); + expect(cleanupFn.fn).toHaveBeenCalled(); + }); + + it("skips to cleanup functions when next.fail is called", function() { + var queueableFn = { fn: function(done) { + done.fail('nope'); + } }, + nextQueueableFn = { fn: jasmine.createSpy('nextFunction') }, + cleanupFn = { fn: jasmine.createSpy('cleanup') }, + queueRunner = new jasmineUnderTest.QueueRunner({ + queueableFns: [queueableFn, nextQueueableFn], + cleanupFns: [cleanupFn], + completeOnFirstError: true, + }); + + queueRunner.execute(); + jasmine.clock().tick(); + expect(nextQueueableFn.fn).not.toHaveBeenCalled(); + expect(cleanupFn.fn).toHaveBeenCalled(); + }); + }); + }); + it("calls a provided complete callback when done", function() { var queueableFn = { fn: jasmine.createSpy('fn') }, completeCallback = jasmine.createSpy('completeCallback'), diff --git a/spec/core/SpecSpec.js b/spec/core/SpecSpec.js index 913a54fb..ac94ead7 100644 --- a/spec/core/SpecSpec.js +++ b/spec/core/SpecSpec.js @@ -103,8 +103,26 @@ describe("Spec", function() { spec.execute(); - var allSpecFns = fakeQueueRunner.calls.mostRecent().args[0].queueableFns; - expect(allSpecFns).toEqual([before, queueableFn, after]); + var options = fakeQueueRunner.calls.mostRecent().args[0]; + expect(options.queueableFns).toEqual([before, queueableFn]); + expect(options.cleanupFns).toEqual([after]); + }); + + it("tells the queue runner that it's a leaf node", function() { + var fakeQueueRunner = jasmine.createSpy('fakeQueueRunner'), + spec = new jasmineUnderTest.Spec({ + queueableFn: { fn: function() {} }, + beforeAndAfterFns: function() { + return {befores: [], afters: []} + }, + queueRunnerFactory: fakeQueueRunner + }); + + spec.execute(); + + expect(fakeQueueRunner).toHaveBeenCalledWith(jasmine.objectContaining({ + isLeaf: true + })); }); it("is marked pending if created without a function body", function() { diff --git a/spec/core/integration/SpecRunningSpec.js b/spec/core/integration/SpecRunningSpec.js index 6c2be5ce..9950f722 100644 --- a/spec/core/integration/SpecRunningSpec.js +++ b/spec/core/integration/SpecRunningSpec.js @@ -836,4 +836,127 @@ describe("jasmine spec running", function () { env.execute(); }); + describe("When throwOnExpectationFailure is set", function() { + it("skips to cleanup functions after an error", function(done) { + var actions = []; + + env.describe('Something', function() { + env.beforeEach(function() { + actions.push('outer beforeEach'); + throw new Error("error"); + }); + + env.afterEach(function() { + actions.push('outer afterEach'); + }); + + env.describe('Inner', function() { + env.beforeEach(function() { + actions.push('inner beforeEach'); + }); + + env.afterEach(function() { + actions.push('inner afterEach'); + }); + + env.it('does it' , function() { + actions.push('inner it'); + }); + }); + }); + + env.throwOnExpectationFailure(true); + + var assertions = function() { + expect(actions).toEqual([ + 'outer beforeEach', + 'inner afterEach', + 'outer afterEach' + ]); + done(); + }; + + env.addReporter({jasmineDone: assertions}); + + env.execute(); + }); + + it("skips to cleanup functions after done.fail is called", function(done) { + var actions = []; + + env.describe('Something', function() { + env.beforeEach(function(done) { + actions.push('beforeEach'); + done.fail('error'); + actions.push('after done.fail'); + }); + + env.afterEach(function() { + actions.push('afterEach'); + }); + + env.it('does it' , function() { + actions.push('it'); + }); + }); + + env.throwOnExpectationFailure(true); + + var assertions = function() { + expect(actions).toEqual([ + 'beforeEach', + 'afterEach' + ]); + done(); + }; + + env.addReporter({jasmineDone: assertions}); + + env.execute(); + }); + + describe("When an async function times out", function() { + beforeEach(function() { + jasmine.clock().install(); + env = new jasmineUnderTest.Env(); + }); + + afterEach(function() { + jasmine.clock().uninstall(); + }); + + it("skips to cleanup functions", function(done) { + var actions = []; + + env.describe('Something', function() { + env.beforeEach(function(done) { + actions.push('beforeEach'); + }); + + env.afterEach(function() { + actions.push('afterEach'); + }); + + env.it('does it' , function() { + actions.push('it'); + }); + }); + + env.throwOnExpectationFailure(true); + + var assertions = function() { + expect(actions).toEqual([ + 'beforeEach', + 'afterEach' + ]); + done(); + }; + + env.addReporter({jasmineDone: assertions}); + + env.execute(); + jasmine.clock().tick(jasmineUnderTest.DEFAULT_TIMEOUT_INTERVAL); + }); + }); + }); }); diff --git a/src/core/Env.js b/src/core/Env.js index 903a2115..2bdbdde1 100644 --- a/src/core/Env.js +++ b/src/core/Env.js @@ -196,6 +196,7 @@ getJasmineRequireObj().Env = function(j$) { options.timeout = {setTimeout: realSetTimeout, clearTimeout: realClearTimeout}; options.fail = self.fail; options.globalErrors = globalErrors; + options.completeOnFirstError = throwOnExpectationFailure && options.isLeaf; new j$.QueueRunner(options).execute(); }; @@ -520,6 +521,7 @@ getJasmineRequireObj().Env = function(j$) { this.afterEach = function(afterEachFunction, timeout) { ensureIsFunctionOrAsync(afterEachFunction, 'afterEach'); + afterEachFunction.isCleanup = true; currentDeclarationSuite.afterEach({ fn: afterEachFunction, timeout: function() { return timeout || j$.DEFAULT_TIMEOUT_INTERVAL; } @@ -568,6 +570,10 @@ getJasmineRequireObj().Env = function(j$) { message: message, error: error && error.message ? error : null }); + + if (self.throwingExpectationFailures()) { + throw new Error(message); + } }; } diff --git a/src/core/QueueRunner.js b/src/core/QueueRunner.js index d01ad46e..9cdef983 100644 --- a/src/core/QueueRunner.js +++ b/src/core/QueueRunner.js @@ -12,7 +12,9 @@ getJasmineRequireObj().QueueRunner = function(j$) { } function QueueRunner(attrs) { - this.queueableFns = attrs.queueableFns || []; + var queueableFns = attrs.queueableFns || []; + this.queueableFns = queueableFns.concat(attrs.cleanupFns || []); + this.firstCleanupIx = queueableFns.length; this.onComplete = attrs.onComplete || function() {}; this.clearStack = attrs.clearStack || function(fn) {fn();}; this.onException = attrs.onException || function() {}; @@ -21,6 +23,7 @@ getJasmineRequireObj().QueueRunner = function(j$) { this.timeout = attrs.timeout || {setTimeout: setTimeout, clearTimeout: clearTimeout}; this.fail = attrs.fail || function() {}; this.globalErrors = attrs.globalErrors || { pushListener: function() {}, popListener: function() {} }; + this.completeOnFirstError = !!attrs.completeOnFirstError; } QueueRunner.prototype.execute = function() { @@ -29,20 +32,32 @@ getJasmineRequireObj().QueueRunner = function(j$) { self.onException(error); }; this.globalErrors.pushListener(this.handleFinalError); - this.run(this.queueableFns, 0); + this.run(0); }; - QueueRunner.prototype.run = function(queueableFns, recursiveIndex) { - var length = queueableFns.length, + QueueRunner.prototype.skipToCleanup = function(lastRanIndex) { + if (lastRanIndex < this.firstCleanupIx) { + this.run(this.firstCleanupIx); + } else { + this.run(lastRanIndex + 1); + } + }; + + QueueRunner.prototype.run = function(recursiveIndex) { + var length = this.queueableFns.length, self = this, iterativeIndex; for(iterativeIndex = recursiveIndex; iterativeIndex < length; iterativeIndex++) { - var queueableFn = queueableFns[iterativeIndex]; - var completedSynchronously = attempt(queueableFn); + var result = attempt(iterativeIndex); - if (!completedSynchronously) { + if (!result.completedSynchronously) { + return; + } + + if (this.completeOnFirstError && result.errored) { + this.skipToCleanup(iterativeIndex); return; } } @@ -52,7 +67,7 @@ getJasmineRequireObj().QueueRunner = function(j$) { self.onComplete(); }); - function attempt(queueableFn) { + function attempt() { var clearTimeout = function () { Function.prototype.apply.apply(self.timeout.clearTimeout, [j$.getGlobal(), [timeoutId]]); }, @@ -70,18 +85,28 @@ getJasmineRequireObj().QueueRunner = function(j$) { }), next = once(function () { cleanup(); + + function runNext() { + if (self.completeOnFirstError && errored) { + self.skipToCleanup(iterativeIndex); + } else { + self.run(iterativeIndex + 1); + } + } + if (completedSynchronously) { - setTimeout(function() { - self.run(queueableFns, iterativeIndex + 1); - }); + setTimeout(runNext); } else { - self.run(queueableFns, iterativeIndex + 1); + runNext(); } }), + errored = false, + queueableFn = self.queueableFns[iterativeIndex], timeoutId; next.fail = function() { self.fail.apply(null, arguments); + errored = true; next(); }; @@ -102,31 +127,33 @@ getJasmineRequireObj().QueueRunner = function(j$) { if (maybeThenable && j$.isFunction_(maybeThenable.then)) { maybeThenable.then(next, next.fail); completedSynchronously = false; - return false; + return { completedSynchronously: false }; } } else { queueableFn.fn.call(self.userContext, next); completedSynchronously = false; - return false; + return { completedSynchronously: false }; } } catch (e) { handleException(e, queueableFn); + errored = true; } cleanup(); - return true; - } + return { completedSynchronously: true, errored: errored }; - function onException(e) { - self.onException(e); - } + function onException(e) { + self.onException(e); + errored = true; + } - function handleException(e, queueableFn) { - onException(e); - 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.. - throw e; + function handleException(e, queueableFn) { + onException(e); + 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.. + throw e; + } } } }; diff --git a/src/core/Spec.js b/src/core/Spec.js index a7235d9f..57ba0d34 100644 --- a/src/core/Spec.js +++ b/src/core/Spec.js @@ -56,10 +56,12 @@ getJasmineRequireObj().Spec = function(j$) { } var fns = this.beforeAndAfterFns(); - var allFns = fns.befores.concat(this.queueableFn).concat(fns.afters); + var regularFns = fns.befores.concat(this.queueableFn); this.queueRunnerFactory({ - queueableFns: allFns, + isLeaf: true, + queueableFns: regularFns, + cleanupFns: fns.afters, onException: function() { self.onException.apply(self, arguments); }, onComplete: complete, userContext: this.userContext()