Skip to content

Commit

Permalink
When stop on failure is enabled, skip subsequent it() and beforeEach().
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@sgravrock
sgravrock committed Jun 19, 2017
1 parent de862f8 commit 585287b
Show file tree
Hide file tree
Showing 7 changed files with 368 additions and 56 deletions.
89 changes: 62 additions & 27 deletions lib/jasmine-core/jasmine.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 comment has been minimized.

Sign in to view

Copy link
@NicBright

NicBright Aug 31, 2017

I'm sorry for polluting this PR, specifically adding a comment to this line, but I didn't find a way to add a comment for the whole PR.

My question is: what is this "stop on failure" feature about? I can neither find any code changes in this PR (except a "throwOnExpectationFailures" which obviously means something else) nor documentation in the official docs that would describe this feature. How do you enable it?

This comment has been minimized.

Sign in to view

Copy link
@ArnaudPel

ArnaudPel May 11, 2018 •
edited
Loading

I assume this is referring to either

  • --stop-on-failure=[true|false] on the command line
  • jasmineNodeOpts: {stopSpecOnExpectationFailure: true} in the conf file.

Source: https://jasmine.github.io/setup/nodejs.html - I wish I could be more specific but there is no anchor on the page.

But this PR is not the end of the story anyway: #1533

this.queueRunnerFactory({
queueableFns: allFns,
isLeaf: true,
queueableFns: regularFns,
cleanupFns: fns.afters,
onException: function() { self.onException.apply(self, arguments); },
onComplete: complete,
userContext: this.userContext()
Expand Down Expand Up @@ -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();
};
Expand Down Expand Up @@ -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; }
Expand Down Expand Up @@ -1189,6 +1193,10 @@ getJasmineRequireObj().Env = function(j$) {
message: message,
error: error && error.message ? error : null
});

if (self.throwingExpectationFailures()) {
throw new Error(message);
}
};
}

Expand Down Expand Up @@ -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() {};
Expand All @@ -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() {
Expand All @@ -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.skipToCleanup = function(lastRanIndex) {
if (lastRanIndex < this.firstCleanupIx) {
this.run(this.firstCleanupIx);
} else {
this.run(lastRanIndex + 1);
}
};

QueueRunner.prototype.run = function(queueableFns, recursiveIndex) {
var length = queueableFns.length,
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;
}
}
Expand All @@ -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]]);
},
Expand All @@ -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();
};

Expand All @@ -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;
}
}
}
};
Expand Down
101 changes: 101 additions & 0 deletions spec/core/QueueRunnerSpec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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') },
Expand Down Expand Up @@ -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'),
Expand Down
22 changes: 20 additions & 2 deletions spec/core/SpecSpec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
Loading

0 comments on commit 585287b

Please sign in to comment.