From 23e8898b4574a6fb2722f8176ddfc7bcce87b933 Mon Sep 17 00:00:00 2001 From: nedsalk Date: Sun, 7 Jul 2024 15:39:37 +0200 Subject: [PATCH 01/24] test: add test --- .../src/test-utils/launchNode-singular-test.test.ts | 13 +++++++++++++ 1 file changed, 13 insertions(+) create mode 100644 packages/account/src/test-utils/launchNode-singular-test.test.ts diff --git a/packages/account/src/test-utils/launchNode-singular-test.test.ts b/packages/account/src/test-utils/launchNode-singular-test.test.ts new file mode 100644 index 0000000000..f5b8afc2d7 --- /dev/null +++ b/packages/account/src/test-utils/launchNode-singular-test.test.ts @@ -0,0 +1,13 @@ +import { launchNode } from './launchNode'; + +describe('only one test in this file to verify correct behavior', () => { + let killedNodeUrl = ''; + afterAll(async () => { + await expect(fetch(killedNodeUrl)).rejects.toThrow('fetch failed'); + }); + test('synchronous cleanup kills node before test runner exits', async () => { + const { cleanup, url } = await launchNode(); + killedNodeUrl = url; + cleanup(); + }); +}); From 41de3f24d0d2119992ee4bf73786e9ade0f900a6 Mon Sep 17 00:00:00 2001 From: nedsalk Date: Sun, 7 Jul 2024 15:40:17 +0200 Subject: [PATCH 02/24] fix: the problem --- packages/account/src/test-utils/launchNode.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/account/src/test-utils/launchNode.ts b/packages/account/src/test-utils/launchNode.ts index 3fd1ceddb1..04f4482f77 100644 --- a/packages/account/src/test-utils/launchNode.ts +++ b/packages/account/src/test-utils/launchNode.ts @@ -68,11 +68,11 @@ export type KillNodeParams = { }; export const killNode = (params: KillNodeParams) => { - const { child, configPath, state, killFn } = params; + const { child, configPath, state } = params; if (!state.isDead) { if (child.pid) { state.isDead = true; - killFn(Number(child.pid)); + process.kill(-child.pid); } // Remove all the listeners we've added. @@ -232,7 +232,7 @@ export const launchNode = async ({ '--debug', ...remainingArgs, ].flat(), - { stdio: 'pipe' } + { stdio: 'pipe', detached: true } ); if (loggingEnabled) { From 3db78c06ab094be543b6815ce8e45d3e699a7b0a Mon Sep 17 00:00:00 2001 From: nedsalk Date: Sun, 7 Jul 2024 15:43:01 +0200 Subject: [PATCH 03/24] add comment --- .../account/src/test-utils/launchNode-singular-test.test.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/account/src/test-utils/launchNode-singular-test.test.ts b/packages/account/src/test-utils/launchNode-singular-test.test.ts index f5b8afc2d7..0c5a3ce4d1 100644 --- a/packages/account/src/test-utils/launchNode-singular-test.test.ts +++ b/packages/account/src/test-utils/launchNode-singular-test.test.ts @@ -1,6 +1,10 @@ import { launchNode } from './launchNode'; -describe('only one test in this file to verify correct behavior', () => { +/** + * The test runner creates an test environment per file, + * which we can use to isolate the faulty behavior. + */ +describe('launchNode-singular-test', () => { let killedNodeUrl = ''; afterAll(async () => { await expect(fetch(killedNodeUrl)).rejects.toThrow('fetch failed'); From b64dcd4dd1d16320a6b943ff88e8d40b49bafcbe Mon Sep 17 00:00:00 2001 From: nedsalk Date: Sun, 7 Jul 2024 16:04:53 +0200 Subject: [PATCH 04/24] remove `tree-kill` dependency and refactor accordingly --- packages/account/package.json | 1 - .../account/src/test-utils/launchNode.test.ts | 64 +++-------------- packages/account/src/test-utils/launchNode.ts | 69 +++++++------------ pnpm-lock.yaml | 56 ++++++--------- 4 files changed, 56 insertions(+), 134 deletions(-) diff --git a/packages/account/package.json b/packages/account/package.json index e426048edf..29ed92bb8c 100644 --- a/packages/account/package.json +++ b/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": { diff --git a/packages/account/src/test-utils/launchNode.test.ts b/packages/account/src/test-utils/launchNode.test.ts index 07564cc721..44f1fe08b7 100644 --- a/packages/account/src/test-utils/launchNode.test.ts +++ b/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.only('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); }); }); diff --git a/packages/account/src/test-utils/launchNode.ts b/packages/account/src/test-utils/launchNode.ts index 04f4482f77..7ef20f34c6 100644 --- a/packages/account/src/test-utils/launchNode.ts +++ b/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 } = params; - if (!state.isDead) { - if (child.pid) { - state.isDead = true; - process.kill(-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, @@ -242,13 +213,25 @@ export const launchNode = async ({ }); } - const cleanupConfig: KillNodeParams = { - child, - configPath: tempDir, - killFn: treeKill, - state: { - isDead: false, - }, + const childState = { + isDead: false, + }; + + const cleanup = () => { + if (!childState.isDead) { + if (child.pid) { + childState.isDead = true; + process.kill(-child.pid); + } + + // 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 }); + } + } }; // Look for a specific graphql start point in the output. @@ -264,7 +247,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 +262,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); }); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 04522b8c37..2e5d0f39d7 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -725,9 +725,6 @@ importers: ramda: specifier: ^0.29.0 version: 0.29.0 - tree-kill: - specifier: ^1.2.2 - version: 1.2.2 uuid: specifier: ^10.0.0 version: 10.0.0 @@ -6416,7 +6413,6 @@ packages: bun@1.1.17: resolution: {integrity: sha512-x2vUqI75XQ11Qxb3FzQCd/AkbA8A3AiJ35xfw49JeNgu0MTi0RCuW+1zOyFf5iJM0xU07LKf2H69n4ASuEqhtQ==} - cpu: [arm64, x64] os: [darwin, linux, win32] hasBin: true @@ -18892,8 +18888,6 @@ snapshots: chalk: 4.1.2 execa: 5.1.1 fast-glob: 3.3.2 - transitivePeerDependencies: - - encoding '@react-native-community/cli-config@13.6.6': dependencies: @@ -18903,8 +18897,6 @@ snapshots: deepmerge: 4.3.1 fast-glob: 3.3.2 joi: 17.13.1 - transitivePeerDependencies: - - encoding '@react-native-community/cli-debugger-ui@13.6.6': dependencies: @@ -18931,8 +18923,6 @@ snapshots: strip-ansi: 5.2.0 wcwidth: 1.0.1 yaml: 2.3.1 - transitivePeerDependencies: - - encoding '@react-native-community/cli-hermes@13.6.6': dependencies: @@ -18940,8 +18930,6 @@ snapshots: '@react-native-community/cli-tools': 13.6.6 chalk: 4.1.2 hermes-profile-transformer: 0.0.6 - transitivePeerDependencies: - - encoding '@react-native-community/cli-platform-android@13.6.6': dependencies: @@ -18951,8 +18939,6 @@ snapshots: fast-glob: 3.3.2 fast-xml-parser: 4.4.0 logkitty: 0.7.1 - transitivePeerDependencies: - - encoding '@react-native-community/cli-platform-apple@13.6.6': dependencies: @@ -18962,14 +18948,10 @@ snapshots: fast-glob: 3.3.2 fast-xml-parser: 4.4.0 ora: 5.4.1 - transitivePeerDependencies: - - encoding '@react-native-community/cli-platform-ios@13.6.6': dependencies: '@react-native-community/cli-platform-apple': 13.6.6 - transitivePeerDependencies: - - encoding '@react-native-community/cli-server-api@13.6.6(bufferutil@4.0.8)(utf-8-validate@6.0.4)': dependencies: @@ -18984,7 +18966,6 @@ snapshots: ws: 6.2.2(bufferutil@4.0.8)(utf-8-validate@6.0.4) transitivePeerDependencies: - bufferutil - - encoding - supports-color - utf-8-validate @@ -18995,14 +18976,12 @@ snapshots: execa: 5.1.1 find-up: 5.0.0 mime: 2.6.0 - node-fetch: 2.7.0 + node-fetch: 3.3.2 open: 6.4.0 ora: 5.4.1 semver: 7.5.4 shell-quote: 1.8.1 sudo-prompt: 9.2.1 - transitivePeerDependencies: - - encoding '@react-native-community/cli-types@13.6.6': dependencies: @@ -19029,7 +19008,6 @@ snapshots: semver: 7.5.4 transitivePeerDependencies: - bufferutil - - encoding - supports-color - utf-8-validate @@ -20296,7 +20274,7 @@ snapshots: '@vitest/utils': 1.6.0 magic-string: 0.30.5 sirv: 2.0.4 - vitest: 1.6.0(@types/node@18.15.3)(@vitest/browser@1.6.0)(jsdom@16.7.0(bufferutil@4.0.8)(utf-8-validate@6.0.4))(terser@5.18.2) + vitest: 1.6.0(@types/node@20.11.13)(@vitest/browser@1.6.0)(jsdom@16.7.0(bufferutil@4.0.8)(utf-8-validate@6.0.4))(terser@5.18.2) optionalDependencies: playwright: 1.44.0 webdriverio: 8.39.0(bufferutil@4.0.8)(typescript@5.4.5)(utf-8-validate@6.0.4) @@ -23396,8 +23374,8 @@ snapshots: '@typescript-eslint/parser': 6.9.1(eslint@8.54.0)(typescript@5.2.2) eslint: 8.54.0 eslint-import-resolver-node: 0.3.9 - eslint-import-resolver-typescript: 3.6.1(@typescript-eslint/parser@6.9.1(eslint@8.54.0)(typescript@5.2.2))(eslint-import-resolver-node@0.3.9)(eslint-plugin-import@2.29.0(@typescript-eslint/parser@6.9.1(eslint@8.57.0)(typescript@5.4.5))(eslint@8.54.0))(eslint@8.54.0) - eslint-plugin-import: 2.29.0(@typescript-eslint/parser@6.9.1(eslint@8.54.0)(typescript@5.2.2))(eslint-import-resolver-typescript@3.6.1)(eslint@8.54.0) + eslint-import-resolver-typescript: 3.6.1(@typescript-eslint/parser@6.9.1(eslint@8.54.0)(typescript@5.2.2))(eslint-import-resolver-node@0.3.9)(eslint-plugin-import@2.29.0)(eslint@8.54.0) + eslint-plugin-import: 2.29.0(@typescript-eslint/parser@6.9.1(eslint@8.57.0)(typescript@5.4.5))(eslint@8.54.0) eslint-plugin-jsx-a11y: 6.7.1(eslint@8.54.0) eslint-plugin-react: 7.33.2(eslint@8.54.0) eslint-plugin-react-hooks: 4.6.0(eslint@8.54.0) @@ -23446,13 +23424,13 @@ snapshots: transitivePeerDependencies: - supports-color - eslint-import-resolver-typescript@3.6.1(@typescript-eslint/parser@6.9.1(eslint@8.54.0)(typescript@5.2.2))(eslint-import-resolver-node@0.3.9)(eslint-plugin-import@2.29.0(@typescript-eslint/parser@6.9.1(eslint@8.57.0)(typescript@5.4.5))(eslint@8.54.0))(eslint@8.54.0): + eslint-import-resolver-typescript@3.6.1(@typescript-eslint/parser@6.9.1(eslint@8.54.0)(typescript@5.2.2))(eslint-import-resolver-node@0.3.9)(eslint-plugin-import@2.29.0)(eslint@8.54.0): dependencies: debug: 4.3.5 enhanced-resolve: 5.15.0 eslint: 8.54.0 - eslint-module-utils: 2.8.0(@typescript-eslint/parser@6.9.1(eslint@8.54.0)(typescript@5.2.2))(eslint-import-resolver-node@0.3.9)(eslint-import-resolver-typescript@3.6.1(@typescript-eslint/parser@6.9.1(eslint@8.54.0)(typescript@5.2.2))(eslint-import-resolver-node@0.3.9)(eslint-plugin-import@2.29.0(@typescript-eslint/parser@6.9.1(eslint@8.57.0)(typescript@5.4.5))(eslint@8.54.0))(eslint@8.54.0))(eslint@8.54.0) - eslint-plugin-import: 2.29.0(@typescript-eslint/parser@6.9.1(eslint@8.54.0)(typescript@5.2.2))(eslint-import-resolver-typescript@3.6.1)(eslint@8.54.0) + eslint-module-utils: 2.8.0(@typescript-eslint/parser@6.9.1(eslint@8.54.0)(typescript@5.2.2))(eslint-import-resolver-node@0.3.9)(eslint-import-resolver-typescript@3.6.1(@typescript-eslint/parser@6.9.1(eslint@8.54.0)(typescript@5.2.2))(eslint-import-resolver-node@0.3.9)(eslint-plugin-import@2.29.0)(eslint@8.54.0))(eslint@8.54.0) + eslint-plugin-import: 2.29.0(@typescript-eslint/parser@6.9.1(eslint@8.57.0)(typescript@5.4.5))(eslint@8.54.0) fast-glob: 3.3.2 get-tsconfig: 4.7.2 is-core-module: 2.13.1 @@ -23473,14 +23451,24 @@ snapshots: transitivePeerDependencies: - supports-color - eslint-module-utils@2.8.0(@typescript-eslint/parser@6.9.1(eslint@8.54.0)(typescript@5.2.2))(eslint-import-resolver-node@0.3.9)(eslint-import-resolver-typescript@3.6.1(@typescript-eslint/parser@6.9.1(eslint@8.54.0)(typescript@5.2.2))(eslint-import-resolver-node@0.3.9)(eslint-plugin-import@2.29.0(@typescript-eslint/parser@6.9.1(eslint@8.57.0)(typescript@5.4.5))(eslint@8.54.0))(eslint@8.54.0))(eslint@8.54.0): + eslint-module-utils@2.8.0(@typescript-eslint/parser@6.9.1(eslint@8.54.0)(typescript@5.2.2))(eslint-import-resolver-node@0.3.9)(eslint-import-resolver-typescript@3.6.1(@typescript-eslint/parser@6.9.1(eslint@8.54.0)(typescript@5.2.2))(eslint-import-resolver-node@0.3.9)(eslint-plugin-import@2.29.0)(eslint@8.54.0))(eslint@8.54.0): dependencies: debug: 3.2.7 optionalDependencies: '@typescript-eslint/parser': 6.9.1(eslint@8.54.0)(typescript@5.2.2) eslint: 8.54.0 eslint-import-resolver-node: 0.3.9 - eslint-import-resolver-typescript: 3.6.1(@typescript-eslint/parser@6.9.1(eslint@8.54.0)(typescript@5.2.2))(eslint-import-resolver-node@0.3.9)(eslint-plugin-import@2.29.0(@typescript-eslint/parser@6.9.1(eslint@8.57.0)(typescript@5.4.5))(eslint@8.54.0))(eslint@8.54.0) + eslint-import-resolver-typescript: 3.6.1(@typescript-eslint/parser@6.9.1(eslint@8.54.0)(typescript@5.2.2))(eslint-import-resolver-node@0.3.9)(eslint-plugin-import@2.29.0)(eslint@8.54.0) + transitivePeerDependencies: + - supports-color + + eslint-module-utils@2.8.0(@typescript-eslint/parser@6.9.1(eslint@8.57.0)(typescript@5.4.5))(eslint-import-resolver-node@0.3.9)(eslint@8.54.0): + dependencies: + debug: 3.2.7 + optionalDependencies: + '@typescript-eslint/parser': 6.9.1(eslint@8.57.0)(typescript@5.4.5) + eslint: 8.54.0 + eslint-import-resolver-node: 0.3.9 transitivePeerDependencies: - supports-color @@ -23531,7 +23519,7 @@ snapshots: - eslint-import-resolver-webpack - supports-color - eslint-plugin-import@2.29.0(@typescript-eslint/parser@6.9.1(eslint@8.54.0)(typescript@5.2.2))(eslint-import-resolver-typescript@3.6.1)(eslint@8.54.0): + eslint-plugin-import@2.29.0(@typescript-eslint/parser@6.9.1(eslint@8.57.0)(typescript@5.4.5))(eslint@8.54.0): dependencies: array-includes: 3.1.7 array.prototype.findlastindex: 1.2.3 @@ -23541,7 +23529,7 @@ snapshots: doctrine: 2.1.0 eslint: 8.54.0 eslint-import-resolver-node: 0.3.9 - eslint-module-utils: 2.8.0(@typescript-eslint/parser@6.9.1(eslint@8.54.0)(typescript@5.2.2))(eslint-import-resolver-node@0.3.9)(eslint-import-resolver-typescript@3.6.1(@typescript-eslint/parser@6.9.1(eslint@8.54.0)(typescript@5.2.2))(eslint-import-resolver-node@0.3.9)(eslint-plugin-import@2.29.0(@typescript-eslint/parser@6.9.1(eslint@8.57.0)(typescript@5.4.5))(eslint@8.54.0))(eslint@8.54.0))(eslint@8.54.0) + eslint-module-utils: 2.8.0(@typescript-eslint/parser@6.9.1(eslint@8.57.0)(typescript@5.4.5))(eslint-import-resolver-node@0.3.9)(eslint@8.54.0) hasown: 2.0.0 is-core-module: 2.13.1 is-glob: 4.0.3 @@ -23552,7 +23540,7 @@ snapshots: semver: 6.3.1 tsconfig-paths: 3.14.2 optionalDependencies: - '@typescript-eslint/parser': 6.9.1(eslint@8.54.0)(typescript@5.2.2) + '@typescript-eslint/parser': 6.9.1(eslint@8.57.0)(typescript@5.4.5) transitivePeerDependencies: - eslint-import-resolver-typescript - eslint-import-resolver-webpack From 40615643a542f178b00228e2da448e367e3b5370 Mon Sep 17 00:00:00 2001 From: nedsalk Date: Sun, 7 Jul 2024 16:10:37 +0200 Subject: [PATCH 05/24] chore: changeset --- .changeset/breezy-tables-jam.md | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .changeset/breezy-tables-jam.md diff --git a/.changeset/breezy-tables-jam.md b/.changeset/breezy-tables-jam.md new file mode 100644 index 0000000000..2115f4b874 --- /dev/null +++ b/.changeset/breezy-tables-jam.md @@ -0,0 +1,6 @@ +--- +"@fuel-ts/account": minor +"fuels": minor +--- + +fix!: `launchNode.cleanup` not killing node in single test From f19b83fb9d1dbf0b5e5e51f460ba115d8b939855 Mon Sep 17 00:00:00 2001 From: nedsalk Date: Sun, 7 Jul 2024 16:16:18 +0200 Subject: [PATCH 06/24] chore: changeset v2 --- .changeset/breezy-tables-jam.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/breezy-tables-jam.md b/.changeset/breezy-tables-jam.md index 2115f4b874..d3ac919ea4 100644 --- a/.changeset/breezy-tables-jam.md +++ b/.changeset/breezy-tables-jam.md @@ -3,4 +3,4 @@ "fuels": minor --- -fix!: `launchNode.cleanup` not killing node in single test +fix!: `launchNode.cleanup` not killing node in last test of test group From b87cb98d233aee59ebe57084ce62f03d5ae5671c Mon Sep 17 00:00:00 2001 From: nedsalk Date: Sun, 7 Jul 2024 16:28:05 +0200 Subject: [PATCH 07/24] remove unused type --- packages/fuels/src/cli/commands/dev/autoStartFuelCore.ts | 9 --------- 1 file changed, 9 deletions(-) diff --git a/packages/fuels/src/cli/commands/dev/autoStartFuelCore.ts b/packages/fuels/src/cli/commands/dev/autoStartFuelCore.ts index b82496f56b..303cf60004 100644 --- a/packages/fuels/src/cli/commands/dev/autoStartFuelCore.ts +++ b/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; From db7404af594705e1fa8abd37f831b82953196d8a Mon Sep 17 00:00:00 2001 From: nedsalk Date: Sun, 7 Jul 2024 16:33:03 +0200 Subject: [PATCH 08/24] return early --- packages/account/src/test-utils/launchNode.ts | 27 ++++++++++--------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/packages/account/src/test-utils/launchNode.ts b/packages/account/src/test-utils/launchNode.ts index 7ef20f34c6..f69fa75fd5 100644 --- a/packages/account/src/test-utils/launchNode.ts +++ b/packages/account/src/test-utils/launchNode.ts @@ -218,19 +218,20 @@ export const launchNode = async ({ }; const cleanup = () => { - if (!childState.isDead) { - if (child.pid) { - childState.isDead = true; - process.kill(-child.pid); - } - - // 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 }); - } + if (childState.isDead) { + return; + } + childState.isDead = true; + + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + process.kill(-child.pid!); + + // 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 }); } }; From 48cb1ebc0b6bbc1524f2c7b620de4b4166e4f46a Mon Sep 17 00:00:00 2001 From: nedsalk Date: Sun, 7 Jul 2024 16:33:57 +0200 Subject: [PATCH 09/24] clean up all side effects and then kill --- packages/account/src/test-utils/launchNode.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/account/src/test-utils/launchNode.ts b/packages/account/src/test-utils/launchNode.ts index f69fa75fd5..9e6cb15df8 100644 --- a/packages/account/src/test-utils/launchNode.ts +++ b/packages/account/src/test-utils/launchNode.ts @@ -223,9 +223,6 @@ export const launchNode = async ({ } childState.isDead = true; - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - process.kill(-child.pid!); - // Remove all the listeners we've added. child.stderr.removeAllListeners(); @@ -233,6 +230,9 @@ export const launchNode = async ({ 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. From 16ddc834cd0a56db85277652444eec618cccac17 Mon Sep 17 00:00:00 2001 From: nedsalk Date: Sun, 7 Jul 2024 16:35:47 +0200 Subject: [PATCH 10/24] "a test", not "an test" --- .../account/src/test-utils/launchNode-singular-test.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/account/src/test-utils/launchNode-singular-test.test.ts b/packages/account/src/test-utils/launchNode-singular-test.test.ts index 0c5a3ce4d1..a3c5f61229 100644 --- a/packages/account/src/test-utils/launchNode-singular-test.test.ts +++ b/packages/account/src/test-utils/launchNode-singular-test.test.ts @@ -1,7 +1,7 @@ import { launchNode } from './launchNode'; /** - * The test runner creates an test environment per file, + * The test runner creates a test environment per file, * which we can use to isolate the faulty behavior. */ describe('launchNode-singular-test', () => { From 07d2f6c9f00da34950c7633c73ced288a4200ad2 Mon Sep 17 00:00:00 2001 From: nedsalk Date: Sun, 7 Jul 2024 16:37:11 +0200 Subject: [PATCH 11/24] oops, remove `test.only` --- packages/account/src/test-utils/launchNode.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/account/src/test-utils/launchNode.test.ts b/packages/account/src/test-utils/launchNode.test.ts index 44f1fe08b7..163d45fde9 100644 --- a/packages/account/src/test-utils/launchNode.test.ts +++ b/packages/account/src/test-utils/launchNode.test.ts @@ -128,7 +128,7 @@ describe('launchNode', () => { cleanup(); }); - test.only('calling cleanup multiple times does not retry process killing', async () => { + test('calling cleanup multiple times does not retry process killing', async () => { const killSpy = vi.spyOn(process, 'kill'); const { cleanup } = await launchNode(); From b454fa3b14771334e467765329ce347c5ac46331 Mon Sep 17 00:00:00 2001 From: nedsalk Date: Sun, 7 Jul 2024 16:45:05 +0200 Subject: [PATCH 12/24] add test group --- .../account/src/test-utils/launchNode-singular-test.test.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/account/src/test-utils/launchNode-singular-test.test.ts b/packages/account/src/test-utils/launchNode-singular-test.test.ts index a3c5f61229..7300edf680 100644 --- a/packages/account/src/test-utils/launchNode-singular-test.test.ts +++ b/packages/account/src/test-utils/launchNode-singular-test.test.ts @@ -4,6 +4,9 @@ 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 () => { From aeb7ff8bc9d6fc04cbadd063cdb7ab84b8682900 Mon Sep 17 00:00:00 2001 From: nedsalk Date: Sun, 7 Jul 2024 16:45:37 +0200 Subject: [PATCH 13/24] add `.only` to be explicit about it --- .../account/src/test-utils/launchNode-singular-test.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/account/src/test-utils/launchNode-singular-test.test.ts b/packages/account/src/test-utils/launchNode-singular-test.test.ts index 7300edf680..f11b675797 100644 --- a/packages/account/src/test-utils/launchNode-singular-test.test.ts +++ b/packages/account/src/test-utils/launchNode-singular-test.test.ts @@ -12,7 +12,7 @@ describe('launchNode-singular-test', () => { afterAll(async () => { await expect(fetch(killedNodeUrl)).rejects.toThrow('fetch failed'); }); - test('synchronous cleanup kills node before test runner exits', async () => { + test.only('synchronous cleanup kills node before test runner exits', async () => { const { cleanup, url } = await launchNode(); killedNodeUrl = url; cleanup(); From 44271a10797d52d447e5af8fed89912305dde926 Mon Sep 17 00:00:00 2001 From: nedsalk Date: Mon, 8 Jul 2024 09:28:54 +0200 Subject: [PATCH 14/24] remove `tree-kill` from `fuels` --- packages/fuels/package.json | 1 - pnpm-lock.yaml | 3 --- 2 files changed, 4 deletions(-) diff --git a/packages/fuels/package.json b/packages/fuels/package.json index 782564cae2..1cbe54b98e 100644 --- a/packages/fuels/package.json +++ b/packages/fuels/package.json @@ -84,7 +84,6 @@ "lodash.camelcase": "^4.3.0", "portfinder": "^1.0.32", "toml": "^3.0.0", - "tree-kill": "^1.2.2", "yup": "^1.4.0" }, "devDependencies": { diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 2e5d0f39d7..69935f79fd 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -978,9 +978,6 @@ importers: toml: specifier: ^3.0.0 version: 3.0.0 - tree-kill: - specifier: ^1.2.2 - version: 1.2.2 yup: specifier: ^1.4.0 version: 1.4.0 From 3640d19d26335ab76fdd8aa97b049856f01fa338 Mon Sep 17 00:00:00 2001 From: nedsalk Date: Mon, 8 Jul 2024 09:34:54 +0200 Subject: [PATCH 15/24] re-add accidental `bun` removal --- .knip.json | 1 + 1 file changed, 1 insertion(+) diff --git a/.knip.json b/.knip.json index cb020b4084..2a209555f5 100644 --- a/.knip.json +++ b/.knip.json @@ -7,6 +7,7 @@ "templates/**" ], "ignoreDependencies": [ + "bun", "@/sway-api/*", "@fuel-ts/*", "@internal/fuel-core", From 4e33a343881413449a843fc6976b9d4fdae0111d Mon Sep 17 00:00:00 2001 From: nedsalk Date: Mon, 8 Jul 2024 10:18:43 +0200 Subject: [PATCH 16/24] use object --- .../src/test-utils/launchNode-singular-test.test.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/account/src/test-utils/launchNode-singular-test.test.ts b/packages/account/src/test-utils/launchNode-singular-test.test.ts index f11b675797..6018336983 100644 --- a/packages/account/src/test-utils/launchNode-singular-test.test.ts +++ b/packages/account/src/test-utils/launchNode-singular-test.test.ts @@ -8,13 +8,15 @@ import { launchNode } from './launchNode'; * @group node */ describe('launchNode-singular-test', () => { - let killedNodeUrl = ''; + const killedNode = { + url: '', + }; afterAll(async () => { - await expect(fetch(killedNodeUrl)).rejects.toThrow('fetch failed'); + await expect(fetch(killedNode.url)).rejects.toThrow('fetch failed'); }); test.only('synchronous cleanup kills node before test runner exits', async () => { const { cleanup, url } = await launchNode(); - killedNodeUrl = url; + killedNode.url = url; cleanup(); }); }); From eb091155536ea6eb3ed70056f44423c0fd3e35df Mon Sep 17 00:00:00 2001 From: nedsalk Date: Mon, 8 Jul 2024 10:26:41 +0200 Subject: [PATCH 17/24] works now? --- .../account/src/test-utils/launchNode-singular-test.test.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/account/src/test-utils/launchNode-singular-test.test.ts b/packages/account/src/test-utils/launchNode-singular-test.test.ts index 6018336983..17237e0069 100644 --- a/packages/account/src/test-utils/launchNode-singular-test.test.ts +++ b/packages/account/src/test-utils/launchNode-singular-test.test.ts @@ -11,11 +11,11 @@ describe('launchNode-singular-test', () => { const killedNode = { url: '', }; - afterAll(async () => { + afterEach(async () => { await expect(fetch(killedNode.url)).rejects.toThrow('fetch failed'); }); - test.only('synchronous cleanup kills node before test runner exits', async () => { - const { cleanup, url } = await launchNode(); + test('synchronous cleanup kills node before test runner exits', async () => { + const { cleanup, url } = await launchNode({ loggingEnabled: false }); killedNode.url = url; cleanup(); }); From 1babf7068d5c19f0b23eda05a664c72f5f49ddc9 Mon Sep 17 00:00:00 2001 From: nedsalk Date: Tue, 9 Jul 2024 16:31:20 +0200 Subject: [PATCH 18/24] remove side effects on `'error'` and `'exit'` effects --- .../account/src/test-utils/launchNode.test.ts | 61 ++++++++++++++++++- packages/account/src/test-utils/launchNode.ts | 20 +++--- 2 files changed, 70 insertions(+), 11 deletions(-) diff --git a/packages/account/src/test-utils/launchNode.test.ts b/packages/account/src/test-utils/launchNode.test.ts index 163d45fde9..f3c396811f 100644 --- a/packages/account/src/test-utils/launchNode.test.ts +++ b/packages/account/src/test-utils/launchNode.test.ts @@ -1,8 +1,9 @@ import { ErrorCode } from '@fuel-ts/errors'; import { safeExec, expectToThrowFuelError } from '@fuel-ts/errors/test-utils'; -import { defaultSnapshotConfigs } from '@fuel-ts/utils'; +import { defaultSnapshotConfigs, sleep } from '@fuel-ts/utils'; import { waitUntilUnreachable } from '@fuel-ts/utils/test-utils'; import * as childProcessMod from 'child_process'; +import * as fsMod from 'fs'; import { Provider } from '../providers'; @@ -16,6 +17,14 @@ vi.mock('child_process', async () => { }; }); +vi.mock('fs', async () => { + const mod = await vi.importActual('fs'); + return { + __esModule: true, + ...mod, + }; +}); + /** * @group node */ @@ -128,10 +137,58 @@ describe('launchNode', () => { cleanup(); }); + test('cleanup removes temporary directory', async () => { + const mkdirSyncSpy = vi.spyOn(fsMod, 'mkdirSync'); + const { cleanup } = await launchNode(); + + expect(mkdirSyncSpy).toHaveBeenCalledTimes(1); + const tempDirPath = mkdirSyncSpy.mock.calls[0][0]; + cleanup(); + + // wait until cleanup finishes (done via events) + await sleep(1500); + expect(fsMod.existsSync(tempDirPath)).toBeFalsy(); + }); + + test('temporary directory gets removed on error', async () => { + const mkdirSyncSpy = vi.spyOn(fsMod, 'mkdirSync'); + + const invalidCoin = { + asset_id: 'whatever', + tx_id: '', + output_index: 0, + tx_pointer_block_height: 0, + tx_pointer_tx_idx: 0, + owner: '', + amount: 0, + }; + + const { error } = await safeExec(async () => + launchNode({ + loggingEnabled: false, + snapshotConfig: { + ...defaultSnapshotConfigs, + stateConfig: { + coins: [invalidCoin], + messages: [], + }, + }, + }) + ); + expect(error).toBeDefined(); + + expect(mkdirSyncSpy).toHaveBeenCalledTimes(1); + const tempDirPath = mkdirSyncSpy.mock.calls[0][0]; + + // wait until cleanup finishes (done via events) + await sleep(1500); + expect(fsMod.existsSync(tempDirPath)).toBeFalsy(); + }); + test('calling cleanup multiple times does not retry process killing', async () => { const killSpy = vi.spyOn(process, 'kill'); - const { cleanup } = await launchNode(); + const { cleanup } = await launchNode({ loggingEnabled: false }); cleanup(); diff --git a/packages/account/src/test-utils/launchNode.ts b/packages/account/src/test-utils/launchNode.ts index 9e6cb15df8..62acfe6136 100644 --- a/packages/account/src/test-utils/launchNode.ts +++ b/packages/account/src/test-utils/launchNode.ts @@ -213,6 +213,17 @@ export const launchNode = async ({ }); } + const removeSideffects = () => { + child.stderr.removeAllListeners(); + + if (existsSync(tempDir)) { + rmSync(tempDir, { recursive: true }); + } + }; + + child.on('error', removeSideffects); + child.on('exit', removeSideffects); + const childState = { isDead: false, }; @@ -222,15 +233,6 @@ export const launchNode = async ({ 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!); }; From a73ff578105cfabf4f7e4a3837b6b5210e2e176b Mon Sep 17 00:00:00 2001 From: nedsalk Date: Tue, 9 Jul 2024 16:48:45 +0200 Subject: [PATCH 19/24] cover `child.pid` as `undefined` --- packages/account/src/test-utils/launchNode.ts | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/packages/account/src/test-utils/launchNode.ts b/packages/account/src/test-utils/launchNode.ts index 62acfe6136..1021035861 100644 --- a/packages/account/src/test-utils/launchNode.ts +++ b/packages/account/src/test-utils/launchNode.ts @@ -215,7 +215,6 @@ export const launchNode = async ({ const removeSideffects = () => { child.stderr.removeAllListeners(); - if (existsSync(tempDir)) { rmSync(tempDir, { recursive: true }); } @@ -233,8 +232,14 @@ export const launchNode = async ({ return; } childState.isDead = true; - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - process.kill(-child.pid!); + + if (child.pid !== undefined) { + process.kill(child.pid); + } else { + removeSideffects(); + // eslint-disable-next-line no-console + console.error('No PID available for the child process, unable to kill launched node'); + } }; // Look for a specific graphql start point in the output. From 823c33a83fd0474ead05e18889fc6578f8f7ec50 Mon Sep 17 00:00:00 2001 From: nedsalk Date: Wed, 10 Jul 2024 09:30:31 +0200 Subject: [PATCH 20/24] fix: kill process group --- packages/account/src/test-utils/launchNode.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/account/src/test-utils/launchNode.ts b/packages/account/src/test-utils/launchNode.ts index 1021035861..850c500f6b 100644 --- a/packages/account/src/test-utils/launchNode.ts +++ b/packages/account/src/test-utils/launchNode.ts @@ -234,7 +234,7 @@ export const launchNode = async ({ childState.isDead = true; if (child.pid !== undefined) { - process.kill(child.pid); + process.kill(-child.pid); } else { removeSideffects(); // eslint-disable-next-line no-console From ebcf55c6aa0440a851ddabd6c828b4e64813bacc Mon Sep 17 00:00:00 2001 From: nedsalk Date: Wed, 10 Jul 2024 13:37:00 +0200 Subject: [PATCH 21/24] verify external killing removes side-effects --- .../account/src/test-utils/launchNode.test.ts | 19 +++++++++++++++++++ packages/account/src/test-utils/launchNode.ts | 2 ++ 2 files changed, 21 insertions(+) diff --git a/packages/account/src/test-utils/launchNode.test.ts b/packages/account/src/test-utils/launchNode.test.ts index f3c396811f..bba6449b6f 100644 --- a/packages/account/src/test-utils/launchNode.test.ts +++ b/packages/account/src/test-utils/launchNode.test.ts @@ -198,4 +198,23 @@ describe('launchNode', () => { expect(killSpy).toHaveBeenCalledTimes(1); }); + + test( + 'external killing of node runs side-effect cleanup', + async () => { + const mkdirSyncSpy = vi.spyOn(fsMod, 'mkdirSync'); + + const { pid } = await launchNode({ loggingEnabled: false }); + + expect(mkdirSyncSpy).toHaveBeenCalledTimes(1); + const tempDirPath = mkdirSyncSpy.mock.calls[0][0]; + + expect(tempDirPath).toBeTruthy(); + childProcessMod.execSync(`kill -- -${pid}`); + // wait until cleanup finishes (done via events) + await sleep(1500); + expect(fsMod.existsSync(tempDirPath)).toBeFalsy(); + }, + { timeout: 1000000 } + ); }); diff --git a/packages/account/src/test-utils/launchNode.ts b/packages/account/src/test-utils/launchNode.ts index 850c500f6b..f39de0da7c 100644 --- a/packages/account/src/test-utils/launchNode.ts +++ b/packages/account/src/test-utils/launchNode.ts @@ -54,6 +54,7 @@ export type LaunchNodeResult = Promise<{ port: string; url: string; snapshotDir: string; + pid: number; }>; function getFinalStateConfigJSON({ stateConfig, chainConfig }: SnapshotConfigs) { @@ -260,6 +261,7 @@ export const launchNode = async ({ port: realPort, url: `http://${realIp}:${realPort}/v1/graphql`, snapshotDir: snapshotDirToUse as string, + pid: child.pid as number, }); } if (/error/i.test(text)) { From b7cfab64e486735e0aff3b436b57b38a8afa1291 Mon Sep 17 00:00:00 2001 From: nedsalk Date: Wed, 10 Jul 2024 13:39:36 +0200 Subject: [PATCH 22/24] remove unnecessary truthy check --- packages/account/src/test-utils/launchNode.test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/account/src/test-utils/launchNode.test.ts b/packages/account/src/test-utils/launchNode.test.ts index bba6449b6f..0c787c20a9 100644 --- a/packages/account/src/test-utils/launchNode.test.ts +++ b/packages/account/src/test-utils/launchNode.test.ts @@ -209,7 +209,6 @@ describe('launchNode', () => { expect(mkdirSyncSpy).toHaveBeenCalledTimes(1); const tempDirPath = mkdirSyncSpy.mock.calls[0][0]; - expect(tempDirPath).toBeTruthy(); childProcessMod.execSync(`kill -- -${pid}`); // wait until cleanup finishes (done via events) await sleep(1500); From 3e1b704b6c913f0b9e1164c652fa63a7029bcc3b Mon Sep 17 00:00:00 2001 From: nedsalk Date: Wed, 10 Jul 2024 13:39:48 +0200 Subject: [PATCH 23/24] remove enormous timeout --- .../account/src/test-utils/launchNode.test.ts | 30 ++++++++----------- 1 file changed, 13 insertions(+), 17 deletions(-) diff --git a/packages/account/src/test-utils/launchNode.test.ts b/packages/account/src/test-utils/launchNode.test.ts index 0c787c20a9..c18ac7dd5c 100644 --- a/packages/account/src/test-utils/launchNode.test.ts +++ b/packages/account/src/test-utils/launchNode.test.ts @@ -199,21 +199,17 @@ describe('launchNode', () => { expect(killSpy).toHaveBeenCalledTimes(1); }); - test( - 'external killing of node runs side-effect cleanup', - async () => { - const mkdirSyncSpy = vi.spyOn(fsMod, 'mkdirSync'); - - const { pid } = await launchNode({ loggingEnabled: false }); - - expect(mkdirSyncSpy).toHaveBeenCalledTimes(1); - const tempDirPath = mkdirSyncSpy.mock.calls[0][0]; - - childProcessMod.execSync(`kill -- -${pid}`); - // wait until cleanup finishes (done via events) - await sleep(1500); - expect(fsMod.existsSync(tempDirPath)).toBeFalsy(); - }, - { timeout: 1000000 } - ); + test('external killing of node runs side-effect cleanup', async () => { + const mkdirSyncSpy = vi.spyOn(fsMod, 'mkdirSync'); + + const { pid } = await launchNode({ loggingEnabled: false }); + + expect(mkdirSyncSpy).toHaveBeenCalledTimes(1); + const tempDirPath = mkdirSyncSpy.mock.calls[0][0]; + + childProcessMod.execSync(`kill -- -${pid}`); + // wait until cleanup finishes (done via events) + await sleep(1500); + expect(fsMod.existsSync(tempDirPath)).toBeFalsy(); + }); }); From e7abaeed48483675abe3c913397c8045a3e5e54a Mon Sep 17 00:00:00 2001 From: nedsalk Date: Wed, 10 Jul 2024 17:55:23 +0200 Subject: [PATCH 24/24] avoid dirty exit when node already killed --- .../account/src/test-utils/launchNode.test.ts | 17 +++++++++++++++++ packages/account/src/test-utils/launchNode.ts | 16 ++++++++++++++-- 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/packages/account/src/test-utils/launchNode.test.ts b/packages/account/src/test-utils/launchNode.test.ts index c18ac7dd5c..8353078094 100644 --- a/packages/account/src/test-utils/launchNode.test.ts +++ b/packages/account/src/test-utils/launchNode.test.ts @@ -212,4 +212,21 @@ describe('launchNode', () => { await sleep(1500); expect(fsMod.existsSync(tempDirPath)).toBeFalsy(); }); + + test('calling cleanup on externally killed node does not throw', async () => { + const mkdirSyncSpy = vi.spyOn(fsMod, 'mkdirSync'); + const logSpy = vi.spyOn(console, 'log'); + + const { pid, cleanup } = await launchNode({ loggingEnabled: false }); + expect(mkdirSyncSpy).toHaveBeenCalledTimes(1); + + childProcessMod.execSync(`kill -- -${pid}`); + // wait until cleanup finishes (done via events) + await sleep(1500); + cleanup(); + + expect(logSpy).toHaveBeenCalledWith( + `fuel-core node under pid ${pid} does not exist. The node might have been killed before cleanup was called. Exiting cleanly.` + ); + }); }); diff --git a/packages/account/src/test-utils/launchNode.ts b/packages/account/src/test-utils/launchNode.ts index f39de0da7c..0554acca8f 100644 --- a/packages/account/src/test-utils/launchNode.ts +++ b/packages/account/src/test-utils/launchNode.ts @@ -234,10 +234,22 @@ export const launchNode = async ({ } childState.isDead = true; + removeSideffects(); if (child.pid !== undefined) { - process.kill(-child.pid); + try { + process.kill(-child.pid); + } catch (e) { + const error = e as Error & { code: string }; + if (error.code === 'ESRCH') { + // eslint-disable-next-line no-console + console.log( + `fuel-core node under pid ${child.pid} does not exist. The node might have been killed before cleanup was called. Exiting cleanly.` + ); + } else { + throw e; + } + } } else { - removeSideffects(); // eslint-disable-next-line no-console console.error('No PID available for the child process, unable to kill launched node'); }