From fe806f4e5da6b6460c86f71137fba681dcafd79a Mon Sep 17 00:00:00 2001 From: Andrew Bradley Date: Sun, 26 Jul 2020 14:56:57 -0400 Subject: [PATCH 01/13] fix #4348 --- lib/runner.js | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/lib/runner.js b/lib/runner.js index 1214121e1e..6716c16c11 100644 --- a/lib/runner.js +++ b/lib/runner.js @@ -419,20 +419,6 @@ Runner.prototype.fail = function(test, err, force) { * @param {Error} err */ Runner.prototype.failHook = function(hook, err) { - hook.originalTitle = hook.originalTitle || hook.title; - if (hook.ctx && hook.ctx.currentTest) { - hook.title = - hook.originalTitle + ' for ' + dQuote(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 ' + dQuote(parentTitle); - } - this.fail(hook, err); }; @@ -456,6 +442,20 @@ Runner.prototype.hook = function(name, fn) { } self.currentRunnable = hook; + hook.originalTitle = hook.originalTitle || hook.title; + if (hook.ctx && hook.ctx.currentTest) { + hook.title = + hook.originalTitle + ' for ' + dQuote(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 ' + dQuote(parentTitle); + } + if (name === HOOK_TYPE_BEFORE_ALL) { hook.ctx.currentTest = hook.parent.tests[0]; } else if (name === HOOK_TYPE_AFTER_ALL) { From 2e03e57dae652c859bf5be7b6daa1e1e4b58eb1a Mon Sep 17 00:00:00 2001 From: Andrew Bradley Date: Sun, 26 Jul 2020 15:45:39 -0400 Subject: [PATCH 02/13] fix --- lib/runner.js | 16 +++++++++------- test/unit/runner.spec.js | 35 +++++++++++++++++++++++------------ 2 files changed, 32 insertions(+), 19 deletions(-) diff --git a/lib/runner.js b/lib/runner.js index 6716c16c11..0c03f21350 100644 --- a/lib/runner.js +++ b/lib/runner.js @@ -442,6 +442,14 @@ Runner.prototype.hook = function(name, fn) { } self.currentRunnable = hook; + if (name === HOOK_TYPE_BEFORE_ALL) { + hook.ctx.currentTest = hook.parent.tests[0]; + } else if (name === HOOK_TYPE_AFTER_ALL) { + hook.ctx.currentTest = hook.parent.tests[hook.parent.tests.length - 1]; + } else { + hook.ctx.currentTest = self.test; + } + hook.originalTitle = hook.originalTitle || hook.title; if (hook.ctx && hook.ctx.currentTest) { hook.title = @@ -456,13 +464,7 @@ Runner.prototype.hook = function(name, fn) { hook.title = hook.originalTitle + ' in ' + dQuote(parentTitle); } - if (name === HOOK_TYPE_BEFORE_ALL) { - hook.ctx.currentTest = hook.parent.tests[0]; - } else if (name === HOOK_TYPE_AFTER_ALL) { - hook.ctx.currentTest = hook.parent.tests[hook.parent.tests.length - 1]; - } else { - hook.ctx.currentTest = self.test; - } + console.log([hook.title, hook.originalTitle]); hook.allowUncaught = self.allowUncaught; diff --git a/test/unit/runner.spec.js b/test/unit/runner.spec.js index 28a152fc1e..5f7954037f 100644 --- a/test/unit/runner.spec.js +++ b/test/unit/runner.spec.js @@ -252,6 +252,29 @@ describe('Runner', function() { runner.hook('afterEach', noop); runner.hook('afterAll', noop); }); + + it('should augment hook title with current test title', function(done) { + suite.beforeEach(function() {}); + var hook = suite._beforeEach[0]; + + suite.addTest(new Test('should behave', noop)); + suite.addTest(new Test('should obey', noop)); + + runner.suite = suite; + runner.test = suite.tests[0]; + runner.hook('beforeEach', function(err) { + if (err) return done(err); + expect(hook.title, 'to be', '"before each" hook for "should behave"'); + + runner.test = suite.tests[1]; + runner.hook('beforeEach', function(err) { + if (err) return done(err); + expect(hook.title, 'to be', '"before each" hook for "should obey"'); + + done(); + }); + }); + }); }); describe('fail()', function() { @@ -431,18 +454,6 @@ describe('Runner', function() { expect(runner.failures, 'to be', 2); }); - it('should augment hook title with current test title', function() { - var hook = new Hook('"before each" hook'); - hook.ctx = {currentTest: new Test('should behave', noop)}; - - runner.failHook(hook, {}); - expect(hook.title, 'to be', '"before each" hook for "should behave"'); - - hook.ctx.currentTest = new Test('should obey', noop); - runner.failHook(hook, {}); - expect(hook.title, 'to be', '"before each" hook for "should obey"'); - }); - it('should emit "fail"', function(done) { var hook = new Hook(); hook.parent = suite; From 597597e608b214c6f0f57473fc474903e0591ba2 Mon Sep 17 00:00:00 2001 From: Andrew Bradley Date: Sun, 26 Jul 2020 15:47:17 -0400 Subject: [PATCH 03/13] fix --- lib/runner.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/runner.js b/lib/runner.js index 0c03f21350..c4cf51ee2f 100644 --- a/lib/runner.js +++ b/lib/runner.js @@ -464,8 +464,6 @@ Runner.prototype.hook = function(name, fn) { hook.title = hook.originalTitle + ' in ' + dQuote(parentTitle); } - console.log([hook.title, hook.originalTitle]); - hook.allowUncaught = self.allowUncaught; self.emit(constants.EVENT_HOOK_BEGIN, hook); From 002c432e0f45429d21b1f8a5ae5ebe704a74a59c Mon Sep 17 00:00:00 2001 From: Andrew Bradley Date: Sun, 26 Jul 2020 17:33:48 -0400 Subject: [PATCH 04/13] fix --- lib/errors.js | 32 ++++++++++++++++++-------- lib/runnable.js | 5 ++-- lib/runner.js | 31 ++++++++++++++----------- test/integration/multiple-done.spec.js | 4 ++-- test/unit/runner.spec.js | 2 +- 5 files changed, 46 insertions(+), 28 deletions(-) diff --git a/lib/errors.js b/lib/errors.js index 929f56399c..afd1e90ee7 100644 --- a/lib/errors.js +++ b/lib/errors.js @@ -270,6 +270,10 @@ function createMochaInstanceAlreadyRunningError(message, instance) { * @returns {Error} instance detailing the error condition */ function createMultipleDoneError(runnable, originalErr) { + return createMultipleDoneErrorFactory(runnable)(originalErr); +} + +function createMultipleDoneErrorFactory(runnable) { var title; try { title = format('<%s>', runnable.fullTitle()); @@ -279,23 +283,30 @@ function createMultipleDoneError(runnable, originalErr) { } catch (ignored) { title = format('<%s> (of unknown suite)', runnable.title); } - var message = format( + var messagePrefix = format( 'done() called multiple times in %s %s', runnable.type ? runnable.type : 'unknown runnable', title ); if (runnable.file) { - message += format(' of file %s', runnable.file); - } - if (originalErr) { - message += format('; in addition, done() received error: %s', originalErr); + messagePrefix += format(' of file %s', runnable.file); } - var err = new Error(message); - err.code = constants.MULTIPLE_DONE; - err.valueType = typeof originalErr; - err.value = originalErr; - return err; + return function multipleDoneErrorFactory(originalErr) { + var message = messagePrefix; + if (originalErr) { + message += format( + '; in addition, done() received error: %s', + originalErr + ); + } + + var err = new Error(message); + err.code = constants.MULTIPLE_DONE; + err.valueType = typeof originalErr; + err.value = originalErr; + return err; + }; } /** @@ -329,6 +340,7 @@ module.exports = { createMochaInstanceAlreadyRunningError: createMochaInstanceAlreadyRunningError, createFatalError: createFatalError, createMultipleDoneError: createMultipleDoneError, + createMultipleDoneErrorFactory: createMultipleDoneErrorFactory, createForbiddenExclusivityError: createForbiddenExclusivityError, constants: constants }; diff --git a/lib/runnable.js b/lib/runnable.js index 023481dd69..417286d189 100644 --- a/lib/runnable.js +++ b/lib/runnable.js @@ -7,7 +7,7 @@ var milliseconds = require('ms'); var utils = require('./utils'); var errors = require('./errors'); var createInvalidExceptionError = errors.createInvalidExceptionError; -var createMultipleDoneError = errors.createMultipleDoneError; +var createMultipleDoneErrorFactory = errors.createMultipleDoneErrorFactory; /** * Save timer references to avoid Sinon interfering (see GH-237). @@ -274,12 +274,13 @@ Runnable.prototype.run = function(fn) { } // called multiple times + var multipleDoneErrorFactory = createMultipleDoneErrorFactory(self); function multiple(err) { if (errorWasHandled) { return; } errorWasHandled = true; - self.emit('error', createMultipleDoneError(self, err)); + self.emit('error', multipleDoneErrorFactory(err)); } // finished diff --git a/lib/runner.js b/lib/runner.js index c4cf51ee2f..51215eba84 100644 --- a/lib/runner.js +++ b/lib/runner.js @@ -450,19 +450,7 @@ Runner.prototype.hook = function(name, fn) { hook.ctx.currentTest = self.test; } - hook.originalTitle = hook.originalTitle || hook.title; - if (hook.ctx && hook.ctx.currentTest) { - hook.title = - hook.originalTitle + ' for ' + dQuote(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 ' + dQuote(parentTitle); - } + setHookTitle(hook); hook.allowUncaught = self.allowUncaught; @@ -514,8 +502,25 @@ Runner.prototype.hook = function(name, fn) { } self.emit(constants.EVENT_HOOK_END, hook); delete hook.ctx.currentTest; + setHookTitle(hook); next(++i); }); + + function setHookTitle(hook) { + hook.originalTitle = hook.originalTitle || hook.title; + if (hook.ctx && hook.ctx.currentTest) { + hook.title = + hook.originalTitle + ' for ' + dQuote(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 ' + dQuote(parentTitle); + } + } } Runner.immediately(function() { diff --git a/test/integration/multiple-done.spec.js b/test/integration/multiple-done.spec.js index 2019df25c5..d09ccfde18 100644 --- a/test/integration/multiple-done.spec.js +++ b/test/integration/multiple-done.spec.js @@ -121,13 +121,13 @@ describe('multiple calls to done()', function() { it('correctly attributes the errors', function() { expect(res.failures, 'to satisfy', [ { - fullTitle: 'suite "before each" hook in "suite"', + fullTitle: 'suite "before each" hook for "test1"', err: { message: /done\(\) called multiple times in hook of file.+multiple-done-before-each\.fixture\.js/ } }, { - fullTitle: 'suite "before each" hook in "suite"', + fullTitle: 'suite "before each" hook for "test2"', err: { message: /done\(\) called multiple times in hook of file.+multiple-done-before-each\.fixture\.js/ } diff --git a/test/unit/runner.spec.js b/test/unit/runner.spec.js index 5f7954037f..4489032416 100644 --- a/test/unit/runner.spec.js +++ b/test/unit/runner.spec.js @@ -271,7 +271,7 @@ describe('Runner', function() { if (err) return done(err); expect(hook.title, 'to be', '"before each" hook for "should obey"'); - done(); + return done(); }); }); }); From 680b14f697bdc9cb5a7a831017445ef51463fd2a Mon Sep 17 00:00:00 2001 From: Andrew Bradley Date: Sun, 26 Jul 2020 17:55:29 -0400 Subject: [PATCH 05/13] fix --- test/unit/runner.spec.js | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/test/unit/runner.spec.js b/test/unit/runner.spec.js index 4489032416..b4aced0adf 100644 --- a/test/unit/runner.spec.js +++ b/test/unit/runner.spec.js @@ -254,23 +254,25 @@ describe('Runner', function() { }); it('should augment hook title with current test title', function(done) { - suite.beforeEach(function() {}); + var expectedHookTitle; + suite.beforeEach(function() { + expect(hook.title, 'to be', expectedHookTitle); + }); var hook = suite._beforeEach[0]; suite.addTest(new Test('should behave', noop)); suite.addTest(new Test('should obey', noop)); - runner.suite = suite; + runner.test = suite.tests[0]; + expectedHookTitle = '"before each" hook for "should behave"'; runner.hook('beforeEach', function(err) { if (err) return done(err); - expect(hook.title, 'to be', '"before each" hook for "should behave"'); runner.test = suite.tests[1]; + expectedHookTitle = '"before each" hook for "should obey"'; runner.hook('beforeEach', function(err) { if (err) return done(err); - expect(hook.title, 'to be', '"before each" hook for "should obey"'); - return done(); }); }); From aea7b657c66f61a9b42a9c2312bd0dbadd3399c3 Mon Sep 17 00:00:00 2001 From: Andrew Bradley Date: Mon, 27 Jul 2020 00:19:41 -0400 Subject: [PATCH 06/13] fix --- lib/runnable.js | 4 ++-- test/integration/multiple-done.spec.js | 24 +++++++++++------------- 2 files changed, 13 insertions(+), 15 deletions(-) diff --git a/lib/runnable.js b/lib/runnable.js index 417286d189..e65acafc89 100644 --- a/lib/runnable.js +++ b/lib/runnable.js @@ -274,13 +274,13 @@ Runnable.prototype.run = function(fn) { } // called multiple times - var multipleDoneErrorFactory = createMultipleDoneErrorFactory(self); + var createMultipleDoneError = createMultipleDoneErrorFactory(self); function multiple(err) { if (errorWasHandled) { return; } errorWasHandled = true; - self.emit('error', multipleDoneErrorFactory(err)); + self.emit('error', createMultipleDoneError(err)); } // finished diff --git a/test/integration/multiple-done.spec.js b/test/integration/multiple-done.spec.js index d09ccfde18..5803575ef6 100644 --- a/test/integration/multiple-done.spec.js +++ b/test/integration/multiple-done.spec.js @@ -119,20 +119,18 @@ describe('multiple calls to done()', function() { }); it('correctly attributes the errors', function() { - expect(res.failures, 'to satisfy', [ - { - fullTitle: 'suite "before each" hook for "test1"', - err: { - message: /done\(\) called multiple times in hook of file.+multiple-done-before-each\.fixture\.js/ - } - }, - { - fullTitle: 'suite "before each" hook for "test2"', - err: { - message: /done\(\) called multiple times in hook of file.+multiple-done-before-each\.fixture\.js/ - } + expect(res.failures[0], 'to deep equal', res.failures[1]); + expect(res.failures[0], 'to satisfy', { + fullTitle: 'suite "before each" hook in "suite"', + err: { + message: /done\(\) called multiple times in hook of file.+multiple-done-before-each\.fixture\.js/, + multiple: [ + { + code: 'ERR_MOCHA_MULTIPLE_DONE' + } + ] } - ]); + }); }); }); From 3ad9fa35c368b0b9c5d1b62585afcd42784e41f3 Mon Sep 17 00:00:00 2001 From: Andrew Bradley Date: Mon, 27 Jul 2020 00:21:29 -0400 Subject: [PATCH 07/13] fix --- test/integration/multiple-done.spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/multiple-done.spec.js b/test/integration/multiple-done.spec.js index 5803575ef6..7e8b1c2b53 100644 --- a/test/integration/multiple-done.spec.js +++ b/test/integration/multiple-done.spec.js @@ -119,7 +119,7 @@ describe('multiple calls to done()', function() { }); it('correctly attributes the errors', function() { - expect(res.failures[0], 'to deep equal', res.failures[1]); + expect(res.failures[0], 'to equal', res.failures[1]); expect(res.failures[0], 'to satisfy', { fullTitle: 'suite "before each" hook in "suite"', err: { From 31febbfb29c70643ba6b47f0f787df15bdd8c39c Mon Sep 17 00:00:00 2001 From: Andrew Bradley Date: Mon, 27 Jul 2020 00:31:50 -0400 Subject: [PATCH 08/13] fix --- test/integration/multiple-done.spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/multiple-done.spec.js b/test/integration/multiple-done.spec.js index 7e8b1c2b53..fb74f559ab 100644 --- a/test/integration/multiple-done.spec.js +++ b/test/integration/multiple-done.spec.js @@ -97,7 +97,7 @@ describe('multiple calls to done()', function() { expect(res.failures[0], 'to satisfy', { fullTitle: 'suite "before all" hook in "suite"', err: { - message: /done\(\) called multiple times in hook of file.+multiple-done-before\.fixture\.js/ + message: /done\(\) called multiple times in hook of file.+multiple-done-before\.fixture\.js/ } }); }); From c149bdb0e13e47396dbbd8ef6c05d976635e9b42 Mon Sep 17 00:00:00 2001 From: Andrew Bradley Date: Mon, 27 Jul 2020 01:27:44 -0400 Subject: [PATCH 09/13] fix --- lib/errors.js | 31 +++++++++----------------- lib/runnable.js | 5 ++--- test/integration/multiple-done.spec.js | 4 ++-- test/unit/runner.spec.js | 11 ++++++++- 4 files changed, 24 insertions(+), 27 deletions(-) diff --git a/lib/errors.js b/lib/errors.js index afd1e90ee7..f4c68150d9 100644 --- a/lib/errors.js +++ b/lib/errors.js @@ -270,10 +270,6 @@ function createMochaInstanceAlreadyRunningError(message, instance) { * @returns {Error} instance detailing the error condition */ function createMultipleDoneError(runnable, originalErr) { - return createMultipleDoneErrorFactory(runnable)(originalErr); -} - -function createMultipleDoneErrorFactory(runnable) { var title; try { title = format('<%s>', runnable.fullTitle()); @@ -283,30 +279,24 @@ function createMultipleDoneErrorFactory(runnable) { } catch (ignored) { title = format('<%s> (of unknown suite)', runnable.title); } - var messagePrefix = format( + var message = format( 'done() called multiple times in %s %s', runnable.type ? runnable.type : 'unknown runnable', title ); if (runnable.file) { - messagePrefix += format(' of file %s', runnable.file); + message += format(' of file %s', runnable.file); } - return function multipleDoneErrorFactory(originalErr) { - var message = messagePrefix; - if (originalErr) { - message += format( - '; in addition, done() received error: %s', - originalErr - ); - } + if (originalErr) { + message += format('; in addition, done() received error: %s', originalErr); + } - var err = new Error(message); - err.code = constants.MULTIPLE_DONE; - err.valueType = typeof originalErr; - err.value = originalErr; - return err; - }; + var err = new Error(message); + err.code = constants.MULTIPLE_DONE; + err.valueType = typeof originalErr; + err.value = originalErr; + return err; } /** @@ -340,7 +330,6 @@ module.exports = { createMochaInstanceAlreadyRunningError: createMochaInstanceAlreadyRunningError, createFatalError: createFatalError, createMultipleDoneError: createMultipleDoneError, - createMultipleDoneErrorFactory: createMultipleDoneErrorFactory, createForbiddenExclusivityError: createForbiddenExclusivityError, constants: constants }; diff --git a/lib/runnable.js b/lib/runnable.js index e65acafc89..023481dd69 100644 --- a/lib/runnable.js +++ b/lib/runnable.js @@ -7,7 +7,7 @@ var milliseconds = require('ms'); var utils = require('./utils'); var errors = require('./errors'); var createInvalidExceptionError = errors.createInvalidExceptionError; -var createMultipleDoneErrorFactory = errors.createMultipleDoneErrorFactory; +var createMultipleDoneError = errors.createMultipleDoneError; /** * Save timer references to avoid Sinon interfering (see GH-237). @@ -274,13 +274,12 @@ Runnable.prototype.run = function(fn) { } // called multiple times - var createMultipleDoneError = createMultipleDoneErrorFactory(self); function multiple(err) { if (errorWasHandled) { return; } errorWasHandled = true; - self.emit('error', createMultipleDoneError(err)); + self.emit('error', createMultipleDoneError(self, err)); } // finished diff --git a/test/integration/multiple-done.spec.js b/test/integration/multiple-done.spec.js index fb74f559ab..97fad9d3a5 100644 --- a/test/integration/multiple-done.spec.js +++ b/test/integration/multiple-done.spec.js @@ -97,7 +97,7 @@ describe('multiple calls to done()', function() { expect(res.failures[0], 'to satisfy', { fullTitle: 'suite "before all" hook in "suite"', err: { - message: /done\(\) called multiple times in hook of file.+multiple-done-before\.fixture\.js/ + message: /done\(\) called multiple times in hook of file.+multiple-done-before\.fixture\.js/ } }); }); @@ -123,7 +123,7 @@ describe('multiple calls to done()', function() { expect(res.failures[0], 'to satisfy', { fullTitle: 'suite "before each" hook in "suite"', err: { - message: /done\(\) called multiple times in hook of file.+multiple-done-before-each\.fixture\.js/, + message: /done\(\) called multiple times in hook of file.+multiple-done-before-each\.fixture\.js/, multiple: [ { code: 'ERR_MOCHA_MULTIPLE_DONE' diff --git a/test/unit/runner.spec.js b/test/unit/runner.spec.js index b4aced0adf..0f04f4418a 100644 --- a/test/unit/runner.spec.js +++ b/test/unit/runner.spec.js @@ -12,7 +12,9 @@ var Hook = Mocha.Hook; var noop = Mocha.utils.noop; var errors = require('../../lib/errors'); var EVENT_HOOK_BEGIN = Runner.constants.EVENT_HOOK_BEGIN; +var EVENT_HOOK_END = Runner.constants.EVENT_HOOK_END; var EVENT_TEST_FAIL = Runner.constants.EVENT_TEST_FAIL; +var EVENT_TEST_PASS = Runner.constants.EVENT_TEST_PASS; var EVENT_TEST_RETRY = Runner.constants.EVENT_TEST_RETRY; var EVENT_TEST_END = Runner.constants.EVENT_TEST_END; var EVENT_RUN_END = Runner.constants.EVENT_RUN_END; @@ -255,9 +257,16 @@ describe('Runner', function() { it('should augment hook title with current test title', function(done) { var expectedHookTitle; - suite.beforeEach(function() { + function assertHookTitle() { expect(hook.title, 'to be', expectedHookTitle); + } + suite.beforeEach(function() { + assertHookTitle(); }); + runner.on(EVENT_HOOK_BEGIN, assertHookTitle); + runner.on(EVENT_HOOK_END, assertHookTitle); + runner.on(EVENT_TEST_FAIL, assertHookTitle); + runner.on(EVENT_TEST_PASS, assertHookTitle); var hook = suite._beforeEach[0]; suite.addTest(new Test('should behave', noop)); From 46cf90a7a792b1f7fd92d098650a73a771bf50b7 Mon Sep 17 00:00:00 2001 From: Andrew Bradley Date: Mon, 27 Jul 2020 01:34:47 -0400 Subject: [PATCH 10/13] fix --- lib/errors.js | 1 - test/unit/runner.spec.js | 10 ++++++++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/lib/errors.js b/lib/errors.js index f4c68150d9..929f56399c 100644 --- a/lib/errors.js +++ b/lib/errors.js @@ -287,7 +287,6 @@ function createMultipleDoneError(runnable, originalErr) { if (runnable.file) { message += format(' of file %s', runnable.file); } - if (originalErr) { message += format('; in addition, done() received error: %s', originalErr); } diff --git a/test/unit/runner.spec.js b/test/unit/runner.spec.js index 0f04f4418a..017c3893aa 100644 --- a/test/unit/runner.spec.js +++ b/test/unit/runner.spec.js @@ -260,8 +260,13 @@ describe('Runner', function() { function assertHookTitle() { expect(hook.title, 'to be', expectedHookTitle); } + var failHook = false; + var hookError = new Error('failed hook'); suite.beforeEach(function() { assertHookTitle(); + if (failHook) { + throw hookError; + } }); runner.on(EVENT_HOOK_BEGIN, assertHookTitle); runner.on(EVENT_HOOK_END, assertHookTitle); @@ -276,12 +281,13 @@ describe('Runner', function() { runner.test = suite.tests[0]; expectedHookTitle = '"before each" hook for "should behave"'; runner.hook('beforeEach', function(err) { - if (err) return done(err); + if (err && err !== hookError) return done(err); runner.test = suite.tests[1]; + failHook = true; expectedHookTitle = '"before each" hook for "should obey"'; runner.hook('beforeEach', function(err) { - if (err) return done(err); + if (err && err !== hookError) return done(err); return done(); }); }); From 299cfa46466bf7d5c0a47e5af0017618d4553a3e Mon Sep 17 00:00:00 2001 From: Andrew Bradley Date: Tue, 28 Jul 2020 17:15:21 -0400 Subject: [PATCH 11/13] Addressing PR feedback --- lib/runner.js | 43 +++++++++++++++------------------------- test/unit/runner.spec.js | 18 ++++++++--------- 2 files changed, 25 insertions(+), 36 deletions(-) diff --git a/lib/runner.js b/lib/runner.js index 51215eba84..cd91173e10 100644 --- a/lib/runner.js +++ b/lib/runner.js @@ -360,6 +360,19 @@ Runner.prototype.checkGlobals = function(test) { /** * Fail the given `test`. * + * If `test` is a hook, failures work in the following pattern: + * - If bail, run corresponding `after each` and `after` hooks, + * then exit + * - Failed `before` hook skips all tests in a suite and subsuites, + * but jumps to corresponding `after` hook + * - Failed `before each` hook skips remaining tests in a + * suite and jumps to corresponding `after each` hook, + * which is run only once + * - Failed `after` hook does not alter execution order + * - Failed `after each` hook skips remaining tests in a + * suite and subsuites, but executes other `after each` + * hooks + * * @private * @param {Runnable} test * @param {Error} err @@ -398,30 +411,6 @@ Runner.prototype.fail = function(test, err, force) { this.emit(constants.EVENT_TEST_FAIL, test, err); }; -/** - * Fail the given `hook` with `err`. - * - * Hook failures work in the following pattern: - * - If bail, run corresponding `after each` and `after` hooks, - * then exit - * - Failed `before` hook skips all tests in a suite and subsuites, - * but jumps to corresponding `after` hook - * - Failed `before each` hook skips remaining tests in a - * suite and jumps to corresponding `after each` hook, - * which is run only once - * - Failed `after` hook does not alter execution order - * - Failed `after each` hook skips remaining tests in a - * suite and subsuites, but executes other `after each` - * hooks - * - * @private - * @param {Hook} hook - * @param {Error} err - */ -Runner.prototype.failHook = function(hook, err) { - this.fail(hook, err); -}; - /** * Run hook `name` callbacks and then invoke `fn()`. * @@ -458,7 +447,7 @@ Runner.prototype.hook = function(name, fn) { if (!hook.listeners('error').length) { self._addEventListener(hook, 'error', function(err) { - self.failHook(hook, err); + self.fail(hook, err); }); } @@ -492,11 +481,11 @@ Runner.prototype.hook = function(name, fn) { } else { hook.pending = false; var errForbid = createUnsupportedError('`this.skip` forbidden'); - self.failHook(hook, errForbid); + self.fail(hook, errForbid); return fn(errForbid); } } else if (err) { - self.failHook(hook, err); + self.fail(hook, err); // stop executing hooks, notify callee of hook err return fn(err); } diff --git a/test/unit/runner.spec.js b/test/unit/runner.spec.js index 017c3893aa..730c40003d 100644 --- a/test/unit/runner.spec.js +++ b/test/unit/runner.spec.js @@ -458,16 +458,16 @@ describe('Runner', function() { }); }); - describe('.failHook(hook, err)', function() { + describe('.fail(hook, err)', function() { it('should increment .failures', function() { expect(runner.failures, 'to be', 0); var test1 = new Test('fail hook 1', noop); var test2 = new Test('fail hook 2', noop); suite.addTest(test1); suite.addTest(test2); - runner.failHook(test1, new Error('error1')); + runner.fail(test1, new Error('error1')); expect(runner.failures, 'to be', 1); - runner.failHook(test2, new Error('error2')); + runner.fail(test2, new Error('error2')); expect(runner.failures, 'to be', 2); }); @@ -480,7 +480,7 @@ describe('Runner', function() { expect(_err, 'to be', err); done(); }); - runner.failHook(hook, err); + runner.fail(hook, err); }); it('should not emit "end" if suite bail is not true', function(done) { @@ -490,7 +490,7 @@ describe('Runner', function() { suite.bail(false); expect( function() { - runner.failHook(hook, err); + runner.fail(hook, err); }, 'not to emit from', hook, @@ -755,7 +755,7 @@ describe('Runner', function() { expect(_err.stack, 'to be', stack.slice(0, 3).join('\n')); done(); }); - runner.failHook(hook, err); + runner.fail(hook, err); }); }); @@ -773,7 +773,7 @@ describe('Runner', function() { expect(_err.stack, 'to be', stack.join('\n')); done(); }); - runner.failHook(hook, err); + runner.fail(hook, err); }); }); @@ -824,7 +824,7 @@ describe('Runner', function() { ); done(); }); - runner.failHook(hook, err); + runner.fail(hook, err); }); it('should not hang if overlong error message is multiple lines', function(done) { @@ -844,7 +844,7 @@ describe('Runner', function() { ); done(); }); - runner.failHook(hook, err); + runner.fail(hook, err); }); }); }); From 564b1894414eca31901b41d7be38c389c67bb87f Mon Sep 17 00:00:00 2001 From: Andrew Bradley Date: Tue, 28 Jul 2020 17:32:18 -0400 Subject: [PATCH 12/13] rename "suite" to "suite1" to make test cases clearer --- .../fixtures/multiple-done-before-each.fixture.js | 2 +- test/integration/fixtures/multiple-done-before.fixture.js | 2 +- test/integration/multiple-done.spec.js | 8 ++++---- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/test/integration/fixtures/multiple-done-before-each.fixture.js b/test/integration/fixtures/multiple-done-before-each.fixture.js index 32de2bf442..6c93167cfb 100644 --- a/test/integration/fixtures/multiple-done-before-each.fixture.js +++ b/test/integration/fixtures/multiple-done-before-each.fixture.js @@ -1,6 +1,6 @@ 'use strict'; -describe('suite', function () { +describe('suite1', function () { beforeEach(function (done) { setTimeout(done, 10); setTimeout(done, 20); diff --git a/test/integration/fixtures/multiple-done-before.fixture.js b/test/integration/fixtures/multiple-done-before.fixture.js index 1e1bc71a16..cfa18de0c0 100644 --- a/test/integration/fixtures/multiple-done-before.fixture.js +++ b/test/integration/fixtures/multiple-done-before.fixture.js @@ -1,6 +1,6 @@ 'use strict'; -describe('suite', function () { +describe('suite1', function () { before(function (done) { setTimeout(done, 10); setTimeout(done, 30); diff --git a/test/integration/multiple-done.spec.js b/test/integration/multiple-done.spec.js index 97fad9d3a5..b9106a58bc 100644 --- a/test/integration/multiple-done.spec.js +++ b/test/integration/multiple-done.spec.js @@ -95,9 +95,9 @@ describe('multiple calls to done()', function() { it('correctly attributes the error', function() { expect(res.failures[0], 'to satisfy', { - fullTitle: 'suite "before all" hook in "suite"', + fullTitle: 'suite1 "before all" hook in "suite1"', err: { - message: /done\(\) called multiple times in hook of file.+multiple-done-before\.fixture\.js/ + message: /done\(\) called multiple times in hook of file.+multiple-done-before\.fixture\.js/ } }); }); @@ -121,9 +121,9 @@ describe('multiple calls to done()', function() { it('correctly attributes the errors', function() { expect(res.failures[0], 'to equal', res.failures[1]); expect(res.failures[0], 'to satisfy', { - fullTitle: 'suite "before each" hook in "suite"', + fullTitle: 'suite1 "before each" hook in "suite1"', err: { - message: /done\(\) called multiple times in hook of file.+multiple-done-before-each\.fixture\.js/, + message: /done\(\) called multiple times in hook of file.+multiple-done-before-each\.fixture\.js/, multiple: [ { code: 'ERR_MOCHA_MULTIPLE_DONE' From fe88c0300313327668759367c4bdcb6da2662258 Mon Sep 17 00:00:00 2001 From: Andrew Bradley Date: Tue, 28 Jul 2020 17:40:10 -0400 Subject: [PATCH 13/13] addressing PR feedback --- test/integration/multiple-done.spec.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/integration/multiple-done.spec.js b/test/integration/multiple-done.spec.js index b9106a58bc..b7348ab488 100644 --- a/test/integration/multiple-done.spec.js +++ b/test/integration/multiple-done.spec.js @@ -119,8 +119,7 @@ describe('multiple calls to done()', function() { }); it('correctly attributes the errors', function() { - expect(res.failures[0], 'to equal', res.failures[1]); - expect(res.failures[0], 'to satisfy', { + expect(res.failures[0], 'to equal', res.failures[1]).and('to satisfy', { fullTitle: 'suite1 "before each" hook in "suite1"', err: { message: /done\(\) called multiple times in hook of file.+multiple-done-before-each\.fixture\.js/,