diff --git a/run-tests.js b/run-tests.js index 67925e4c87ee7..c270b72ef78d4 100644 --- a/run-tests.js +++ b/run-tests.js @@ -555,7 +555,8 @@ ${ENDGROUP}`) // If environment is CI and if this test execution is failed after retry, preserve test traces // to upload into github actions artifacts for debugging purpose const shouldPreserveTracesOutput = - process.env.CI && isRetry && isChildExitWithNonZero + (process.env.CI && isRetry && isChildExitWithNonZero) || + process.env.PRESERVE_TRACES_OUTPUT if (!shouldPreserveTracesOutput) { await fsp .rm( diff --git a/test/lib/browsers/playwright.ts b/test/lib/browsers/playwright.ts index f54062d3c45b2..c665b133042e4 100644 --- a/test/lib/browsers/playwright.ts +++ b/test/lib/browsers/playwright.ts @@ -21,13 +21,25 @@ let websocketFrames: Array<{ payload: string | Buffer }> = [] const tracePlaywright = process.env.TRACE_PLAYWRIGHT +// loose global to register teardown functions before quitting the browser instance. +// This is due to `quit` can be called anytime outside of BrowserInterface's lifecycle, +// which can create corrupted state by terminating the context. +// [TODO] global `quit` might need to be removed, instead should introduce per-instance teardown +const pendingTeardown = [] export async function quit() { + await Promise.all(pendingTeardown.map((fn) => fn())) await context?.close() await browser?.close() context = undefined browser = undefined } +async function teardown(tearDownFn: () => Promise) { + pendingTeardown.push(tearDownFn) + await tearDownFn() + pendingTeardown.splice(pendingTeardown.indexOf(tearDownFn), 1) +} + interface ElementHandleExt extends ElementHandle { getComputedCss(prop: string): Promise text(): Promise @@ -38,7 +50,6 @@ export class Playwright extends BrowserInterface { private eventCallbacks: Record void>> = { request: new Set(), } - private async initContextTracing( url: string, page: Page, @@ -50,7 +61,7 @@ export class Playwright extends BrowserInterface { try { // Clean up if any previous traces are still active - await this.teardownTracing() + await teardown(this.teardownTracing.bind(this)) await context.tracing.start({ screenshots: true, @@ -60,7 +71,7 @@ export class Playwright extends BrowserInterface { this.activeTrace = encodeURIComponent(url) page.on('close', async () => { - await this.teardownTracing() + await teardown(this.teardownTracing.bind(this)) }) } catch (e) { this.activeTrace = undefined @@ -87,6 +98,7 @@ export class Playwright extends BrowserInterface { path: traceOutputPath, }) } catch (e) { + require('console').warn('Failed to teardown playwright tracing', e) } finally { this.activeTrace = undefined } @@ -128,6 +140,7 @@ export class Playwright extends BrowserInterface { if (browser) { if (contextHasJSEnabled !== javaScriptEnabled) { // If we have switched from having JS enable/disabled we need to recreate the context. + await teardown(this.teardownTracing.bind(this)) await context?.close() context = await browser.newContext({ locale, @@ -135,9 +148,6 @@ export class Playwright extends BrowserInterface { ignoreHTTPSErrors, ...device, }) - context.once('close', async () => { - await this.teardownTracing() - }) contextHasJSEnabled = javaScriptEnabled } return @@ -150,9 +160,6 @@ export class Playwright extends BrowserInterface { ignoreHTTPSErrors, ...device, }) - context.once('close', async () => { - await this.teardownTracing() - }) contextHasJSEnabled = javaScriptEnabled }