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.

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

Forbid this.skip() within afterAll hooks #4136

Merged
merged 3 commits into from Jan 3, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions docs/index.md
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
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
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
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