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

Fix Jest Memory leaks #1043

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix Jest Memory leaks #1043

wants to merge 1 commit into from

Conversation

kelvinkipruto
Copy link
Contributor

Description

Our current Jest setup has a memory leak. You can check it using pnpm run jest -- --detectLeaks.
This is not an isolated case based on this open issue
This causes performance issues. in my case, running the jest command has used up to 20GB of RAM and even caused my machine to shut down when generating new snapshots.

This PR attempts to fix that by doing garbage collection after each test run. It also limits the max RAM usage to 4GB among other improvements.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation

@kelvinkipruto kelvinkipruto marked this pull request as ready for review February 4, 2025 13:01
@kilemensi
Copy link
Member

  1. Instead of node --expose-gc --max-old-space-size=4096 node_modules/jest/bin/jest.js, doesn't NODE_OPTIONS="--expose-gc --max-old-space-size=4096" jest work? I think we should avoid hard-coding paths in node_modules, etc.
  2. Can't we add this as as script in the project root dir i.e.<project root>/scripts/jest.sh that every package.json in packages/apps refers to rather than copy/pasting the full command in every package/app?

@@ -0,0 +1,6 @@
/* eslint-env jest */
/* eslint-disable no-undef */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can just ignore this file in eslint

@m453h
Copy link
Contributor

m453h commented Feb 5, 2025

🚀

I tested locally and it does significantly reduce the load on my machine I did notice this warning related to --forceExit

Force exiting Jest: Have you considered using `--detectOpenHandles` to detect async operations that kept running after all tests finished?

I haven't played much with these cli options but I was a bit concerned with using --forceExit, I saw these combinations of flags which are pretty close to what we currently have, I did notice when we use --runInBand the maximum heap does tend to increase significantly

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

Successfully merging this pull request may close these issues.

4 participants