diff --git a/docs/src/test-api/class-testconfig.md b/docs/src/test-api/class-testconfig.md index 6e98c8631fa72..11806e4e6c5e9 100644 --- a/docs/src/test-api/class-testconfig.md +++ b/docs/src/test-api/class-testconfig.md @@ -656,7 +656,7 @@ export default config; - `port` ?<[int]> The port that your http server is expected to appear on. It does wait until it accepts connections. Exactly one of `port` or `url` is required. - `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. Exactly one of `port` or `url` is required. - `ignoreHTTPSErrors` ?<[boolean]> Whether to ignore HTTPS errors when fetching the `url`. Defaults to `false`. - - `timeout` ?<[int]> How long to wait for the process to start up and be available in milliseconds. Defaults to 60000. + - `timeout` ?<[int]> How long to wait for the process to start up and be available in milliseconds. The same timeout is also used to terminate the process. Defaults to 60000. - `reuseExistingServer` ?<[boolean]> If true, it will re-use an existing server on the `port` or `url` when available. If no server is running on that `port` or `url`, it will run the command to start a new server. If `false`, it will throw if an existing process is listening on the `port` or `url`. This should be commonly set to `!process.env.CI` to allow the local dev server when running tests locally. - `cwd` ?<[string]> Current working directory of the spawned process, defaults to the directory of the configuration file. - `env` ?<[Object]<[string], [string]>> Environment variables to set for the command, `process.env` by default. diff --git a/packages/playwright-core/src/utils/processLauncher.ts b/packages/playwright-core/src/utils/processLauncher.ts index e043ebe854bde..b83e440f72eb8 100644 --- a/packages/playwright-core/src/utils/processLauncher.ts +++ b/packages/playwright-core/src/utils/processLauncher.ts @@ -47,7 +47,7 @@ export type LaunchProcessOptions = { type LaunchResult = { launchedProcess: childProcess.ChildProcess, gracefullyClose: () => Promise, - kill: () => Promise, + kill: (sendSigtermBeforeSigkillTimeout?: number) => Promise, }; export const gracefullyCloseSet = new Set<() => Promise>(); @@ -188,6 +188,21 @@ export async function launchProcess(options: LaunchProcessOptions): Promise`); + try { + // Send SIGTERM to process tree. + process.kill(-spawnedProcess.pid, 'SIGTERM'); + } catch (e) { + // The process might have already stopped. + options.log(`[pid=${spawnedProcess.pid}] exception while trying to SIGTERM process: ${e}`); + } + } else { + options.log(`[pid=${spawnedProcess.pid}] `); + } + } + function killProcessAndCleanup() { killProcess(); options.log(`[pid=${spawnedProcess.pid || 'N/A'}] starting temporary directories cleanup`); @@ -202,9 +217,16 @@ export async function launchProcess(options: LaunchProcessOptions): Promise Promise; - private _killProcess?: () => Promise; + private _killProcess?: (presendSigtermBeforeSigkillTimeout?: number) => Promise; private _processExitedPromise!: Promise; private _options: WebServerPluginOptions; private _checkPortOnly: boolean; private _reporter?: Reporter; + private _launchTerminateTimeout: number; name = 'playwright:webserver'; constructor(options: WebServerPluginOptions, checkPortOnly: boolean) { this._options = options; + this._launchTerminateTimeout = this._options.timeout || 60 * 1000; this._checkPortOnly = checkPortOnly; } @@ -72,7 +74,8 @@ export class WebServerPlugin implements TestRunnerPlugin { } public async teardown() { - await this._killProcess?.(); + // Send SIGTERM and wait for it to gracefully close. + await this._killProcess?.(this._launchTerminateTimeout); } private async _startProcess(): Promise { @@ -122,15 +125,14 @@ export class WebServerPlugin implements TestRunnerPlugin { } private async _waitForAvailability() { - const launchTimeout = this._options.timeout || 60 * 1000; const cancellationToken = { canceled: false }; const { timedOut } = (await Promise.race([ - raceAgainstTimeout(() => waitFor(this._isAvailable!, cancellationToken), launchTimeout), + raceAgainstTimeout(() => waitFor(this._isAvailable!, cancellationToken), this._launchTerminateTimeout), this._processExitedPromise, ])); cancellationToken.canceled = true; if (timedOut) - throw new Error(`Timed out waiting ${launchTimeout}ms from config.webServer.`); + throw new Error(`Timed out waiting ${this._launchTerminateTimeout}ms from config.webServer.`); } } diff --git a/packages/playwright-test/types/test.d.ts b/packages/playwright-test/types/test.d.ts index 16da91372c28b..0e3061debb3af 100644 --- a/packages/playwright-test/types/test.d.ts +++ b/packages/playwright-test/types/test.d.ts @@ -4599,7 +4599,8 @@ interface TestConfigWebServer { ignoreHTTPSErrors?: boolean; /** - * How long to wait for the process to start up and be available in milliseconds. Defaults to 60000. + * How long to wait for the process to start up and be available in milliseconds. The same timeout is also used to + * terminate the process. Defaults to 60000. */ timeout?: number; diff --git a/tests/playwright-test/assets/simple-server-ignores-sigterm.js b/tests/playwright-test/assets/simple-server-ignores-sigterm.js new file mode 100644 index 0000000000000..209f4c98bf2ef --- /dev/null +++ b/tests/playwright-test/assets/simple-server-ignores-sigterm.js @@ -0,0 +1,13 @@ +const http = require('http'); + +const port = process.argv[2] || 3000; + +const server = http.createServer(function (req, res) { + res.end('running!'); +}); +process.on('SIGTERM', () => console.log('received SIGTERM - ignoring')); +process.on('SIGINT', () => console.log('received SIGINT - ignoring')); + +server.listen(port, () => { + console.log('listening on port', port); +}); diff --git a/tests/playwright-test/web-server.spec.ts b/tests/playwright-test/web-server.spec.ts index e70b197b4dbff..3fc8c0a774199 100644 --- a/tests/playwright-test/web-server.spec.ts +++ b/tests/playwright-test/web-server.spec.ts @@ -19,6 +19,7 @@ import path from 'path'; import { test, expect } from './playwright-test-fixtures'; const SIMPLE_SERVER_PATH = path.join(__dirname, 'assets', 'simple-server.js'); +const SIMPLE_SERVER_THAT_IGNORES_SIGTERM_PATH = path.join(__dirname, 'assets', 'simple-server-ignores-sigterm.js'); test('should create a server', async ({ runInlineTest }, { workerIndex }) => { const port = workerIndex + 10500; @@ -601,3 +602,27 @@ test('should treat 3XX as available server', async ({ runInlineTest }, { workerI expect(result.output).toContain('[WebServer] listening'); expect(result.output).toContain('[WebServer] error from server'); }); + +test('should be able to kill process that ignores SIGTERM', async ({ runInlineTest }, { workerIndex }) => { + test.skip(process.platform === 'win32', 'there is no SIGTERM on Windows'); + const port = workerIndex + 10500; + const result = await runInlineTest({ + 'test.spec.ts': ` + const { test } = pwt; + test('pass', async ({}) => {}); + `, + 'playwright.config.ts': ` + module.exports = { + webServer: { + command: 'node ${JSON.stringify(SIMPLE_SERVER_THAT_IGNORES_SIGTERM_PATH)} ${port}', + port: ${port}, + timeout: 1000, + } + }; + `, + }, {}, { DEBUG: 'pw:webserver' }); + expect(result.exitCode).toBe(0); + expect(result.passed).toBe(1); + expect(result.output).toContain('[WebServer] listening'); + expect(result.output).toContain('[WebServer] received SIGTERM - ignoring'); +});