From 9f82ec86b40b16e54204bed2c96b8ef0b8e3b416 Mon Sep 17 00:00:00 2001 From: Christopher Hiller Date: Fri, 16 Sep 2016 16:47:17 -0700 Subject: [PATCH] always halt execution when async skip() called; closes #2465 (#2479) - always halt execution when async skip() called; closes #2465 - also fixes lack of support for async `skip()` when `allowUncaught` option is used - adds some debuggability into `this.skip()` calls - avoid double-failure conditions when using this.skip() - fix bad Runner tests; lint & add test/runner.js to check --- .eslintignore | 2 + Makefile | 2 +- lib/runnable.js | 15 ++- lib/runner.js | 10 +- test/runner.js | 319 ++++++++++++++++++++++++++---------------------- 5 files changed, 191 insertions(+), 157 deletions(-) diff --git a/.eslintignore b/.eslintignore index e9067bb70d..2b550ca81d 100644 --- a/.eslintignore +++ b/.eslintignore @@ -1 +1,3 @@ lib/to-iso-string/ +test/**/* +!test/runner.js diff --git a/Makefile b/Makefile index 3627f046ab..4cd9692269 100644 --- a/Makefile +++ b/Makefile @@ -25,7 +25,7 @@ clean: lint: @printf "==> [Test :: Lint]\n" - $(ESLINT) browser-entry.js index.js karma.conf.js bin/mocha bin/_mocha "lib/**/*.js" "scripts/**/*.js" + $(ESLINT) browser-entry.js index.js karma.conf.js bin/mocha bin/_mocha "lib/**/*.js" "scripts/**/*.js" test test-node: test-bdd test-tdd test-qunit test-exports test-unit test-integration test-jsapi test-compilers test-glob test-requires test-reporters test-only test-global-only diff --git a/lib/runnable.js b/lib/runnable.js index d89a7dd5d7..52ee900b22 100644 --- a/lib/runnable.js +++ b/lib/runnable.js @@ -133,7 +133,7 @@ Runnable.prototype.enableTimeouts = function(enabled) { * @api public */ Runnable.prototype.skip = function() { - throw new Pending(); + throw new Pending('sync skip'); }; /** @@ -298,14 +298,19 @@ Runnable.prototype.run = function(fn) { if (this.async) { this.resetTimeout(); + // 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. + throw new Pending('async skip; aborting execution'); + }; + if (this.allowUncaught) { return callFnAsync(this.fn); } try { - // allows skip() to be used in an explicit async context - this.skip = function() { - done(new Pending()); - }; callFnAsync(this.fn); } catch (err) { done(utils.getError(err)); diff --git a/lib/runner.js b/lib/runner.js index 8831c8e1cb..22907f570f 100644 --- a/lib/runner.js +++ b/lib/runner.js @@ -220,6 +220,10 @@ Runner.prototype.checkGlobals = function(test) { * @param {Error} err */ Runner.prototype.fail = function(test, err) { + if (test.isPending()) { + return; + } + ++this.failures; test.state = 'failed'; @@ -309,6 +313,8 @@ Runner.prototype.hook = function(name, fn) { suite.tests.forEach(function(test) { test.pending = true; }); + // a pending hook won't be executed twice. + hook.pending = true; } } else { self.failHook(hook, err); @@ -695,8 +701,8 @@ Runner.prototype.uncaught = function(err) { runnable.clearTimeout(); - // Ignore errors if complete - if (runnable.state) { + // Ignore errors if complete or pending + if (runnable.state || runnable.isPending()) { return; } this.fail(runnable, err); diff --git a/test/runner.js b/test/runner.js index 6168bb3e34..ffe4d70e8c 100644 --- a/test/runner.js +++ b/test/runner.js @@ -1,55 +1,59 @@ -var mocha = require('../') - , Suite = mocha.Suite - , Runner = mocha.Runner - , Test = mocha.Test; +var mocha = require('../'); +var Suite = mocha.Suite; +var Runner = mocha.Runner; +var Test = mocha.Test; +var Hook = mocha.Hook; -describe('Runner', function(){ - var suite, runner; +function noop() {} - beforeEach(function(){ +describe('Runner', function() { + var suite; + var runner; + + beforeEach(function() { suite = new Suite('Suite', 'root'); runner = new Runner(suite); - }) + }); - describe('.grep()', function(){ - it('should update the runner.total with number of matched tests', function(){ - suite.addTest(new Test('im a test about lions')); - suite.addTest(new Test('im another test about lions')); - suite.addTest(new Test('im a test about bears')); + describe('.grep()', function() { + it('should update the runner.total with number of matched tests', function() { + suite.addTest(new Test('im a test about lions', noop)); + suite.addTest(new Test('im another test about lions', noop)); + suite.addTest(new Test('im a test about bears', noop)); var newRunner = new Runner(suite); newRunner.grep(/lions/); newRunner.total.should.equal(2); - }) + }); - it('should update the runner.total with number of matched tests when inverted', function(){ - suite.addTest(new Test('im a test about lions')); - suite.addTest(new Test('im another test about lions')); - suite.addTest(new Test('im a test about bears')); + it('should update the runner.total with number of matched tests when inverted', function() { + suite.addTest(new Test('im a test about lions', noop)); + suite.addTest(new Test('im another test about lions', noop)); + suite.addTest(new Test('im a test about bears', noop)); var newRunner = new Runner(suite); newRunner.grep(/lions/, true); newRunner.total.should.equal(1); - }) - }) - - describe('.grepTotal()', function(){ - it('should return the total number of matched tests', function(){ - suite.addTest(new Test('im a test about lions')); - suite.addTest(new Test('im another test about lions')); - suite.addTest(new Test('im a test about bears')); + }); + }); + + describe('.grepTotal()', function() { + it('should return the total number of matched tests', function() { + suite.addTest(new Test('im a test about lions', noop)); + suite.addTest(new Test('im another test about lions', noop)); + suite.addTest(new Test('im a test about bears', noop)); runner.grep(/lions/); runner.grepTotal(suite).should.equal(2); - }) + }); - it('should return the total number of matched tests when inverted', function(){ - suite.addTest(new Test('im a test about lions')); - suite.addTest(new Test('im another test about lions')); - suite.addTest(new Test('im a test about bears')); + it('should return the total number of matched tests when inverted', function() { + suite.addTest(new Test('im a test about lions', noop)); + suite.addTest(new Test('im another test about lions', noop)); + suite.addTest(new Test('im a test about bears', noop)); runner.grep(/lions/, true); runner.grepTotal(suite).should.equal(1); - }) - }) + }); + }); - describe('.globalProps()', function(){ + describe('.globalProps()', function() { it('should include common non enumerable globals', function() { var props = runner.globalProps(); props.should.containEql('setTimeout'); @@ -61,19 +65,19 @@ describe('Runner', function(){ }); }); - describe('.globals()', function(){ - it('should default to the known globals', function(){ + describe('.globals()', function() { + it('should default to the known globals', function() { runner.globals().length.should.be.above(16); - }) + }); - it('should white-list globals', function(){ + it('should white-list globals', function() { runner.globals(['foo', 'bar']); runner.globals().should.containEql('foo'); runner.globals().should.containEql('bar'); - }) - }) + }); + }); - describe('.checkGlobals(test)', function(){ + describe('.checkGlobals(test)', function() { it('should allow variables that match a wildcard', function(done) { runner.globals(['foo*', 'giz*']); global.foo = 'baz'; @@ -81,43 +85,44 @@ describe('Runner', function(){ runner.checkGlobals(); delete global.foo; delete global.gizmo; - done() - }) + done(); + }); - it('should emit "fail" when a new global is introduced', function(done){ + it('should emit "fail" when a new global is introduced', function(done) { + var test = new Test('im a test', noop); runner.checkGlobals(); global.foo = 'bar'; - runner.on('fail', function(test, err){ - test.should.equal('im a test'); + runner.on('fail', function(_test, err) { + _test.should.equal(test); err.message.should.equal('global leak detected: foo'); delete global.foo; done(); }); - runner.checkGlobals('im a test'); - }) + runner.checkGlobals(test); + }); it('should emit "fail" when a single new disallowed global is introduced after a single extra global is allowed', function(done) { var doneCalled = false; runner.globals('good'); global.bad = 1; - runner.on('fail', function(test, err) { + runner.on('fail', function() { delete global.bad; done(); doneCalled = true; }); - runner.checkGlobals('test'); + runner.checkGlobals(new Test('yet another test', noop)); if (!doneCalled) { - done(Error("Expected test failure did not occur.")); + done(Error('Expected test failure did not occur.')); } }); - it ('should not fail when a new common global is introduced', function(){ + it('should not fail when a new common global is introduced', function() { // verify that the prop isn't enumerable delete global.XMLHttpRequest; global.propertyIsEnumerable('XMLHttpRequest').should.not.be.ok(); // create a new runner and keep a reference to the test. - var test = new Test('im a test about bears'); + var test = new Test('im a test about bears', noop); suite.addTest(test); var newRunner = new Runner(suite); @@ -133,22 +138,23 @@ describe('Runner', function(){ delete global.XMLHttpRequest; }); - it('should pluralize the error message when several are introduced', function(done){ + it('should pluralize the error message when several are introduced', function(done) { + var test = new Test('im a test', noop); runner.checkGlobals(); global.foo = 'bar'; global.bar = 'baz'; - runner.on('fail', function(test, err){ - test.should.equal('im a test'); + runner.on('fail', function(_test, err) { + _test.should.equal(test); err.message.should.equal('global leaks detected: foo, bar'); delete global.foo; delete global.bar; done(); }); - runner.checkGlobals('im a test'); - }) + runner.checkGlobals(test); + }); it('should respect per test whitelisted globals', function() { - var test = new Test('im a test about lions'); + var test = new Test('im a test about lions', noop); test.globals(['foo']); suite.addTest(test); @@ -161,166 +167,179 @@ describe('Runner', function(){ test.should.not.have.key('state'); delete global.foo; - }) + }); it('should respect per test whitelisted globals but still detect other leaks', function(done) { - var test = new Test('im a test about lions'); + var test = new Test('im a test about lions', noop); test.globals(['foo']); suite.addTest(test); global.foo = 'bar'; global.bar = 'baz'; - runner.on('fail', function(test, err){ + runner.on('fail', function(test, err) { test.title.should.equal('im a test about lions'); err.message.should.equal('global leak detected: bar'); delete global.foo; done(); }); runner.checkGlobals(test); - }) + }); it('should emit "fail" when a global beginning with d is introduced', function(done) { global.derp = 'bar'; - runner.on('fail', function(test, err){ + runner.on('fail', function() { delete global.derp; done(); }); - runner.checkGlobals('im a test'); + runner.checkGlobals(new Test('herp', function() {})); }); - }) + }); - describe('.hook(name, fn)', function(){ - it('should execute hooks after failed test if suite bail is true', function(done){ - runner.fail({}); + describe('.hook(name, fn)', function() { + it('should execute hooks after failed test if suite bail is true', function(done) { + runner.fail(new Test('failed test', noop)); suite.bail(true); - suite.afterEach(function(){ + suite.afterEach(function() { suite.afterAll(function() { done(); - }) + }); }); - runner.hook('afterEach', function(){}); - runner.hook('afterAll', function(){}); - }) - }) + runner.hook('afterEach', function() {}); + runner.hook('afterAll', function() {}); + }); + }); - describe('.fail(test, err)', function(){ - it('should increment .failures', function(){ + describe('.fail(test, err)', function() { + it('should increment .failures', function() { runner.failures.should.equal(0); - runner.fail({}, {}); + runner.fail(new Test('one', noop), {}); runner.failures.should.equal(1); - runner.fail({}, {}); + runner.fail(new Test('two', noop), {}); runner.failures.should.equal(2); - }) + }); - it('should set test.state to "failed"', function(){ - var test = {}; + it('should set test.state to "failed"', function() { + var test = new Test('some test', noop); runner.fail(test, 'some error'); test.state.should.equal('failed'); - }) + }); - it('should emit "fail"', function(done){ - var test = {}, err = {}; - runner.on('fail', function(test, err){ + it('should emit "fail"', function(done) { + var test = new Test('some other test', noop); + var err = {}; + runner.on('fail', function(test, err) { test.should.equal(test); err.should.equal(err); done(); }); runner.fail(test, err); - }) + }); - it('should emit a helpful message when failed with a string', function(done){ - var test = {}, err = 'string'; - runner.on('fail', function(test, err){ + it('should emit a helpful message when failed with a string', function(done) { + var test = new Test('helpful test', noop); + var err = 'string'; + runner.on('fail', function(test, err) { err.message.should.equal('the string "string" was thrown, throw an Error :)'); done(); }); runner.fail(test, err); - }) + }); - it('should emit a the error when failed with an Error instance', function(done){ - var test = {}, err = new Error('an error message'); - runner.on('fail', function(test, err){ + it('should emit a the error when failed with an Error instance', function(done) { + var test = new Test('a test', noop); + var err = new Error('an error message'); + runner.on('fail', function(test, err) { err.message.should.equal('an error message'); done(); }); runner.fail(test, err); - }) + }); - it('should emit the error when failed with an Error-like object', function(done){ - var test = {}, err = {message: 'an error message'}; - runner.on('fail', function(test, err){ + it('should emit the error when failed with an Error-like object', function(done) { + var test = new Test('a test', noop); + var err = { message: 'an error message' }; + runner.on('fail', function(test, err) { err.message.should.equal('an error message'); done(); }); runner.fail(test, err); - }) + }); - it('should emit a helpful message when failed with an Object', function(done){ - var test = {}, err = { x: 1 }; - runner.on('fail', function(test, err){ + it('should emit a helpful message when failed with an Object', function(done) { + var test = new Test('a test', noop); + var err = { x: 1 }; + runner.on('fail', function(test, err) { err.message.should.equal('the object {\n "x": 1\n} was thrown, throw an Error :)'); done(); }); runner.fail(test, err); - }) + }); - it('should emit a helpful message when failed with an Array', function(done){ - var test = {}, err = [1,2]; - runner.on('fail', function(test, err){ + it('should emit a helpful message when failed with an Array', function(done) { + var test = new Test('a test', noop); + var err = [ + 1, + 2 + ]; + runner.on('fail', function(test, err) { err.message.should.equal('the array [\n 1\n 2\n] was thrown, throw an Error :)'); done(); }); runner.fail(test, err); - }) - }) + }); + }); - describe('.failHook(hook, err)', function(){ - it('should increment .failures', function(){ + describe('.failHook(hook, err)', function() { + it('should increment .failures', function() { runner.failures.should.equal(0); - runner.failHook({}, {}); + runner.failHook(new Test('fail hook 1', noop), {}); runner.failures.should.equal(1); - runner.failHook({}, {}); + runner.failHook(new Test('fail hook 2', noop), {}); runner.failures.should.equal(2); - }) + }); + + it('should augment hook title with current test title', function() { + var hook = new Hook('"before each" hook'); + hook.ctx = { currentTest: new Test('should behave', noop) }; - it('should augment hook title with current test title', function(){ - var hook = { - title: '"before each" hook', - ctx: { currentTest: new Test('should behave') } - }; runner.failHook(hook, {}); hook.title.should.equal('"before each" hook for "should behave"'); - hook.ctx.currentTest = new Test('should obey'); + hook.ctx.currentTest = new Test('should obey', noop); runner.failHook(hook, {}); hook.title.should.equal('"before each" hook for "should obey"'); - }) + }); - it('should emit "fail"', function(done){ - var hook = {}, err = {}; - runner.on('fail', function(hook, err){ + it('should emit "fail"', function(done) { + var hook = new Hook(); + var err = {}; + runner.on('fail', function(hook, err) { hook.should.equal(hook); err.should.equal(err); done(); }); runner.failHook(hook, err); - }) + }); - it('should emit "end" if suite bail is true', function(done){ - var hook = {}, err = {}; + it('should emit "end" if suite bail is true', function(done) { + var hook = new Hook(); + var err = {}; suite.bail(true); runner.on('end', done); runner.failHook(hook, err); - }) + }); - it('should not emit "end" if suite bail is not true', function(done){ - var hook = {}, err = {}; + it('should not emit "end" if suite bail is not true', function(done) { + var hook = new Hook(); + var err = {}; suite.bail(false); - runner.on('end', function() { throw new Error('"end" was emit, but the bail is false'); }); + runner.on('end', function() { + throw new Error('"end" was emit, but the bail is false'); + }); runner.failHook(hook, err); done(); - }) + }); }); describe('allowUncaught', function() { @@ -339,27 +358,29 @@ describe('Runner', function(){ }); describe('stackTrace', function() { - var stack = [ 'AssertionError: foo bar' - , 'at EventEmitter. (/usr/local/dev/test.js:16:12)' - , 'at Context. (/usr/local/dev/test.js:19:5)' - , 'Test.Runnable.run (/usr/local/lib/node_modules/mocha/lib/runnable.js:244:7)' - , 'Runner.runTest (/usr/local/lib/node_modules/mocha/lib/runner.js:374:10)' - , '/usr/local/lib/node_modules/mocha/lib/runner.js:452:12' - , 'next (/usr/local/lib/node_modules/mocha/lib/runner.js:299:14)' - , '/usr/local/lib/node_modules/mocha/lib/runner.js:309:7' - , 'next (/usr/local/lib/node_modules/mocha/lib/runner.js:248:23)' - , 'Immediate._onImmediate (/usr/local/lib/node_modules/mocha/lib/runner.js:276:5)' - , 'at processImmediate [as _immediateCallback] (timers.js:321:17)']; + var stack = [ + 'AssertionError: foo bar', + 'at EventEmitter. (/usr/local/dev/test.js:16:12)', + 'at Context. (/usr/local/dev/test.js:19:5)', + 'Test.Runnable.run (/usr/local/lib/node_modules/mocha/lib/runnable.js:244:7)', + 'Runner.runTest (/usr/local/lib/node_modules/mocha/lib/runner.js:374:10)', + '/usr/local/lib/node_modules/mocha/lib/runner.js:452:12', + 'next (/usr/local/lib/node_modules/mocha/lib/runner.js:299:14)', + '/usr/local/lib/node_modules/mocha/lib/runner.js:309:7', + 'next (/usr/local/lib/node_modules/mocha/lib/runner.js:248:23)', + 'Immediate._onImmediate (/usr/local/lib/node_modules/mocha/lib/runner.js:276:5)', + 'at processImmediate [as _immediateCallback] (timers.js:321:17)' + ]; describe('shortStackTrace', function() { it('should prettify the stack-trace', function(done) { - var hook = {}, - err = new Error(); + var hook = new Hook(); + var err = new Error(); // Fake stack-trace err.stack = stack.join('\n'); - runner.on('fail', function(hook, err){ - err.stack.should.equal(stack.slice(0,3).join('\n')); + runner.on('fail', function(hook, err) { + err.stack.should.equal(stack.slice(0, 3).join('\n')); done(); }); runner.failHook(hook, err); @@ -368,14 +389,14 @@ describe('Runner', function(){ describe('longStackTrace', function() { it('should display the full stack-trace', function(done) { - var hook = {}, - err = new Error(); + var hook = new Hook(); + var err = new Error(); // Fake stack-trace err.stack = stack.join('\n'); // Add --stack-trace option runner.fullStackTrace = true; - runner.on('fail', function(hook, err){ + runner.on('fail', function(hook, err) { err.stack.should.equal(stack.join('\n')); done(); });