From 907bfd2c80beb7308e02d73ed9bc340c28f4150c Mon Sep 17 00:00:00 2001
From: Simon Knott <info@simonknott.de>
Date: Sun, 22 Dec 2024 11:45:37 +0100
Subject: [PATCH] simplify again

---
 .../playwright/src/plugins/webServerPlugin.ts | 22 ++++---------------
 tests/playwright-test/web-server.spec.ts      |  7 +++---
 2 files changed, 7 insertions(+), 22 deletions(-)

diff --git a/packages/playwright/src/plugins/webServerPlugin.ts b/packages/playwright/src/plugins/webServerPlugin.ts
index 97554e5331..ac0eb3809d 100644
--- a/packages/playwright/src/plugins/webServerPlugin.ts
+++ b/packages/playwright/src/plugins/webServerPlugin.ts
@@ -15,7 +15,6 @@
  */
 import path from 'path';
 import net from 'net';
-import child_process from 'child_process'
 
 import { colors, debug } from 'playwright-core/lib/utilsBundle';
 import { raceAgainstDeadline, launchProcess, monotonicTime, isURLAvailable } from 'playwright-core/lib/utils';
@@ -125,18 +124,10 @@ export class WebServerPlugin implements TestRunnerPlugin {
         if (!signal)
           throw new Error('skip graceful shutdown');
 
-        if (signal === "SIGINT") // proper usage of SIGINT is to send it to the entire process group, see https://www.cons.org/cracauer/sigint.html
-          process.kill(-launchedProcess.pid!, "SIGINT");
-        else { // SIGTERM is sent to the top process only, which then decides what to do
-          let pid = launchedProcess.pid!;
-          if (process.platform === "linux") {
-            for (const webServerPID of this._queryChildProcesses(launchedProcess.pid!))
-              process.kill(webServerPID, 'SIGTERM');
-          } else {
-            process.kill(-pid, "SIGTERM");
-          }
-        }
-          
+        // proper usage of SIGINT is to send it to the entire process group, see https://www.cons.org/cracauer/sigint.html
+        // there's no such convention for SIGTERM, but we follow GNU bash behaviour, which is also sending to the entire process group
+        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)
@@ -165,11 +156,6 @@ export class WebServerPlugin implements TestRunnerPlugin {
     });
   }
 
-  private _queryChildProcesses(parentPID: number): number[] {
-    const output = child_process.execSync(`ps --ppid ${parentPID} -o pid`, { encoding: 'utf-8' });
-    return output.split("\n").map(l => parseInt(l.trim(), 10)).filter(l => !Number.isNaN(l))
-  }
-
   private async _waitForProcess() {
     if (!this._isAvailableCallback) {
       this._processExitedPromise.catch(() => {});
diff --git a/tests/playwright-test/web-server.spec.ts b/tests/playwright-test/web-server.spec.ts
index d120e55119..ce7e6d4a8e 100644
--- a/tests/playwright-test/web-server.spec.ts
+++ b/tests/playwright-test/web-server.spec.ts
@@ -755,17 +755,16 @@ test.describe('kill option', () => {
       'child.js': `
         process.on('SIGINT', () => { console.log('%%childprocess received SIGINT'); setTimeout(() => process.exit(), 10) })
         process.on('SIGTERM', () => { console.log('%%childprocess received SIGTERM'); setTimeout(() => process.exit(), 10) })
-        process.on('message', msg => console.log(msg))
+        process.on('message', () => {}) // somehow, this line is needed to receive signals in a forked child process
       `,
       'web-server.js': `
-        const child = require("node:child_process").fork('./child.js', { silent: false })
+        require("node:child_process").fork('./child.js', { silent: false })
         
         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')
-          child.kill('SIGINT')
         })
 
         const server = require("node:http").createServer((req, res) => { res.end("ok"); })
@@ -801,7 +800,7 @@ test.describe('kill option', () => {
 
   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', 'childprocess received SIGINT']);
+    expect(parseOutputLines(result).sort()).toEqual(['childprocess received SIGTERM', 'webserver received SIGTERM but stubbornly refuses to wind down']);
   });
 
   test('can be configured to send SIGINT', async ({ runInlineTest }) => {