From fe723d5af73b962f654e0f8a21a7cd7931ecad47 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gast=C3=B3n=20I=2E=20Silva?= Date: Sat, 12 Jan 2019 00:34:27 -0800 Subject: [PATCH 1/3] set allowUncaught on hooks --- lib/runner.js | 2 ++ test/unit/runner.spec.js | 41 ++++++++++++++++++++++++++++++++++++++-- 2 files changed, 41 insertions(+), 2 deletions(-) diff --git a/lib/runner.js b/lib/runner.js index fc69a687cf..5cfeba7b32 100644 --- a/lib/runner.js +++ b/lib/runner.js @@ -371,6 +371,8 @@ Runner.prototype.hook = function(name, fn) { hook.ctx.currentTest = self.test; } + hook.allowUncaught = self.allowUncaught; + self.emit(constants.EVENT_HOOK_BEGIN, hook); if (!hook.listeners('error').length) { diff --git a/test/unit/runner.spec.js b/test/unit/runner.spec.js index d524ea9ba2..61be99cdf6 100644 --- a/test/unit/runner.spec.js +++ b/test/unit/runner.spec.js @@ -443,7 +443,20 @@ describe('Runner', function() { }); describe('allowUncaught', function() { - it('should allow unhandled errors to propagate through', function(done) { + var immediately; + + before(function() { + immediately = Runner.immediately; + Runner.immediately = function(fn) { + fn(); + }; + }); + + after(function() { + Runner.immediately = immediately; + }); + + it('should allow unhandled errors to propagate through', function() { var newRunner = new Runner(suite); newRunner.allowUncaught = true; newRunner.test = new Test('failing test', function() { @@ -453,7 +466,31 @@ describe('Runner', function() { newRunner.runTest(); } expect(fail, 'to throw', 'allow unhandled errors'); - done(); + }); + + it('should allow unhandled errors in sync hooks to propagate through', function() { + suite.beforeEach(function() { + throw new Error('allow unhandled errors in sync hooks'); + }); + var runner = new Runner(suite); + runner.allowUncaught = true; + function throwError() { + runner.hook('beforeEach', function() {}); + } + expect(throwError, 'to throw', 'allow unhandled errors in sync hooks'); + }); + + it('async - should allow unhandled errors in hooks to propagate through', function() { + suite.beforeEach(function(done) { + // having `done` triggers the async path + throw new Error('allow unhandled errors in async hooks'); + }); + var runner = new Runner(suite); + runner.allowUncaught = true; + function throwError() { + runner.hook('beforeEach', function() {}); + } + expect(throwError, 'to throw', 'allow unhandled errors in async hooks'); }); }); From 5b9633ee60358525d24d09767f7d5bbc608f2c3a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gast=C3=B3n=20I=2E=20Silva?= Date: Sat, 12 Jan 2019 11:02:53 -0800 Subject: [PATCH 2/3] add test: hooks swallow uncaught exceptions --- test/unit/runner.spec.js | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/test/unit/runner.spec.js b/test/unit/runner.spec.js index 61be99cdf6..c80b781946 100644 --- a/test/unit/runner.spec.js +++ b/test/unit/runner.spec.js @@ -468,6 +468,19 @@ describe('Runner', function() { expect(fail, 'to throw', 'allow unhandled errors'); }); + it('should not allow unhandled errors in hooks to propagate through', function() { + suite.beforeEach(function() { + throw new Error('this error will not propagate'); + }); + var runner = new Runner(suite); + try { + runner.hook('beforeEach', function() {}); + expect(true, 'to be', true); + } catch (err) { + expect(false, 'to be', true); + } + }); + it('should allow unhandled errors in sync hooks to propagate through', function() { suite.beforeEach(function() { throw new Error('allow unhandled errors in sync hooks'); @@ -481,8 +494,8 @@ describe('Runner', function() { }); it('async - should allow unhandled errors in hooks to propagate through', function() { + // having `done` triggers the async path suite.beforeEach(function(done) { - // having `done` triggers the async path throw new Error('allow unhandled errors in async hooks'); }); var runner = new Runner(suite); From de9cc7823d5e4e3b4ae78d86cd52fdc878ff361a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gast=C3=B3n=20I=2E=20Silva?= Date: Sun, 17 Feb 2019 07:08:29 -0800 Subject: [PATCH 3/3] PR feedback --- test/unit/runner.spec.js | 83 ++++++++++++++++++++++++---------------- 1 file changed, 51 insertions(+), 32 deletions(-) diff --git a/test/unit/runner.spec.js b/test/unit/runner.spec.js index c80b781946..ec2b8a0f34 100644 --- a/test/unit/runner.spec.js +++ b/test/unit/runner.spec.js @@ -9,6 +9,7 @@ var Test = Mocha.Test; var Runnable = Mocha.Runnable; var Hook = Mocha.Hook; var noop = Mocha.utils.noop; +var EVENT_HOOK_BEGIN = Runner.constants.EVENT_HOOK_BEGIN; var EVENT_TEST_FAIL = Runner.constants.EVENT_TEST_FAIL; var EVENT_TEST_RETRY = Runner.constants.EVENT_TEST_RETRY; var EVENT_RUN_END = Runner.constants.EVENT_RUN_END; @@ -443,19 +444,6 @@ describe('Runner', function() { }); describe('allowUncaught', function() { - var immediately; - - before(function() { - immediately = Runner.immediately; - Runner.immediately = function(fn) { - fn(); - }; - }); - - after(function() { - Runner.immediately = immediately; - }); - it('should allow unhandled errors to propagate through', function() { var newRunner = new Runner(suite); newRunner.allowUncaught = true; @@ -468,42 +456,73 @@ describe('Runner', function() { expect(fail, 'to throw', 'allow unhandled errors'); }); - it('should not allow unhandled errors in hooks to propagate through', function() { + it('should not allow unhandled errors in sync hooks to propagate through', function(done) { suite.beforeEach(function() { - throw new Error('this error will not propagate'); + throw new Error(); }); var runner = new Runner(suite); - try { - runner.hook('beforeEach', function() {}); - expect(true, 'to be', true); - } catch (err) { - expect(false, 'to be', true); - } + runner.allowUncaught = false; + + // We are monkey patching here with runner.once and a hook.run wrapper to effectively + // capture thrown errors within the event loop phase where Runner.immediately executes + runner.once(EVENT_HOOK_BEGIN, function(hook) { + var _run = hook.run; + hook.run = function(fn) { + function throwError() { + _run.call(hook, fn); + } + expect(throwError, 'not to throw'); + done(); + }; + }); + + runner.hook('beforeEach', noop); }); - it('should allow unhandled errors in sync hooks to propagate through', function() { + it('should allow unhandled errors in sync hooks to propagate through', function(done) { suite.beforeEach(function() { throw new Error('allow unhandled errors in sync hooks'); }); var runner = new Runner(suite); runner.allowUncaught = true; - function throwError() { - runner.hook('beforeEach', function() {}); - } - expect(throwError, 'to throw', 'allow unhandled errors in sync hooks'); + + runner.once(EVENT_HOOK_BEGIN, function(hook) { + var _run = hook.run; + hook.run = function(fn) { + function throwError() { + _run.call(hook, fn); + } + var expected = 'allow unhandled errors in sync hooks'; + expect(throwError, 'to throw', expected); + done(); + }; + }); + + runner.hook('beforeEach', noop); }); - it('async - should allow unhandled errors in hooks to propagate through', function() { - // having `done` triggers the async path + it('async - should allow unhandled errors in hooks to propagate through', function(done) { + // the `done` argument, although unused, it triggers the async path + // see this.async in the Runnable constructor suite.beforeEach(function(done) { throw new Error('allow unhandled errors in async hooks'); }); var runner = new Runner(suite); runner.allowUncaught = true; - function throwError() { - runner.hook('beforeEach', function() {}); - } - expect(throwError, 'to throw', 'allow unhandled errors in async hooks'); + + runner.once(EVENT_HOOK_BEGIN, function(hook) { + var _run = hook.run; + hook.run = function(fn) { + function throwError() { + _run.call(hook, fn); + } + var expected = 'allow unhandled errors in async hooks'; + expect(throwError, 'to throw', expected); + done(); + }; + }); + + runner.hook('beforeEach', noop); }); });