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

[jest-cli] Please update node-notifier to fix memory leak #5365

Closed
alexilyaev opened this issue Jan 22, 2018 · 4 comments · May be fixed by manekinekko/devoxx-assistant#3
Closed

[jest-cli] Please update node-notifier to fix memory leak #5365

alexilyaev opened this issue Jan 22, 2018 · 4 comments · May be fixed by manekinekko/devoxx-assistant#3

Comments

@alexilyaev
Copy link
Contributor

Do you want to request a feature or report a bug?
Bug

What is the current behavior?
When running tests with --notify, in some situations there's a memory leak in the terminal-notifier process which eats up all of the computer RAM.

If the current behavior is a bug, please provide the steps to reproduce and
either a repl.it demo through https://repl.it/languages/jest or a minimal
repository on GitHub that we can yarn install and yarn test.

This has been reported in:
#2999

I've started experiencing the issue with my team recently, after upgrading to Jest v22.x.x (not sure if it's related, and probably doesn't matter now).

What is the expected behavior?
Looks like the issue has been fixed in latest node-notifier:
mikaelbr/node-notifier#183

So, if we bump node-notifier, the issue should go away.

Please provide your exact Jest configuration and mention your Jest, node,
yarn/npm version and operating system.

I couldn't pin down exactly in what scenario it happens, #2999 has a repro though.

Mac OS X 10.12.6
Jest v22.1.2
Node v9.3.0

@thymikee
Copy link
Collaborator

Mind sending a PR? 🙂

alexilyaev added a commit to alexilyaev/jest that referenced this issue Jan 22, 2018
@SimenB
Copy link
Member

SimenB commented Jan 22, 2018

We are within semver range (fix is apparently in 5.2.1), so not sure we need to do anything? https://github.com/facebook/jest/blob/139f976b67f40b286a73860c1f59def712ec323d/packages/jest-cli/package.json#L32

Doesn't hurt to update, of course.

@alexilyaev
Copy link
Contributor Author

Ahm, did a PR (should I update CHANGELOG.md under msater?).

Indeed it's in semver range, after removing node_modules and yarn.lock and running yarn it did install node-notifier@5.2.1.

But for most people it will not update and already installed node-notifier, perhaps we should still make the change. The RAM eat up is quite severe.

@github-actions
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 13, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants