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

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.

packages/playwright-core/src/utils/processLauncher.ts

@@ -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 {
-      killProcess();
-      await waitForCleanup;
-    }
+  function killAndWait() {
+    killProcess();
+    return waitForCleanup;
   }
 
   return { launchedProcess: spawnedProcess, gracefullyClose, kill: killAndWait };

packages/playwright-test/src/plugins/webServerPlugin.ts

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

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;
 

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