From 018467911b27108dc999685d72ed7317f7274a09 Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Tue, 21 Sep 2021 16:24:48 -0700 Subject: [PATCH] test: introduce some common test fixtures (#9060) --- tests/config/baseTest.ts | 3 +- tests/config/browserTest.ts | 7 +- tests/config/commonFixtures.ts | 138 ++++++++++++++++++ tests/config/remoteServer.ts | 51 +++---- tests/inspector/inspectorTest.ts | 42 ++---- .../playwright-test-fixtures.ts | 135 ++--------------- 6 files changed, 192 insertions(+), 184 deletions(-) create mode 100644 tests/config/commonFixtures.ts diff --git a/tests/config/baseTest.ts b/tests/config/baseTest.ts index 062ba168dd..ff8d9ed665 100644 --- a/tests/config/baseTest.ts +++ b/tests/config/baseTest.ts @@ -25,6 +25,7 @@ import { start } from '../../lib/outofprocess'; import { PlaywrightClient } from '../../lib/remote/playwrightClient'; import type { LaunchOptions } from '../../index'; import { TestProxy } from './proxy'; +import { commonFixtures, CommonFixtures } from './commonFixtures'; export type BrowserName = 'chromium' | 'firefox' | 'webkit'; type Mode = 'default' | 'driver' | 'service'; @@ -242,4 +243,4 @@ const coverageFixtures: Fixtures<{}, CoverageOptions & { __collectCoverage: void export type CommonOptions = BaseOptions & ServerOptions & CoverageOptions; export type CommonWorkerFixtures = CommonOptions & BaseFixtures; -export const baseTest = _baseTest.extend<{}, CoverageOptions>(coverageFixtures).extend(serverFixtures).extend<{}, BaseOptions & BaseFixtures>(baseFixtures); +export const baseTest = _baseTest.extend(commonFixtures).extend<{}, CoverageOptions>(coverageFixtures).extend(serverFixtures as any).extend<{}, BaseOptions & BaseFixtures>(baseFixtures); diff --git a/tests/config/browserTest.ts b/tests/config/browserTest.ts index d0972c313f..b66250f114 100644 --- a/tests/config/browserTest.ts +++ b/tests/config/browserTest.ts @@ -22,6 +22,7 @@ import * as fs from 'fs'; import * as os from 'os'; import { RemoteServer, RemoteServerOptions } from './remoteServer'; import { baseTest, CommonWorkerFixtures } from './baseTest'; +import { CommonFixtures } from './commonFixtures'; type PlaywrightWorkerOptions = { executablePath: LaunchOptions['executablePath']; @@ -48,7 +49,7 @@ type PlaywrightTestFixtures = { }; export type PlaywrightOptions = PlaywrightWorkerOptions & PlaywrightTestOptions; -export const playwrightFixtures: Fixtures = { +export const playwrightFixtures: Fixtures = { executablePath: [ undefined, { scope: 'worker' } ], proxy: [ undefined, { scope: 'worker' } ], args: [ undefined, { scope: 'worker' } ], @@ -109,13 +110,13 @@ export const playwrightFixtures: Fixtures { + startRemoteServer: async ({ childProcess, browserType, browserOptions }, run) => { let remoteServer: RemoteServer | undefined; await run(async options => { if (remoteServer) throw new Error('can only start one remote server'); remoteServer = new RemoteServer(); - await remoteServer._start(browserType, browserOptions, options); + await remoteServer._start(childProcess, browserType, browserOptions, options); return remoteServer; }); if (remoteServer) diff --git a/tests/config/commonFixtures.ts b/tests/config/commonFixtures.ts new file mode 100644 index 0000000000..591847e62c --- /dev/null +++ b/tests/config/commonFixtures.ts @@ -0,0 +1,138 @@ +/** + * Copyright (c) Microsoft Corporation. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import type { Fixtures } from './test-runner'; +import { spawn, ChildProcess, execSync } from 'child_process'; +import net from 'net'; + +type TestChildParams = { + command: string[], + cwd?: string, + env?: { [key: string]: string | number | boolean | undefined }, + shell?: boolean, + onOutput?: () => void; +}; + +export class TestChildProcess { + params: TestChildParams; + process: ChildProcess; + output = ''; + onOutput?: () => void; + exited: Promise<{ exitCode: number | null, signal: string | null }>; + exitCode: Promise; + + constructor(params: TestChildParams) { + this.params = params; + this.process = spawn(params.command[0], params.command.slice(1), { + env: { + ...process.env, + ...params.env, + } as any, + cwd: params.cwd, + shell: params.shell, + }); + if (process.env.PWTEST_DEBUG) + process.stdout.write(`\n\nLaunching ${params.command.join(' ')}\n`); + this.onOutput = params.onOutput; + + const appendChunk = (chunk: string | Buffer) => { + this.output += String(chunk); + if (process.env.PWTEST_DEBUG) + process.stdout.write(String(chunk)); + this.onOutput?.(); + }; + + this.process.stderr.on('data', appendChunk); + this.process.stdout.on('data', appendChunk); + + const onExit = () => { + if (!this.process.pid || this.process.killed) + return; + try { + if (process.platform === 'win32') + execSync(`taskkill /pid ${this.process.pid} /T /F /FI "MEMUSAGE gt 0"`); + else + process.kill(-this.process.pid, 'SIGKILL'); + } catch (e) { + // the process might have already stopped + } + }; + process.on('exit', onExit); + this.exited = new Promise(f => { + this.process.on('exit', (exitCode, signal) => f({ exitCode, signal })); + process.off('exit', onExit); + }); + this.exitCode = this.exited.then(r => r.exitCode); + } + + async close() { + if (!this.process.killed) + this.process.kill(); + return this.exited; + } + + async cleanExit() { + const r = await this.exited; + if (r.exitCode) + throw new Error(`Process failed with exit code ${r.exitCode}`); + if (r.signal) + throw new Error(`Process recieved signal: ${r.signal}`); + } +} + +export type CommonFixtures = { + childProcess: (params: TestChildParams) => TestChildProcess; + waitForPort: (port: number) => Promise; +}; + +export const commonFixtures: Fixtures = { + childProcess: async ({}, use, testInfo) => { + const processes: TestChildProcess[] = []; + await use(params => { + const process = new TestChildProcess(params); + processes.push(process); + return process; + }); + await Promise.all(processes.map(child => child.close())); + if (testInfo.status !== 'passed' && !process.env.PW_RUNNER_DEBUG) { + for (const process of processes) { + console.log('====== ' + process.params.command.join(' ')); + console.log(process.output); + console.log('========================================='); + } + } + }, + + waitForPort: async ({}, use) => { + const token = { canceled: false }; + await use(async port => { + while (!token.canceled) { + const promise = new Promise(resolve => { + const conn = net.connect(port) + .on('error', () => resolve(false)) + .on('connect', () => { + conn.end(); + resolve(true); + }); + }); + if (await promise) + return; + await new Promise(x => setTimeout(x, 100)); + } + }); + token.canceled = true; + }, +}; diff --git a/tests/config/remoteServer.ts b/tests/config/remoteServer.ts index 89495e55d3..cc2f677f39 100644 --- a/tests/config/remoteServer.ts +++ b/tests/config/remoteServer.ts @@ -15,8 +15,8 @@ */ import path from 'path'; -import { spawn } from 'child_process'; import type { BrowserType, Browser, LaunchOptions } from '../../index'; +import { CommonFixtures, TestChildProcess } from './commonFixtures'; const playwrightPath = path.join(__dirname, '..', '..'); @@ -28,20 +28,17 @@ export type RemoteServerOptions = { }; export class RemoteServer { - _output: Map; - _outputCallback: Map; + private _process: TestChildProcess; + _output: Map; + _outputCallback: Map void>; _browserType: BrowserType; - _child: import('child_process').ChildProcess; - _exitPromise: Promise; _exitAndDisconnectPromise: Promise; _browser: Browser; - _didExit: boolean; _wsEndpoint: string; - async _start(browserType: BrowserType, browserOptions: LaunchOptions, remoteServerOptions: RemoteServerOptions = {}) { + async _start(childProcess: CommonFixtures['childProcess'], browserType: BrowserType, browserOptions: LaunchOptions, remoteServerOptions: RemoteServerOptions = {}) { this._output = new Map(); this._outputCallback = new Map(); - this._didExit = false; this._browserType = browserType; // Copy options to prevent a large JSON string when launching subprocess. @@ -62,29 +59,20 @@ export class RemoteServer { launchOptions, ...remoteServerOptions, }; - this._child = spawn('node', [path.join(__dirname, 'remote-server-impl.js'), JSON.stringify(options)], { env: process.env }); - this._child.on('error', (...args) => console.log('ERROR', ...args)); - this._exitPromise = new Promise(resolve => this._child.on('exit', (exitCode, signal) => { - this._didExit = true; - resolve(exitCode); - })); + this._process = childProcess({ + command: ['node', path.join(__dirname, 'remote-server-impl.js'), JSON.stringify(options)], + }); - let outputString = ''; - this._child.stdout.on('data', data => { - outputString += data.toString(); - // Uncomment to debug. - // console.log(data.toString()); + let index = 0; + this._process.onOutput = () => { let match; - while ((match = outputString.match(/\(([^()]+)=>([^()]+)\)/))) { + while ((match = this._process.output.substring(index).match(/\(([^()]+)=>([^()]+)\)/))) { const key = match[1]; const value = match[2]; this._addOutput(key, value); - outputString = outputString.substring(match.index + match[0].length); + index += match.index + match[0].length; } - }); - this._child.stderr.on('data', data => { - console.log(data.toString()); - }); + }; this._wsEndpoint = await this.out('wsEndpoint'); @@ -95,7 +83,7 @@ export class RemoteServer { } } - _addOutput(key, value) { + _addOutput(key: string, value: string) { this._output.set(key, value); const cb = this._outputCallback.get(key); this._outputCallback.delete(key); @@ -103,9 +91,9 @@ export class RemoteServer { cb(); } - async out(key) { + async out(key: string) { if (!this._output.has(key)) - await new Promise(f => this._outputCallback.set(key, f)); + await new Promise(f => this._outputCallback.set(key, f)); return this._output.get(key); } @@ -114,11 +102,11 @@ export class RemoteServer { } child() { - return this._child; + return this._process.process; } async childExitCode() { - return await this._exitPromise; + return await this._process.exitCode; } async close() { @@ -126,8 +114,7 @@ export class RemoteServer { await this._browser.close(); this._browser = undefined; } - if (!this._didExit) - this._child.kill(); + await this._process.close(); return await this.childExitCode(); } } diff --git a/tests/inspector/inspectorTest.ts b/tests/inspector/inspectorTest.ts index 683cc467e5..436906bb25 100644 --- a/tests/inspector/inspectorTest.ts +++ b/tests/inspector/inspectorTest.ts @@ -18,8 +18,8 @@ import { contextTest } from '../config/browserTest'; import type { Page } from '../../index'; import * as path from 'path'; import type { Source } from '../../src/server/supplements/recorder/recorderTypes'; -import { ChildProcess, spawn } from 'child_process'; import { chromium } from '../../index'; +import { CommonFixtures, TestChildProcess } from '../config/commonFixtures'; export { expect } from '../config/test-runner'; type CLITestArgs = { @@ -49,13 +49,13 @@ export const test = contextTest.extend({ }); }, - runCLI: async ({ browserName, channel, headless, mode, executablePath }, run, testInfo) => { + runCLI: async ({ childProcess, browserName, channel, headless, mode, executablePath }, run, testInfo) => { process.env.PWTEST_RECORDER_PORT = String(10907 + testInfo.workerIndex); testInfo.skip(mode === 'service'); let cli: CLIMock | undefined; await run(cliArgs => { - cli = new CLIMock(browserName, channel, headless, cliArgs, executablePath); + cli = new CLIMock(childProcess, browserName, channel, headless, cliArgs, executablePath); return cli; }); if (cli) @@ -177,15 +177,14 @@ class Recorder { } class CLIMock { - private process: ChildProcess; - private data: string; + private process: TestChildProcess; private waitForText: string; private waitForCallback: () => void; exited: Promise; - constructor(browserName: string, channel: string | undefined, headless: boolean | undefined, args: string[], executablePath: string | undefined) { - this.data = ''; + constructor(childProcess: CommonFixtures['childProcess'], browserName: string, channel: string | undefined, headless: boolean | undefined, args: string[], executablePath: string | undefined) { const nodeArgs = [ + 'node', path.join(__dirname, '..', '..', 'lib', 'cli', 'cli.js'), 'codegen', ...args, @@ -193,36 +192,23 @@ class CLIMock { ]; if (channel) nodeArgs.push(`--channel=${channel}`); - this.process = spawn('node', nodeArgs, { + this.process = childProcess({ + command: nodeArgs, env: { - ...process.env, PWTEST_CLI_EXIT: '1', PWTEST_CLI_HEADLESS: headless ? '1' : undefined, PWTEST_CLI_EXECUTABLE_PATH: executablePath, }, - stdio: 'pipe' }); - this.process.stdout.on('data', data => { - this.data = data.toString(); - if (this.waitForCallback && this.data.includes(this.waitForText)) + this.process.onOutput = () => { + if (this.waitForCallback && this.process.output.includes(this.waitForText)) this.waitForCallback(); - }); - this.exited = new Promise((f, r) => { - this.process.stderr.on('data', data => { - console.error(data.toString()); - }); - this.process.on('exit', (exitCode, signal) => { - if (exitCode) - r(new Error(`Process failed with exit code ${exitCode}`)); - if (signal) - r(new Error(`Process recieved signal: ${signal}`)); - f(); - }); - }); + }; + this.exited = this.process.cleanExit(); } async waitFor(text: string, timeout = 10_000): Promise { - if (this.data.includes(text)) + if (this.process.output.includes(text)) return Promise.resolve(); this.waitForText = text; return new Promise((f, r) => { @@ -236,7 +222,7 @@ class CLIMock { } text() { - return removeAnsiColors(this.data); + return removeAnsiColors(this.process.output); } } diff --git a/tests/playwright-test/playwright-test-fixtures.ts b/tests/playwright-test/playwright-test-fixtures.ts index a81f053019..980a2405b1 100644 --- a/tests/playwright-test/playwright-test-fixtures.ts +++ b/tests/playwright-test/playwright-test-fixtures.ts @@ -15,7 +15,7 @@ */ import { TestInfo, test as base } from './stable-test-runner'; -import { spawn, ChildProcess, execSync } from 'child_process'; +import { CommonFixtures, commonFixtures } from '../config/commonFixtures'; import * as fs from 'fs'; import * as path from 'path'; import * as os from 'os'; @@ -23,7 +23,6 @@ import type { JSONReport, JSONReportSuite } from '../../src/test/reporters/json' import rimraf from 'rimraf'; import { promisify } from 'util'; import * as url from 'url'; -import net from 'net'; const removeFolderAsync = promisify(rimraf); @@ -47,76 +46,6 @@ type Files = { [key: string]: string | Buffer }; type Params = { [key: string]: string | number | boolean | string[] }; type Env = { [key: string]: string | number | boolean | undefined }; -type ChildParams = { - command: string[], - cwd?: string, - env?: Env, - shell?: boolean, - sendSIGINTAfter?: number, -}; - -class Child { - params: ChildParams; - process: ChildProcess; - output = ''; - exited: Promise; - - constructor(params: ChildParams) { - this.params = params; - this.process = spawn(params.command[0], params.command.slice(1), { - env: { - ...process.env, - ...params.env, - } as any, - cwd: params.cwd, - shell: params.shell, - }); - if (process.env.PW_RUNNER_DEBUG) - process.stdout.write(`\n\nLaunching ${params.command.join(' ')}\n`); - - this.process.stderr.on('data', chunk => { - this.output += String(chunk); - if (process.env.PW_RUNNER_DEBUG) - process.stdout.write(String(chunk)); - }); - - let didSendSigint = false; - this.process.stdout.on('data', chunk => { - this.output += String(chunk); - if (params.sendSIGINTAfter && !didSendSigint && countTimes(this.output, '%%SEND-SIGINT%%') >= params.sendSIGINTAfter) { - didSendSigint = true; - process.kill(this.process.pid, 'SIGINT'); - } - if (process.env.PW_RUNNER_DEBUG) - process.stdout.write(String(chunk)); - }); - - const onExit = () => { - if (!this.process.pid || this.process.killed) - return; - try { - if (process.platform === 'win32') - execSync(`taskkill /pid ${this.process.pid} /T /F /FI "MEMUSAGE gt 0"`); - else - process.kill(-this.process.pid, 'SIGKILL'); - } catch (e) { - // the process might have already stopped - } - }; - process.on('exit', onExit); - this.exited = new Promise(f => { - this.process.on('exit', (code, signal) => f(code)); - process.off('exit', onExit); - }); - } - - async close() { - if (!this.process.killed) - this.process.kill(); - return this.exited; - } -} - async function writeFiles(testInfo: TestInfo, files: Files) { const baseDir = testInfo.outputPath(); @@ -162,7 +91,7 @@ async function writeFiles(testInfo: TestInfo, files: Files) { return baseDir; } -async function runPlaywrightTest(childProcess: (params: ChildParams) => Promise, baseDir: string, params: any, env: Env, options: RunOptions): Promise { +async function runPlaywrightTest(childProcess: CommonFixtures['childProcess'], baseDir: string, params: any, env: Env, options: RunOptions): Promise { const paramList = []; for (const key of Object.keys(params)) { for (const value of Array.isArray(params[key]) ? params[key] : [params[key]]) { @@ -183,7 +112,7 @@ async function runPlaywrightTest(childProcess: (params: ChildParams) => Promise< if (options.additionalArgs) args.push(...options.additionalArgs); const cacheDir = fs.mkdtempSync(path.join(os.tmpdir(), 'playwright-test-cache-')); - const testProcess = await childProcess({ + const testProcess = childProcess({ command: args, env: { ...process.env, @@ -194,9 +123,15 @@ async function runPlaywrightTest(childProcess: (params: ChildParams) => Promise< ...env, }, cwd: baseDir, - sendSIGINTAfter: options.sendSIGINTAfter, }); - const exitCode = await testProcess.exited; + let didSendSigint = false; + testProcess.onOutput = () => { + if (options.sendSIGINTAfter && !didSendSigint && countTimes(testProcess.output, '%%SEND-SIGINT%%') >= options.sendSIGINTAfter) { + didSendSigint = true; + process.kill(testProcess.process.pid, 'SIGINT'); + } + }; + const { exitCode } = await testProcess.exited; await removeFolderAsync(cacheDir); const outputString = testProcess.output.toString(); @@ -256,11 +191,10 @@ type Fixtures = { writeFiles: (files: Files) => Promise; runInlineTest: (files: Files, params?: Params, env?: Env, options?: RunOptions) => Promise; runTSC: (files: Files) => Promise; - childProcess: (params: ChildParams) => Promise; - waitForPort: (port: number) => Promise; }; -export const test = base.extend({ +const common = base.extend(commonFixtures as any); +export const test = common.extend({ writeFiles: async ({}, use, testInfo) => { await use(files => writeFiles(testInfo, files)); }, @@ -275,54 +209,15 @@ export const test = base.extend({ runTSC: async ({ childProcess }, use, testInfo) => { await use(async files => { const baseDir = await writeFiles(testInfo, { 'tsconfig.json': JSON.stringify(TSCONFIG), ...files }); - const tsc = await childProcess({ + const tsc = childProcess({ command: ['npx', 'tsc', '-p', baseDir], cwd: baseDir, shell: true, }); - const exitCode = await tsc.exited; + const { exitCode } = await tsc.exited; return { exitCode, output: tsc.output }; }); }, - - childProcess: async ({}, use, testInfo) => { - const children: Child[] = []; - await use(async params => { - const child = new Child(params); - children.push(child); - return child; - }); - await Promise.all(children.map(child => child.close())); - if (testInfo.status !== 'passed' && !process.env.PW_RUNNER_DEBUG) { - for (const child of children) { - console.log('====== ' + child.params.command.join(' ')); - console.log(child.output); - console.log('========================================='); - } - } - }, - - waitForPort: async ({}, use) => { - const token = { canceled: false }; - await use(async port => { - await test.step(`waiting for port ${port}`, async () => { - while (!token.canceled) { - const promise = new Promise(resolve => { - const conn = net.connect(port) - .on('error', () => resolve(false)) - .on('connect', () => { - conn.end(); - resolve(true); - }); - }); - if (await promise) - return; - await new Promise(x => setTimeout(x, 100)); - } - }); - }); - token.canceled = true; - }, }); const TSCONFIG = {