Skip to content

Commit 8f5d316

Browse files
committed
fix: prevent Playwright tests from hanging when BROWSER_RESTART=browser
- Add timeout protection to _afterSuite() and _cleanup() methods - Fix restart strategy detection in lib/listener/helpers.js to include 'browser' and 'session' restart modes - Add teardown timeout (10s) and force exit (500ms) in run command - Skip _finishTest and _cleanup hooks when browser restart is enabled This prevents double cleanup and hanging when using BROWSER_RESTART=browser env variable. Tests now complete and exit properly instead of hanging indefinitely.
1 parent 746333b commit 8f5d316

File tree

3 files changed

+54
-17
lines changed

3 files changed

+54
-17
lines changed

lib/command/run.js

Lines changed: 20 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -41,20 +41,28 @@ export default async function (test, options) {
4141
printError(err)
4242
process.exitCode = 1
4343
} finally {
44-
await codecept.teardown()
44+
// Add timeout to teardown to prevent hanging
45+
const teardownPromise = codecept.teardown()
46+
const teardownTimeout = new Promise((_, reject) =>
47+
setTimeout(() => reject(new Error('Teardown timeout')), 10000)
48+
)
4549

46-
// Force exit if event loop doesn't clear naturally
47-
// This is needed because some helpers (like Playwright) may leave handles open
48-
// even after proper cleanup, preventing natural process termination
50+
try {
51+
await Promise.race([teardownPromise, teardownTimeout])
52+
} catch (e) {
53+
if (e.message === 'Teardown timeout') {
54+
console.warn('Teardown timed out after 10 seconds, forcing exit')
55+
} else {
56+
console.warn('Teardown error:', e.message)
57+
}
58+
}
59+
60+
// Force exit immediately after teardown completes or times out
4961
if (!options.noExit) {
50-
// Use beforeExit to ensure we run after all other exit handlers (including Mocha's)
51-
// have set the correct exit code
52-
process.once('beforeExit', (code) => {
53-
// Give cleanup a moment to complete, then force exit with the correct code
54-
setTimeout(() => {
55-
process.exit(code || process.exitCode || 0)
56-
}, 100)
57-
})
62+
// Give a very short grace period then force exit
63+
setTimeout(() => {
64+
process.exit(process.exitCode || 0)
65+
}, 500)
5866
}
5967
}
6068
}

lib/helper/Playwright.js

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -808,12 +808,20 @@ class Playwright extends Helper {
808808
// For session mode (restart:false): stop after the last suite
809809
if (this.isRunning) {
810810
try {
811-
await this._stopBrowser()
811+
// Add timeout protection to prevent hanging
812+
await Promise.race([
813+
this._stopBrowser(),
814+
new Promise((_, reject) => setTimeout(() => reject(new Error('Browser stop timeout in afterSuite')), 10000)),
815+
])
812816
} catch (e) {
813817
console.warn('Warning during suite cleanup:', e.message)
814818
// Track suite cleanup failures
815819
this.hasCleanupError = true
816820
this.testFailures.push(`Suite cleanup failed: ${e.message}`)
821+
// Force cleanup on timeout
822+
this.browser = null
823+
this.browserContext = null
824+
this.isRunning = false
817825
} finally {
818826
this.isRunning = false
819827
}
@@ -914,17 +922,32 @@ class Playwright extends Helper {
914922
// Final cleanup when test run completes
915923
if (this.isRunning) {
916924
try {
917-
await this._stopBrowser()
925+
// Add timeout protection to prevent hanging
926+
await Promise.race([
927+
this._stopBrowser(),
928+
new Promise((_, reject) => setTimeout(() => reject(new Error('Browser stop timeout in cleanup')), 10000)),
929+
])
918930
} catch (e) {
919931
console.warn('Warning during final cleanup:', e.message)
932+
// Force cleanup on timeout
933+
this.browser = null
934+
this.browserContext = null
935+
this.isRunning = false
920936
}
921937
} else {
922938
// Check if we still have a browser object despite isRunning being false
923939
if (this.browser) {
924940
try {
925-
await this._stopBrowser()
941+
// Add timeout protection to prevent hanging
942+
await Promise.race([
943+
this._stopBrowser(),
944+
new Promise((_, reject) => setTimeout(() => reject(new Error('Browser stop timeout in forced cleanup')), 10000)),
945+
])
926946
} catch (e) {
927947
console.warn('Warning during forced cleanup:', e.message)
948+
// Force cleanup on timeout
949+
this.browser = null
950+
this.browserContext = null
928951
}
929952
}
930953
}

lib/listener/helpers.js

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,10 @@ export default function () {
7474

7575
event.dispatcher.on(event.all.result, () => {
7676
// Skip _finishTest for all helpers if any browser helper restarts to avoid double cleanup
77-
const hasBrowserRestart = Object.values(helpers).some(helper => (helper.config && helper.config.restart) || (helper.options && helper.options.restart === 'context'))
77+
const hasBrowserRestart = Object.values(helpers).some(helper =>
78+
(helper.config && (helper.config.restart === 'browser' || helper.config.restart === 'context' || helper.config.restart === true)) ||
79+
(helper.options && (helper.options.restart === 'browser' || helper.options.restart === 'context' || helper.options.restart === true))
80+
)
7881

7982
Object.keys(helpers).forEach(key => {
8083
const helper = helpers[key]
@@ -86,7 +89,10 @@ export default function () {
8689

8790
event.dispatcher.on(event.all.after, () => {
8891
// Skip _cleanup for all helpers if any browser helper restarts to avoid double cleanup
89-
const hasBrowserRestart = Object.values(helpers).some(helper => (helper.config && helper.config.restart) || (helper.options && helper.options.restart === 'context'))
92+
const hasBrowserRestart = Object.values(helpers).some(helper =>
93+
(helper.config && (helper.config.restart === 'browser' || helper.config.restart === 'context' || helper.config.restart === true)) ||
94+
(helper.options && (helper.options.restart === 'browser' || helper.options.restart === 'context' || helper.options.restart === true))
95+
)
9096

9197
Object.keys(helpers).forEach(key => {
9298
const helper = helpers[key]

0 commit comments

Comments
 (0)