Skip to content

Commit f5682e7

Browse files
Copilotkobenguyent
andcommitted
Fix browser close hanging by ensuring all pages and contexts are closed first
Co-authored-by: kobenguyent <7845001+kobenguyent@users.noreply.github.com>
1 parent fc96ccc commit f5682e7

File tree

4 files changed

+29
-58
lines changed

4 files changed

+29
-58
lines changed

lib/helper/Playwright.js

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1389,6 +1389,29 @@ class Playwright extends Helper {
13891389
}
13901390
}
13911391

1392+
// Close all contexts and pages before closing browser to prevent hanging
1393+
if (this.browser) {
1394+
try {
1395+
const contexts = await Promise.race([
1396+
this.browser.contexts(),
1397+
new Promise((_, reject) => setTimeout(() => reject(new Error('Get contexts timeout')), 1000))
1398+
])
1399+
// Close all pages in all contexts first
1400+
await Promise.allSettled(contexts.map(async (ctx) => {
1401+
try {
1402+
const pages = await ctx.pages()
1403+
await Promise.allSettled(pages.map(p => p.close().catch(() => {})))
1404+
} catch (e) {
1405+
// Ignore errors getting or closing pages
1406+
}
1407+
}))
1408+
// Then close all contexts
1409+
await Promise.allSettled(contexts.map(c => c.close().catch(() => {})))
1410+
} catch (e) {
1411+
// Ignore errors if browser is already closed or timeout getting contexts
1412+
}
1413+
}
1414+
13921415
if (this.options.recordHar && this.browserContext) {
13931416
try {
13941417
await this.browserContext.close()

lib/helper/extras/PlaywrightRestartOpts.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,9 @@ export function setRestartStrategy(options) {
1414
return (restarts = restart)
1515
}
1616

17-
// When restart is false, map to 'context' restart (as per documentation)
17+
// When restart is false, don't restart anything
1818
if (restart === false) {
19-
restarts = 'context'
19+
restarts = null
2020
return
2121
}
2222

lib/listener/helpers.js

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -74,10 +74,9 @@ 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-
// Note: restart: false is equivalent to restart: 'context' per documentation
7877
const hasBrowserRestart = Object.values(helpers).some(helper =>
79-
(helper.config && (helper.config.restart === 'browser' || helper.config.restart === 'context' || helper.config.restart === false || helper.config.restart === true)) ||
80-
(helper.options && (helper.options.restart === 'browser' || helper.options.restart === 'context' || helper.options.restart === false || helper.options.restart === true))
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))
8180
)
8281

8382
Object.keys(helpers).forEach(key => {
@@ -90,10 +89,9 @@ export default function () {
9089

9190
event.dispatcher.on(event.all.after, () => {
9291
// Skip _cleanup for all helpers if any browser helper restarts to avoid double cleanup
93-
// Note: restart: false is equivalent to restart: 'context' per documentation
9492
const hasBrowserRestart = Object.values(helpers).some(helper =>
95-
(helper.config && (helper.config.restart === 'browser' || helper.config.restart === 'context' || helper.config.restart === false || helper.config.restart === true)) ||
96-
(helper.options && (helper.options.restart === 'browser' || helper.options.restart === 'context' || helper.options.restart === false || helper.options.restart === true))
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))
9795
)
9896

9997
Object.keys(helpers).forEach(key => {

test/helper/PlaywrightRestartOpts_test.js

Lines changed: 0 additions & 50 deletions
This file was deleted.

0 commit comments

Comments
 (0)