Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
arboleya opened this issue Jul 5, 2024 · 4 comments · Fixed by #2718
Closed

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

arboleya opened this issue Jul 5, 2024 · 4 comments · Fixed by #2718
Assignees
Labels
bug Issue is a bug
Milestone

Comments

@arboleya
Copy link
Member

arboleya commented Jul 5, 2024

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 arboleya added the bug Issue is a bug label Jul 5, 2024
@arboleya arboleya added this to the 0.x mainnet milestone Jul 5, 2024
@arboleya arboleya added the p0 label Jul 5, 2024
@maschad maschad assigned maschad and unassigned maschad Jul 5, 2024
@arboleya
Copy link
Member Author

arboleya commented Jul 6, 2024

@maschad Have you experienced this in your PR?

@maschad
Copy link
Member

maschad commented Jul 6, 2024

@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
Copy link
Contributor

nedsalk commented Jul 7, 2024

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.

@nedsalk nedsalk self-assigned this Jul 7, 2024
@maschad
Copy link
Member

maschad commented Jul 7, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue is a bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants