Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(webserver): customize shutdown with new kill option #33379

Closed
wants to merge 19 commits into from
Closed
3 changes: 3 additions & 0 deletions docs/src/test-api/class-testconfig.md
Original file line number Diff line number Diff line change
Expand Up @@ -619,6 +619,9 @@ 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 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.

Launch a development web server (or multiple) during the tests.
Expand Down
3 changes: 2 additions & 1 deletion docs/src/test-webserver-js.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,8 @@ export default defineConfig({
| `cwd` | Current working directory of the spawned process, defaults to the directory of the configuration file. |
| `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. |
| `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 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

Expand Down
4 changes: 2 additions & 2 deletions packages/playwright-core/src/utils/processLauncher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ export async function launchProcess(options: LaunchProcessOptions): Promise<Laun
let processClosed = false;
let fulfillCleanup = () => {};
const waitForCleanup = new Promise<void>(f => fulfillCleanup = f);
spawnedProcess.once('exit', (exitCode, signal) => {
spawnedProcess.once('close', (exitCode, signal) => {
options.log(`[pid=${spawnedProcess.pid}] <process did exit: exitCode=${exitCode}, signal=${signal}>`);
processClosed = true;
gracefullyCloseSet.delete(gracefullyClose);
Expand Down Expand Up @@ -226,7 +226,7 @@ export async function launchProcess(options: LaunchProcessOptions): Promise<Laun
killSet.delete(killProcessAndCleanup);
removeProcessHandlersIfNeeded();
options.log(`[pid=${spawnedProcess.pid}] <kill>`);
if (spawnedProcess.pid && !spawnedProcess.killed && !processClosed) {
Copy link
Member Author

@Skn0tt Skn0tt Nov 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.killed means that the process was sent a signal before, not that it was killed. processClosed is better for that, especially now that it's set by the close event and not the exit event. exit is fired whenever the process receives a signal, close only after the process exited. some confusing terminology there.

if (spawnedProcess.pid && !processClosed) {
options.log(`[pid=${spawnedProcess.pid}] <will force kill>`);
// Force kill the browser.
try {
Expand Down
41 changes: 36 additions & 5 deletions packages/playwright/src/plugins/webServerPlugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
url?: string;
ignoreHTTPSErrors?: boolean;
timeout?: number;
kill?: { SIGINT?: number, SIGTERM?: number };
reuseExistingServer?: boolean;
cwd?: string;
env?: { [key: string]: string; };
Expand Down Expand Up @@ -91,8 +92,23 @@
throw new Error(`${this._options.url ?? `http://localhost${port ? ':' + port : ''}`} is already used, make sure that nothing is running on the port/url or set reuseExistingServer:true in config.webServer.`);
}

let signal: 'SIGINT' | 'SIGTERM' | undefined = undefined;
let timeout = 0;
if (this._options.kill) {
if (typeof this._options.kill.SIGINT === 'number') {
signal = 'SIGINT';
timeout = this._options.kill.SIGINT;
}
if (typeof this._options.kill.SIGTERM === 'number') {
if (signal)
throw new Error('Only one of SIGINT or SIGTERM can be specified in config.webServer.kill');
signal = 'SIGTERM';
timeout = this._options.kill.SIGTERM;
}
}

debugWebServer(`Starting WebServer process ${this._options.command}...`);
const { launchedProcess, kill } = await launchProcess({
const { launchedProcess, gracefullyClose } = await launchProcess({
command: this._options.command,
env: {
...DEFAULT_ENVIRONMENT_VARIABLES,
Expand All @@ -102,14 +118,29 @@
cwd: this._options.cwd,
stdio: 'stdin',
shell: true,
// Reject to indicate that we cannot close the web server gracefully
// and should fallback to non-graceful shutdown.
attemptToGracefullyClose: () => Promise.reject(),
attemptToGracefullyClose: async () => {
if (process.platform === 'win32')
throw new Error('Graceful shutdown is not supported on Windows');
if (!signal)
throw new Error('skip graceful shutdown');

process.kill(-launchedProcess.pid!, signal);
return new Promise<void>((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('close', (...args) => {
console.log("closing", ...args)

Check failure on line 133 in packages/playwright/src/plugins/webServerPlugin.ts

View workflow job for this annotation

GitHub Actions / docs & lint

Unexpected console statement

Check failure on line 133 in packages/playwright/src/plugins/webServerPlugin.ts

View workflow job for this annotation

GitHub Actions / docs & lint

Strings must use singlequote

Check failure on line 133 in packages/playwright/src/plugins/webServerPlugin.ts

View workflow job for this annotation

GitHub Actions / docs & lint

Missing semicolon
clearTimeout(timer);
resolve();
});
});
},
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._killProcess = kill;
this._killProcess = gracefullyClose;

debugWebServer(`Process started`);

Expand Down
12 changes: 12 additions & 0 deletions packages/playwright/types/test.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9371,6 +9371,18 @@ interface TestConfigWebServer {
*/
timeout?: number;

/**
* 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.
*/
kill?: {
SIGINT?: number;

SIGTERM?: number;
};

/**
* 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
Expand Down
58 changes: 58 additions & 0 deletions tests/playwright-test/web-server.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import type http from 'http';
import path from 'path';
import { test, expect, parseTestRunnerOutput } from './playwright-test-fixtures';
import type { RunResult } from './playwright-test-fixtures';
import { createHttpServer } from '../../packages/playwright-core/lib/utils/network';

const SIMPLE_SERVER_PATH = path.join(__dirname, 'assets', 'simple-server.js');
Expand Down Expand Up @@ -744,3 +745,60 @@ test('should forward stdout when set to "pipe" before server is ready', async ({
expect(result.output).toContain('[WebServer] output from server');
expect(result.output).not.toContain('Timed out waiting 3000ms');
});

test.describe('kill option', () => {
test.skip(process.platform === 'win32', 'No sending SIGINT on Windows');

const files = (additionalOptions = {}) => {
const port = test.info().workerIndex * 2 + 10510;
return {
'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"); })
server.listen(process.argv[2]);
`,
'test.spec.ts': `
import { test, expect } from '@playwright/test';
test('pass', async ({}) => {});
`,
'playwright.config.ts': `
module.exports = {
webServer: {
command: 'node web-server.js ${port}',
port: ${port},
stdout: 'pipe',
timeout: 3000,
...${JSON.stringify(additionalOptions)}
},
};
`,
};
};

function parseOutputLines(result: RunResult): string[] {
const prefix = '[WebServer] %%';
return result.output.split('\n').filter(line => line.startsWith(prefix)).map(line => line.substring(prefix.length));
}

test('sends SIGKILL by default', async ({ runInlineTest }) => {
const result = await runInlineTest(files(), { workers: 1 });
expect(parseOutputLines(result)).toEqual([]);
});

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']);
});

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']);
});

test('throws when mixed', async ({ runInlineTest }) => {
const result = await runInlineTest(files({ kill: { SIGINT: 500, SIGTERM: 500 } }), { workers: 1 });
expect(result.exitCode).toBe(1);
expect(result.output).toContain('Only one of SIGINT or SIGTERM can be specified in config.webServer.kill');
});
});
Loading