Skip to content

fix(playwright): reset cleanup state at start of _afterSuite to prevent worker crashes#5552

Merged
DavertMik merged 1 commit intocodeceptjs:4.xfrom
gololdf1sh:fix/aftersuite-stale-cleanup-state
May 8, 2026
Merged

fix(playwright): reset cleanup state at start of _afterSuite to prevent worker crashes#5552
DavertMik merged 1 commit intocodeceptjs:4.xfrom
gololdf1sh:fix/aftersuite-stale-cleanup-state

Conversation

@gololdf1sh
Copy link
Copy Markdown

@gololdf1sh gololdf1sh commented May 8, 2026

Problem

When running tests with run-workers, workers can crash silently due to stale cleanup state leaking from test-level hooks into _afterSuite().

Root cause

  1. A test fails → the screenshot plugin calls saveScreenshot()
  2. If the screenshot also fails (timeout, closed page, protocol error), saveScreenshot() sets this.hasCleanupError = true and pushes to this.testFailures[] (Playwright.js L2991-2992)
  3. The error is caught and the method returns gracefully — the test runner continues
  4. hasCleanupError / testFailures are reset per-test in _beforeTest() (L567-568), but not at the start of _afterSuite()
  5. When the last test in a suite has a screenshot failure, the stale state carries into _afterSuite()
  6. At the end of _afterSuite(), the guard clause if (this.hasCleanupError && this.testFailures.length > 0) evaluates to true and throws (L841-844) — even if the suite's own cleanup completed successfully
  7. In a run-workers context, this throw kills the worker thread. All remaining test files assigned to that worker are silently dropped — they never execute and never appear in the results

Fix

Reset hasCleanupError and testFailures at the top of _afterSuite(), before any suite-level cleanup runs. This ensures the guard clause at L841 only evaluates errors from _afterSuite's own cleanup operations, not leftover test-level errors.

  async _afterSuite() {
+   // Reset leftover test-level cleanup state (e.g. screenshot failures)
+   // so only errors from this suite teardown are evaluated below.
+   this.hasCleanupError = false
+   this.testFailures = []
+
    // Stop browser after suite completes

…nt worker crashes

When a test fails and its screenshot also fails (timeout, closed page, etc.),
`saveScreenshot()` sets `hasCleanupError = true` and pushes to `testFailures[]`.

These flags are reset per-test in `_beforeTest()` (line 567-568), but NOT at the
start of `_afterSuite()`. So when the last test in a suite has a screenshot failure,
the stale error state carries over into `_afterSuite`.

At the end of `_afterSuite` (line 841-844), the method checks
`if (this.hasCleanupError && this.testFailures.length > 0)` and throws — even if
the suite's own cleanup completed successfully. This throw is fatal in a
`run-workers` context: it kills the worker thread and silently drops all remaining
test files assigned to that worker.

Observed impact: in a 4-worker parallel run (133 scenarios), Worker 01 crashed
at minute 1 due to this bug. Its remaining ~21 tests were never executed and
never reported — a silent 16% test loss with no indication in the CI summary.

The fix resets `hasCleanupError` and `testFailures` at the top of `_afterSuite`
so the guard clause only evaluates errors from the suite teardown itself, not
leftover test-level errors.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@DavertMik DavertMik merged commit b60be21 into codeceptjs:4.x May 8, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants