From c187c5ecfc3e207cc4acc2bd13a6ee203c6ab1dc Mon Sep 17 00:00:00 2001 From: nedsalk Date: Fri, 22 Nov 2024 13:13:21 +0100 Subject: [PATCH 1/5] =?UTF-8?q?fix:=20=E2=9C=85=20Your=20fuels=20version?= =?UTF-8?q?=20is=20up=20to=20date:=200.97.0?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Config file not found! cleanup not killing node --- .changeset/kind-students-kneel.md | 6 ++ .github/actions/test-setup/action.yaml | 6 ++ packages/account/src/test-utils/launchNode.ts | 1 - packages/fuels/src/cli.ts | 4 + .../proxy/types/Src14OwnedProxyFactory.ts | 2 +- .../src/cli/commands/dev/autoStartFuelCore.ts | 6 +- packages/fuels/src/cli/commands/init/index.ts | 3 +- .../fuels/src/cli/templates/fuels.config.hbs | 3 + .../fuels/src/cli/templates/fuels.config.ts | 1 + packages/fuels/test/features/dev-2.test.ts | 79 +++++++++++++++++++ .../fuels/test/utils/findChildProcessPid.ts | 25 ++++++ packages/fuels/test/utils/isProcessRunning.ts | 19 +++++ 12 files changed, 148 insertions(+), 7 deletions(-) create mode 100644 .changeset/kind-students-kneel.md create mode 100644 packages/fuels/test/features/dev-2.test.ts create mode 100644 packages/fuels/test/utils/findChildProcessPid.ts create mode 100644 packages/fuels/test/utils/isProcessRunning.ts diff --git a/.changeset/kind-students-kneel.md b/.changeset/kind-students-kneel.md new file mode 100644 index 00000000000..dfbaf4f9f50 --- /dev/null +++ b/.changeset/kind-students-kneel.md @@ -0,0 +1,6 @@ +--- +"@fuel-ts/account": patch +"fuels": patch +--- + +fix: `fuels dev` cleanup not killing node diff --git a/.github/actions/test-setup/action.yaml b/.github/actions/test-setup/action.yaml index 8d7c3d6cf0c..744b2a7ad1b 100644 --- a/.github/actions/test-setup/action.yaml +++ b/.github/actions/test-setup/action.yaml @@ -33,6 +33,12 @@ runs: version: ${{ inputs.pnpm-version }} run_install: true + - name: Setup forc and fuel-core paths + shell: bash + run: | + echo "$PWD/internal/forc/forc-binaries" >> $GITHUB_PATH + echo "$PWD/internal/fuel-core/fuel-core-binaries" >> $GITHUB_PATH + - name: Setup Bun if: ${{ inputs.should-install-bun == 'true' }} uses: oven-sh/setup-bun@v1 diff --git a/packages/account/src/test-utils/launchNode.ts b/packages/account/src/test-utils/launchNode.ts index 9a5a6f61ddc..21c47cd3ce2 100644 --- a/packages/account/src/test-utils/launchNode.ts +++ b/packages/account/src/test-utils/launchNode.ts @@ -225,7 +225,6 @@ export const launchNode = async ({ } childState.isDead = true; - removeSideffects(); if (child.pid !== undefined) { try { process.kill(-child.pid); diff --git a/packages/fuels/src/cli.ts b/packages/fuels/src/cli.ts index 522665cd7f8..3c37a6f6de5 100644 --- a/packages/fuels/src/cli.ts +++ b/packages/fuels/src/cli.ts @@ -60,6 +60,10 @@ export const configureCli = () => { .option('--forc-path ', 'Path to the `forc` binary') .option('--fuel-core-path ', 'Path to the `fuel-core` binary') .option('--auto-start-fuel-core', 'Auto-starts a `fuel-core` node during `dev` command') + .option( + '--fuel-core-port ', + 'Port to use when starting a local `fuel-core` node for dev mode' + ) .action(withProgram(command, Commands.init, init)); (command = program.command(Commands.dev)) diff --git a/packages/fuels/src/cli/commands/deploy/proxy/types/Src14OwnedProxyFactory.ts b/packages/fuels/src/cli/commands/deploy/proxy/types/Src14OwnedProxyFactory.ts index 0a142a18e5d..f21c3c8038a 100644 --- a/packages/fuels/src/cli/commands/deploy/proxy/types/Src14OwnedProxyFactory.ts +++ b/packages/fuels/src/cli/commands/deploy/proxy/types/Src14OwnedProxyFactory.ts @@ -31,7 +31,7 @@ export class Src14OwnedProxyFactory extends ContractFactory { static deploy ( wallet: Account, options: DeployContractOptions = {} - ): ReturnType { + ) { const factory = new Src14OwnedProxyFactory(wallet); return factory.deploy(options); } diff --git a/packages/fuels/src/cli/commands/dev/autoStartFuelCore.ts b/packages/fuels/src/cli/commands/dev/autoStartFuelCore.ts index 303cf600047..3e9047a297f 100644 --- a/packages/fuels/src/cli/commands/dev/autoStartFuelCore.ts +++ b/packages/fuels/src/cli/commands/dev/autoStartFuelCore.ts @@ -25,9 +25,7 @@ export const autoStartFuelCore = async (config: FuelsConfig) => { const port = config.fuelCorePort ?? (await getPortPromise({ port: 4000 })); - const providerUrl = `http://${accessIp}:${port}/v1/graphql`; - - const { cleanup, snapshotDir } = await launchNode({ + const { cleanup, snapshotDir, url } = await launchNode({ args: [ ['--snapshot', config.snapshotDir], ['--db-type', 'in-memory'], @@ -43,7 +41,7 @@ export const autoStartFuelCore = async (config: FuelsConfig) => { bindIp, accessIp, port, - providerUrl, + providerUrl: url, snapshotDir, killChildProcess: cleanup, }; diff --git a/packages/fuels/src/cli/commands/init/index.ts b/packages/fuels/src/cli/commands/init/index.ts index 0f537c5431a..cf15d11af49 100644 --- a/packages/fuels/src/cli/commands/init/index.ts +++ b/packages/fuels/src/cli/commands/init/index.ts @@ -10,7 +10,7 @@ import { log } from '../../utils/logger'; export function init(program: Command) { const options = program.opts(); - const { path, autoStartFuelCore, forcPath, fuelCorePath } = options; + const { path, autoStartFuelCore, forcPath, fuelCorePath, fuelCorePort } = options; let workspace: string | undefined; let absoluteWorkspace: string | undefined; @@ -61,6 +61,7 @@ export function init(program: Command) { forcPath, fuelCorePath, autoStartFuelCore, + fuelCorePort, }); writeFileSync(fuelsConfigPath, renderedConfig); diff --git a/packages/fuels/src/cli/templates/fuels.config.hbs b/packages/fuels/src/cli/templates/fuels.config.hbs index dc351121c38..33ceaa33b18 100644 --- a/packages/fuels/src/cli/templates/fuels.config.hbs +++ b/packages/fuels/src/cli/templates/fuels.config.hbs @@ -36,6 +36,9 @@ export default createConfig({ {{#if (isDefined autoStartFuelCore)}} autoStartFuelCore: {{autoStartFuelCore}}, {{/if}} + {{#if (isDefined fuelCorePort)}} + fuelCorePort: {{fuelCorePort}}, + {{/if}} }); /** diff --git a/packages/fuels/src/cli/templates/fuels.config.ts b/packages/fuels/src/cli/templates/fuels.config.ts index 71930e975a9..cda74e22e1a 100644 --- a/packages/fuels/src/cli/templates/fuels.config.ts +++ b/packages/fuels/src/cli/templates/fuels.config.ts @@ -16,6 +16,7 @@ export function renderFuelsConfigTemplate(props: { forcPath?: string; fuelCorePath?: string; autoStartFuelCore?: boolean; + fuelCorePort?: string; }) { const renderTemplate = Handlebars.compile(fuelsConfigTemplate, { strict: true, diff --git a/packages/fuels/test/features/dev-2.test.ts b/packages/fuels/test/features/dev-2.test.ts new file mode 100644 index 00000000000..a40487cde5a --- /dev/null +++ b/packages/fuels/test/features/dev-2.test.ts @@ -0,0 +1,79 @@ +import { execSync, spawn } from 'child_process'; +import { mkdirSync, rmSync } from 'fs'; +import { tmpdir } from 'os'; +import path from 'path'; + +import { deferPromise, randomUUID } from '../../src'; +import { findChildProcessPid } from '../utils/findChildProcessPid'; +import { isProcessRunning } from '../utils/isProcessRunning'; + +function runInit() { + const fuelsPath = path.join(process.cwd(), 'packages/fuels'); + + const init = path.join(tmpdir(), '.fuels', 'tests', randomUUID()); + + mkdirSync(init, { recursive: true }); + + execSync('pnpm init', { cwd: init }); + execSync(`pnpm link ${fuelsPath}`, { cwd: init }); + + const contractDir = path.join(init, 'contract'); + const outputDir = path.join(init, 'output'); + mkdirSync(contractDir); + mkdirSync(outputDir); + + execSync(`${process.env.FORC_PATH} init`, { cwd: contractDir }); + execSync(`pnpm fuels init -o ${outputDir} -c ${contractDir} --fuel-core-port 0`, { cwd: init }); + + return { + init, + [Symbol.dispose]: () => { + rmSync(init, { recursive: true }); + }, + }; +} + +/** + * @group node + */ +describe('dev', () => { + it( + 'cleans up resources on graceful shutdown', + async () => { + using paths = runInit(); + + const devProcess = spawn('pnpm fuels dev', { + shell: 'bash', + detached: true, + cwd: paths.init, + }); + + const devCompleted = deferPromise(); + + devProcess.stdout.on('data', (chunk) => { + const text = chunk.toString(); + if (text.indexOf('Dev completed successfully!') !== -1) { + devCompleted.resolve(undefined); + } + }); + + await devCompleted.promise; + + const devExited = deferPromise(); + devProcess.on('exit', () => { + devExited.resolve(undefined); + }); + + const devPid = devProcess.pid as number; + + const fuelCorePid = findChildProcessPid(devPid, 'fuel-core') as number; + + process.kill(-devPid, 'SIGINT'); + + await devExited.promise; + + expect(isProcessRunning(fuelCorePid)).toBe(false); + }, + { timeout: 15000 } + ); +}); diff --git a/packages/fuels/test/utils/findChildProcessPid.ts b/packages/fuels/test/utils/findChildProcessPid.ts new file mode 100644 index 00000000000..69b35a1fdb1 --- /dev/null +++ b/packages/fuels/test/utils/findChildProcessPid.ts @@ -0,0 +1,25 @@ +import { execSync } from 'child_process'; + +export function findChildProcessPid( + parentPid: number, + childProcessName: string +): number | undefined { + const childProcesses = execSync(`ps --ppid ${parentPid} -o pid,cmd --no-headers || true`) + .toString() + .split('\n') + .map((s) => s.trim()) + .filter((s) => s !== ''); + + for (const cp of childProcesses) { + const [pid, name] = cp.split(' '); + if (name.indexOf(childProcessName) !== -1) { + return +pid; + } + const childPid = findChildProcessPid(+pid, childProcessName); + if (childPid) { + return childPid; + } + } + + return undefined; +} diff --git a/packages/fuels/test/utils/isProcessRunning.ts b/packages/fuels/test/utils/isProcessRunning.ts new file mode 100644 index 00000000000..eb7ec7be39c --- /dev/null +++ b/packages/fuels/test/utils/isProcessRunning.ts @@ -0,0 +1,19 @@ +export function isProcessRunning(pid: number) { + try { + // Check if the process exists + process.kill(pid, 0); + return true; // If no error, the process is running + } catch (e) { + const error = e as Error & { code: string }; + // Error codes: + // ESRCH: No such process + // EPERM: Permission denied (you don't have permissions to check) + if (error.code === 'ESRCH') { + return false; // No such process + } + if (error.code === 'EPERM') { + return true; // Process exists, but we don't have permission to send a signal + } + throw error; // Some other unexpected error + } +} From 1b81f2add8e451f1b12ed7488ed7f2de6fa03c11 Mon Sep 17 00:00:00 2001 From: nedsalk Date: Fri, 22 Nov 2024 13:27:10 +0100 Subject: [PATCH 2/5] fix missing FORC_PATH --- vitest.global-setup.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/vitest.global-setup.ts b/vitest.global-setup.ts index 48593227974..76715c2a703 100644 --- a/vitest.global-setup.ts +++ b/vitest.global-setup.ts @@ -1,3 +1,4 @@ export default function setup() { process.env.FUEL_CORE_PATH = 'fuels-core'; + process.env.FORC_PATH = 'fuels-forc'; } From 05f3dec682f62fe8ea4cc977c7e2b80f09b93119 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nedim=20Salki=C4=87?= Date: Fri, 22 Nov 2024 13:28:18 +0100 Subject: [PATCH 3/5] Fix code scanning alert no. 78: Shell command built from environment values Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> --- packages/fuels/test/features/dev-2.test.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/fuels/test/features/dev-2.test.ts b/packages/fuels/test/features/dev-2.test.ts index a40487cde5a..51215d2b04a 100644 --- a/packages/fuels/test/features/dev-2.test.ts +++ b/packages/fuels/test/features/dev-2.test.ts @@ -1,4 +1,4 @@ -import { execSync, spawn } from 'child_process'; +import { execSync, execFileSync, spawn } from 'child_process'; import { mkdirSync, rmSync } from 'fs'; import { tmpdir } from 'os'; import path from 'path'; @@ -14,8 +14,8 @@ function runInit() { mkdirSync(init, { recursive: true }); - execSync('pnpm init', { cwd: init }); - execSync(`pnpm link ${fuelsPath}`, { cwd: init }); + execFileSync('pnpm', ['init'], { cwd: init }); + execFileSync('pnpm', ['link', fuelsPath], { cwd: init }); const contractDir = path.join(init, 'contract'); const outputDir = path.join(init, 'output'); From 0c11d3bd926744d82a2d4625853ba0053d5a0fcc Mon Sep 17 00:00:00 2001 From: nedsalk Date: Fri, 22 Nov 2024 14:28:04 +0100 Subject: [PATCH 4/5] fix test --- packages/fuels/test/features/dev-2.test.ts | 6 +-- .../fuels/test/utils/findChildProcessPid.ts | 25 --------- packages/fuels/test/utils/isProcessRunning.ts | 19 ------- packages/fuels/test/utils/processUtils.ts | 52 +++++++++++++++++++ 4 files changed, 55 insertions(+), 47 deletions(-) delete mode 100644 packages/fuels/test/utils/findChildProcessPid.ts delete mode 100644 packages/fuels/test/utils/isProcessRunning.ts create mode 100644 packages/fuels/test/utils/processUtils.ts diff --git a/packages/fuels/test/features/dev-2.test.ts b/packages/fuels/test/features/dev-2.test.ts index 51215d2b04a..ffbfdc4f780 100644 --- a/packages/fuels/test/features/dev-2.test.ts +++ b/packages/fuels/test/features/dev-2.test.ts @@ -4,8 +4,7 @@ import { tmpdir } from 'os'; import path from 'path'; import { deferPromise, randomUUID } from '../../src'; -import { findChildProcessPid } from '../utils/findChildProcessPid'; -import { isProcessRunning } from '../utils/isProcessRunning'; +import { findChildProcessPid, waitProcessEnd } from '../utils/processUtils'; function runInit() { const fuelsPath = path.join(process.cwd(), 'packages/fuels'); @@ -72,7 +71,8 @@ describe('dev', () => { await devExited.promise; - expect(isProcessRunning(fuelCorePid)).toBe(false); + // if it finishes before timeout, it means the process was killed successfully + await waitProcessEnd(fuelCorePid); }, { timeout: 15000 } ); diff --git a/packages/fuels/test/utils/findChildProcessPid.ts b/packages/fuels/test/utils/findChildProcessPid.ts deleted file mode 100644 index 69b35a1fdb1..00000000000 --- a/packages/fuels/test/utils/findChildProcessPid.ts +++ /dev/null @@ -1,25 +0,0 @@ -import { execSync } from 'child_process'; - -export function findChildProcessPid( - parentPid: number, - childProcessName: string -): number | undefined { - const childProcesses = execSync(`ps --ppid ${parentPid} -o pid,cmd --no-headers || true`) - .toString() - .split('\n') - .map((s) => s.trim()) - .filter((s) => s !== ''); - - for (const cp of childProcesses) { - const [pid, name] = cp.split(' '); - if (name.indexOf(childProcessName) !== -1) { - return +pid; - } - const childPid = findChildProcessPid(+pid, childProcessName); - if (childPid) { - return childPid; - } - } - - return undefined; -} diff --git a/packages/fuels/test/utils/isProcessRunning.ts b/packages/fuels/test/utils/isProcessRunning.ts deleted file mode 100644 index eb7ec7be39c..00000000000 --- a/packages/fuels/test/utils/isProcessRunning.ts +++ /dev/null @@ -1,19 +0,0 @@ -export function isProcessRunning(pid: number) { - try { - // Check if the process exists - process.kill(pid, 0); - return true; // If no error, the process is running - } catch (e) { - const error = e as Error & { code: string }; - // Error codes: - // ESRCH: No such process - // EPERM: Permission denied (you don't have permissions to check) - if (error.code === 'ESRCH') { - return false; // No such process - } - if (error.code === 'EPERM') { - return true; // Process exists, but we don't have permission to send a signal - } - throw error; // Some other unexpected error - } -} diff --git a/packages/fuels/test/utils/processUtils.ts b/packages/fuels/test/utils/processUtils.ts new file mode 100644 index 00000000000..1ebc049a1bb --- /dev/null +++ b/packages/fuels/test/utils/processUtils.ts @@ -0,0 +1,52 @@ +import { sleep } from '@fuel-ts/utils'; +import { execSync } from 'child_process'; + +export function findChildProcessPid( + parentPid: number, + childProcessName: string +): number | undefined { + const childProcesses = execSync(`ps --ppid ${parentPid} -o pid,cmd --no-headers || true`) + .toString() + .split('\n') + .map((s) => s.trim()) + .filter((s) => s !== ''); + + for (const cp of childProcesses) { + const [pid, name] = cp.split(' '); + if (name.indexOf(childProcessName) !== -1) { + return +pid; + } + const childPid = findChildProcessPid(+pid, childProcessName); + if (childPid) { + return childPid; + } + } + + return undefined; +} + +function isProcessRunning(pid: number) { + try { + // Check if the process exists + process.kill(pid, 0); + return true; // If no error, the process is running + } catch (e) { + const error = e as Error & { code: string }; + // Error codes: + // ESRCH: No such process + // EPERM: Permission denied (you don't have permissions to check) + if (error.code === 'ESRCH') { + return false; // No such process + } + if (error.code === 'EPERM') { + return true; // Process exists, but we don't have permission to send a signal + } + throw error; // Some other unexpected error + } +} + +export async function waitProcessEnd(pid: number) { + while (isProcessRunning(pid)) { + await sleep(100); + } +} From 3f525ceae7770503c4cf268e37d68c4877696e90 Mon Sep 17 00:00:00 2001 From: nedsalk Date: Fri, 22 Nov 2024 14:32:11 +0100 Subject: [PATCH 5/5] add explanatory comment --- packages/fuels/test/features/dev-2.test.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/fuels/test/features/dev-2.test.ts b/packages/fuels/test/features/dev-2.test.ts index ffbfdc4f780..01924679182 100644 --- a/packages/fuels/test/features/dev-2.test.ts +++ b/packages/fuels/test/features/dev-2.test.ts @@ -67,6 +67,8 @@ describe('dev', () => { const fuelCorePid = findChildProcessPid(devPid, 'fuel-core') as number; + // we kill the pnpm fuels dev process group + // and we want to verify that the fuel-core process is also killed process.kill(-devPid, 'SIGINT'); await devExited.promise;