From 1288519915d5886d2d0ff8174578758edbd93808 Mon Sep 17 00:00:00 2001 From: Max Schmitt Date: Wed, 19 Jul 2023 17:50:25 +0200 Subject: [PATCH] fix(ui-mode): run teardown handlers with Command + C (#24267) Fixes https://github.com/microsoft/playwright/issues/23907 --- .../src/server/trace/viewer/traceViewer.ts | 6 ++- packages/playwright-test/src/runner/uiMode.ts | 28 ++++++++----- tests/playwright-test/ui-mode-fixtures.ts | 39 +++++++++++++------ .../ui-mode-test-setup.spec.ts | 34 ++++++++++++++++ 4 files changed, 86 insertions(+), 21 deletions(-) diff --git a/packages/playwright-core/src/server/trace/viewer/traceViewer.ts b/packages/playwright-core/src/server/trace/viewer/traceViewer.ts index 12caf129b5..52e99e7a10 100644 --- a/packages/playwright-core/src/server/trace/viewer/traceViewer.ts +++ b/packages/playwright-core/src/server/trace/viewer/traceViewer.ts @@ -25,6 +25,7 @@ import { createPlaywright } from '../../playwright'; import { ProgressController } from '../../progress'; import { open, wsServer } from '../../../utilsBundle'; import type { Page } from '../../page'; +import type { BrowserType } from '../../browserType'; export type Transport = { sendEvent?: (method: string, params: any) => void; @@ -40,6 +41,7 @@ export type OpenTraceViewerOptions = { port?: number; isServer?: boolean; transport?: Transport; + persistentContextOptions?: Parameters[2]; }; async function startTraceViewerServer(traceUrls: string[], options?: OpenTraceViewerOptions): Promise<{ server: HttpServer, url: string }> { @@ -144,6 +146,7 @@ export async function openTraceViewerApp(traceUrls: string[], browserName: strin ignoreDefaultArgs: ['--enable-automation'], colorScheme: 'no-override', useWebSocket: isUnderTest(), + ...options?.persistentContextOptions, }); const controller = new ProgressController(serverSideCallMetadata(), context._browser); @@ -171,7 +174,8 @@ export async function openTraceInBrowser(traceUrls: string[], options?: OpenTrac const { url } = await startTraceViewerServer(traceUrls, options); // eslint-disable-next-line no-console console.log('\nListening on ' + url); - await open(url).catch(() => {}); + if (!isUnderTest()) + await open(url).catch(() => {}); } class StdinServer implements Transport { diff --git a/packages/playwright-test/src/runner/uiMode.ts b/packages/playwright-test/src/runner/uiMode.ts index d231ede2a1..940d76d28c 100644 --- a/packages/playwright-test/src/runner/uiMode.ts +++ b/packages/playwright-test/src/runner/uiMode.ts @@ -29,6 +29,7 @@ import { open } from 'playwright-core/lib/utilsBundle'; import ListReporter from '../reporters/list'; import type { OpenTraceViewerOptions, Transport } from 'playwright-core/lib/server/trace/viewer/traceViewer'; import { Multiplexer } from '../reporters/multiplexer'; +import { SigIntWatcher } from './sigIntWatcher'; class UIMode { private _config: FullConfigInternal; @@ -79,7 +80,7 @@ class UIMode { return status; } - async showUI(options: { host?: string, port?: number }) { + async showUI(options: { host?: string, port?: number }, cancelPromise: ManualPromise) { let queue = Promise.resolve(); this._transport = { @@ -118,13 +119,15 @@ class UIMode { transport: this._transport, host: options.host, port: options.port, + persistentContextOptions: { + handleSIGINT: false, + }, }; - const exitPromise = new ManualPromise(); if (options.host !== undefined || options.port !== undefined) { await openTraceInBrowser([], openOptions); } else { const page = await openTraceViewerApp([], 'chromium', openOptions); - page.on('close', () => exitPromise.resolve()); + page.on('close', () => cancelPromise.resolve()); } if (!process.env.PWTEST_DEBUG) { @@ -137,7 +140,7 @@ class UIMode { return true; }; } - await exitPromise; + await cancelPromise; if (!process.env.PWTEST_DEBUG) { process.stdout.write = this._originalStdoutWrite; @@ -218,11 +221,18 @@ class UIMode { export async function runUIMode(config: FullConfigInternal, options: { host?: string, port?: number }): Promise { const uiMode = new UIMode(config); - const status = await uiMode.runGlobalSetup(); - if (status !== 'passed') - return status; - await uiMode.showUI(options); - return await uiMode.globalCleanup?.() || 'passed'; + const globalSetupStatus = await uiMode.runGlobalSetup(); + if (globalSetupStatus !== 'passed') + return globalSetupStatus; + const cancelPromise = new ManualPromise(); + const sigintWatcher = new SigIntWatcher(); + void sigintWatcher.promise().then(() => cancelPromise.resolve()); + try { + await uiMode.showUI(options, cancelPromise); + } finally { + sigintWatcher.disarm(); + } + return await uiMode.globalCleanup?.() || (sigintWatcher.hadSignal() ? 'interrupted' : 'passed'); } type StdioPayload = { diff --git a/tests/playwright-test/ui-mode-fixtures.ts b/tests/playwright-test/ui-mode-fixtures.ts index 78e1d17cdb..ef016f047b 100644 --- a/tests/playwright-test/ui-mode-fixtures.ts +++ b/tests/playwright-test/ui-mode-fixtures.ts @@ -21,7 +21,7 @@ import type { TestChildProcess } from '../config/commonFixtures'; import { rimraf } from '../../packages/playwright-core/lib/utilsBundle'; import { cleanEnv, cliEntrypoint, test as base, writeFiles } from './playwright-test-fixtures'; import type { Files, RunOptions } from './playwright-test-fixtures'; -import type { Browser, Page, TestInfo } from './stable-test-runner'; +import type { Browser, BrowserType, Page, TestInfo } from './stable-test-runner'; import { createGuid } from '../../packages/playwright-core/src/utils/crypto'; type Latch = { @@ -30,8 +30,12 @@ type Latch = { close: () => void; }; +type UIModeOptions = RunOptions & { + useWeb?: boolean +}; + type Fixtures = { - runUITest: (files: Files, env?: NodeJS.ProcessEnv, options?: RunOptions) => Promise<{ page: Page, testProcess: TestChildProcess }>; + runUITest: (files: Files, env?: NodeJS.ProcessEnv, options?: UIModeOptions) => Promise<{ page: Page, testProcess: TestChildProcess }>; createLatch: () => Latch; }; @@ -82,16 +86,16 @@ export function dumpTestTree(page: Page, options: { time?: boolean } = {}): () = export const test = base .extend({ - runUITest: async ({ childProcess, playwright, headless }, use, testInfo: TestInfo) => { + runUITest: async ({ childProcess, headless }, use, testInfo: TestInfo) => { if (process.env.CI) testInfo.slow(); const cacheDir = await fs.promises.mkdtemp(path.join(os.tmpdir(), 'playwright-test-cache-')); let testProcess: TestChildProcess | undefined; let browser: Browser | undefined; - await use(async (files: Files, env: NodeJS.ProcessEnv = {}, options: RunOptions = {}) => { + await use(async (files: Files, env: NodeJS.ProcessEnv = {}, options: UIModeOptions = {}) => { const baseDir = await writeFiles(testInfo, files, true); testProcess = childProcess({ - command: ['node', cliEntrypoint, 'test', '--ui', '--workers=1', ...(options.additionalArgs || [])], + command: ['node', cliEntrypoint, 'test', (options.useWeb ? '--ui-host=127.0.0.1' : '--ui'), '--workers=1', ...(options.additionalArgs || [])], env: { ...cleanEnv(env), PWTEST_UNDER_TEST: '1', @@ -101,12 +105,25 @@ export const test = base }, cwd: options.cwd ? path.resolve(baseDir, options.cwd) : baseDir, }); - await testProcess.waitForOutput('DevTools listening on'); - const line = testProcess.output.split('\n').find(l => l.includes('DevTools listening on')); - const wsEndpoint = line!.split(' ')[3]; - browser = await playwright.chromium.connectOverCDP(wsEndpoint); - const [context] = browser.contexts(); - const [page] = context.pages(); + let page: Page; + // We want to have ToT playwright-core here, since we install it's browsers and otherwise + // don't have the right browser revision (ToT revisions != stable-test-runner revisions). + const chromium: BrowserType = require('../../packages/playwright-core').chromium; + if (options.useWeb) { + await testProcess.waitForOutput('Listening on'); + const line = testProcess.output.split('\n').find(l => l.includes('Listening on')); + const uiAddress = line!.split(' ')[2]; + browser = await chromium.launch(); + page = await browser.newPage(); + await page.goto(uiAddress); + } else { + await testProcess.waitForOutput('DevTools listening on'); + const line = testProcess.output.split('\n').find(l => l.includes('DevTools listening on')); + const wsEndpoint = line!.split(' ')[3]; + browser = await chromium.connectOverCDP(wsEndpoint); + const [context] = browser.contexts(); + [page] = context.pages(); + } return { page, testProcess }; }); await browser?.close(); diff --git a/tests/playwright-test/ui-mode-test-setup.spec.ts b/tests/playwright-test/ui-mode-test-setup.spec.ts index 45ac9b3f94..702fbf7640 100644 --- a/tests/playwright-test/ui-mode-test-setup.spec.ts +++ b/tests/playwright-test/ui-mode-test-setup.spec.ts @@ -193,3 +193,37 @@ test('should run part of the setup only', async ({ runUITest }) => { ◯ test `); }); + +for (const useWeb of [true, false]) { + test.describe(`web-mode: ${useWeb}`, () => { + test('should run teardown with SIGINT', async ({ runUITest }) => { + test.skip(process.platform === 'win32', 'No sending SIGINT on Windows'); + const { page, testProcess } = await runUITest({ + 'playwright.config.ts': ` + import { defineConfig } from '@playwright/test'; + export default defineConfig({ + globalTeardown: './globalTeardown.ts', + }); + `, + 'globalTeardown.ts': ` + export default async () => { + console.log('\\n%%from-global-teardown0000') + await new Promise(f => setTimeout(f, 3000)); + console.log('\\n%%from-global-teardown3000') + }; + `, + 'a.test.js': ` + import { test, expect } from '@playwright/test'; + test('should work', async ({}) => {}); + ` + }, null, { useWeb }); + await page.getByTitle('Run all').click(); + await expect(page.getByTestId('status-line')).toHaveText('1/1 passed (100%)'); + await testProcess.kill('SIGINT'); + await expect.poll(() => testProcess.outputLines()).toEqual([ + 'from-global-teardown0000', + 'from-global-teardown3000', + ]); + }); + }); +} \ No newline at end of file