-
-
Notifications
You must be signed in to change notification settings - Fork 737
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
Memory leak issue with jest #1817
Comments
@juhosuominen thanks for this. I've started to play with your test repo a bit, but I'll need to spend more time with it this weekend. One thing I've noticed is that I can't reproduce your statement "Even a single require/import of nock seems to cause the issue.". So on my machine simply importing Nock isn't the issue, it's the repeated use of Nock that does it. Doing a quick dive into a heap dump points to Babel/generator, but I need to more time exploring to see if that is a red herring. |
Ahh now I see what your statement means. A single require of Nock in each test suite causes the issue. require('nock');
it('should do nothing', async () => {
// do nothing
}); So far I've been able to determine that this has to do with Jest not respecting the I'm not sure why there is a memory leak. I'm still leaning towards something related to Babel. |
Just a hunch, any chance invoking We do keep a reference to the original http & https functions inside nock. Though I’m still not really clear on what Jest is doing, or how the combination of Jest and Nock creates leaks. |
yes, I have seen that adding Right now I'm playing around with different ways of tacking on the original ClientRequest onto a global so Nock doesn't keep extending its own subclass. But I haven't figured out the quirks of Jest's and/or Babel's caching mechanisms yet. I'm going to call it a night. |
We end up extending our own subclass over and over? |
yes. |
It's hard to call this a bug, since Jest is the one breaking from how Node is suppose to work. |
What's confusing is that if I do the dance to store the original This issue also exists for the http[s].request and get methods. |
The discussion in Jest jestjs/jest#6814 And a nice write up on it. https://chanind.github.io/javascript/2019/10/12/jest-tests-memory-leak.html |
I wonder if we could detect Jest and then print a warning if we try to extend and discover that it’s already been overridden. We could also automatically reset but I don’t like that idea too much; at least not without understanding better what Jest is doing. Another option is to avoid patching if it’s already patched by setting a flag in the global module. (Perhaps a flag works where saving original request doesn’t?) Though I wonder if that would work. Nock has its own global state, and I’m not quite clear whether request patched in a previous test would work with mocks defined in a new test. |
@mastermatt Yes, it seems I was mistaken about the "Even a single require/import of nock seems to cause the issue" part. We were probably importing nock in some test boilerplate file. I did manage to fix the memory leak by calling |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. We try to do our best, but nock is maintained by volunteers and there is only so much we can do at a time. Thank you for your contributions. |
Any update on this? |
Isn't this the fix? |
Yes, probably. I wasn't able to confirm it (Jest still reports leaks and actually leaks 100s of Mbs of memory per test file). But that's probably due to other dependencies I have. So, ignore my last comment please:) |
Just a question, will |
|
Thanks! |
@mentierd yes, at this time we don't plan on "fixing" this as it's an issue with Jest. jestjs/jest#6814 as noted in the comment above. #1817 (comment). We have updated our docs to describe how to avoid this when using Nock in Jest suites. Unfortunately, the comment just above that one describes an attempt to get around it, in the same way graceful-fs and agent-base did. Those libs have slightly different use cases. graceful-fs was caching whole stdlib modules and agent-base was only monkey patching If anyone comes across a way to get around this or force Babel to stop its caching in a way that doesn't make things worse, we'd love a PR. |
This still seems to be an issue, though I can confirm that when I use node |
What is the expected behavior?
Node heap size shouldn't grow in every test.
What is the actual behavior?
Heap size increases after every test file.
With nock:
Without nock:
How to reproduce the issue
Even a single require/import of nock seems to cause the issue.
Minimal test case: https://github.com/juhosuominen/jest-nock-memory-leak
Versions
The text was updated successfully, but these errors were encountered: