From da238fa294a7958d09c752182f7a7822a4349e8b Mon Sep 17 00:00:00 2001 From: juergba Date: Mon, 18 Feb 2019 15:22:33 +0100 Subject: [PATCH 1/2] fix this.skip() in beforeEach hooks --- lib/runner.js | 28 ++++++++++++++++++++++------ 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/lib/runner.js b/lib/runner.js index 374a6143bf..5f148d8abd 100644 --- a/lib/runner.js +++ b/lib/runner.js @@ -315,8 +315,7 @@ Runner.prototype.fail = function(test, err) { * - Failed `before each` hook skips remaining tests in a * suite and jumps to corresponding `after each` hook, * which is run only once - * - Failed `after` hook does not alter - * execution order + * - Failed `after` hook does not alter execution order * - Failed `after each` hook skips remaining tests in a * suite and subsuites, but executes other `after each` * hooks @@ -386,7 +385,7 @@ Runner.prototype.hook = function(name, fn) { if (testError) { self.fail(self.test, testError); } - // conditional this.skip() + // conditional skip if (hook.pending) { if (name === HOOK_TYPE_AFTER_ALL) { utils.deprecate( @@ -394,11 +393,20 @@ Runner.prototype.hook = function(name, fn) { 'Use a return statement or other means to abort hook execution.' ); } - if (name === HOOK_TYPE_BEFORE_EACH || name === HOOK_TYPE_AFTER_EACH) { + if (name === HOOK_TYPE_AFTER_EACH) { + // TODO define and implement use case if (self.test) { self.test.pending = true; } + } else if (name === HOOK_TYPE_BEFORE_EACH) { + if (self.test) { + self.test.pending = true; + } + self.emit(constants.EVENT_HOOK_END, hook); + hook.pending = false; // activates hook for next test + return fn(new Error('abort hookDown')); } else { + // TODO throw error for afterAll suite.tests.forEach(function(test) { test.pending = true; }); @@ -619,6 +627,7 @@ Runner.prototype.runTests = function(suite, fn) { return; } + // static skip, no hooks are executed if (test.isPending()) { if (self.forbidPending) { test.isPending = alwaysFalse; @@ -634,6 +643,7 @@ Runner.prototype.runTests = function(suite, fn) { // execute test and hook(s) self.emit(constants.EVENT_TEST_BEGIN, (self.test = test)); self.hookDown(HOOK_TYPE_BEFORE_EACH, function(err, errSuite) { + // conditional skip within beforeEach if (test.isPending()) { if (self.forbidPending) { test.isPending = alwaysFalse; @@ -643,7 +653,13 @@ Runner.prototype.runTests = function(suite, fn) { self.emit(constants.EVENT_TEST_PENDING, test); } self.emit(constants.EVENT_TEST_END, test); - return next(); + // skip inner afterEach hooks below errSuite level + var origSuite = self.suite; + self.suite = errSuite; + return self.hookUp(HOOK_TYPE_AFTER_EACH, function(e, eSuite) { + self.suite = origSuite; + next(e, eSuite); + }); } if (err) { return hookErr(err, errSuite, false); @@ -651,7 +667,7 @@ Runner.prototype.runTests = function(suite, fn) { self.currentRunnable = self.test; self.runTest(function(err) { test = self.test; - // conditional this.skip() + // conditional skip within it if (test.pending) { if (self.forbidPending) { test.isPending = alwaysFalse; From dc9bf3baea8eccb9a0a82366371549b2c160b62e Mon Sep 17 00:00:00 2001 From: juergba Date: Tue, 19 Feb 2019 10:59:23 +0100 Subject: [PATCH 2/2] additional tests --- .../pending/skip-async-beforeEach.fixture.js | 46 ++++++++++++++---- .../skip-sync-beforeEach-cond.fixture.js | 28 +++++++++++ .../pending/skip-sync-beforeEach.fixture.js | 40 ++++++++++++++-- test/integration/pending.spec.js | 48 ++++++++++++++----- 4 files changed, 135 insertions(+), 27 deletions(-) create mode 100644 test/integration/fixtures/pending/skip-sync-beforeEach-cond.fixture.js diff --git a/test/integration/fixtures/pending/skip-async-beforeEach.fixture.js b/test/integration/fixtures/pending/skip-async-beforeEach.fixture.js index a450e44bdd..73f188d100 100644 --- a/test/integration/fixtures/pending/skip-async-beforeEach.fixture.js +++ b/test/integration/fixtures/pending/skip-async-beforeEach.fixture.js @@ -1,20 +1,48 @@ 'use strict'; +var assert = require('assert'); -describe('skip in beforeEach', function () { - beforeEach(function (done) { +describe('skip in beforeEach', function() { + var runOrder = []; + beforeEach(function(done) { + runOrder.push('beforeEach'); var self = this; - setTimeout(function () { + setTimeout(function() { self.skip(); // done() is not required - }, 0); + }, 10); }); - it('should skip this test-1', function () { + it('should skip this test-1', function() { throw new Error('never run this test'); }); - it('should skip this test-2', function () { - throw new Error('never run this test'); + + describe('inner', function() { + beforeEach(function() { + runOrder.push('should not run'); + }); + + it('should skip this test-2', function() { + throw new Error('never run this test'); + }); + it('should skip this test-3', function() { + throw new Error('never run this test'); + }); + + afterEach(function() { + runOrder.push('should not run'); + }); }); - it('should skip this test-3', function () { - throw new Error('never run this test'); + + afterEach(function() { + runOrder.push('afterEach'); + }); + after(function() { + runOrder.push('after'); + assert.deepStrictEqual(runOrder, [ + 'beforeEach', 'afterEach', + 'beforeEach', 'afterEach', + 'beforeEach', 'afterEach', + 'after' + ]); + throw new Error('should throw this error'); }); }); diff --git a/test/integration/fixtures/pending/skip-sync-beforeEach-cond.fixture.js b/test/integration/fixtures/pending/skip-sync-beforeEach-cond.fixture.js new file mode 100644 index 0000000000..ff7d3c8ea6 --- /dev/null +++ b/test/integration/fixtures/pending/skip-sync-beforeEach-cond.fixture.js @@ -0,0 +1,28 @@ +'use strict'; + +describe('skip conditionally in beforeEach', function() { + var n = 1; + beforeEach(function() { + if (n !== 2) { + this.skip(); + } + }); + + it('should skip this test-1', function() { + throw new Error('never run this test'); + }); + it('should run this test-2', function() {}); + + describe('inner suite', function() { + it('should skip this test-3', function() { + throw new Error('never run this test'); + }); + }); + + afterEach(function() { n++; }); + after(function() { + if (n === 4) { + throw new Error('should throw this error'); + } + }); +}); diff --git a/test/integration/fixtures/pending/skip-sync-beforeEach.fixture.js b/test/integration/fixtures/pending/skip-sync-beforeEach.fixture.js index 2312265613..43ad644224 100644 --- a/test/integration/fixtures/pending/skip-sync-beforeEach.fixture.js +++ b/test/integration/fixtures/pending/skip-sync-beforeEach.fixture.js @@ -1,15 +1,45 @@ 'use strict'; +var assert = require('assert'); -describe('skip in beforeEach', function () { - beforeEach(function () { +describe('skip in beforeEach', function() { + var runOrder = []; + beforeEach(function() { + runOrder.push('beforeEach'); this.skip(); }); - it('should never run this test', function () { + it('should skip this test-1', function() { throw new Error('never run this test'); }); - it('should never run this test', function () { - throw new Error('never run this test'); + describe('inner', function() { + beforeEach(function() { + runOrder.push('should not run'); + }); + + it('should skip this test-2', function() { + throw new Error('never run this test'); + }); + it('should skip this test-3', function() { + throw new Error('never run this test'); + }); + + afterEach(function() { + runOrder.push('should not run'); + }); + }); + + afterEach(function() { + runOrder.push('afterEach'); + }); + after(function() { + runOrder.push('after'); + assert.deepStrictEqual(runOrder, [ + 'beforeEach', 'afterEach', + 'beforeEach', 'afterEach', + 'beforeEach', 'afterEach', + 'after' + ]); + throw new Error('should throw this error'); }); }); diff --git a/test/integration/pending.spec.js b/test/integration/pending.spec.js index 98620c8c9a..3232fb0805 100644 --- a/test/integration/pending.spec.js +++ b/test/integration/pending.spec.js @@ -170,18 +170,40 @@ describe('pending', function() { describe('in beforeEach', function() { it('should skip all suite specs', function(done) { - run('pending/skip-sync-beforeEach.fixture.js', args, function( - err, - res - ) { + var fixture = 'pending/skip-sync-beforeEach.fixture.js'; + run(fixture, args, function(err, res) { if (err) { - done(err); - return; + return done(err); } - assert.strictEqual(res.stats.pending, 2); - assert.strictEqual(res.stats.passes, 0); - assert.strictEqual(res.stats.failures, 0); - assert.strictEqual(res.code, 0); + expect(res, 'to have failed with error', 'should throw this error') + .and('to have failed test count', 1) + .and('to have pending test count', 3) + .and( + 'to have pending test order', + 'should skip this test-1', + 'should skip this test-2', + 'should skip this test-3' + ) + .and('to have passed test count', 0); + done(); + }); + }); + it('should skip only two suite specs', function(done) { + var fixture = 'pending/skip-sync-beforeEach-cond.fixture.js'; + run(fixture, args, function(err, res) { + if (err) { + return done(err); + } + expect(res, 'to have failed with error', 'should throw this error') + .and('to have failed test count', 1) + .and('to have pending test count', 2) + .and( + 'to have pending test order', + 'should skip this test-1', + 'should skip this test-3' + ) + .and('to have passed test count', 1) + .and('to have passed test', 'should run this test-2'); done(); }); }); @@ -287,8 +309,8 @@ describe('pending', function() { if (err) { return done(err); } - expect(res, 'to have passed') - .and('to have passed test count', 0) + expect(res, 'to have failed with error', 'should throw this error') + .and('to have failed test count', 1) .and('to have pending test count', 3) .and( 'to have pending test order', @@ -296,7 +318,7 @@ describe('pending', function() { 'should skip this test-2', 'should skip this test-3' ) - .and('to have failed test count', 0); + .and('to have passed test count', 0); done(); }); });