diff --git a/lib/mocha/asyncWrapper.js b/lib/mocha/asyncWrapper.js index 1457a5ce1..33ee8ba3e 100644 --- a/lib/mocha/asyncWrapper.js +++ b/lib/mocha/asyncWrapper.js @@ -165,6 +165,9 @@ export function injected(fn, suite, hookName) { retry(err) } finally { if (number < retries) { + // drain the failed chain so in-flight steps (sessions, listeners) + // finish before the next attempt starts queueing new ones + await recorder.promise().catch(() => {}) recorder.stop() recorder.start() } diff --git a/lib/recorder.js b/lib/recorder.js index a83e0baa1..fe09b2808 100644 --- a/lib/recorder.js +++ b/lib/recorder.js @@ -125,6 +125,15 @@ export default { * @inner */ start(name) { + if (sessionId === name) { + // re-starting the same session abandons the current (broken) chain + // without polluting the session stack + debug(`${currentQueue()}Session <${name}> already started, abandoning its chain`) + tasks.push('--->') + this.running = true + promise = Promise.resolve() + return + } if (sessionId) { debug(`${currentQueue()}Session already started as ${sessionId}, nesting session ${name}`) // Push current session to stack instead of restoring it @@ -151,7 +160,8 @@ export default { debug(`${currentQueue()}Finalize <${name}> session`) this.running = false this.catch(errFn) - promise = promise.then(() => oldPromises.pop()) + const restored = oldPromises.pop() + if (restored && restored !== promise) promise = promise.then(() => restored) // Restore parent session from stack if available if (sessionStack.length > 0) { diff --git a/test/unit/recorder_test.js b/test/unit/recorder_test.js index 04096ee6d..872326ef0 100644 --- a/test/unit/recorder_test.js +++ b/test/unit/recorder_test.js @@ -28,6 +28,38 @@ describe('Recorder', () => { recorder.add(() => (order += 'b')) return recorder.promise().then(() => expect(order).is.equal('acdb')) }) + + it('re-starting the same session name does not pollute the session stack', () => { + let order = '' + recorder.add(() => (order += 'a')) + recorder.add(() => { + recorder.session.start('teardown') + recorder.add(() => (order += 'c')) + recorder.session.start('teardown') // leaked teardown re-start: no sessionStack/oldPromises push + recorder.add(() => (order += 'd')) + }) + recorder.add(() => recorder.session.restore('teardown')) + recorder.add(() => (order += 'b')) + // steps already attached to the live chain still run; a broken one would be abandoned + return recorder.promise().then(() => expect(order).is.equal('acdb')) + }) + + it('does not chain a promise onto itself when restoring an unbalanced stack', async () => { + const unhandled = [] + const onUnhandled = e => unhandled.push(e) + process.on('unhandledRejection', onUnhandled) + try { + recorder.session.start('s1') + recorder.add(() => 'step') + recorder.session.restore('s1') + recorder.session.restore('extra') // nothing left to pop, must not create a cycle + await recorder.promise() + await new Promise(r => setTimeout(r, 20)) + expect(unhandled).to.have.length(0) + } finally { + process.removeListener('unhandledRejection', onUnhandled) + } + }) }) describe('#add', () => {