diff --git a/packages/playwright/src/plugins/webServerPlugin.ts b/packages/playwright/src/plugins/webServerPlugin.ts index 096629e1f5..97554e5331 100644 --- a/packages/playwright/src/plugins/webServerPlugin.ts +++ b/packages/playwright/src/plugins/webServerPlugin.ts @@ -15,6 +15,7 @@ */ import path from 'path'; import net from 'net'; +import child_process from 'child_process' import { colors, debug } from 'playwright-core/lib/utilsBundle'; import { raceAgainstDeadline, launchProcess, monotonicTime, isURLAvailable } from 'playwright-core/lib/utils'; @@ -124,10 +125,18 @@ export class WebServerPlugin implements TestRunnerPlugin { if (!signal) throw new Error('skip graceful shutdown'); - // launchedProcess is a shell. not all shells forward signals to their child processes. - // we need to send the signal to the webserver inside the shell. - const webserverPid = launchedProcess.pid!; // TODO: get the actual pid of the webserver - process.kill(webserverPid, signal); + if (signal === "SIGINT") // proper usage of SIGINT is to send it to the entire process group, see https://www.cons.org/cracauer/sigint.html + process.kill(-launchedProcess.pid!, "SIGINT"); + else { // SIGTERM is sent to the top process only, which then decides what to do + let pid = launchedProcess.pid!; + if (process.platform === "linux") { + for (const webServerPID of this._queryChildProcesses(launchedProcess.pid!)) + process.kill(webServerPID, 'SIGTERM'); + } else { + process.kill(-pid, "SIGTERM"); + } + } + return new Promise((resolve, reject) => { const timer = timeout !== 0 ? setTimeout(() => reject(new Error(`process didn't close gracefully within timeout, falling back to SIGKILL`)), timeout) @@ -156,6 +165,11 @@ export class WebServerPlugin implements TestRunnerPlugin { }); } + private _queryChildProcesses(parentPID: number): number[] { + const output = child_process.execSync(`ps --ppid ${parentPID} -o pid`, { encoding: 'utf-8' }); + return output.split("\n").map(l => parseInt(l.trim(), 10)).filter(l => !Number.isNaN(l)) + } + private async _waitForProcess() { if (!this._isAvailableCallback) { this._processExitedPromise.catch(() => {}); diff --git a/tests/playwright-test/web-server.spec.ts b/tests/playwright-test/web-server.spec.ts index 3a6ab192f0..d120e55119 100644 --- a/tests/playwright-test/web-server.spec.ts +++ b/tests/playwright-test/web-server.spec.ts @@ -752,10 +752,23 @@ test.describe('kill option', () => { const files = (additionalOptions = {}) => { const port = test.info().workerIndex * 2 + 10510; return { + 'child.js': ` + process.on('SIGINT', () => { console.log('%%childprocess received SIGINT'); setTimeout(() => process.exit(), 10) }) + process.on('SIGTERM', () => { console.log('%%childprocess received SIGTERM'); setTimeout(() => process.exit(), 10) }) + process.on('message', msg => console.log(msg)) + `, 'web-server.js': ` - process.on('SIGINT', () => { console.log('%%webserver received SIGINT but stubbornly refuses to wind down') }) - process.on('SIGTERM', () => { console.log('%%webserver received SIGTERM but stubbornly refuses to wind down') }) - const server = require('http').createServer((req, res) => { res.end("ok"); }) + const child = require("node:child_process").fork('./child.js', { silent: false }) + + process.on('SIGINT', () => { + console.log('%%webserver received SIGINT but stubbornly refuses to wind down') + }) + process.on('SIGTERM', () => { + console.log('%%webserver received SIGTERM but stubbornly refuses to wind down') + child.kill('SIGINT') + }) + + const server = require("node:http").createServer((req, res) => { res.end("ok"); }) server.listen(process.argv[2]); `, 'test.spec.ts': ` @@ -765,7 +778,7 @@ test.describe('kill option', () => { 'playwright.config.ts': ` module.exports = { webServer: { - command: 'node web-server.js ${port}', + command: 'echo some-precondition && node web-server.js ${port}', port: ${port}, stdout: 'pipe', timeout: 3000, @@ -788,12 +801,12 @@ test.describe('kill option', () => { test('can be configured to send SIGTERM', async ({ runInlineTest }) => { const result = await runInlineTest(files({ kill: { SIGTERM: 500 } }), { workers: 1 }); - expect(parseOutputLines(result)).toEqual(['webserver received SIGTERM but stubbornly refuses to wind down']); + expect(parseOutputLines(result)).toEqual(['webserver received SIGTERM but stubbornly refuses to wind down', 'childprocess received SIGINT']); }); test('can be configured to send SIGINT', async ({ runInlineTest }) => { - const result = await runInlineTest(files({ kill: { SIGINT: 500 } }), { workers: 1 }); - expect(parseOutputLines(result)).toEqual(['webserver received SIGINT but stubbornly refuses to wind down']); + const result = await runInlineTest(files({ kill: { SIGINT: 5000 } }), { workers: 1 }); + expect(parseOutputLines(result).sort()).toEqual(['childprocess received SIGINT', 'webserver received SIGINT but stubbornly refuses to wind down']); }); test('throws when mixed', async ({ runInlineTest }) => {