From b2443346f8f7c3e1170189e729e01ada0b40207b Mon Sep 17 00:00:00 2001 From: Andrey Lushnikov Date: Mon, 21 Nov 2022 11:14:17 -1000 Subject: [PATCH] Revert "fix(webserver): do not spawn webserver as a new process group (#18564)" This reverts commit 26044c11c03d13fa076b14a98758d61ede013245. Fixes #18865 --- .../src/plugins/webServerPlugin.ts | 55 ++++++++----------- 1 file changed, 22 insertions(+), 33 deletions(-) diff --git a/packages/playwright-test/src/plugins/webServerPlugin.ts b/packages/playwright-test/src/plugins/webServerPlugin.ts index d7df9212d681f..6a3f5b5b97ced 100644 --- a/packages/playwright-test/src/plugins/webServerPlugin.ts +++ b/packages/playwright-test/src/plugins/webServerPlugin.ts @@ -17,11 +17,10 @@ import http from 'http'; import https from 'https'; import path from 'path'; import net from 'net'; -import { spawn, spawnSync } from 'child_process'; -import type { ChildProcess } from 'child_process'; import { debug } from 'playwright-core/lib/utilsBundle'; import { raceAgainstTimeout } from 'playwright-core/lib/utils/timeoutRunner'; +import { launchProcess } from 'playwright-core/lib/utils/processLauncher'; import type { FullConfig, Reporter, Suite } from '../../types/testReporter'; import type { TestRunnerPlugin } from '.'; @@ -47,8 +46,8 @@ const debugWebServer = debug('pw:webserver'); export class WebServerPlugin implements TestRunnerPlugin { private _isAvailable?: () => Promise; - private _processExitedPromise?: Promise<{ code: number|null, signal: NodeJS.Signals|null }>; - private _childProcess?: ChildProcess; + private _killProcess?: () => Promise; + private _processExitedPromise!: Promise; private _options: WebServerPluginOptions; private _checkPortOnly: boolean; private _reporter?: Reporter; @@ -65,6 +64,7 @@ export class WebServerPlugin implements TestRunnerPlugin { this._options.cwd = this._options.cwd ? path.resolve(configDir, this._options.cwd) : configDir; try { await this._startProcess(); + await this._waitForProcess(); } catch (error) { await this.teardown(); throw error; @@ -72,27 +72,13 @@ export class WebServerPlugin implements TestRunnerPlugin { } public async teardown() { - if (!this._childProcess || !this._childProcess.pid || this._childProcess.killed) - return; - // Send SIGTERM and wait for it to gracefully close. - try { - if (process.platform === 'win32') { - const taskkillProcess = spawnSync(`taskkill /pid ${this._childProcess.pid} /T /F`, { shell: true }); - const [stdout, stderr] = [taskkillProcess.stdout.toString(), taskkillProcess.stderr.toString()]; - if (stdout) - debugWebServer(`[pid=${this._childProcess.pid}] taskkill stdout: ${stdout}`); - if (stderr) - debugWebServer(`[pid=${this._childProcess.pid}] taskkill stderr: ${stderr}`); - } else { - process.kill(this._childProcess.pid, 'SIGTERM'); - } - } catch (e) { - // the process might have already stopped - } - await this._processExitedPromise; + await this._killProcess?.(); } private async _startProcess(): Promise { + let processExitedReject = (error: Error) => { }; + this._processExitedPromise = new Promise((_, reject) => processExitedReject = reject); + const isAlreadyAvailable = await this._isAvailable!(); if (isAlreadyAvailable) { debugWebServer(`WebServer is already available`); @@ -103,28 +89,33 @@ export class WebServerPlugin implements TestRunnerPlugin { } debugWebServer(`Starting WebServer process ${this._options.command}...`); - - this._childProcess = spawn(this._options.command, [], { + const { launchedProcess, kill } = await launchProcess({ + command: this._options.command, env: { ...DEFAULT_ENVIRONMENT_VARIABLES, ...envWithoutExperimentalLoaderOptions(), ...this._options.env, }, cwd: this._options.cwd, - stdio: 'pipe', + stdio: 'stdin', shell: true, + attemptToGracefullyClose: async () => {}, + log: () => {}, + onExit: code => processExitedReject(new Error(code ? `Process from config.webServer was not able to start. Exit code: ${code}` : 'Process from config.webServer exited early.')), + tempDirectories: [], }); - this._processExitedPromise = new Promise((resolve, reject) => { - this._childProcess!.once('exit', (code, signal) => resolve({ code, signal })); - }); + this._killProcess = kill; debugWebServer(`Process started`); - this._childProcess.stderr!.on('data', line => this._reporter!.onStdErr?.('[WebServer] ' + line.toString())); - this._childProcess.stdout!.on('data', line => { + launchedProcess.stderr!.on('data', line => this._reporter!.onStdErr?.('[WebServer] ' + line.toString())); + launchedProcess.stdout!.on('data', line => { if (debugWebServer.enabled) this._reporter!.onStdOut?.('[WebServer] ' + line.toString()); }); + } + + private async _waitForProcess() { debugWebServer(`Waiting for availability...`); await this._waitForAvailability(); debugWebServer(`WebServer available`); @@ -135,9 +126,7 @@ export class WebServerPlugin implements TestRunnerPlugin { const cancellationToken = { canceled: false }; const { timedOut } = (await Promise.race([ raceAgainstTimeout(() => waitFor(this._isAvailable!, cancellationToken), launchTimeout), - this._processExitedPromise!.then(({ code, signal }) => { - throw new Error(`Process from config.webServer terminated with exit code "${code}" and signal "${signal}"`); - }), + this._processExitedPromise, ])); cancellationToken.canceled = true; if (timedOut)