fix!: launchNode.cleanup not killing node in last test of test group

.changeset/breezy-tables-jam.md

@@ -0,0 +1,6 @@
+---
+"@fuel-ts/account": minor
+"fuels": minor
+---
+
+fix!: `launchNode.cleanup` not killing node in last test of test group

packages/account/package.json

@@ -67,7 +67,6 @@
     "graphql-tag": "^2.12.6",
     "portfinder": "^1.0.32",
     "ramda": "^0.29.0",
-    "tree-kill": "^1.2.2",
     "uuid": "^10.0.0"
   },
   "devDependencies": {

packages/account/src/test-utils/launchNode-singular-test.test.ts

@@ -0,0 +1,20 @@
+import { launchNode } from './launchNode';
+
+/**
+ * The test runner creates a test environment per file,
+ * which we can use to isolate the faulty behavior.
+ */
+/**
+ * @group node
+ */
+describe('launchNode-singular-test', () => {
+  let killedNodeUrl = '';
+  afterAll(async () => {
+    await expect(fetch(killedNodeUrl)).rejects.toThrow('fetch failed');
+  });
+  test.only('synchronous cleanup kills node before test runner exits', async () => {
+    const { cleanup, url } = await launchNode();
+    killedNodeUrl = url;
+    cleanup();
+  });
+});

packages/account/src/test-utils/launchNode.test.ts

@@ -6,9 +6,7 @@ import * as childProcessMod from 'child_process';
 
 import { Provider } from '../providers';
 
-import { killNode, launchNode } from './launchNode';
-
-type ChildProcessWithoutNullStreams = childProcessMod.ChildProcessWithoutNullStreams;
+import { launchNode } from './launchNode';
 
 vi.mock('child_process', async () => {
   const mod = await vi.importActual('child_process');
@@ -130,63 +128,17 @@ describe('launchNode', () => {
     cleanup();
   });
 
-  test('should kill process only if PID exists and node is alive', () => {
-    const killFn = vi.fn();
-    const state = { isDead: true };
-
-    // should not kill
-    let child = {
-      pid: undefined,
-      stdout: {
-        removeAllListeners: () => {},
-      },
-      stderr: {
-        removeAllListeners: () => {},
-      },
-    } as ChildProcessWithoutNullStreams;
-
-    killNode({
-      child,
-      configPath: '',
-      killFn,
-      state,
-    });
+  test('calling cleanup multiple times does not retry process killing', async () => {
+    const killSpy = vi.spyOn(process, 'kill');
 
-    expect(killFn).toHaveBeenCalledTimes(0);
-    expect(state.isDead).toEqual(true);
-
-    // should not kill
-    child = {
-      pid: 1,
-      stdout: {
-        removeAllListeners: () => {},
-      },
-      stderr: {
-        removeAllListeners: () => {},
-      },
-    } as ChildProcessWithoutNullStreams;
-
-    killNode({
-      child,
-      configPath: '',
-      killFn,
-      state,
-    });
+    const { cleanup } = await launchNode();
 
-    expect(killFn).toHaveBeenCalledTimes(0);
-    expect(state.isDead).toEqual(true);
+    cleanup();
 
-    // should kill
-    state.isDead = false;
+    expect(killSpy).toHaveBeenCalledTimes(1);
 
-    killNode({
-      child,
-      configPath: '',
-      killFn,
-      state,
-    });
+    cleanup();
 
-    expect(killFn).toHaveBeenCalledTimes(1);
-    expect(state.isDead).toEqual(true);
+    expect(killSpy).toHaveBeenCalledTimes(1);
   });
 });

packages/account/src/test-utils/launchNode.ts

@@ -3,13 +3,11 @@ import { randomBytes } from '@fuel-ts/crypto';
 import { FuelError } from '@fuel-ts/errors';
 import type { SnapshotConfigs } from '@fuel-ts/utils';
 import { defaultConsensusKey, hexlify, defaultSnapshotConfigs } from '@fuel-ts/utils';
-import type { ChildProcessWithoutNullStreams } from 'child_process';
 import { randomUUID } from 'crypto';
 import { existsSync, mkdirSync, rmSync, writeFileSync } from 'fs';
 import os from 'os';
 import path from 'path';
 import { getPortPromise } from 'portfinder';
-import treeKill from 'tree-kill';
 
 import { Provider } from '../providers';
 import { Signer } from '../signer';
@@ -58,33 +56,6 @@ export type LaunchNodeResult = Promise<{
   snapshotDir: string;
 }>;
 
-export type KillNodeParams = {
-  child: ChildProcessWithoutNullStreams;
-  configPath: string;
-  killFn: (pid: number) => void;
-  state: {
-    isDead: boolean;
-  };
-};
-
-export const killNode = (params: KillNodeParams) => {
-  const { child, configPath, state, killFn } = params;
-  if (!state.isDead) {
-    if (child.pid) {
-      state.isDead = true;
-      killFn(Number(child.pid));
-    }
-
-    // Remove all the listeners we've added.
-    child.stderr.removeAllListeners();
-
-    // Remove the temporary folder and all its contents.
-    if (existsSync(configPath)) {
-      rmSync(configPath, { recursive: true });
-    }
-  }
-};
-
 function getFinalStateConfigJSON({ stateConfig, chainConfig }: SnapshotConfigs) {
   const defaultCoins = defaultSnapshotConfigs.stateConfig.coins.map((coin) => ({
     ...coin,
@@ -232,7 +203,7 @@ export const launchNode = async ({
         '--debug',
         ...remainingArgs,
       ].flat(),
-      { stdio: 'pipe' }
+      { stdio: 'pipe', detached: true }
     );
 
     if (loggingEnabled) {
@@ -242,13 +213,26 @@ export const launchNode = async ({
       });
     }
 
-    const cleanupConfig: KillNodeParams = {
-      child,
-      configPath: tempDir,
-      killFn: treeKill,
-      state: {
-        isDead: false,
-      },
+    const childState = {
+      isDead: false,
+    };
+
+    const cleanup = () => {
+      if (childState.isDead) {
+        return;
+      }
+      childState.isDead = true;
+
+      // Remove all the listeners we've added.
+      child.stderr.removeAllListeners();
+
+      // Remove the temporary folder and all its contents.
+      if (existsSync(tempDir)) {
+        rmSync(tempDir, { recursive: true });
+      }
+
+      // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
+      process.kill(-child.pid!);
     };
 
     // Look for a specific graphql start point in the output.
@@ -264,7 +248,7 @@ export const launchNode = async ({
 
         // Resolve with the cleanup method.
         resolve({
-          cleanup: () => killNode(cleanupConfig),
+          cleanup,
           ip: realIp,
           port: realPort,
           url: `http://${realIp}:${realPort}/v1/graphql`,
@@ -279,18 +263,18 @@ export const launchNode = async ({
     });
 
     // Process exit.
-    process.on('exit', () => killNode(cleanupConfig));
+    process.on('exit', cleanup);
 
     // Catches ctrl+c event.
-    process.on('SIGINT', () => killNode(cleanupConfig));
+    process.on('SIGINT', cleanup);
 
     // Catches "kill pid" (for example: nodemon restart).
-    process.on('SIGUSR1', () => killNode(cleanupConfig));
-    process.on('SIGUSR2', () => killNode(cleanupConfig));
+    process.on('SIGUSR1', cleanup);
+    process.on('SIGUSR2', cleanup);
 
     // Catches uncaught exceptions.
-    process.on('beforeExit', () => killNode(cleanupConfig));
-    process.on('uncaughtException', () => killNode(cleanupConfig));
+    process.on('beforeExit', cleanup);
+    process.on('uncaughtException', cleanup);
 
     child.on('error', reject);
   });

packages/fuels/src/cli/commands/dev/autoStartFuelCore.ts

@@ -1,5 +1,4 @@
 import { defaultConsensusKey } from '@fuel-ts/utils';
-import type { ChildProcessWithoutNullStreams } from 'child_process';
 import { getPortPromise } from 'portfinder';
 
 import { launchNode } from '../../../test-utils';
@@ -15,14 +14,6 @@ export type FuelCoreNode = {
   killChildProcess: () => void;
 };
 
-export type KillNodeParams = {
-  core: ChildProcessWithoutNullStreams;
-  killFn: (pid: number) => void;
-  state: {
-    isDead: boolean;
-  };
-};
-
 export const autoStartFuelCore = async (config: FuelsConfig) => {
   let fuelCore: FuelCoreNode | undefined;