Revert "feat: send SIGTERM to webserver before SIGKILL'ing it. (#18220)" (#18661)

This reverts commit c63a0b536d1f0119794a909e4f9c420c8506b4d5.

Reason: https://github.com/microsoft/playwright/pull/18564
This commit is contained in:
Andrey Lushnikov 2022-11-09 09:18:33 -08:00 committed by GitHub
parent a7f1c8cb65
commit 9bcb28f25a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 11 additions and 74 deletions

View File

@ -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.

View File

@ -47,7 +47,7 @@ export type LaunchProcessOptions = {
type LaunchResult = {
launchedProcess: childProcess.ChildProcess,
gracefullyClose: () => Promise<void>,
kill: (sendSigtermBeforeSigkillTimeout?: number) => Promise<void>,
kill: () => Promise<void>,
};
export const gracefullyCloseSet = new Set<() => Promise<void>>();
@ -188,21 +188,6 @@ export async function launchProcess(options: LaunchProcessOptions): Promise<Laun
}
}
function sendPosixSIGTERM() {
if (spawnedProcess.pid && !spawnedProcess.killed && !processClosed) {
options.log(`[pid=${spawnedProcess.pid}] <will sigterm>`);
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}] <skipped sigterm spawnedProcess=${spawnedProcess.killed} processClosed=${processClosed}>`);
}
}
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<Laun
options.log(`[pid=${spawnedProcess.pid || 'N/A'}] finished temporary directories cleanup`);
}
async function killAndWait(sendSigtermBeforeSigkillTimeout?: number) {
if (process.platform !== 'win32' && sendSigtermBeforeSigkillTimeout) {
sendPosixSIGTERM();
const sigtermTimeoutId = setTimeout(killProcess, sendSigtermBeforeSigkillTimeout);
await waitForCleanup;
clearTimeout(sigtermTimeoutId);
} else {
function killAndWait() {
killProcess();
await waitForCleanup;
}
return waitForCleanup;
}
return { launchedProcess: spawnedProcess, gracefullyClose, kill: killAndWait };

View File

@ -46,17 +46,15 @@ const debugWebServer = debug('pw:webserver');
export class WebServerPlugin implements TestRunnerPlugin {
private _isAvailable?: () => Promise<boolean>;
private _killProcess?: (presendSigtermBeforeSigkillTimeout?: number) => Promise<void>;
private _killProcess?: () => Promise<void>;
private _processExitedPromise!: Promise<any>;
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<void> {
@ -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.`);
}
}

View File

@ -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;

View File

@ -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);
});

View File

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