From 5bf5fcf61d2e8409bc27a6c387dc0eda3e3f8073 Mon Sep 17 00:00:00 2001 From: Simon Knott Date: Thu, 7 Nov 2024 12:16:36 +0000 Subject: [PATCH] send to the entire process group --- docs/src/test-api/class-testconfig.md | 2 +- docs/src/test-webserver-js.md | 2 +- packages/playwright-core/src/utils/processLauncher.ts | 8 ++++---- packages/playwright/src/plugins/webServerPlugin.ts | 8 +++----- packages/playwright/types/test.d.ts | 2 +- 5 files changed, 10 insertions(+), 12 deletions(-) diff --git a/docs/src/test-api/class-testconfig.md b/docs/src/test-api/class-testconfig.md index 343d72950f..663bfe23fa 100644 --- a/docs/src/test-api/class-testconfig.md +++ b/docs/src/test-api/class-testconfig.md @@ -619,7 +619,7 @@ export default defineConfig({ - `stdout` ?<["pipe"|"ignore"]> If `"pipe"`, it will pipe the stdout of the command to the process stdout. If `"ignore"`, it will ignore the stdout of the command. Default to `"ignore"`. - `stderr` ?<["pipe"|"ignore"]> Whether to pipe the stderr of the command to the process stderr or ignore it. Defaults to `"pipe"`. - `timeout` ?<[int]> How long to wait for the process to start up and be available in milliseconds. Defaults to 60000. - - `kill` ?<[Object]> How to shut down the process gracefully. If unspecified, the process group is forcefully `SIGKILL`ed. If set to `{ SIGINT: 500 }`, the top process is sent a `SIGINT` signal, followed by `SIGKILL` if it doesn't exit within 500ms. You can also use `SIGTERM` instead. A `0` timeout means no `SIGKILL` will be sent. Windows doesn't support `SIGINT` and `SIGTERM` signals, so this option is ignored. + - `kill` ?<[Object]> How to shut down the process gracefully. If unspecified, the process group is forcefully `SIGKILL`ed. If set to `{ SIGINT: 500 }`, the process group is sent a `SIGINT` signal, followed by `SIGKILL` if it doesn't exit within 500ms. You can also use `SIGTERM` instead. A `0` timeout means no `SIGKILL` will be sent. Windows doesn't support `SIGINT` and `SIGTERM` signals, so this option is ignored. - `SIGINT` ?<[int]> - `SIGTERM` ?<[int]> - `url` ?<[string]> The url on your http server that is expected to return a 2xx, 3xx, 400, 401, 402, or 403 status code when the server is ready to accept connections. Redirects (3xx status codes) are being followed and the new location is checked. Either `port` or `url` should be specified. diff --git a/docs/src/test-webserver-js.md b/docs/src/test-webserver-js.md index 8739207e7f..fa1113b3b3 100644 --- a/docs/src/test-webserver-js.md +++ b/docs/src/test-webserver-js.md @@ -37,7 +37,7 @@ export default defineConfig({ | `stdout` | If `"pipe"`, it will pipe the stdout of the command to the process stdout. If `"ignore"`, it will ignore the stdout of the command. Default to `"ignore"`. | | `stderr` | Whether to pipe the stderr of the command to the process stderr or ignore it. Defaults to `"pipe"`. | | `timeout` | How long to wait for the process to start up and be available in milliseconds. Defaults to 60000. | -| `kill` | How to shut down the process gracefully. If unspecified, the process group is forcefully `SIGKILL`ed. If set to `{ SIGINT: 500 }`, the top process is sent a `SIGINT` signal, followed by `SIGKILL` if it doesn't exit within 500ms. You can also use `SIGTERM` instead. A `0` timeout means no `SIGKILL` will be sent. Windows doesn't support `SIGINT` and `SIGTERM` signals, so this option is ignored. | +| `kill` | How to shut down the process gracefully. If unspecified, the process group is forcefully `SIGKILL`ed. If set to `{ SIGINT: 500 }`, the process group is sent a `SIGINT` signal, followed by `SIGKILL` if it doesn't exit within 500ms. You can also use `SIGTERM` instead. A `0` timeout means no `SIGKILL` will be sent. Windows doesn't support `SIGINT` and `SIGTERM` signals, so this option is ignored. | ## Adding a server timeout diff --git a/packages/playwright-core/src/utils/processLauncher.ts b/packages/playwright-core/src/utils/processLauncher.ts index d64589b79b..19c6531c6b 100644 --- a/packages/playwright-core/src/utils/processLauncher.ts +++ b/packages/playwright-core/src/utils/processLauncher.ts @@ -180,7 +180,7 @@ export async function launchProcess(options: LaunchProcessOptions): Promise {}; const waitForCleanup = new Promise(f => fulfillCleanup = f); - spawnedProcess.once('exit', (exitCode, signal) => { + spawnedProcess.once('close', (exitCode, signal) => { options.log(`[pid=${spawnedProcess.pid}] `); processClosed = true; gracefullyCloseSet.delete(gracefullyClose); @@ -215,18 +215,18 @@ export async function launchProcess(options: LaunchProcessOptions): Promise`); - await options.attemptToGracefullyClose().catch(() => killProcess(true)); + await options.attemptToGracefullyClose().catch((e) => killProcess()); await waitForCleanup; // Ensure the process is dead and we have cleaned up. options.log(`[pid=${spawnedProcess.pid}] `); } // This method has to be sync to be used in the 'exit' event handler. - function killProcess(evenIfAlreadyKilled = false) { + function killProcess() { gracefullyCloseSet.delete(gracefullyClose); killSet.delete(killProcessAndCleanup); removeProcessHandlersIfNeeded(); options.log(`[pid=${spawnedProcess.pid}] `); - if (spawnedProcess.pid && (evenIfAlreadyKilled || !spawnedProcess.killed) && !processClosed) { + if (spawnedProcess.pid && !processClosed) { options.log(`[pid=${spawnedProcess.pid}] `); // Force kill the browser. try { diff --git a/packages/playwright/src/plugins/webServerPlugin.ts b/packages/playwright/src/plugins/webServerPlugin.ts index e879f109fa..55a5038f82 100644 --- a/packages/playwright/src/plugins/webServerPlugin.ts +++ b/packages/playwright/src/plugins/webServerPlugin.ts @@ -124,15 +124,13 @@ export class WebServerPlugin implements TestRunnerPlugin { if (!signal) throw new Error('skip graceful shutdown'); - const success = launchedProcess.kill(signal); - if (!success) - throw new Error(`signal didn't succeed, fall back to non-graceful shutdown`); - + process.kill(-launchedProcess.pid!, signal); 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) : undefined; - launchedProcess.once('exit', () => { + launchedProcess.once('close', (...args) => { + console.log("closing", ...args) clearTimeout(timer); resolve(); }); diff --git a/packages/playwright/types/test.d.ts b/packages/playwright/types/test.d.ts index 0c33f8dadb..4c6cfae95b 100644 --- a/packages/playwright/types/test.d.ts +++ b/packages/playwright/types/test.d.ts @@ -9373,7 +9373,7 @@ interface TestConfigWebServer { /** * How to shut down the process gracefully. If unspecified, the process group is forcefully `SIGKILL`ed. If set to `{ - * SIGINT: 500 }`, the top process is sent a `SIGINT` signal, followed by `SIGKILL` if it doesn't exit within 500ms. + * SIGINT: 500 }`, the process group is sent a `SIGINT` signal, followed by `SIGKILL` if it doesn't exit within 500ms. * You can also use `SIGTERM` instead. A `0` timeout means no `SIGKILL` will be sent. Windows doesn't support `SIGINT` * and `SIGTERM` signals, so this option is ignored. */