diff --git a/docs/src/test-api/class-testconfig.md b/docs/src/test-api/class-testconfig.md index 11806e4e6c5e9..6e98c8631fa72 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. The same timeout is also used to terminate the process. Defaults to 60000. + - `timeout` ?<[int]> How long to wait for the process to start up and be available in milliseconds. 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 b83e440f72eb8..e043ebe854bde 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: (sendSigtermBeforeSigkillTimeout?: number) => Promise, + kill: () => Promise, }; export const gracefullyCloseSet = new Set<() => Promise>(); @@ -188,21 +188,6 @@ 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`); @@ -217,16 +202,9 @@ export async function launchProcess(options: LaunchProcessOptions): Promise Promise; - private _killProcess?: (presendSigtermBeforeSigkillTimeout?: number) => Promise; + private _killProcess?: () => 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; } @@ -74,8 +72,7 @@ export class WebServerPlugin implements TestRunnerPlugin { } public async teardown() { - // Send SIGTERM and wait for it to gracefully close. - await this._killProcess?.(this._launchTerminateTimeout); + await this._killProcess?.(); } private async _startProcess(): Promise { @@ -125,14 +122,15 @@ 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), this._launchTerminateTimeout), + raceAgainstTimeout(() => waitFor(this._isAvailable!, cancellationToken), launchTimeout), this._processExitedPromise, ])); cancellationToken.canceled = true; if (timedOut) - throw new Error(`Timed out waiting ${this._launchTerminateTimeout}ms from config.webServer.`); + throw new Error(`Timed out waiting ${launchTimeout}ms from config.webServer.`); } } diff --git a/packages/playwright-test/types/test.d.ts b/packages/playwright-test/types/test.d.ts index 67f6e50ec9ba4..9a3aa471caff6 100644 --- a/packages/playwright-test/types/test.d.ts +++ b/packages/playwright-test/types/test.d.ts @@ -4648,8 +4648,7 @@ interface TestConfigWebServer { ignoreHTTPSErrors?: boolean; /** - * 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. + * How long to wait for the process to start up and be available in milliseconds. 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 deleted file mode 100644 index 209f4c98bf2ef..0000000000000 --- a/tests/playwright-test/assets/simple-server-ignores-sigterm.js +++ /dev/null @@ -1,13 +0,0 @@ -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 3fc8c0a774199..e70b197b4dbff 100644 --- a/tests/playwright-test/web-server.spec.ts +++ b/tests/playwright-test/web-server.spec.ts @@ -19,7 +19,6 @@ 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; @@ -602,27 +601,3 @@ 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'); -});