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 --detectleaks shows memory leak while using SuperTest with Express #634

Open
kamrankhanwastaken opened this issue Mar 12, 2020 · 13 comments · May be fixed by #651
Open

Jest --detectleaks shows memory leak while using SuperTest with Express #634

kamrankhanwastaken opened this issue Mar 12, 2020 · 13 comments · May be fixed by #651

Comments

@kamrankhanwastaken
Copy link

kamrankhanwastaken commented Mar 12, 2020

Hey everyone,

I have been experiencing memory leaks in any test files that require supertest when using Jest with the --detectLeaks option on an express server.

Versions

node: 12.16.1
node: 6.14.2

Modules

jest: 25.1.0
supertest: 4.0.2
weak-napi: 1.0.3 (jest complains without this module available when using detectLeaks)
express: 4.17.1

To isolate the issue from any custom development code in my particular application, I have created this repository to highlight the problem:

https://github.com/fakekamrankhan/express-supertest-jest-detect-memory-leak

If you run npm test you'll see the memory leak warning. Removing supertest gets rid of the memory leak flag. The test.js file in question:

const express = require('express');
const request = require('supertest');
describe('leak', () => {

	let port;
	let server;

	beforeAll(() => {
		port = 3000;
		server = new express().listen(port);
	});

	afterAll(() => {
		server.close();
	})
	test('leak test', async () => {
		let res = await request(server).get('/');
		expect(res.status).toBe(404);
	});
});

The error:

    EXPERIMENTAL FEATURE!
    Your test suite is leaking memory. Please ensure all references are cleaned.

    There is a number of things that can leak memory:
      - Async operations that have not finished (e.g. fs.readFile).
      - Timers not properly mocked (e.g. setInterval, setTimeout).
      - Keeping references to the global scope.

      at onResult (node_modules/@jest/core/build/TestScheduler.js:188:18)

I have been wracking by brain for the last four days trying to pinpoint the issue. Does anyone know if this could be anything in the test or if this is a bug in superest or jest?

@jonathansamines
Copy link

@fakekamrankhan Both .listen() and .close() receive an optional callback, to properly wait for those operations to complete. You need to manually provide those callbacks, so that Jest waits for those operations.

beforeAll((done) => {
  port = 3000;
  server = new express().listen(port, done);
});

afterAll((done) => {
  server.close(done);
})

@kamrankhanwastaken
Copy link
Author

Thanks for the response!

That is one of the many variations I did try in my troubleshooting to close off functions before Jest prematurely exits but it still complains of memory leak. I just tried the code you mentioned and it is still throwing the same error.

Both of these triggers Jest to complain of memory leak:

beforeAll((done) => {
  port = 3000;
  server = new express().listen(port, done);
});

afterAll((done) => {
  server.close(done);
});
beforeAll( async () => {
  port = 3000;
  server = await new express().listen(port, done);
});

afterAll( async () => {
  await server.close();
})

@kirillgroshkov
Copy link

Any updates on this?

@kamrankhanwastaken
Copy link
Author

Still the same problem! :(

@Ehbraheem
Copy link

Any update on this issue?

1 similar comment
@oleksiygontariownit
Copy link

Any update on this issue?

@jonathansamines
Copy link

Given the last reproduction example sent by @fakekamrankhan (which only involves express). Seems unlikely that this is an issue with this library.

@nunofgs nunofgs linked a pull request May 15, 2020 that will close this issue
@nunofgs
Copy link

nunofgs commented May 15, 2020

@jonathansamines I'm actually not sure that that's the case. I was able to reproduce this issue with Koa since I was passing it an instance of the (previously started) server.

I've submitted PR #651 which completely fixed my issue.

@mainfraame
Copy link

In case you guys haven't looked in the jest issues, it looks like this is occurring because one of the modules you're using is mutating a native node module, eg. http, https, etc. I drove myself crazy hunting down this issue in my integration tests; the culprit turned out to a library called follow-redirects. It was mutating the http and https modules.

@kirillgroshkov
Copy link

In case you guys haven't looked in the jest issues, it looks like this is occurring because one of the modules you're using is mutating a native node module, eg. http, https, etc. I drove myself crazy hunting down this issue in my integration tests; the culprit turned out to a library called follow-redirects. It was mutating the http and https modules.

Interesting. Is it solved in follow-redirects, or is there an issue opened on them?

@imjordanxd
Copy link

Still an issue in 2021 👎

@nfantone
Copy link

In case you guys haven't looked in the jest issues, it looks like this is occurring because one of the modules you're using is mutating a native node module, eg. http, https, etc. I drove myself crazy hunting down this issue in my integration tests; the culprit turned out to a library called follow-redirects. It was mutating the http and https modules.

I don't think this is the bottom of it. I've tried the most basic setup (empty project with express, jest and supertest) and --detectLeaks fails for me still. See #520 (comment).

@rcbevans
Copy link

For anyone seeing this now, I do not see a handle leaking in my supertest jest tests to which I pass an already constructed express app instance.

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 a pull request may close this issue.

10 participants