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

Fix Hook Test Names #3573

Merged
merged 3 commits into from Jan 28, 2019
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
18 changes: 16 additions & 2 deletions lib/runner.js
Expand Up @@ -265,10 +265,18 @@ Runner.prototype.fail = function(test, err) {
* @param {Error} err
*/
Runner.prototype.failHook = function(hook, err) {
hook.originalTitle = hook.originalTitle || hook.title;
if (hook.ctx && hook.ctx.currentTest) {
hook.originalTitle = hook.originalTitle || hook.title;
hook.title =
hook.originalTitle + ' for "' + hook.ctx.currentTest.title + '"';
} else {
var parentTitle;
if (hook.parent.title) {
parentTitle = hook.parent.title;
} else {
parentTitle = hook.parent.root ? '{root}' : '';
}
hook.title = hook.originalTitle + ' in "' + parentTitle + '"';
}

this.fail(hook, err);
Expand All @@ -294,7 +302,13 @@ Runner.prototype.hook = function(name, fn) {
}
self.currentRunnable = hook;

hook.ctx.currentTest = self.test;
if (name === 'beforeAll') {
Copy link
Member

Choose a reason for hiding this comment

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

note to self: replace with constant, see #3655

hook.ctx.currentTest = hook.parent.tests[0];
} else if (name === 'afterAll') {
hook.ctx.currentTest = hook.parent.tests[hook.parent.tests.length - 1];
} else {
hook.ctx.currentTest = self.test;
}

self.emit('hook', hook);

Expand Down
47 changes: 47 additions & 0 deletions test/integration/fixtures/current-test-title.fixture.js
@@ -0,0 +1,47 @@
'use strict';
var assert = require('assert');

function getTitle(ctx) {
return ctx.currentTest && ctx.currentTest.title;
};

before(function () {
assert.equal(getTitle(this), undefined);
});

describe('suite A', () => {

before(function () {
assert.equal(getTitle(this), undefined);
});

describe('suite B', () => {

it('test1 B', () => {});

describe('suite C', function () {
var lap = 0;

before(function () {
assert.equal(getTitle(this), 'test1 C');
});
beforeEach(function () {
assert.equal(getTitle(this), ++lap === 1 ? 'test1 C' : 'test2 C');
});

it('test1 C', function () {});
it('test2 C', function () {});

afterEach(function () {
assert.equal(getTitle(this), lap === 1 ? 'test1 C' : 'test2 C');
});
after(function () {
assert.equal(getTitle(this), 'test2 C');
});
});
});
});

after(function () {
assert.equal(getTitle(this), undefined);
});
@@ -0,0 +1,13 @@
'use strict';

describe('spec 1', function () {
it('should pass', function () { });
describe('spec 2 nested - this title should be used', function () {
after(function() {
throw new Error('after hook nested error');
});
describe('spec 3 nested', function () {
it('it nested - this title should not be used', function () { });
});
});
});
14 changes: 14 additions & 0 deletions test/integration/fixtures/hooks/after-hook-nested-error.fixture.js
@@ -0,0 +1,14 @@
'use strict';

describe('spec 1', function () {
it('should pass', function () { });
describe('spec nested', function () {
after(function() {
throw new Error('after hook nested error');
});
it('it nested - this title should be used', function () { });
});
describe('spec 2 nested', function () {
it('it nested - not this title', function () { });
});
});
@@ -0,0 +1,13 @@
'use strict';

describe('spec 1', function () {
it('should pass', function () { });
describe('spec 2 nested - this title should be used', function () {
before(function() {
throw new Error('before hook nested error');
});
describe('spec 3 nested', function () {
it('it nested - this title should not be used', function () { });
});
});
});
@@ -0,0 +1,11 @@
'use strict';

describe('spec 1', function () {
it('should pass', function () { });
describe('spec nested', function () {
before(function() {
throw new Error('before hook nested error');
});
it('it nested - this title should be used', function () { });
});
});
@@ -0,0 +1,9 @@
'use strict';

before(function() {
throw new Error('before hook root error');
});

describe('spec 1', function () {
it('should not be called', function () { });
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this be an assertion?

Copy link
Member Author

Choose a reason for hiding this comment

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

The assertions are done in "hook-err.spec.js":

expect(res, 'to have failed with error', 'before hook root error')
          .and('to have failed test', '"before all" hook in "{root}"')
          .and('to have passed test count', 0);

});
Expand Up @@ -50,7 +50,7 @@ describe('suite2', function () {
before('before suite2', function () {});
beforeEach('beforeEach suite2', function () {});
it('test suite2', function () {
runOrder.push('test suite2 - should not run');
console.log('test suite2 - should not run');
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this be an assertion?

Copy link
Member Author

Choose a reason for hiding this comment

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

describe('suite2', function () {...} ) is skipped by the --bail flag. So this blog does never run.

});
afterEach('afterEach suite2', function () {});
after('after suite2', function () {});
Expand Down
Expand Up @@ -37,7 +37,7 @@ describe('suite2', function () {
before('before suite2', function () {});
beforeEach('beforeEach suite2', function () {});
it('test suite2', function () {
runOrder.push('test suite2 - should not run');
console.log('test suite2 - should not run');
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this be an assertion?

Copy link
Member Author

@juergba juergba Jan 30, 2019

Choose a reason for hiding this comment

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

describe('suite2', function () {...} ) is skipped by the --bail flag.
So this blog never runs. Checking "failed/passed test count" and "passed test order" in the "spec" file confirms that.

});
afterEach('afterEach suite2', function () {});
after('after suite2', function () {});
Expand Down
106 changes: 105 additions & 1 deletion test/integration/hook-err.spec.js
Expand Up @@ -2,6 +2,7 @@

var helpers = require('./helpers');
var runMocha = helpers.runMocha;
var runMochaJSON = require('./helpers').runMochaJSON;
var splitRegExp = helpers.splitRegExp;
var bang = require('../../lib/reporters/base').symbols.bang;

Expand All @@ -18,7 +19,63 @@ describe('hook error handling', function() {
describe('before hook error tip', function() {
before(run('hooks/before-hook-error-tip.fixture.js', onlyErrorTitle()));
it('should verify results', function() {
expect(lines, 'to equal', ['1) spec 2', '"before all" hook:']);
expect(lines, 'to equal', [
'1) spec 2',
'"before all" hook for "skipped":'
]);
});
});

describe('before hook root error', function() {
it('should verify results', function(done) {
var fixture = 'hooks/before-hook-root-error.fixture.js';
runMochaJSON(fixture, [], function(err, res) {
if (err) {
return done(err);
}
expect(res, 'to have failed with error', 'before hook root error')
.and('to have failed test', '"before all" hook in "{root}"')
.and('to have passed test count', 0);
done();
});
});
});

describe('before hook nested error', function() {
it('should verify results', function(done) {
var fixture = 'hooks/before-hook-nested-error.fixture.js';
runMochaJSON(fixture, [], function(err, res) {
if (err) {
return done(err);
}
expect(res, 'to have failed with error', 'before hook nested error')
.and(
'to have failed test',
'"before all" hook for "it nested - this title should be used"'
)
.and('to have passed test count', 1)
.and('to have passed test', 'should pass');
done();
});
});
});

describe('before hook deepnested error', function() {
it('should verify results', function(done) {
var fixture = 'hooks/before-hook-deepnested-error.fixture.js';
runMochaJSON(fixture, [], function(err, res) {
if (err) {
return done(err);
}
expect(res, 'to have failed with error', 'before hook nested error')
.and(
'to have failed test',
'"before all" hook in "spec 2 nested - this title should be used"'
)
.and('to have passed test count', 1)
.and('to have passed test', 'should pass');
done();
});
});
});

Expand All @@ -36,6 +93,53 @@ describe('hook error handling', function() {
});
});

describe('after hook nested error', function() {
it('should verify results', function(done) {
var fixture = 'hooks/after-hook-nested-error.fixture.js';
runMochaJSON(fixture, [], function(err, res) {
if (err) {
return done(err);
}
expect(res, 'to have failed with error', 'after hook nested error')
.and(
'to have failed test',
'"after all" hook for "it nested - this title should be used"'
)
.and('to have passed test count', 3)
.and(
'to have passed test order',
'should pass',
'it nested - this title should be used',
'it nested - not this title'
);
done();
});
});
});

describe('after hook deepnested error', function() {
it('should verify results', function(done) {
var fixture = 'hooks/after-hook-deepnested-error.fixture.js';
runMochaJSON(fixture, [], function(err, res) {
if (err) {
return done(err);
}
expect(res, 'to have failed with error', 'after hook nested error')
.and(
'to have failed test',
'"after all" hook in "spec 2 nested - this title should be used"'
)
.and('to have passed test count', 2)
.and(
'to have passed test order',
'should pass',
'it nested - this title should not be used'
);
done();
});
});
});

describe('after each hook error', function() {
before(run('hooks/afterEach-hook-error.fixture.js'));
it('should verify results', function() {
Expand Down
13 changes: 13 additions & 0 deletions test/integration/hooks.spec.js
Expand Up @@ -2,6 +2,7 @@

var assert = require('assert');
var runMocha = require('./helpers').runMocha;
var runMochaJSON = require('./helpers').runMochaJSON;
var splitRegExp = require('./helpers').splitRegExp;
var args = ['--reporter', 'dot'];

Expand Down Expand Up @@ -49,4 +50,16 @@ describe('hooks', function() {
done();
});
});

it('current test title of all hooks', function(done) {
runMochaJSON('current-test-title.fixture.js', [], function(err, res) {
if (err) {
return done(err);
}
expect(res, 'to have passed')
.and('to have passed test count', 3)
.and('to have passed test order', 'test1 B', 'test1 C', 'test2 C');
done();
});
});
});
10 changes: 8 additions & 2 deletions test/integration/multiple-done.spec.js
Expand Up @@ -90,7 +90,10 @@ describe('multiple calls to done()', function() {
});

it('correctly attributes the error', function() {
assert.strictEqual(res.failures[0].fullTitle, 'suite "before all" hook');
assert.strictEqual(
res.failures[0].fullTitle,
'suite "before all" hook in "suite"'
);
assert.strictEqual(
res.failures[0].err.message,
'done() called multiple times'
Expand All @@ -116,7 +119,10 @@ describe('multiple calls to done()', function() {
it('correctly attributes the errors', function() {
assert.strictEqual(res.failures.length, 2);
res.failures.forEach(function(failure) {
assert.strictEqual(failure.fullTitle, 'suite "before each" hook');
assert.strictEqual(
failure.fullTitle,
'suite "before each" hook in "suite"'
);
assert.strictEqual(failure.err.message, 'done() called multiple times');
});
});
Expand Down
10 changes: 8 additions & 2 deletions test/integration/options/bail.spec.js
Expand Up @@ -52,7 +52,10 @@ describe('--bail', function() {

expect(res, 'to have failed')
.and('to have failed test count', 1)
.and('to have failed test', '"before all" hook: before suite1')
.and(
'to have failed test',
'"before all" hook: before suite1 for "test suite1"'
)
.and('to have passed test count', 0);
done();
});
Expand Down Expand Up @@ -100,7 +103,10 @@ describe('--bail', function() {

expect(res, 'to have failed')
.and('to have failed test count', 1)
.and('to have failed test', '"after all" hook: after suite1A')
.and(
'to have failed test',
'"after all" hook: after suite1A for "test suite1A"'
)
.and('to have passed test count', 2)
.and('to have passed test order', 'test suite1', 'test suite1A');
done();
Expand Down