Errors in beforeEach not reported in ci mode in mocha

[job13er]

With the most recent versions of mocha, the test end event is no longer emitted when there is an error thrown in a beforeEach(), the result is that in CI mode. Previously the test end event was emitted whenever there's a fail event, but that's no longer the case. As a result, testem no longer shows the error. This manifested for my team when using ember-cli and ember-cli-mocha.

I was able to reproduce with a very simple test:

/**
 * This is a test that forces a failure in a beforeEach to make sure
 * that it is properly reported. We were seeing an issue with the latest `ember-mocha` where
 * testem wasn't reporting errors in this case
 */

import {expect} from 'chai'
import {beforeEach, describe, it} from 'mocha'

describe('Outer test that passes', function () {
  let foo
  beforeEach(function () {
    foo = 1
  })

  it('should fail', function () {
    expect(foo).to.equal(1)
  })

  describe('Inner test with bad beforeEach', function () {
    beforeEach(function () {
      throw new Error('error in beforeEach')
    })

    it('should have failed before it got here', function () {
      expect(foo).to.equal(1)
    })
  })
})

When running in the browser (dev mode) I see

However, the console output is:

And when running in CI mode, I get:

ok 1 Chrome 55.0 - Outer test that passes should fail

1..1
# tests 1
# pass  1
# skip  0
# fail  0

# ok

I locally tried updating the mocha_adapter here to do:

var name = getFullName(test);
test.err = test.err || err;
testFail(test, err);

instead of just

test.err = test.err || err;

And the result was promising. I then saw

in dev mode and for ci mode I got:

not ok 1 Chrome 55.0 - Outer test that passes Inner test with bad beforeEach "before each" hook for "should have failed before it got here"
    ---
        message: >
            error in beforeEach
        stack: >
            Error: error in beforeEach
                at Context.<anonymous> (http://localhost:7357/assets/tests.js:718:15)
                at http://localhost:7357/assets/test-support.js:36339:25
                at Backburner.run (http://localhost:7357/assets/vendor.js:10849:25)
                at Object.run (http://localhost:7357/assets/vendor.js:34283:27)
                at Context.<anonymous> (http://localhost:7357/assets/test-support.js:36337:24)
                at invoke (http://localhost:7357/assets/test-support.js:21640:21)
                at Context.asyncFn (http://localhost:7357/assets/test-support.js:21625:11)
                at callFnAsync (http://localhost:7357/assets/test-support.js:6768:8)
                at Hook.Runnable.run (http://localhost:7357/assets/test-support.js:6720:7)
                at next (http://localhost:7357/assets/test-support.js:7084:10)
        Log: |
    ...
ok 2 Chrome 55.0 - Outer test that passes should fail

1..2
# tests 2
# pass  1
# skip  0
# fail  1
Not all tests passed.
Error: Not all tests passed.
    at EventEmitter.getExitCode (/Users/ameadows/Documents/Repos/ember-frost-test/node_modules/testem/lib/app.js:434:15)
    at EventEmitter.exit (/Users/ameadows/Documents/Repos/ember-frost-test/node_modules/testem/lib/app.js:189:23)
    at /Users/ameadows/Documents/Repos/ember-frost-test/node_modules/testem/lib/app.js:103:14
    at tryCatcher (/Users/ameadows/Documents/Repos/ember-frost-test/node_modules/testem/node_modules/bluebird/js/release/util.js:16:23)
    at Promise._settlePromiseFromHandler (/Users/ameadows/Documents/Repos/ember-frost-test/node_modules/testem/node_modules/bluebird/js/release/promise.js:510:31)
    at Promise._settlePromise (/Users/ameadows/Documents/Repos/ember-frost-test/node_modules/testem/node_modules/bluebird/js/release/promise.js:567:18)
    at Promise._settlePromise0 (/Users/ameadows/Documents/Repos/ember-frost-test/node_modules/testem/node_modules/bluebird/js/release/promise.js:612:10)
    at Promise._settlePromises (/Users/ameadows/Documents/Repos/ember-frost-test/node_modules/testem/node_modules/bluebird/js/release/promise.js:691:18)
    at Async._drainQueue (/Users/ameadows/Documents/Repos/ember-frost-test/node_modules/testem/node_modules/bluebird/js/release/async.js:138:16)
    at Async._drainQueues (/Users/ameadows/Documents/Repos/ember-frost-test/node_modules/testem/node_modules/bluebird/js/release/async.js:148:10)
    at Immediate.Async.drainQueues (/Users/ameadows/Documents/Repos/ember-frost-test/node_modules/testem/node_modules/bluebird/js/release/async.js:17:14)
    at runCallback (timers.js:637:20)
    at tryOnImmediate (timers.js:610:5)
    at processImmediate [as _immediateCallback] (timers.js:582:5)

I would have directly submitted a PR with the above change, but the contribution guidelines wisely suggested accompanying a change with a test. I was disheartened to see that no tests currently exist for the mocha_adapter.js code, so I'm starting to write some to accompany this change. The first will be the one related to this change of course, but then I'll try to throw in some more.

While I'm working on it, I thought I'd at least post this issue so that others who may be baffled as to why their tests aren't failing as they expect them to in CI mode may have some insight into the problem.

A colleague of mine (@sophypal) even tracked down the commit which made the change in mocha

You'll notice that it's present in both v2.5.3 and v3.2.0 as well as the current master branch