Merge pull request #125 from gemini-testing/fix/event.throws

fix: fail instead of hang on throws from event handlers

Author: j0tunn

lib/runner/mocha-runner/mocha-adapter.js

@@ -11,6 +11,7 @@ const path = require('path');
 const clearRequire = require('clear-require');
 const q = require('q');
 const _ = require('lodash');
+const EventEmitter = require('events').EventEmitter;
 
 // Avoid mochajs warning about possible EventEmitter memory leak
 // https://nodejs.org/docs/latest/api/events.html#events_emitter_setmaxlisteners_n
@@ -46,6 +47,8 @@ module.exports = class MochaAdapter {
         this._injectSkip();
 
         _.extend(global.hermione, {ctx});
+
+        this._errMonitor = new EventEmitter();
     }
 
     applySkip(testSkipper) {
@@ -92,23 +95,48 @@ module.exports = class MochaAdapter {
     }
 
     attachEmitFn(emit) {
+        const _this = this;
+        function monitoredEmit() {
+            try {
+                emit.apply(null, arguments);
+            } catch (e) {
+                _this._errMonitor.emit('err', e);
+                throw e;
+            }
+        }
+
+        this._attachProxyReporter(monitoredEmit);
+        this._passthroughFileEvents(monitoredEmit);
+
+        return this;
+    }
+
+    _attachProxyReporter(emit) {
         const Reporter = _.partial(ProxyReporter, emit, () => this._getBrowser());
         this._mocha.reporter(Reporter);
+    }
 
+    _passthroughFileEvents(emit) {
         const emit_ = (event, file) => emit(event, {
             file,
             hermione: global.hermione,
             browser: this._browserAgent.browserId,
             suite: this.suite
         });
+
         this.suite.on('pre-require', (ctx, file) => emit_(RunnerEvents.BEFORE_FILE_READ, file));
         this.suite.on('post-require', (ctx, file) => emit_(RunnerEvents.AFTER_FILE_READ, file));
 
         return this;
     }
 
     run() {
-        return q.Promise(this._mocha.run.bind(this._mocha));
+        const defer = q.defer();
+
+        this._errMonitor.on('err', (err) => defer.reject(err));
+        this._mocha.run((err) => err ? defer.reject(err) : defer.resolve());
+
+        return defer.promise;
     }
 
     _injectSkip() {

test/lib/runner/mocha-runner/mocha-adapter.js

@@ -72,7 +72,7 @@ describe('mocha-runner/mocha-adapter', () => {
         browserAgent = sinon.createStubInstance(BrowserAgent);
 
         sandbox.stub(MochaStub.prototype);
-        MochaStub.prototype.run.yields();
+        MochaStub.prototype.run = (cb) => process.nextTick(cb);
         MochaStub.prototype.suite = mkSuiteStub_();
 
         MochaAdapter = proxyquire('../../../../lib/runner/mocha-runner/mocha-adapter', {
@@ -524,8 +524,11 @@ describe('mocha-runner/mocha-adapter', () => {
 
             attachEmitFn_(emitFn);
 
-            assert.calledOnce(ProxyReporter.prototype.__constructor);
-            assert.calledWith(ProxyReporter.prototype.__constructor, emitFn);
+            const emit_ = ProxyReporter.prototype.__constructor.firstCall.args[0];
+            emit_('some-event', {some: 'data'});
+
+            assert.calledOnce(emitFn);
+            assert.calledWith(emitFn, 'some-event', sinon.match({some: 'data'}));
         });
 
         it('should pass to proxy reporter getter for requested browser', () => {
@@ -552,6 +555,40 @@ describe('mocha-runner/mocha-adapter', () => {
             assert.deepEqual(getBrowser(), {id: 'some-browser'});
         });
 
+        describe('if event handler throws', () => {
+            const initBadHandler_ = (event, handler) => {
+                const emitter = new EventEmitter();
+                emitter.on(event, handler);
+
+                attachEmitFn_(emitter.emit.bind(emitter));
+                return ProxyReporter.prototype.__constructor.firstCall.args[0];
+            };
+
+            it('proxy should rethrow error', () => {
+                const emit_ = initBadHandler_('foo', () => {
+                    throw new Error(new Error('bar'));
+                });
+
+                assert.throws(() => emit_('foo'), /bar/);
+            });
+
+            it('run should be rejected', () => {
+                const emit_ = initBadHandler_('foo', () => {
+                    throw new Error('bar');
+                });
+
+                const promise = mochaAdapter.run();
+
+                try {
+                    emit_('foo');
+                } catch (e) {
+                    // eslint иди лесом
+                }
+
+                return assert.isRejected(promise, /bar/);
+            });
+        });
+
         describe('file events', () => {
             beforeEach(() => MochaAdapter.init());
             afterEach(() => delete global.hermione);