diff --git a/lib/runnable.js b/lib/runnable.js index 026ab06b40..d0f3aebf18 100644 --- a/lib/runnable.js +++ b/lib/runnable.js @@ -135,6 +135,7 @@ Runnable.prototype.enableTimeouts = function(enabled) { * @public */ Runnable.prototype.skip = function() { + this.pending = true; throw new Pending('sync skip'); }; @@ -343,34 +344,27 @@ Runnable.prototype.run = function(fn) { // allows skip() to be used in an explicit async context this.skip = function asyncSkip() { - done(new Pending('async skip call')); - // halt execution. the Runnable will be marked pending - // by the previous call, and the uncaught handler will ignore - // the failure. + this.pending = true; + done(); + // halt execution, the uncaught handler will ignore the failure. throw new Pending('async skip; aborting execution'); }; - if (this.allowUncaught) { - return callFnAsync(this.fn); - } try { callFnAsync(this.fn); } catch (err) { + // handles async runnables which actually run synchronously emitted = true; + if (err instanceof Pending) { + return; // done() is already called in this.skip() + } else if (this.allowUncaught) { + throw err; + } done(Runnable.toValueOrError(err)); } return; } - if (this.allowUncaught) { - if (this.isPending()) { - done(); - } else { - callFn(this.fn); - } - return; - } - // sync or promise-returning try { if (this.isPending()) { @@ -380,6 +374,11 @@ Runnable.prototype.run = function(fn) { } } catch (err) { emitted = true; + if (err instanceof Pending) { + return done(); + } else if (this.allowUncaught) { + throw err; + } done(Runnable.toValueOrError(err)); } diff --git a/lib/runner.js b/lib/runner.js index e4bb329af3..ca447f139b 100644 --- a/lib/runner.js +++ b/lib/runner.js @@ -386,34 +386,30 @@ Runner.prototype.hook = function(name, fn) { if (testError) { self.fail(self.test, testError); } - if (err) { - if (err instanceof Pending) { - if (name === HOOK_TYPE_AFTER_ALL) { - utils.deprecate( - 'Skipping a test within an "after all" hook is DEPRECATED and will throw an exception in a future version of Mocha. ' + - 'Use a return statement or other means to abort hook execution.' - ); - } - if (name === HOOK_TYPE_BEFORE_EACH || name === HOOK_TYPE_AFTER_EACH) { - if (self.test) { - self.test.pending = true; - } - } else { - suite.tests.forEach(function(test) { - test.pending = true; - }); - suite.suites.forEach(function(suite) { - suite.pending = true; - }); - // a pending hook won't be executed twice. - hook.pending = true; + // conditional this.skip() + if (hook.pending) { + if (name === HOOK_TYPE_AFTER_ALL) { + utils.deprecate( + 'Skipping a test within an "after all" hook is DEPRECATED and will throw an exception in a future version of Mocha. ' + + 'Use a return statement or other means to abort hook execution.' + ); + } + if (name === HOOK_TYPE_BEFORE_EACH || name === HOOK_TYPE_AFTER_EACH) { + if (self.test) { + self.test.pending = true; } } else { - self.failHook(hook, err); - - // stop executing hooks, notify callee of hook err - return fn(err); + suite.tests.forEach(function(test) { + test.pending = true; + }); + suite.suites.forEach(function(suite) { + suite.pending = true; + }); } + } else if (err) { + self.failHook(hook, err); + // stop executing hooks, notify callee of hook err + return fn(err); } self.emit(constants.EVENT_HOOK_END, hook); delete hook.ctx.currentTest; @@ -652,14 +648,20 @@ Runner.prototype.runTests = function(suite, fn) { self.currentRunnable = self.test; self.runTest(function(err) { test = self.test; - if (err) { - var retry = test.currentRetry(); - if (err instanceof Pending && self.forbidPending) { + // conditional this.skip() + if (test.pending) { + if (self.forbidPending) { + test.isPending = alwaysFalse; self.fail(test, new Error('Pending test forbidden')); - } else if (err instanceof Pending) { - test.pending = true; + delete test.isPending; + } else { self.emit(constants.EVENT_TEST_PENDING, test); - } else if (retry < test.retries()) { + } + self.emit(constants.EVENT_TEST_END, test); + return self.hookUp(HOOK_TYPE_AFTER_EACH, next); + } else if (err) { + var retry = test.currentRetry(); + if (retry < test.retries()) { var clonedTest = test.clone(); clonedTest.currentRetry(retry + 1); tests.unshift(clonedTest); @@ -910,6 +912,12 @@ Runner.prototype.run = function(fn) { this.on(constants.EVENT_RUN_END, function() { debug(constants.EVENT_RUN_END); process.removeListener('uncaughtException', uncaught); + process.on('uncaughtException', function(err) { + if (err instanceof Pending) { + return; + } + throw err; + }); fn(self.failures); }); diff --git a/test/integration/fixtures/options/allow-uncaught/this-skip-it.fixture.js b/test/integration/fixtures/options/allow-uncaught/this-skip-it.fixture.js new file mode 100644 index 0000000000..ce1c3b0a96 --- /dev/null +++ b/test/integration/fixtures/options/allow-uncaught/this-skip-it.fixture.js @@ -0,0 +1,21 @@ +'use strict'; + +describe('test suite', () => { + it('test1', function () { }); + it('test2', function (done) { + var self = this; + setTimeout(function () { + self.skip(); + throw new Error("should not throw"); + }, 10); + }); + it('test3', function () { + this.skip(); + throw new Error("should not throw"); + }); + it('test4', function () { }); + it('test5', function () { + this.skip(); + throw new Error("should not throw"); + }); +}); diff --git a/test/integration/options/allowUncaught.spec.js b/test/integration/options/allowUncaught.spec.js new file mode 100644 index 0000000000..a3d8739ffe --- /dev/null +++ b/test/integration/options/allowUncaught.spec.js @@ -0,0 +1,25 @@ +'use strict'; + +var path = require('path').posix; +var helpers = require('../helpers'); +var runMochaJSON = helpers.runMochaJSON; + +describe('--allow-uncaught', function() { + var args = ['--allow-uncaught']; + + it('should run with conditional `this.skip()`', function(done) { + var fixture = path.join('options', 'allow-uncaught', 'this-skip-it'); + runMochaJSON(fixture, args, function(err, res) { + if (err) { + return done(err); + } + + expect(res, 'to have passed') + .and('to have passed test count', 2) + .and('to have pending test count', 3) + .and('to have passed test', 'test1', 'test4') + .and('to have pending test order', 'test2', 'test3', 'test5'); + done(); + }); + }); +}); diff --git a/test/unit/runnable.spec.js b/test/unit/runnable.spec.js index f912bbaf54..40ccc12c7e 100644 --- a/test/unit/runnable.spec.js +++ b/test/unit/runnable.spec.js @@ -4,7 +4,6 @@ var Mocha = require('../../lib/mocha'); var Runnable = Mocha.Runnable; var Suite = Mocha.Suite; var sinon = require('sinon'); -var Pending = require('../../lib/pending'); var STATE_FAILED = Runnable.constants.STATE_FAILED; describe('Runnable(title, fn)', function() { @@ -644,13 +643,14 @@ describe('Runnable(title, fn)', function() { }); describe('if async', function() { - it('this.skip() should call callback with Pending', function(done) { + it('this.skip() should set runnable to pending', function(done) { var runnable = new Runnable('foo', function(done) { // normally "this" but it gets around having to muck with a context runnable.skip(); }); runnable.run(function(err) { - expect(err.constructor, 'to be', Pending); + expect(err, 'to be undefined'); + expect(runnable.pending, 'to be true'); done(); }); }); @@ -663,8 +663,10 @@ describe('Runnable(title, fn)', function() { aborted = false; }); runnable.run(function() { - expect(aborted, 'to be true'); - done(); + process.nextTick(function() { + expect(aborted, 'to be true'); + done(); + }); }); }); });