Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix hook pattern of 'this.skip()' in beforeEach hooks #3741

Merged
merged 2 commits into from Jan 1, 2020
Merged

Fix hook pattern of 'this.skip()' in beforeEach hooks #3741

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
da238fa
fix this.skip() in beforeEach hooks
juergba Feb 18, 2019
dc9bf3b
additional tests
juergba Feb 19, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
28 changes: 22 additions & 6 deletions lib/runner.js
View file
Open in desktop
Expand Up @@ -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
Expand Down Expand Up @@ -386,19 +385,28 @@ 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(
'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 (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;
});
Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -643,15 +653,21 @@ 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);
}
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;
Expand Down
46 changes: 37 additions & 9 deletions test/integration/fixtures/pending/skip-async-beforeEach.fixture.js
View file
Open in desktop
@@ -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');
});
});
28 changes: 28 additions & 0 deletions test/integration/fixtures/pending/skip-sync-beforeEach-cond.fixture.js
View file
Open in desktop
@@ -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');
}
});
});
40 changes: 35 additions & 5 deletions test/integration/fixtures/pending/skip-sync-beforeEach.fixture.js
View file
Open in desktop
@@ -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');
});
});
48 changes: 35 additions & 13 deletions test/integration/pending.spec.js
View file
Open in desktop
Expand Up @@ -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();
});
});
Expand Down Expand Up @@ -287,16 +309,16 @@ 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',
'should skip this test-1',
'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();
});
});
Expand Down