Skip to content

Commit

Permalink
Fix hook pattern of this.skip() in it() tests (#3859)
Browse files Browse the repository at this point in the history
  • Loading branch information
juergba authored Aug 11, 2019
1 parent 6d7c32a commit 7297b0a
Show file tree
Hide file tree
Showing 4 changed files with 52 additions and 19 deletions.
5 changes: 0 additions & 5 deletions lib/runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -673,11 +673,6 @@ Runner.prototype.runTests = function(suite, fn) {
self.fail(test, err);
}
self.emit(constants.EVENT_TEST_END, test);

if (err instanceof Pending) {
return next();
}

return self.hookUp(HOOK_TYPE_AFTER_EACH, next);
}

Expand Down
20 changes: 19 additions & 1 deletion test/integration/fixtures/pending/skip-async-spec.fixture.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,30 @@
'use strict';
var assert = require('assert');

describe('skip in test', function () {
var runOrder = [];
beforeEach(function () {
runOrder.push('beforeEach');
});

it('should skip async', function (done) {
var self = this;
setTimeout(function () {
self.skip(); // done() is not required
}, 0);
});
it('should run other tests in suite', function () {});

it('should run other tests in the suite', function () {});
afterEach(function() {
runOrder.push('afterEach');
});
after(function() {
runOrder.push('after');
assert.deepStrictEqual(runOrder, [
'beforeEach', 'afterEach',
'beforeEach', 'afterEach',
'after'
]);
throw new Error('should throw this error');
});
});
20 changes: 19 additions & 1 deletion test/integration/fixtures/pending/skip-sync-spec.fixture.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,28 @@
'use strict';
var assert = require('assert');

describe('skip in test', function () {
var runOrder = [];
beforeEach(function () {
runOrder.push('beforeEach');
});

it('should skip immediately', function () {
this.skip();
throw new Error('never run this test');
});
it('should run other tests in suite', function () {});

it('should run other tests in the suite', function () {});
afterEach(function() {
runOrder.push('afterEach');
});
after(function() {
runOrder.push('after');
assert.deepStrictEqual(runOrder, [
'beforeEach', 'afterEach',
'beforeEach', 'afterEach',
'after'
]);
throw new Error('should throw this error');
});
});
26 changes: 14 additions & 12 deletions test/integration/pending.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,13 +59,14 @@ describe('pending', function() {
it('should immediately skip the spec and run all others', function(done) {
run('pending/skip-sync-spec.fixture.js', args, function(err, res) {
if (err) {
done(err);
return;
return done(err);
}
assert.strictEqual(res.stats.pending, 1);
assert.strictEqual(res.stats.passes, 1);
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', 1)
.and('to have pending test order', 'should skip immediately')
.and('to have passed test count', 1)
.and('to have passed tests', 'should run other tests in suite');
done();
});
});
Expand Down Expand Up @@ -192,13 +193,14 @@ describe('pending', function() {
it('should immediately skip the spec and run all others', function(done) {
run('pending/skip-async-spec.fixture.js', args, function(err, res) {
if (err) {
done(err);
return;
return done(err);
}
assert.strictEqual(res.stats.pending, 1);
assert.strictEqual(res.stats.passes, 1);
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', 1)
.and('to have pending test order', 'should skip async')
.and('to have passed test count', 1)
.and('to have passed tests', 'should run other tests in suite');
done();
});
});
Expand Down

0 comments on commit 7297b0a

Please sign in to comment.