From 7ec1035b983e5d4c428f1a693c101e03649eca0a Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Mon, 20 Sep 2021 17:17:12 -0700 Subject: [PATCH] test: improve child process utilities in tests (#9036) --- .../playwright-test-fixtures.ts | 210 ++++++++++++------ tests/playwright-test/raw-reporter.spec.ts | 25 +-- tests/playwright-test/test-ignore.spec.ts | 18 +- tests/playwright-test/test-output-dir.spec.ts | 2 +- 4 files changed, 154 insertions(+), 101 deletions(-) diff --git a/tests/playwright-test/playwright-test-fixtures.ts b/tests/playwright-test/playwright-test-fixtures.ts index 7654d2215c..a81f053019 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 } from 'child_process'; +import { spawn, ChildProcess, execSync } from 'child_process'; import * as fs from 'fs'; import * as path from 'path'; import * as os from 'os'; @@ -23,6 +23,7 @@ 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); @@ -46,6 +47,76 @@ 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(); @@ -91,39 +162,9 @@ async function writeFiles(testInfo: TestInfo, files: Files) { return baseDir; } -async function runTSC(baseDir: string): Promise { - const tscProcess = spawn('npx', ['tsc', '-p', baseDir], { - cwd: baseDir, - shell: true, - }); - let output = ''; - tscProcess.stderr.on('data', chunk => { - output += String(chunk); - if (process.env.PW_RUNNER_DEBUG) - process.stderr.write(String(chunk)); - }); - tscProcess.stdout.on('data', chunk => { - output += String(chunk); - if (process.env.PW_RUNNER_DEBUG) - process.stdout.write(String(chunk)); - }); - const status = await new Promise(x => tscProcess.on('close', x)); - return { - exitCode: status, - output, - }; -} - -async function runPlaywrightTest(baseDir: string, params: any, env: Env, options: RunOptions): Promise { +async function runPlaywrightTest(childProcess: (params: ChildParams) => Promise, baseDir: string, params: any, env: Env, options: RunOptions): Promise { const paramList = []; - let additionalArgs = ''; for (const key of Object.keys(params)) { - if (key === 'args') { - additionalArgs = params[key]; - continue; - } - if (key === 'usesCustomOutputDir') - continue; for (const value of Array.isArray(params[key]) ? params[key] : [params[key]]) { const k = key.startsWith('-') ? key : '--' + key; paramList.push(params[key] === true ? `${k}` : `${k}=${value}`); @@ -131,18 +172,19 @@ async function runPlaywrightTest(baseDir: string, params: any, env: Env, options } const outputDir = path.join(baseDir, 'test-results'); const reportFile = path.join(outputDir, 'report.json'); - const args = [path.join(__dirname, '..', '..', 'lib', 'cli', 'cli.js'), 'test']; - if (!params.usesCustomOutputDir) + const args = ['node', path.join(__dirname, '..', '..', 'lib', 'cli', 'cli.js'), 'test']; + if (!options.usesCustomOutputDir) args.push('--output=' + outputDir); args.push( '--reporter=dot,json', '--workers=2', ...paramList ); - if (additionalArgs) - args.push(...additionalArgs); + if (options.additionalArgs) + args.push(...options.additionalArgs); const cacheDir = fs.mkdtempSync(path.join(os.tmpdir(), 'playwright-test-cache-')); - const testProcess = spawn('node', args, { + const testProcess = await childProcess({ + command: args, env: { ...process.env, PLAYWRIGHT_JSON_OUTPUT_NAME: reportFile, @@ -151,28 +193,13 @@ async function runPlaywrightTest(baseDir: string, params: any, env: Env, options PWTEST_SKIP_TEST_OUTPUT: '1', ...env, }, - cwd: baseDir + cwd: baseDir, + sendSIGINTAfter: options.sendSIGINTAfter, }); - let output = ''; - let didSendSigint = false; - testProcess.stderr.on('data', chunk => { - output += String(chunk); - if (process.env.PW_RUNNER_DEBUG) - process.stderr.write(String(chunk)); - }); - testProcess.stdout.on('data', chunk => { - output += String(chunk); - if (options.sendSIGINTAfter && !didSendSigint && countTimes(output, '%%SEND-SIGINT%%') >= options.sendSIGINTAfter) { - didSendSigint = true; - process.kill(testProcess.pid, 'SIGINT'); - } - if (process.env.PW_RUNNER_DEBUG) - process.stdout.write(String(chunk)); - }); - const status = await new Promise(x => testProcess.on('close', x)); + const exitCode = await testProcess.exited; await removeFolderAsync(cacheDir); - const outputString = output.toString(); + const outputString = testProcess.output.toString(); const summary = (re: RegExp) => { let result = 0; let match = re.exec(outputString); @@ -190,7 +217,7 @@ async function runPlaywrightTest(baseDir: string, params: any, env: Env, options try { report = JSON.parse(fs.readFileSync(reportFile).toString()); } catch (e) { - output += '\n' + e.toString(); + testProcess.output += '\n' + e.toString(); } const results = []; @@ -209,8 +236,8 @@ async function runPlaywrightTest(baseDir: string, params: any, env: Env, options visitSuites(report.suites); return { - exitCode: status, - output, + exitCode, + output: testProcess.output, passed, failed, flaky, @@ -222,11 +249,15 @@ async function runPlaywrightTest(baseDir: string, params: any, env: Env, options type RunOptions = { sendSIGINTAfter?: number; + usesCustomOutputDir?: boolean; + additionalArgs?: string[]; }; 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({ @@ -234,26 +265,63 @@ export const test = base.extend({ await use(files => writeFiles(testInfo, files)); }, - runInlineTest: async ({}, use, testInfo: TestInfo) => { - let runResult: RunResult | undefined; + runInlineTest: async ({ childProcess }, use, testInfo: TestInfo) => { await use(async (files: Files, params: Params = {}, env: Env = {}, options: RunOptions = {}) => { const baseDir = await writeFiles(testInfo, files); - runResult = await runPlaywrightTest(baseDir, params, env, options); - return runResult; + return await runPlaywrightTest(childProcess, baseDir, params, env, options); }); - if (testInfo.status !== testInfo.expectedStatus && runResult && !process.env.PW_RUNNER_DEBUG) - console.log('\n' + runResult.output + '\n'); }, - runTSC: async ({}, use, testInfo) => { - let tscResult: TSCResult | undefined; + runTSC: async ({ childProcess }, use, testInfo) => { await use(async files => { const baseDir = await writeFiles(testInfo, { 'tsconfig.json': JSON.stringify(TSCONFIG), ...files }); - tscResult = await runTSC(baseDir); - return tscResult; + const tsc = await childProcess({ + command: ['npx', 'tsc', '-p', baseDir], + cwd: baseDir, + shell: true, + }); + const exitCode = await tsc.exited; + return { exitCode, output: tsc.output }; }); - if (testInfo.status !== testInfo.expectedStatus && tscResult && !process.env.PW_RUNNER_DEBUG) - console.log('\n' + tscResult.output + '\n'); + }, + + 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; }, }); diff --git a/tests/playwright-test/raw-reporter.spec.ts b/tests/playwright-test/raw-reporter.spec.ts index fdfce38e16..903eeafa2d 100644 --- a/tests/playwright-test/raw-reporter.spec.ts +++ b/tests/playwright-test/raw-reporter.spec.ts @@ -26,10 +26,7 @@ test('should generate raw report', async ({ runInlineTest }, testInfo) => { const { test } = pwt; test('passes', async ({ page }, testInfo) => {}); `, - }, { - usesCustomOutputDir: true, - reporter: 'dot,' + kRawReporterPath - }); + }, { reporter: 'dot,' + kRawReporterPath }, {}, { usesCustomOutputDir: true }); const json = JSON.parse(fs.readFileSync(testInfo.outputPath('test-results', 'report', 'project.report'), 'utf-8')); expect(json.config).toBeTruthy(); expect(json.project).toBeTruthy(); @@ -50,10 +47,7 @@ test('should use project name', async ({ runInlineTest }, testInfo) => { const { test } = pwt; test('passes', async ({ page }, testInfo) => {}); `, - }, { - usesCustomOutputDir: true, - reporter: 'dot,' + kRawReporterPath - }); + }, { reporter: 'dot,' + kRawReporterPath }, {}, { usesCustomOutputDir: true }); const json = JSON.parse(fs.readFileSync(testInfo.outputPath('output', 'report', 'project-name.report'), 'utf-8')); expect(json.project.name).toBe('project-name'); expect(result.exitCode).toBe(0); @@ -70,10 +64,7 @@ test('should save stdio', async ({ runInlineTest }, testInfo) => { process.stderr.write(Buffer.from([4, 5, 6])); }); `, - }, { - usesCustomOutputDir: true, - reporter: 'dot,' + kRawReporterPath - }); + }, { reporter: 'dot,' + kRawReporterPath }, {}, { usesCustomOutputDir: true }); const json = JSON.parse(fs.readFileSync(testInfo.outputPath('test-results', 'report', 'project.report'), 'utf-8')); const result = json.suites[0].tests[0].results[0]; expect(result.attachments).toEqual([ @@ -109,10 +100,7 @@ test('should save attachments', async ({ runInlineTest }, testInfo) => { }); }); `, - }, { - usesCustomOutputDir: true, - reporter: 'dot,' + kRawReporterPath - }); + }, { reporter: 'dot,' + kRawReporterPath }, {}, { usesCustomOutputDir: true }); const json = JSON.parse(fs.readFileSync(testInfo.outputPath('test-results', 'report', 'project.report'), 'utf-8')); const result = json.suites[0].tests[0].results[0]; expect(result.attachments[0].name).toBe('binary'); @@ -137,10 +125,7 @@ test('dupe project names', async ({ runInlineTest }, testInfo) => { const { test } = pwt; test('passes', async ({ page }, testInfo) => {}); `, - }, { - usesCustomOutputDir: true, - reporter: 'dot,' + kRawReporterPath - }); + }, { reporter: 'dot,' + kRawReporterPath }, {}, { usesCustomOutputDir: true }); const files = fs.readdirSync(testInfo.outputPath('test-results', 'report')); expect(new Set(files)).toEqual(new Set(['project-name.report', 'project-name-1.report', 'project-name-2.report'])); }); diff --git a/tests/playwright-test/test-ignore.spec.ts b/tests/playwright-test/test-ignore.spec.ts index b79d10ec14..3662592de2 100644 --- a/tests/playwright-test/test-ignore.spec.ts +++ b/tests/playwright-test/test-ignore.spec.ts @@ -185,7 +185,7 @@ test('should match cli string argument', async ({ runInlineTest }) => { const { test } = pwt; test('pass', ({}) => {}); ` - }, { args: [`dir\\${path.sep}a`] }); + }, {}, {}, { additionalArgs: [`dir\\${path.sep}a`] }); expect(result.passed).toBe(1); expect(result.report.suites.map(s => s.file).sort()).toEqual(['a.test.ts']); expect(result.exitCode).toBe(0); @@ -205,7 +205,7 @@ test('should match regex string argument', async ({ runInlineTest }) => { const { test } = pwt; test('pass', ({}) => {}); ` - }, { args: ['/filea.*ts/'] }); + }, {}, {}, { additionalArgs: ['/filea.*ts/'] }); expect(result.passed).toBe(2); expect(result.report.suites.map(s => s.file).sort()).toEqual(['dir/filea.test.ts', 'filea.test.ts']); expect(result.exitCode).toBe(0); @@ -222,7 +222,7 @@ test('should match regex string with a colon argument', async ({ runInlineTest } const { test } = pwt; test('pass', ({}) => {}); ` - }, { args: ['/weird:file\.test\.ts/'] }); + }, {}, {}, { additionalArgs: ['/weird:file\.test\.ts/'] }); expect(result.passed).toBe(1); expect(result.report.suites.map(s => s.file).sort()).toEqual(['weird:file.test.ts']); expect(result.exitCode).toBe(0); @@ -242,7 +242,7 @@ test('should match case insensitive', async ({ runInlineTest }) => { const { test } = pwt; test('pass', ({}) => {}); ` - }, { args: ['a.test.ts'] }); + }, {}, {}, { additionalArgs: ['a.test.ts'] }); expect(result.passed).toBe(2); expect(result.report.suites.map(s => s.file).sort()).toEqual(['capital/A.test.ts', 'lowercase/a.test.ts']); expect(result.exitCode).toBe(0); @@ -260,7 +260,7 @@ test('should focus a single test spec', async ({ runInlineTest }) => { const { test } = pwt; test('no-pass1', ({}) => {}); `, - }, { args: ['foo.test.ts:7'] }); + }, {}, {}, { additionalArgs: ['foo.test.ts:7'] }); expect(result.exitCode).toBe(0); expect(result.passed).toBe(1); expect(result.skipped).toBe(0); @@ -287,7 +287,7 @@ test('should focus a single nested test spec', async ({ runInlineTest }) => { const { test } = pwt; test('no-pass1', ({}) => {}); `, - }, { args: ['foo.test.ts:9', 'bar.test.ts'] }); + }, {}, {}, { additionalArgs: ['foo.test.ts:9', 'bar.test.ts'] }); expect(result.exitCode).toBe(0); expect(result.passed).toBe(2); expect(result.skipped).toBe(0); @@ -312,7 +312,7 @@ test('should focus a single test suite', async ({ runInlineTest }) => { const { test } = pwt; test('no-pass1', ({}) => {}); `, - }, { args: ['foo.test.ts:8'] }); + }, {}, {}, { additionalArgs: ['foo.test.ts:8'] }); expect(result.exitCode).toBe(0); expect(result.passed).toBe(2); expect(result.skipped).toBe(0); @@ -338,7 +338,7 @@ test('should match by directory', async ({ runInlineTest }) => { const { test } = pwt; test('pass', ({}) => {}); ` - }, { args: ['dir-b'] }); + }, {}, {}, { additionalArgs: ['dir-b'] }); expect(result.passed).toBe(2); expect(result.report.suites.map(s => s.file).sort()).toEqual(['dir-b/file1.test.ts', 'dir-b/file2.test.ts']); expect(result.exitCode).toBe(0); @@ -438,7 +438,7 @@ test('should always work with unix separators', async ({ runInlineTest }) => { const { test } = pwt; test('pass', ({}) => {}); ` - }, { args: [`dir/a`] }); + }, {}, {}, { additionalArgs: [`dir/a`] }); expect(result.passed).toBe(1); expect(result.report.suites.map(s => s.file).sort()).toEqual(['a.test.ts']); expect(result.exitCode).toBe(0); diff --git a/tests/playwright-test/test-output-dir.spec.ts b/tests/playwright-test/test-output-dir.spec.ts index debbf4c754..2f5d571426 100644 --- a/tests/playwright-test/test-output-dir.spec.ts +++ b/tests/playwright-test/test-output-dir.spec.ts @@ -303,7 +303,7 @@ test('should accept a relative path for outputDir', async ({ runInlineTest }, te { outputDir: './my-output-dir' }, ] }; `, - }, {usesCustomOutputDir: true}); + }, {}, {}, { usesCustomOutputDir: true }); expect(result.exitCode).toBe(0); });