fix(pause): remove leaked dispatcher listeners and guard session restore

lib/pause.js

@@ -19,6 +19,35 @@ let finish
 let next
 let registeredVariables = {}
 let externalHandler = null
+let pauseSessionOpen = false
+
+function onStepAfter() {
+  recorder.add('Start next pause session', () => {
+    // test already finished, nothing to pause
+    if (!store.currentTest) return
+    if (!next) return
+    return pauseSession()
+  })
+}
+
+function onTestFinished() {
+  if (typeof finish === 'function') finish()
+  if (pauseSessionOpen) {
+    recorder.session.restore('pause')
+    pauseSessionOpen = false
+  }
+  if (rl) rl.close()
+  if (!externalHandler) history.save()
+  event.dispatcher.removeListener(event.step.after, onStepAfter)
+  event.dispatcher.removeListener(event.test.finished, onTestFinished)
+}
+
+function registerPauseListeners() {
+  event.dispatcher.removeListener(event.step.after, onStepAfter)
+  event.dispatcher.removeListener(event.test.finished, onTestFinished)
+  event.dispatcher.on(event.step.after, onStepAfter)
+  event.dispatcher.on(event.test.finished, onTestFinished)
+}
 
 /**
  * Pauses test execution and starts interactive shell
@@ -28,35 +57,22 @@ const pause = function (passedObject = {}) {
   if (store.dryRun) return
 
   next = false
-  // add listener to all next steps to provide next() functionality
-  event.dispatcher.on(event.step.after, () => {
-    recorder.add('Start next pause session', () => {
-      // test already finished, nothing to pause
-      if (!store.currentTest) return
-      if (!next) return
-      return pauseSession()
-    })
-  })
-
-  event.dispatcher.on(event.test.finished, () => {
-    if (typeof finish === 'function') finish()
-    recorder.session.restore('pause')
-    if (rl) rl.close()
-    if (!externalHandler) history.save()
-  })
+  registerPauseListeners()
 
   recorder.add('Start new session', () => pauseSession(passedObject))
 }
 
 function pauseSession(passedObject = {}) {
   registeredVariables = passedObject
   recorder.session.start('pause')
+  pauseSessionOpen = true
 
   if (externalHandler) {
     store.onPause = true
     return externalHandler({ registeredVariables }).then(() => {
       store.onPause = false
       recorder.session.restore('pause')
+      pauseSessionOpen = false
     })
   }
 
@@ -107,6 +123,7 @@ async function parseInput(cmd) {
   if (!cmd || cmd === 'resume' || cmd === 'exit') {
     if (typeof finish === 'function') finish()
     recorder.session.restore('pause')
+    pauseSessionOpen = false
     rl.close()
     history.save()
     return nextStep()
@@ -265,6 +282,7 @@ function setPauseHandler(handler) {
  */
 function pauseNow(passedObject = {}) {
   if (store.dryRun) return
+  registerPauseListeners()
   recorder.add('Triggered pause', () => pauseSession(passedObject))
 }
 

test/unit/pause_test.js

@@ -1,5 +1,18 @@
 import { expect } from 'chai'
-import { setPauseHandler } from '../../lib/pause.js'
+import sinon from 'sinon'
+import pause, { setPauseHandler, pauseNow } from '../../lib/pause.js'
+import recorder from '../../lib/recorder.js'
+import event from '../../lib/event.js'
+import store from '../../lib/store.js'
+
+const settles = (promise, ms = 2000) =>
+  Promise.race([
+    promise,
+    new Promise((_, reject) => {
+      const t = setTimeout(() => reject(new Error(`did not settle within ${ms}ms`)), ms)
+      t.unref?.()
+    }),
+  ])
 
 describe('pause external handler hook', () => {
   afterEach(() => {
@@ -26,3 +39,64 @@ describe('pause external handler hook', () => {
     expect(received).to.deep.equal({ registeredVariables: { foo: 1 } })
   })
 })
+
+describe('pause listener lifecycle', () => {
+  beforeEach(() => {
+    store.dryRun = false
+    setPauseHandler(null)
+    recorder.reset()
+    recorder.stop()
+  })
+
+  afterEach(() => {
+    setPauseHandler(null)
+    event.dispatcher.emit(event.test.finished)
+    recorder.reset()
+  })
+
+  it('keeps exactly one listener no matter how many pause() calls', () => {
+    const baseStep = event.dispatcher.listenerCount(event.step.after)
+    const baseFin = event.dispatcher.listenerCount(event.test.finished)
+    pause()
+    pause()
+    pause()
+    expect(event.dispatcher.listenerCount(event.step.after)).to.equal(baseStep + 1)
+    expect(event.dispatcher.listenerCount(event.test.finished)).to.equal(baseFin + 1)
+  })
+
+  it('removes both pause listeners when the test finishes', () => {
+    const baseStep = event.dispatcher.listenerCount(event.step.after)
+    const baseFin = event.dispatcher.listenerCount(event.test.finished)
+    pause()
+    expect(event.dispatcher.listenerCount(event.step.after)).to.equal(baseStep + 1)
+    event.dispatcher.emit(event.test.finished)
+    expect(event.dispatcher.listenerCount(event.step.after)).to.equal(baseStep)
+    expect(event.dispatcher.listenerCount(event.test.finished)).to.equal(baseFin)
+  })
+
+  it('does not restore the pause session when none is open on test.finished', async () => {
+    pause()
+    recorder.start()
+    const restoreSpy = sinon.spy(recorder.session, 'restore')
+    event.dispatcher.emit(event.test.finished)
+    event.dispatcher.emit(event.test.finished)
+    const pauseRestores = restoreSpy.getCalls().filter(c => c.args[0] === 'pause').length
+    restoreSpy.restore()
+    expect(pauseRestores, 'no pause restore when nothing is open').to.equal(0)
+    expect(recorder.getCurrentSessionId()).to.equal(null)
+    await settles(recorder.promise())
+  })
+
+  it('pauseNow drives the external handler and restores the session', async () => {
+    let received = null
+    setPauseHandler(arg => {
+      received = arg
+      return Promise.resolve()
+    })
+    recorder.start()
+    pauseNow({ foo: 1 })
+    await settles(recorder.promise())
+    expect(received).to.deep.equal({ registeredVariables: { foo: 1 } })
+    expect(recorder.getCurrentSessionId()).to.equal(null)
+  })
+})