Skip to content

Commit 1b64d84

Browse files
DavertMikclaude
andcommitted
fix(mocha): fail fast when test hook import chain rejects
In 4.x the per-test setup()/teardown() hooks load ./test.js via a dynamic import() with no .catch(). If that import rejects, or the .then body throws (e.g. enhanceMochaTest on an undefined test, a listener throwing), done() is never called and the mocha hook hangs forever — the silent-hang failure mode the 4.0 release is most concerned with. This mirrors the existing suiteSetup()/suiteTeardown() shape exactly: append a .catch(err => doneFn(err)) to both import chains. No recorder.errHandler is added (the per-test handler owns the single errFn slot). makeDoneCallableOnce already guards against a double done() call. Adds 3 regression tests: setup() and teardown() with a throwing then-body call done with the error within 1s instead of hanging, plus a happy-path check. Reverting the fix makes the two error-path tests hang (verified). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> (cherry picked from commit ac59a40)
1 parent 20607b7 commit 1b64d84

2 files changed

Lines changed: 57 additions & 10 deletions

File tree

lib/mocha/asyncWrapper.js

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -197,23 +197,31 @@ export function setup(suite) {
197197
return function (done) {
198198
const doneFn = makeDoneCallableOnce(done)
199199
recorder.startUnlessRunning()
200-
import('./test.js').then(testModule => {
201-
const { enhanceMochaTest } = testModule.default || testModule
202-
event.emit(event.test.before, enhanceMochaTest(suite?.ctx?.currentTest ?? suite?.currentTest))
203-
recorder.add(() => doneFn())
204-
})
200+
import('./test.js')
201+
.then(testModule => {
202+
const { enhanceMochaTest } = testModule.default || testModule
203+
event.emit(event.test.before, enhanceMochaTest(suite?.ctx?.currentTest ?? suite?.currentTest))
204+
recorder.add(() => doneFn())
205+
})
206+
.catch(err => {
207+
doneFn(err)
208+
})
205209
}
206210
}
207211

208212
export function teardown(suite) {
209213
return function (done) {
210214
const doneFn = makeDoneCallableOnce(done)
211215
recorder.startUnlessRunning()
212-
import('./test.js').then(testModule => {
213-
const { enhanceMochaTest } = testModule.default || testModule
214-
event.emit(event.test.after, enhanceMochaTest(suite?.ctx?.currentTest ?? suite?.currentTest))
215-
recorder.add(() => doneFn())
216-
})
216+
import('./test.js')
217+
.then(testModule => {
218+
const { enhanceMochaTest } = testModule.default || testModule
219+
event.emit(event.test.after, enhanceMochaTest(suite?.ctx?.currentTest ?? suite?.currentTest))
220+
recorder.add(() => doneFn())
221+
})
222+
.catch(err => {
223+
doneFn(err)
224+
})
217225
}
218226
}
219227

test/unit/mocha/asyncWrapper_test.js

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -230,4 +230,43 @@ describe('AsyncWrapper', () => {
230230
expect(suiteTests[0].err, 'the suite test got the hook error attached').to.be.instanceof(Error)
231231
})
232232
})
233+
234+
describe('setup/teardown import hardening (regression)', () => {
235+
beforeEach(() => recorder.start())
236+
237+
it('setup(): a throwing then-body calls done with the error instead of hanging', async () => {
238+
const suite = {
239+
ctx: {
240+
get currentTest() {
241+
throw new Error('setup-boom')
242+
},
243+
},
244+
}
245+
const { count, arg } = await runHook(setup(suite), 1000)
246+
expect(count).to.equal(1)
247+
expect(arg).to.be.instanceof(Error)
248+
expect(arg.message).to.equal('setup-boom')
249+
})
250+
251+
it('teardown(): a throwing then-body calls done with the error instead of hanging', async () => {
252+
const suite = {
253+
ctx: {
254+
get currentTest() {
255+
throw new Error('teardown-boom')
256+
},
257+
},
258+
}
259+
const { count, arg } = await runHook(teardown(suite), 1000)
260+
expect(count).to.equal(1)
261+
expect(arg).to.be.instanceof(Error)
262+
expect(arg.message).to.equal('teardown-boom')
263+
})
264+
265+
it('setup(): happy path calls done with no error', async () => {
266+
const suite = { ctx: { currentTest: { title: 'a sample test' } } }
267+
const { count, arg } = await runHook(setup(suite), 1000)
268+
expect(count).to.equal(1)
269+
expect(arg, 'done called with no error').to.be.undefined
270+
})
271+
})
233272
})

0 commit comments

Comments
 (0)