FuelLabs / fuels-ts

Fuel Network Typescript SDK
https://docs.fuel.network/docs/fuels-ts/
Apache License 2.0
44.16k stars 1.34k forks source link

Node started via `launchTestNode` isn't closed in tests #2712

Closed arboleya closed 2 months ago

arboleya commented 3 months ago

Repro

Create a test with the snippet below and execute it.

pnpm test:filter launching.test.ts

The test finishes and exits, but fuel-core continues running in the background.

// launching.test.ts
import { sleep } from 'fuels';
import { launchTestNode } from 'fuels/test-utils';

test('it works', async () => {
  console.log('before');
  using node = await launchTestNode();
  await sleep(2500);
  console.log('after', node.provider.url);
});

Repro 2

I also tried it with const + cleanup; nothing changed.

import { sleep } from 'fuels';
import { launchTestNode } from 'fuels/test-utils';

test('it works', async () => {
  console.log('before');
  const node = await launchTestNode();
  await sleep(2500);
  console.log('after', node.provider.url);
  node.cleanup();
});

Repro 3

However, it works if I transform this into a regular file (not a test) and execute it.

import { sleep } from 'fuels';
import { launchTestNode } from 'fuels/test-utils';

function main() {
  console.log('before');
  using node = await launchTestNode();
  await sleep(2500);
  console.log('after', node.provider.url);
};

main.catch(console.error);

Conclusion

The utility fails where it will be primarily used: in tests.

arboleya commented 3 months ago

@maschad Have you experienced this in your PR?

maschad commented 3 months ago

@arboleya examining the code snippets you've shared I believe the setup is incorrect. If I add some logs to the Symbol.dispose symbol which is called at the end of the function scope of a variable initialized with the using keyword, I can observe that the resources are properly cleaned up. More info here

For example, If we have the following test:

  it.only('dummy test', async () => {
    console.log('before');
    using node = await launchTestNode();
    console.log('after');
  });

and we add some logs to the [Symbol.dispose] returned by launchTestNode

 [Symbol.dispose]: () => {
      console.log('disposing provider');
      cleanup();
    },

we will observe the following output

stdout | packages/fuel-gauge/src/reentrant-contract-calls.test.ts > Reentrant Contract Calls > dummy test
before

stdout | packages/fuel-gauge/src/reentrant-contract-calls.test.ts > Reentrant Contract Calls > dummy test
after
disposing provider
nedsalk commented 3 months ago

I can reproduce this bug locally. Run the snippet file in the first example and then run ps -A | grep fuel-core after the test finishes and you'll see a fuel-core node still running. You can also run pnpm test:filter packages/fuel-gauge/src/call-test-contract.test.ts which has more tests and you'll find that the last test's node always remains. I have a hunch that it's got to do with cleanup being asynchronous while using is synchronous. I remember doing some manual tests around this and realizing that we can go with Symbole.dispose and using instead of Symbol.asyncDispose and await using - I'll analyze it further again.

maschad commented 3 months ago

Good call @nedsalk I had made the false assumption that when Symbol.dispose was called the process was cleaned up. Thanks for catching this @arboleya