diff --git a/packages/playwright-core/src/cli/cli.ts b/packages/playwright-core/src/cli/cli.ts index ec167e0588..b0a950c84b 100755 --- a/packages/playwright-core/src/cli/cli.ts +++ b/packages/playwright-core/src/cli/cli.ts @@ -414,7 +414,16 @@ async function launchContext(options: Options, headless: boolean, executablePath const browser = await browserType.launch(launchOptions); - if (process.env.PWTEST_CLI_EXIT) { + if (process.env.PWTEST_CLI_IS_UNDER_TEST) { + (process as any)._didSetSourcesForTest = (text: string) => { + process.stdout.write('\n-------------8<-------------\n'); + process.stdout.write(text); + process.stdout.write('\n-------------8<-------------\n'); + const autoExitCondition = process.env.PWTEST_CLI_AUTO_EXIT_WHEN; + if (autoExitCondition && text.includes(autoExitCondition)) + Promise.all(context.pages().map(async p => p.close())); + }; + // Make sure we exit abnormally when browser crashes. const logs: string[] = []; require('playwright-core/lib/utilsBundle').debug.log = (...args: any[]) => { const line = require('util').format(...args) + '\n'; @@ -425,7 +434,6 @@ async function launchContext(options: Options, headless: boolean, executablePath const hasCrashLine = logs.some(line => line.includes('process did exit:') && !line.includes('process did exit: exitCode=0, signal=null')); if (hasCrashLine) { process.stderr.write('Detected browser crash.\n'); - // Make sure we exit abnormally when browser crashes. process.exit(1); } }); @@ -552,7 +560,14 @@ async function openPage(context: BrowserContext, url: string | undefined): Promi url = 'file://' + path.resolve(url); else if (!url.startsWith('http') && !url.startsWith('file://') && !url.startsWith('about:') && !url.startsWith('data:')) url = 'http://' + url; - await page.goto(url); + await page.goto(url).catch(error => { + if (process.env.PWTEST_CLI_AUTO_EXIT_WHEN && error.message.includes('Navigation failed because page was closed')) { + // Tests with PWTEST_CLI_AUTO_EXIT_WHEN might close page too fast, resulting + // in a stray navigation aborted error. We should ignore it. + } else { + throw error; + } + }); } return page; } @@ -567,8 +582,6 @@ async function open(options: Options, url: string | undefined, language: string) saveStorage: options.saveStorage, }); await openPage(context, url); - if (process.env.PWTEST_CLI_EXIT) - await Promise.all(context.pages().map(p => p.close())); } async function codegen(options: Options, url: string | undefined, language: string, outputFile?: string) { @@ -584,8 +597,6 @@ async function codegen(options: Options, url: string | undefined, language: stri handleSIGINT: false, }); await openPage(context, url); - if (process.env.PWTEST_CLI_EXIT) - await Promise.all(context.pages().map(p => p.close())); } async function waitForPage(page: Page, captureOptions: CaptureOptions) { diff --git a/packages/playwright-core/src/server/recorder/recorderApp.ts b/packages/playwright-core/src/server/recorder/recorderApp.ts index e7a7f6b9f3..3ad74f5242 100644 --- a/packages/playwright-core/src/server/recorder/recorderApp.ts +++ b/packages/playwright-core/src/server/recorder/recorderApp.ts @@ -170,13 +170,8 @@ export class RecorderApp extends EventEmitter implements IRecorderApp { }).toString(), true, sources, 'main').catch(() => {}); // Testing harness for runCLI mode. - { - if ((process.env.PWTEST_CLI_IS_UNDER_TEST || process.env.PWTEST_CLI_EXIT) && sources.length) { - process.stdout.write('\n-------------8<-------------\n'); - process.stdout.write(sources[0].text); - process.stdout.write('\n-------------8<-------------\n'); - } - } + if (process.env.PWTEST_CLI_IS_UNDER_TEST && sources.length) + (process as any)._didSetSourcesForTest(sources[0].text); } async setSelector(selector: string, focus?: boolean): Promise { diff --git a/tests/config/commonFixtures.ts b/tests/config/commonFixtures.ts index e9c4932b1e..a904506842 100644 --- a/tests/config/commonFixtures.ts +++ b/tests/config/commonFixtures.ts @@ -28,6 +28,38 @@ type TestChildParams = { onOutput?: () => void; }; +import childProcess from 'child_process'; + +type ProcessData = { + pid: number, // process ID + pgid: number, // process groupd ID + children: Set, // direct children of the process +}; + +function buildProcessTreePosix(pid: number): ProcessData { + const processTree = childProcess.spawnSync('ps', ['-eo', 'pid,pgid,ppid']); + const lines = processTree.stdout.toString().trim().split('\n'); + + const pidToProcess = new Map(); + const edges: { pid: number, ppid: number }[] = []; + for (const line of lines) { + const [pid, pgid, ppid] = line.trim().split(/\s+/).map(token => +token); + // On linux, the very first line of `ps` is the header with "PID PGID PPID". + if (isNaN(pid) || isNaN(pgid) || isNaN(ppid)) + continue; + pidToProcess.set(pid, { pid, pgid, children: new Set() }); + edges.push({ pid, ppid }); + } + for (const { pid, ppid } of edges) { + const parent = pidToProcess.get(ppid); + const child = pidToProcess.get(pid); + // On POSIX, certain processes might not have parent (e.g. PID=1 and occasionally PID=2). + if (parent && child) + parent.children.add(child); + } + return pidToProcess.get(pid); +} + export class TestChildProcess { params: TestChildParams; process: ChildProcess; @@ -72,7 +104,7 @@ export class TestChildProcess { this.process.stderr.on('data', appendChunk); this.process.stdout.on('data', appendChunk); - const killProcessGroup = this._killProcessGroup.bind(this); + const killProcessGroup = this._killProcessTree.bind(this, 'SIGKILL'); process.on('exit', killProcessGroup); this.exited = new Promise(f => { this.process.on('exit', (exitCode, signal) => f({ exitCode, signal })); @@ -86,28 +118,49 @@ export class TestChildProcess { return strippedOutput.split('\n').filter(line => line.startsWith('%%')).map(line => line.substring(2).trim()); } - async close() { - if (this.process.kill(0)) - this._killProcessGroup('SIGINT'); + async kill(signal: 'SIGINT' | 'SIGKILL' = 'SIGKILL') { + this._killProcessTree(signal); return this.exited; } - async kill() { - if (this.process.kill(0)) - this._killProcessGroup('SIGKILL'); - return this.exited; - } - - private _killProcessGroup(signal: 'SIGINT' | 'SIGKILL') { + private _killProcessTree(signal: 'SIGINT' | 'SIGKILL') { if (!this.process.pid || !this.process.kill(0)) return; - try { - if (process.platform === 'win32') + + // On Windows, we always call `taskkill` no matter signal. + if (process.platform === 'win32') { + try { execSync(`taskkill /pid ${this.process.pid} /T /F /FI "MEMUSAGE gt 0"`, { stdio: 'ignore' }); - else - process.kill(-this.process.pid, signal); - } catch (e) { - // the process might have already stopped + } catch (e) { + // the process might have already stopped + } + return; + } + + // In case of POSIX and `SIGINT` signal, send it to the main process group only. + if (signal === 'SIGINT') { + try { + process.kill(-this.process.pid, 'SIGINT'); + } catch (e) { + // the process might have already stopped + } + return; + } + + // In case of POSIX and `SIGKILL` signal, we should send it to all descendant process groups. + const rootProcess = buildProcessTreePosix(this.process.pid); + const descendantProcessGroups = (function flatten(processData: ProcessData, result: Set = new Set()) { + // Process can nullify its own process group with `setpgid`. Use its PID instead. + result.add(processData.pgid || processData.pid); + processData.children.forEach(child => flatten(child, result)); + return result; + })(rootProcess); + for (const pgid of descendantProcessGroups) { + try { + process.kill(-pgid, 'SIGKILL'); + } catch (e) { + // the process might have already stopped + } } } @@ -150,16 +203,7 @@ export const commonFixtures: Fixtures = { processes.push(process); return process; }); - await Promise.all(processes.map(async child => { - await Promise.race([ - child.exited, - new Promise(f => setTimeout(f, 3_000)), - ]); - if (child.process.kill(0)) { - await child.kill(); - throw new Error(`Process ${child.params.command.join(' ')} is still running. Leaking process?\nOutput:${child.output}`); - } - })); + await Promise.all(processes.map(async child => child.kill())); if (testInfo.status !== 'passed' && testInfo.status !== 'skipped' && !process.env.PWTEST_DEBUG) { for (const process of processes) { console.log('====== ' + process.params.command.join(' ')); @@ -176,7 +220,7 @@ export const commonFixtures: Fixtures = { processes.push(process); return process; }); - await Promise.all(processes.map(child => child.close())); + await Promise.all(processes.map(child => child.kill('SIGINT'))); }, { scope: 'worker' }], waitForPort: async ({}, use) => { diff --git a/tests/config/remoteServer.ts b/tests/config/remoteServer.ts index ce1dbb76c4..a088621281 100644 --- a/tests/config/remoteServer.ts +++ b/tests/config/remoteServer.ts @@ -49,8 +49,7 @@ export class RunServer implements PlaywrightServer { } async close() { - await this._process.close(); - await this._process.exitCode; + await this._process.kill('SIGINT'); } } @@ -150,7 +149,7 @@ export class RemoteServer implements PlaywrightServer { await this._browser.close(); this._browser = undefined; } - await this._process.close(); + await this._process.kill('SIGINT'); await this.childExitCode(); } } diff --git a/tests/installation/playwright-cli-codegen.spec.ts b/tests/installation/playwright-cli-codegen.spec.ts index 7562f81809..ecaa0470be 100755 --- a/tests/installation/playwright-cli-codegen.spec.ts +++ b/tests/installation/playwright-cli-codegen.spec.ts @@ -19,20 +19,32 @@ test('codegen should work', async ({ exec }) => { await exec('npm i --foreground-scripts playwright'); await test.step('codegen without arguments', async () => { - const result = await exec('npx playwright codegen', { env: { PWTEST_CLI_EXIT: '1' } }); - expect(result).toContain(`@playwright/test`); + const result = await exec('npx playwright codegen', { + env: { + PWTEST_CLI_IS_UNDER_TEST: '1', + PWTEST_CLI_AUTO_EXIT_WHEN: '@playwright/test', + } + }); expect(result).toContain(`{ page }`); }); await test.step('codegen --target=javascript', async () => { - const result = await exec('npx playwright codegen --target=javascript', { env: { PWTEST_CLI_EXIT: '1' } }); + const result = await exec('npx playwright codegen --target=javascript', { + env: { + PWTEST_CLI_IS_UNDER_TEST: '1', + PWTEST_CLI_AUTO_EXIT_WHEN: 'context.close', + } + }); expect(result).toContain(`playwright`); - expect(result).toContain(`page.close`); }); await test.step('codegen --target=python', async () => { - const result = await exec('npx playwright codegen --target=python', { env: { PWTEST_CLI_EXIT: '1' } }); - expect(result).toContain(`chromium.launch`); + const result = await exec('npx playwright codegen --target=python', { + env: { + PWTEST_CLI_IS_UNDER_TEST: '1', + PWTEST_CLI_AUTO_EXIT_WHEN: 'chromium.launch', + }, + }); expect(result).toContain(`browser.close`); }); }); diff --git a/tests/library/inspector/cli-codegen-2.spec.ts b/tests/library/inspector/cli-codegen-2.spec.ts index 2fd0f89b43..99ac3c7029 100644 --- a/tests/library/inspector/cli-codegen-2.spec.ts +++ b/tests/library/inspector/cli-codegen-2.spec.ts @@ -481,8 +481,10 @@ test.describe('cli codegen', () => { test('should --save-trace', async ({ runCLI }, testInfo) => { const traceFileName = testInfo.outputPath('trace.zip'); - const cli = runCLI([`--save-trace=${traceFileName}`]); - await cli.exited; + const cli = runCLI([`--save-trace=${traceFileName}`], { + autoExitWhen: ' ', + }); + await cli.waitForCleanExit(); expect(fs.existsSync(traceFileName)).toBeTruthy(); }); @@ -492,11 +494,9 @@ test.describe('cli codegen', () => { const traceFileName = testInfo.outputPath('trace.zip'); const storageFileName = testInfo.outputPath('auth.json'); const harFileName = testInfo.outputPath('har.har'); - const cli = runCLI([`--save-trace=${traceFileName}`, `--save-storage=${storageFileName}`, `--save-har=${harFileName}`], { - noAutoExit: true, - }); + const cli = runCLI([`--save-trace=${traceFileName}`, `--save-storage=${storageFileName}`, `--save-har=${harFileName}`]); await cli.waitFor(`import { test, expect } from '@playwright/test'`); - cli.exit('SIGINT'); + cli.process.kill('SIGINT'); const { exitCode } = await cli.process.exited; expect(exitCode).toBe(130); expect(fs.existsSync(traceFileName)).toBeTruthy(); diff --git a/tests/library/inspector/cli-codegen-csharp.spec.ts b/tests/library/inspector/cli-codegen-csharp.spec.ts index 767302114d..c44043cea1 100644 --- a/tests/library/inspector/cli-codegen-csharp.spec.ts +++ b/tests/library/inspector/cli-codegen-csharp.spec.ts @@ -43,8 +43,7 @@ class Program ${launchOptions(channel)} }); var context = await browser.NewContextAsync();`; - await cli.waitFor(expectedResult).catch(e => e); - expect(cli.text()).toContain(expectedResult); + await cli.waitFor(expectedResult); }); test('should print the correct context options for custom settings', async ({ browserName, channel, runCLI }) => { @@ -87,7 +86,6 @@ test('should print the correct context options for custom settings', async ({ br }, });`; await cli.waitFor(expectedResult); - expect(cli.text()).toContain(expectedResult); }); test('should print the correct context options when using a device', async ({ browserName, channel, runCLI }) => { @@ -102,7 +100,6 @@ test('should print the correct context options when using a device', async ({ br }); var context = await browser.NewContextAsync(playwright.Devices["Pixel 2"]);`; await cli.waitFor(expectedResult); - expect(cli.text()).toContain(expectedResult); }); test('should print the correct context options when using a device and additional options', async ({ browserName, channel, runCLI }) => { @@ -147,9 +144,7 @@ test('should print the correct context options when using a device and additiona Width = 1280, }, });`; - await cli.waitFor(expectedResult); - expect(cli.text()).toContain(expectedResult); }); test('should print load/save storageState', async ({ browserName, channel, runCLI }, testInfo) => { @@ -179,7 +174,6 @@ test('should print load/save storageState', async ({ browserName, channel, runCL test('should work with --save-har', async ({ runCLI }, testInfo) => { const harFileName = testInfo.outputPath('har.har'); - const cli = runCLI(['--target=csharp', `--save-har=${harFileName}`]); const expectedResult = ` var context = await browser.NewContextAsync(new BrowserNewContextOptions { @@ -187,9 +181,10 @@ test('should work with --save-har', async ({ runCLI }, testInfo) => { RecordHarPath = ${JSON.stringify(harFileName)}, ServiceWorkers = ServiceWorkerPolicy.Block, });`; - await cli.waitFor(expectedResult).catch(e => e); - expect(cli.text()).toContain(expectedResult); - await cli.exited; + const cli = runCLI(['--target=csharp', `--save-har=${harFileName}`], { + autoExitWhen: expectedResult, + }); + await cli.waitForCleanExit(); const json = JSON.parse(fs.readFileSync(harFileName, 'utf-8')); expect(json.log.creator.name).toBe('Playwright'); }); diff --git a/tests/library/inspector/cli-codegen-java.spec.ts b/tests/library/inspector/cli-codegen-java.spec.ts index d374979839..738ec37cf9 100644 --- a/tests/library/inspector/cli-codegen-java.spec.ts +++ b/tests/library/inspector/cli-codegen-java.spec.ts @@ -37,7 +37,6 @@ public class Example { ${launchOptions(channel)}); BrowserContext context = browser.newContext();`; await cli.waitFor(expectedResult); - expect(cli.text()).toContain(expectedResult); }); test('should print the correct context options for custom settings', async ({ runCLI, browserName }) => { @@ -45,7 +44,6 @@ test('should print the correct context options for custom settings', async ({ ru const expectedResult = `BrowserContext context = browser.newContext(new Browser.NewContextOptions() .setColorScheme(ColorScheme.LIGHT));`; await cli.waitFor(expectedResult); - expect(cli.text()).toContain(expectedResult); }); test('should print the correct context options when using a device', async ({ browserName, runCLI }) => { @@ -93,14 +91,15 @@ test('should print load/save storage_state', async ({ runCLI, browserName }, tes test('should work with --save-har', async ({ runCLI }, testInfo) => { const harFileName = testInfo.outputPath('har.har'); - const cli = runCLI(['--target=java', `--save-har=${harFileName}`]); const expectedResult = `BrowserContext context = browser.newContext(new Browser.NewContextOptions() .setRecordHarMode(HarMode.MINIMAL) .setRecordHarPath(Paths.get(${JSON.stringify(harFileName)})) .setServiceWorkers(ServiceWorkerPolicy.BLOCK));`; - await cli.waitFor(expectedResult).catch(e => e); - expect(cli.text()).toContain(expectedResult); - await cli.exited; + const cli = runCLI(['--target=java', `--save-har=${harFileName}`], { + autoExitWhen: expectedResult, + }); + + await cli.waitForCleanExit(); const json = JSON.parse(fs.readFileSync(harFileName, 'utf-8')); expect(json.log.creator.name).toBe('Playwright'); }); diff --git a/tests/library/inspector/cli-codegen-javascript.spec.ts b/tests/library/inspector/cli-codegen-javascript.spec.ts index 0340f74bfc..f2c34899d8 100644 --- a/tests/library/inspector/cli-codegen-javascript.spec.ts +++ b/tests/library/inspector/cli-codegen-javascript.spec.ts @@ -34,7 +34,6 @@ test('should print the correct imports and context options', async ({ browserNam }); const context = await browser.newContext();`; await cli.waitFor(expectedResult); - expect(cli.text()).toContain(expectedResult); }); test('should print the correct context options for custom settings', async ({ browserName, channel, runCLI }) => { @@ -49,7 +48,6 @@ test('should print the correct context options for custom settings', async ({ br colorScheme: 'light' });`; await cli.waitFor(expectedResult); - expect(cli.text()).toContain(expectedResult); }); @@ -67,7 +65,6 @@ test('should print the correct context options when using a device', async ({ br ...devices['Pixel 2'], });`; await cli.waitFor(expectedResult); - expect(cli.text()).toContain(expectedResult); }); test('should print the correct context options when using a device and additional options', async ({ browserName, channel, runCLI }) => { @@ -85,13 +82,14 @@ test('should print the correct context options when using a device and additiona colorScheme: 'light' });`; await cli.waitFor(expectedResult); - expect(cli.text()).toContain(expectedResult); }); test('should save the codegen output to a file if specified', async ({ browserName, channel, runCLI }, testInfo) => { const tmpFile = testInfo.outputPath('script.js'); - const cli = runCLI(['--output', tmpFile, '--target=javascript', emptyHTML]); - await cli.exited; + const cli = runCLI(['--output', tmpFile, '--target=javascript', emptyHTML], { + autoExitWhen: 'await page.goto', // We have to wait for the initial navigation to be recorded. + }); + await cli.waitForCleanExit(); const content = fs.readFileSync(tmpFile); expect(content.toString()).toBe(`const { ${browserName} } = require('playwright'); diff --git a/tests/library/inspector/cli-codegen-pytest.spec.ts b/tests/library/inspector/cli-codegen-pytest.spec.ts index 968a2d4676..42a1eefa2e 100644 --- a/tests/library/inspector/cli-codegen-pytest.spec.ts +++ b/tests/library/inspector/cli-codegen-pytest.spec.ts @@ -27,15 +27,16 @@ test('should print the correct imports and context options', async ({ runCLI }) def test_example(page: Page) -> None:`; await cli.waitFor(expectedResult); - expect(cli.text()).toContain(expectedResult); }); test('should print the correct context options when using a device and lang', async ({ browserName, runCLI }, testInfo) => { test.skip(browserName !== 'webkit'); const tmpFile = testInfo.outputPath('script.js'); - const cli = runCLI(['--target=python-pytest', '--device=iPhone 11', '--lang=en-US', '--output', tmpFile, emptyHTML]); - await cli.exited; + const cli = runCLI(['--target=python-pytest', '--device=iPhone 11', '--lang=en-US', '--output', tmpFile, emptyHTML], { + autoExitWhen: 'page.goto', + }); + await cli.waitForCleanExit(); const content = fs.readFileSync(tmpFile); expect(content.toString()).toBe(`import pytest @@ -54,8 +55,10 @@ def test_example(page: Page) -> None: test('should save the codegen output to a file if specified', async ({ runCLI }, testInfo) => { const tmpFile = testInfo.outputPath('test_example.py'); - const cli = runCLI(['--target=python-pytest', '--output', tmpFile, emptyHTML]); - await cli.exited; + const cli = runCLI(['--target=python-pytest', '--output', tmpFile, emptyHTML], { + autoExitWhen: 'page.goto', + }); + await cli.waitForCleanExit(); const content = fs.readFileSync(tmpFile); expect(content.toString()).toBe(`from playwright.sync_api import Page, expect diff --git a/tests/library/inspector/cli-codegen-python-async.spec.ts b/tests/library/inspector/cli-codegen-python-async.spec.ts index 05daf4d5ee..d765d55d9c 100644 --- a/tests/library/inspector/cli-codegen-python-async.spec.ts +++ b/tests/library/inspector/cli-codegen-python-async.spec.ts @@ -34,7 +34,6 @@ async def run(playwright: Playwright) -> None: browser = await playwright.${browserName}.launch(${launchOptions(channel)}) context = await browser.new_context()`; await cli.waitFor(expectedResult); - expect(cli.text()).toContain(expectedResult); }); test('should print the correct context options for custom settings', async ({ browserName, channel, runCLI }) => { @@ -48,7 +47,6 @@ async def run(playwright: Playwright) -> None: browser = await playwright.${browserName}.launch(${launchOptions(channel)}) context = await browser.new_context(color_scheme="light")`; await cli.waitFor(expectedResult); - expect(cli.text()).toContain(expectedResult); }); test('should print the correct context options when using a device', async ({ browserName, channel, runCLI }) => { @@ -64,7 +62,6 @@ async def run(playwright: Playwright) -> None: browser = await playwright.chromium.launch(${launchOptions(channel)}) context = await browser.new_context(**playwright.devices["Pixel 2"])`; await cli.waitFor(expectedResult); - expect(cli.text()).toContain(expectedResult); }); test('should print the correct context options when using a device and additional options', async ({ browserName, channel, runCLI }) => { @@ -80,13 +77,14 @@ async def run(playwright: Playwright) -> None: browser = await playwright.webkit.launch(${launchOptions(channel)}) context = await browser.new_context(**playwright.devices["iPhone 11"], color_scheme="light")`; await cli.waitFor(expectedResult); - expect(cli.text()).toContain(expectedResult); }); test('should save the codegen output to a file if specified', async ({ browserName, channel, runCLI }, testInfo) => { const tmpFile = testInfo.outputPath('example.py'); - const cli = runCLI(['--target=python-async', '--output', tmpFile, emptyHTML]); - await cli.exited; + const cli = runCLI(['--target=python-async', '--output', tmpFile, emptyHTML], { + autoExitWhen: 'page.goto', + }); + await cli.waitForCleanExit(); const content = fs.readFileSync(tmpFile); expect(content.toString()).toBe(`import asyncio @@ -148,11 +146,11 @@ asyncio.run(main()) test('should work with --save-har', async ({ runCLI }, testInfo) => { const harFileName = testInfo.outputPath('har.har'); - const cli = runCLI(['--target=python-async', `--save-har=${harFileName}`]); const expectedResult = `context = await browser.new_context(record_har_mode="minimal", record_har_path=${JSON.stringify(harFileName)}, service_workers="block")`; - await cli.waitFor(expectedResult).catch(e => e); - expect(cli.text()).toContain(expectedResult); - await cli.exited; + const cli = runCLI(['--target=python-async', `--save-har=${harFileName}`], { + autoExitWhen: expectedResult, + }); + await cli.waitForCleanExit(); const json = JSON.parse(fs.readFileSync(harFileName, 'utf-8')); expect(json.log.creator.name).toBe('Playwright'); }); diff --git a/tests/library/inspector/cli-codegen-python.spec.ts b/tests/library/inspector/cli-codegen-python.spec.ts index 5b0077468b..5daa340a28 100644 --- a/tests/library/inspector/cli-codegen-python.spec.ts +++ b/tests/library/inspector/cli-codegen-python.spec.ts @@ -32,7 +32,6 @@ def run(playwright: Playwright) -> None: browser = playwright.${browserName}.launch(${launchOptions(channel)}) context = browser.new_context()`; await cli.waitFor(expectedResult); - expect(cli.text()).toContain(expectedResult); }); test('should print the correct context options for custom settings', async ({ runCLI, channel, browserName }) => { @@ -44,7 +43,6 @@ def run(playwright: Playwright) -> None: browser = playwright.${browserName}.launch(${launchOptions(channel)}) context = browser.new_context(color_scheme="light")`; await cli.waitFor(expectedResult); - expect(cli.text()).toContain(expectedResult); }); test('should print the correct context options when using a device', async ({ browserName, channel, runCLI }) => { @@ -58,7 +56,6 @@ def run(playwright: Playwright) -> None: browser = playwright.chromium.launch(${launchOptions(channel)}) context = browser.new_context(**playwright.devices["Pixel 2"])`; await cli.waitFor(expectedResult); - expect(cli.text()).toContain(expectedResult); }); test('should print the correct context options when using a device and additional options', async ({ browserName, channel, runCLI }) => { @@ -72,13 +69,14 @@ def run(playwright: Playwright) -> None: browser = playwright.webkit.launch(${launchOptions(channel)}) context = browser.new_context(**playwright.devices["iPhone 11"], color_scheme="light")`; await cli.waitFor(expectedResult); - expect(cli.text()).toContain(expectedResult); }); test('should save the codegen output to a file if specified', async ({ runCLI, channel, browserName }, testInfo) => { const tmpFile = testInfo.outputPath('example.py'); - const cli = runCLI(['--target=python', '--output', tmpFile, emptyHTML]); - await cli.exited; + const cli = runCLI(['--target=python', '--output', tmpFile, emptyHTML], { + autoExitWhen: 'page.goto', + }); + await cli.waitForCleanExit(); const content = fs.readFileSync(tmpFile); expect(content.toString()).toBe(`from playwright.sync_api import Playwright, sync_playwright, expect diff --git a/tests/library/inspector/cli-codegen-test.spec.ts b/tests/library/inspector/cli-codegen-test.spec.ts index 9066469dd7..a5d5cb6fce 100644 --- a/tests/library/inspector/cli-codegen-test.spec.ts +++ b/tests/library/inspector/cli-codegen-test.spec.ts @@ -27,7 +27,6 @@ test('should print the correct imports and context options', async ({ runCLI }) test('test', async ({ page }) => { });`; await cli.waitFor(expectedResult); - expect(cli.text()).toContain(expectedResult); }); test('should print the correct context options for custom settings', async ({ browserName, channel, runCLI }) => { @@ -40,7 +39,6 @@ test.use({ test('test', async ({ page }) => {`; await cli.waitFor(expectedResult); - expect(cli.text()).toContain(expectedResult); }); @@ -56,7 +54,6 @@ test.use({ test('test', async ({ page }) => {`; await cli.waitFor(expectedResult); - expect(cli.text()).toContain(expectedResult); }); test('should print the correct context options when using a device and additional options', async ({ browserName, channel, runCLI }) => { @@ -72,7 +69,6 @@ test.use({ test('test', async ({ page }) => {`; await cli.waitFor(expectedResult); - expect(cli.text()).toContain(expectedResult); }); test('should print load storageState', async ({ browserName, channel, runCLI }, testInfo) => { @@ -86,21 +82,20 @@ test.use({ }); test('test', async ({ page }) => {`; - await cli.waitFor(expectedResult); }); test('should work with --save-har', async ({ runCLI }, testInfo) => { const harFileName = testInfo.outputPath('har.har'); - const cli = runCLI(['--target=playwright-test', `--save-har=${harFileName}`]); const expectedResult = ` recordHar: { mode: 'minimal', path: '${harFileName.replace(/\\/g, '\\\\')}' }`; - await cli.waitFor(expectedResult).catch(e => e); - expect(cli.text()).toContain(expectedResult); - await cli.exited; + const cli = runCLI(['--target=playwright-test', `--save-har=${harFileName}`], { + autoExitWhen: expectedResult, + }); + await cli.waitForCleanExit(); const json = JSON.parse(fs.readFileSync(harFileName, 'utf-8')); expect(json.log.creator.name).toBe('Playwright'); }); diff --git a/tests/library/inspector/inspectorTest.ts b/tests/library/inspector/inspectorTest.ts index 30e1c65432..58f06fac77 100644 --- a/tests/library/inspector/inspectorTest.ts +++ b/tests/library/inspector/inspectorTest.ts @@ -16,9 +16,11 @@ import { contextTest } from '../../config/browserTest'; import type { Page } from 'playwright-core'; +import { step } from '../../config/baseTest'; import * as path from 'path'; import type { Source } from '../../../packages/recorder/src/recorderTypes'; import type { CommonFixtures, TestChildProcess } from '../../config/commonFixtures'; +import { stripAnsi } from '../../config/utils'; import { expect } from '@playwright/test'; export { expect } from '@playwright/test'; @@ -26,7 +28,7 @@ type CLITestArgs = { recorderPageGetter: () => Promise; closeRecorder: () => Promise; openRecorder: () => Promise; - runCLI: (args: string[], options?: { noAutoExit?: boolean }) => CLIMock; + runCLI: (args: string[], options?: { autoExitWhen?: string }) => CLIMock; }; const codegenLang2Id: Map = new Map([ @@ -68,13 +70,9 @@ export const test = contextTest.extend({ process.env.PWTEST_RECORDER_PORT = String(10907 + testInfo.workerIndex); testInfo.skip(mode === 'service'); - let cli: CLIMock | undefined; - await run((cliArgs, { noAutoExit } = {}) => { - cli = new CLIMock(childProcess, browserName, channel, headless, cliArgs, launchOptions.executablePath, noAutoExit); - return cli; + await run((cliArgs, { autoExitWhen } = {}) => { + return new CLIMock(childProcess, browserName, channel, headless, cliArgs, launchOptions.executablePath, autoExitWhen); }); - // Discard any exit error and let childProcess fixture report leaking processes (processwes which do not exit). - cli?.exited.catch(() => {}); }, openRecorder: async ({ page, recorderPageGetter }, run) => { @@ -135,7 +133,7 @@ class Recorder { const w = window as any; const source = (w.playwrightSourcesEchoForTest || []).find((s: Source) => s.id === params.languageId); return source && source.text.includes(params.text) ? w.playwrightSourcesEchoForTest : null; - }, { text, languageId: codegenLang2Id.get(file) }, { timeout: 8000, polling: 300 }); + }, { text, languageId: codegenLang2Id.get(file) }, { timeout: 0, polling: 300 }); const sources: Source[] = await handle.jsonValue(); for (const source of sources) { if (!codegenLangId2lang.has(source.id)) @@ -199,11 +197,8 @@ class Recorder { class CLIMock { process: TestChildProcess; - private waitForText: string; - private waitForCallback: () => void; - exited: Promise; - constructor(childProcess: CommonFixtures['childProcess'], browserName: string, channel: string | undefined, headless: boolean | undefined, args: string[], executablePath: string | undefined, noAutoExit: boolean | undefined) { + constructor(childProcess: CommonFixtures['childProcess'], browserName: string, channel: string | undefined, headless: boolean | undefined, args: string[], executablePath: string | undefined, autoExitWhen: string | undefined) { const nodeArgs = [ 'node', path.join(__dirname, '..', '..', '..', 'packages', 'playwright-core', 'lib', 'cli', 'cli.js'), @@ -216,47 +211,28 @@ class CLIMock { this.process = childProcess({ command: nodeArgs, env: { + PWTEST_CLI_AUTO_EXIT_WHEN: autoExitWhen, PWTEST_CLI_IS_UNDER_TEST: '1', - PWTEST_CLI_EXIT: !noAutoExit ? '1' : undefined, PWTEST_CLI_HEADLESS: headless ? '1' : undefined, PWTEST_CLI_EXECUTABLE_PATH: executablePath, DEBUG: (process.env.DEBUG ?? '') + ',pw:browser*', }, }); - this.process.onOutput = () => { - if (this.waitForCallback && this.process.output.includes(this.waitForText)) - this.waitForCallback(); - }; - this.exited = this.process.cleanExit(); } - async waitFor(text: string, timeout = 10_000): Promise { - if (this.process.output.includes(text)) - return Promise.resolve(); - this.waitForText = text; - return new Promise((f, r) => { - this.waitForCallback = f; - if (timeout) { - setTimeout(() => { - r(new Error('Timed out waiting for text:\n' + text + '\n\nReceived:\n' + this.text())); - }, timeout); - } - }); + @step + async waitFor(text: string): Promise { + await expect(() => { + expect(this.text()).toContain(text); + }).toPass(); + } + + @step + async waitForCleanExit() { + return this.process.cleanExit(); } text() { - return removeAnsiColors(this.process.output); + return stripAnsi(this.process.output); } - - exit(signal: NodeJS.Signals | number) { - this.process.process.kill(signal); - } -} - -function removeAnsiColors(input: string): string { - const pattern = [ - '[\\u001B\\u009B][[\\]()#;?]*(?:(?:(?:[a-zA-Z\\d]*(?:;[-a-zA-Z\\d\\/#&.:=?%@~_]*)*)?\\u0007)', - '(?:(?:\\d{1,4}(?:;\\d{0,4})*)?[\\dA-PR-TZcf-ntqry=><~]))' - ].join('|'); - return input.replace(new RegExp(pattern, 'g'), ''); -} +} \ No newline at end of file diff --git a/tests/playwright-test/ui-mode-fixtures.ts b/tests/playwright-test/ui-mode-fixtures.ts index 37b5f39bbb..816eeee2c4 100644 --- a/tests/playwright-test/ui-mode-fixtures.ts +++ b/tests/playwright-test/ui-mode-fixtures.ts @@ -109,7 +109,7 @@ export const test = base return { page, testProcess }; }); await browser?.close(); - await testProcess?.close(); + await testProcess?.kill('SIGINT'); await removeFolderAsync(cacheDir); }, createLatch: async ({}, use, testInfo) => { diff --git a/tests/webview2/webView2Test.ts b/tests/webview2/webView2Test.ts index fb4685cef4..38a6c4ca1e 100644 --- a/tests/webview2/webView2Test.ts +++ b/tests/webview2/webView2Test.ts @@ -50,6 +50,6 @@ export const webView2Test = baseTest.extend(traceViewerFixt const browser = await playwright.chromium.connectOverCDP(`http://127.0.0.1:${cdpPort}`); await use(browser); await browser.close(); - await spawnedProcess.close(); + await spawnedProcess.kill('SIGINT'); }, { scope: 'worker' }], });