fix(pause): only pause on fail when the test fails

The fail mode of the pause plugin (and the pauseOnFail alias) set its flag
on step.failed, so a step that failed inside tryTo, hopeThat or retryTo
opened the interactive shell even though the test passed. Track
test.failed instead. Hook failures still pause because they are reported
through test.failed before test.after.

Fixes #4516

Author: luantaraschi

docs/debugging.md

@@ -120,7 +120,7 @@ For automated debugging without modifying test code, use the `pause` plugin. It
 
 ### Pause on Failure
 
-Automatically enters interactive pause when a step fails:
+Automatically enters interactive pause when a test fails. A step that fails inside `tryTo`, `hopeThat` or `retryTo` does not trigger it while the test keeps passing:
 
 ```bash
 npx codeceptjs run -p pause

lib/plugin/pause.js

@@ -27,7 +27,8 @@ import {
  *
  * #### `on=` modes
  *
- * * **fail** — pause when a step fails (default)
+ * * **fail** — pause after a test fails (default). Steps that fail inside
+ *   `tryTo`, `hopeThat` or `retryTo` without failing the test do not trigger a pause
  * * **test** — pause after each test
  * * **step** — pause before the first step (interactive walk-through)
  * * **file** — pause when execution reaches `path=...[;line=...]`
@@ -68,7 +69,7 @@ function initFailMode() {
     failed = false
   })
 
-  event.dispatcher.on(event.step.failed, () => {
+  event.dispatcher.on(event.test.failed, () => {
     failed = true
   })
 

test/unit/plugin/pause_test.js

@@ -0,0 +1,139 @@
+import { expect } from 'chai'
+import pausePlugin from '../../../lib/plugin/pause.js'
+import { setPauseHandler } from '../../../lib/pause.js'
+import { hopeThat, retryTo, tryTo } from '../../../lib/effects.js'
+import recordStep from '../../../lib/step/record.js'
+import Step from '../../../lib/step.js'
+import event from '../../../lib/event.js'
+import recorder from '../../../lib/recorder.js'
+import store from '../../../lib/store.js'
+import { createTest } from '../../../lib/mocha/test.js'
+
+const trackedEvents = [event.test.before, event.test.started, event.test.failed, event.test.after, event.step.failed]
+
+const failingStep = () => recordStep(new Step({ fail: () => Promise.reject(new Error('Ups')) }, 'fail'), [])
+
+describe('pause plugin on=fail', () => {
+  let pauses
+  let listenersBefore
+
+  beforeEach(() => {
+    store.dryRun = false
+    pauses = 0
+    setPauseHandler(() => {
+      pauses++
+      return Promise.resolve()
+    })
+    listenersBefore = Object.fromEntries(trackedEvents.map(name => [name, event.dispatcher.listeners(name)]))
+    recorder.reset()
+    recorder.start()
+    pausePlugin({})
+  })
+
+  afterEach(() => {
+    for (const name of trackedEvents) {
+      for (const listener of event.dispatcher.listeners(name)) {
+        if (!listenersBefore[name].includes(listener)) event.dispatcher.removeListener(name, listener)
+      }
+    }
+    event.dispatcher.emit(event.test.finished, createTest('cleanup'))
+    setPauseHandler(null)
+    recorder.reset()
+  })
+
+  const finishTest = async test => {
+    event.dispatcher.emit(event.test.finished, test)
+    event.dispatcher.emit(event.test.after, test)
+    await recorder.promise()
+  }
+
+  it('does not pause when a step failure is swallowed by tryTo (#4516)', async () => {
+    const test = createTest('tryTo')
+    event.dispatcher.emit(event.test.started, test)
+    const result = await tryTo(failingStep)
+    await recorder.promise()
+    expect(result).to.equal(false)
+    event.dispatcher.emit(event.test.passed, test)
+    await finishTest(test)
+    expect(pauses).to.equal(0)
+  })
+
+  it('does not pause when a step failure is swallowed by hopeThat', async () => {
+    const test = createTest('hopeThat')
+    event.dispatcher.emit(event.test.started, test)
+    const result = await hopeThat(failingStep)
+    await recorder.promise()
+    expect(result).to.equal(false)
+    event.dispatcher.emit(event.test.passed, test)
+    await finishTest(test)
+    expect(pauses).to.equal(0)
+  })
+
+  it('does not pause when retryTo succeeds after a failed attempt', async () => {
+    const test = createTest('retryTo')
+    event.dispatcher.emit(event.test.started, test)
+    await retryTo(tries => (tries === 1 ? failingStep() : recorder.add(() => 'ok')), 2, 0)
+    await recorder.promise()
+    event.dispatcher.emit(event.test.passed, test)
+    await finishTest(test)
+    expect(pauses).to.equal(0)
+  })
+
+  it('pauses when a failed step fails the test', async () => {
+    const test = createTest('failing step')
+    event.dispatcher.emit(event.test.started, test)
+    const err = await failingStep().catch(e => e)
+    recorder.reset()
+    recorder.start()
+    event.dispatcher.emit(event.test.failed, test, err)
+    await finishTest(test)
+    expect(pauses).to.equal(1)
+  })
+
+  it('pauses when a Before hook fails', async () => {
+    const test = createTest('before hook')
+    event.dispatcher.emit(event.test.before, test)
+    const err = await failingStep().catch(e => e)
+    recorder.reset()
+    recorder.start()
+    event.dispatcher.emit(event.test.failed, test, err, 'Before')
+    await finishTest(test)
+    expect(pauses).to.equal(1)
+  })
+
+  it('pauses when an After hook fails a passed test', async () => {
+    const test = createTest('after hook')
+    event.dispatcher.emit(event.test.started, test)
+    event.dispatcher.emit(event.test.passed, test)
+    const err = await failingStep().catch(e => e)
+    recorder.reset()
+    recorder.start()
+    event.dispatcher.emit(event.test.failed, test, err, 'After')
+    await finishTest(test)
+    expect(pauses).to.equal(1)
+  })
+
+  it('pauses when the test fails without a failed step', async () => {
+    const test = createTest('plain error')
+    event.dispatcher.emit(event.test.started, test)
+    event.dispatcher.emit(event.test.failed, test, new Error('Ups'))
+    await finishTest(test)
+    expect(pauses).to.equal(1)
+  })
+
+  it('does not carry a failure over to the next test', async () => {
+    const failed = createTest('failed')
+    event.dispatcher.emit(event.test.started, failed)
+    const err = await failingStep().catch(e => e)
+    recorder.reset()
+    recorder.start()
+    event.dispatcher.emit(event.test.failed, failed, err)
+    await finishTest(failed)
+
+    const passed = createTest('passed')
+    event.dispatcher.emit(event.test.started, passed)
+    event.dispatcher.emit(event.test.passed, passed)
+    await finishTest(passed)
+    expect(pauses).to.equal(1)
+  })
+})