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

Add failing test: after hook is not run if test skipped in beforeEach #2287

Merged
merged 5 commits into from
Jun 11, 2016
Merged

Add failing test: after hook is not run if test skipped in beforeEach #2287

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
2c9966c
Add failing test: after hook not run if beforeEach skipped test
dasilvacontin May 29, 2016
22dbad7
wip Fix suite being marked as pending when beforeEach skips
dasilvacontin May 29, 2016
4f2e454
update pending logic to closely mimic previous behavior
boneskull Jun 9, 2016
d2ba7a9
Clarify nature of meta test
ScottFreeCode Jun 10, 2016
d165c82
Nested suites for meta test
ScottFreeCode Jun 10, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 10 additions & 4 deletions lib/runner.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,13 @@ Runner.prototype.hook = function(name, fn) {
}
if (err) {
if (err instanceof Pending) {
suite.pending = true;
if (name === 'beforeEach' || name === 'afterEach') {
self.test.pending = true;
} else {
suite.tests.forEach(function(test) {
test.pending = true;
});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couldn't it use suite.pending if it's not beforeEach or afterEach? Or in the older versions does after run even if before calls this.skip()?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

setting the suite to be pending, at this point, will cause the after hook to not get executed.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(so, the answer is "no")

}
} else {
self.failHook(hook, err);

Expand Down Expand Up @@ -516,7 +522,7 @@ Runner.prototype.runTests = function(suite, fn) {
// execute test and hook(s)
self.emit('test', self.test = test);
self.hookDown('beforeEach', function(err, errSuite) {
if (suite.isPending()) {
if (test.isPending()) {
self.emit('pending', test);
self.emit('test end', test);
return next();
Expand Down Expand Up @@ -843,8 +849,8 @@ function filterLeaks(ok, globals) {
}

// in firefox
// if runner runs in an iframe, this iframe's window.getInterface method not init at first
// it is assigned in some seconds
// if runner runs in an iframe, this iframe's window.getInterface method
// not init at first it is assigned in some seconds
if (global.navigator && (/^getInterface/).test(key)) {
return false;
}
Expand Down
14 changes: 13 additions & 1 deletion test/integration/regression.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
var assert = require('assert');
var assert = require('assert');
var fs = require('fs');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did this... pick up a UTF-8 BOM or something? I will see if I can fix that when I get another free moment.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or not, if it's already merged into the history and isn't hurting anything...

var path = require('path');
var run = require('./helpers').runMocha;
Expand Down Expand Up @@ -50,4 +50,16 @@ describe('regressions', function() {
done();
});
})

describe('issue-2286: after doesn\'t execute if test was skipped in beforeEach', function () {
var afterWasRun = false;
describe('suite with skipped test for meta test', function () {
beforeEach(function () { this.skip(); });
after(function () { afterWasRun = true; });
it('should be pending', function () {});
})
after('meta test', function () {
afterWasRun.should.be.ok();
});
});
});