Skip to content

Commit

Permalink
fix: enable fuel-core node cleanup to work in Bun (#3438)
Browse files Browse the repository at this point in the history
* fix: issue with bun not kill processes

* chore: removed `console.log`'s from tests

* chore: ensure all process children killed

* chore: changeset

* chore: release

* chore: fix test

* chore: un-release

* chore: pls fix

* chore: include custom .d.ts files

* chore: fix test

* chore: removed flaky check

* chore: suppressing errors
  • Loading branch information
petertonysmith94 authored Dec 2, 2024
1 parent 270b507 commit 4131e74
Show file tree
Hide file tree
Showing 6 changed files with 36 additions and 18 deletions.
5 changes: 5 additions & 0 deletions .changeset/wise-ladybugs-nail.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@fuel-ts/account": patch
---

fix: enable `fuel-core` node cleanup to work in Bun
4 changes: 2 additions & 2 deletions packages/account/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -68,16 +68,16 @@
"ramda": "^0.30.1"
},
"devDependencies": {
"type-fest": "^4.26.1",
"@fuel-ts/hasher": "workspace:*",
"@fuel-ts/math": "workspace:*",
"@fuel-ts/utils": "workspace:*",
"@graphql-codegen/cli": "^5.0.3",
"@graphql-codegen/typescript": "^4.0.9",
"@graphql-codegen/typescript-generic-sdk": "^4.0.1",
"@graphql-codegen/typescript-operations": "^4.2.3",
"@internal/utils": "workspace:*",
"@types/ramda": "^0.30.2",
"get-graphql-schema": "^2.1.2",
"@internal/utils": "workspace:*"
"type-fest": "^4.26.1"
}
}
30 changes: 22 additions & 8 deletions packages/account/src/test-utils/launchNode.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,15 +30,15 @@ vi.mock('fs', async () => {
*/
describe('launchNode', () => {
test('using ephemeral port 0 is possible', async () => {
const { cleanup, port, url } = await launchNode({ port: '0' });
const { cleanup, port, url } = await launchNode({ port: '0', loggingEnabled: false });
expect(await fetch(url)).toBeTruthy();
expect(port).not.toEqual('0');

cleanup();
});

it('cleanup kills the started node', async () => {
const { cleanup, url } = await launchNode();
const { cleanup, url } = await launchNode({ loggingEnabled: false });
expect(await fetch(url)).toBeTruthy();

cleanup();
Expand All @@ -57,7 +57,7 @@ describe('launchNode', () => {
const spawnSpy = vi.spyOn(childProcessMod, 'spawn');
const killSpy = vi.spyOn(process, 'kill');

const { cleanup, pid } = await launchNode();
const { cleanup, pid } = await launchNode({ loggingEnabled: false });

const spawnOptions = spawnSpy.mock.calls[0][2];
expect(spawnOptions.detached).toBeTruthy();
Expand All @@ -74,7 +74,7 @@ describe('launchNode', () => {

process.env.FUEL_CORE_PATH = '';

const { result } = await safeExec(async () => launchNode());
const { result } = await safeExec(async () => launchNode({ loggingEnabled: false }));

const command = spawnSpy.mock.calls[0][0];
expect(command).toEqual('fuel-core');
Expand All @@ -95,7 +95,7 @@ describe('launchNode', () => {

const fuelCorePath = './my-fuel-core-binary-path';
const { error } = await safeExec(async () => {
await launchNode({ fuelCorePath, loggingEnabled: true });
await launchNode({ fuelCorePath, loggingEnabled: false });
});

expect(error).toBeTruthy();
Expand All @@ -107,7 +107,7 @@ describe('launchNode', () => {
test('reads FUEL_CORE_PATH environment variable for fuel-core binary', async () => {
const spawnSpy = vi.spyOn(childProcessMod, 'spawn');
process.env.FUEL_CORE_PATH = 'fuels-core';
const { cleanup, url } = await launchNode();
const { cleanup, url } = await launchNode({ loggingEnabled: false });
await Provider.create(url);

const command = spawnSpy.mock.calls[0][0];
Expand Down Expand Up @@ -152,7 +152,7 @@ describe('launchNode', () => {
});

test('logs fuel-core outputs via console.log', async () => {
const logSpy = vi.spyOn(console, 'log');
const logSpy = vi.spyOn(console, 'log').mockImplementation(() => {});

const { cleanup } = await launchNode({ loggingEnabled: true });
const logs = logSpy.mock.calls.map((call) => call[0]);
Expand All @@ -162,7 +162,7 @@ describe('launchNode', () => {

test('cleanup removes temporary directory', async () => {
const mkdirSyncSpy = vi.spyOn(fsMod, 'mkdirSync');
const { cleanup } = await launchNode();
const { cleanup } = await launchNode({ loggingEnabled: false });

expect(mkdirSyncSpy).toHaveBeenCalledTimes(1);
const tempDirPath = mkdirSyncSpy.mock.calls[0][0];
Expand Down Expand Up @@ -252,4 +252,18 @@ describe('launchNode', () => {
`fuel-core node under pid ${pid} does not exist. The node might have been killed before cleanup was called. Exiting cleanly.`
);
});

test('should clean up when unable to kill process with "RangeError: pid must be a positive integer" error', async () => {
const killSpy = vi.spyOn(process, 'kill').mockImplementationOnce(() => {
throw new RangeError('pid must be a positive integer');
});

const { pid, cleanup } = await launchNode({ loggingEnabled: false });

cleanup();

expect(killSpy).toBeCalledTimes(2);
expect(killSpy).toBeCalledWith(-pid);
expect(killSpy).toBeCalledWith(+pid);
});
});
4 changes: 4 additions & 0 deletions packages/account/src/test-utils/launchNode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,10 @@ export const launchNode = async ({
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 if (error.message.includes('pid must be a positive integer')) {
// This is a workaround for a bug with Bun.
// See: https://github.com/oven-sh/bun/issues/8787
process.kill(+child.pid);
} else {
throw e;
}
Expand Down
9 changes: 2 additions & 7 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion tsconfig.test.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,6 @@
"compilerOptions": {
"noEmit": true
},
"include": ["**/*.test.ts"],
"include": ["**/*.test.ts", "**/*.d.ts"],
"exclude": ["node_modules", "apps/docs/src/**/*.test.ts"]
}

0 comments on commit 4131e74

Please sign in to comment.