Skip to content

Commit

Permalink
Forbid this.skip() within afterAll hooks (#4136)
Browse files Browse the repository at this point in the history
  • Loading branch information
juergba authored Jan 3, 2020
1 parent 24c22be commit 3bd2d28
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 32 deletions.
4 changes: 2 additions & 2 deletions docs/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -666,9 +666,9 @@ describe('outer', function() {
});
```

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.
> _Updated in v7.0.0. Skipping a test within an "after all" hook is disallowed and will throw an exception. Use a return statement or other means to abort hook execution._
> Before Mocha v3.0.0, `this.skip()` was not supported in asynchronous tests and hooks.
Before Mocha v3.0.0, `this.skip()` was not supported in asynchronous tests and hooks.

## Retry Tests

Expand Down
19 changes: 9 additions & 10 deletions lib/runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,9 @@ var sQuote = utils.sQuote;
var stackFilter = utils.stackTraceFilter();
var stringify = utils.stringify;
var type = utils.type;
var createInvalidExceptionError = require('./errors')
.createInvalidExceptionError;
var errors = require('./errors');
var createInvalidExceptionError = errors.createInvalidExceptionError;
var createUnsupportedError = errors.createUnsupportedError;

/**
* Non-enumerable globals.
Expand Down Expand Up @@ -387,12 +388,6 @@ Runner.prototype.hook = function(name, fn) {
}
// 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_AFTER_EACH) {
// TODO define and implement use case
if (self.test) {
Expand All @@ -405,14 +400,18 @@ Runner.prototype.hook = function(name, fn) {
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
} else if (name === HOOK_TYPE_BEFORE_ALL) {
suite.tests.forEach(function(test) {
test.pending = true;
});
suite.suites.forEach(function(suite) {
suite.pending = true;
});
} else {
hook.pending = false;
var errForbid = createUnsupportedError('`this.skip` forbidden');
self.failHook(hook, errForbid);
return fn(errForbid);
}
} else if (err) {
self.failHook(hook, err);
Expand Down
3 changes: 1 addition & 2 deletions test/integration/fixtures/pending/skip-sync-after.fixture.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,8 @@
describe('skip in after', function () {
it('should run this test-1', function () {});

after('should print DeprecationWarning', function () {
after('should throw "this.skip forbidden"', function () {
this.skip();
throw new Error('never throws this error');
});

describe('inner suite', function () {
Expand Down
35 changes: 17 additions & 18 deletions test/integration/pending.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,24 +73,23 @@ describe('pending', function() {
});

describe('in after', function() {
it('should run all tests', function(done) {
runMocha(
'pending/skip-sync-after.fixture.js',
args,
function(err, res) {
if (err) {
return done(err);
}
expect(res, 'to have passed').and('to satisfy', {
passing: 3,
failing: 0,
pending: 0,
output: expect.it('to contain', '"after all" hook is DEPRECATED')
});
done();
},
'pipe'
);
it('should throw, but run all tests', function(done) {
run('pending/skip-sync-after.fixture.js', args, function(err, res) {
if (err) {
return done(err);
}
expect(res, 'to have failed with error', '`this.skip` forbidden')
.and('to have failed test count', 1)
.and('to have pending test count', 0)
.and('to have passed test count', 3)
.and(
'to have passed test order',
'should run this test-1',
'should run this test-2',
'should run this test-3'
);
done();
});
});
});

Expand Down

0 comments on commit 3bd2d28

Please sign in to comment.