Open lucianonooijen opened 5 years ago
@lucianonooijen Does it works if you manually invoke app.close
after each test? (either by individually calling it, or through a afterEach
block)
@jonathansamines I tried adding
afterEach(() => app.close());
and
afterEach(app.close());
but both give the same error:
est the status paths › The GET /status route should give status code 200
TypeError: app.close is not a function
5 | const app = require('../app');
6 |
> 7 | afterEach(() => app.close());
| ^
8 |
9 | describe('Test the status paths', () => {
10 | test('The GET / route should give status code 200', async() => {
at Object.close (tests/supertest.test.js:7:21)
FAIL tests/supertest.test.js
● Test suite failed to run
TypeError: app.close is not a function
5 | const app = require('../app');
6 |
> 7 | afterEach(app.close());
| ^
8 |
9 | describe('Test the status paths', () => {
10 | test('The GET / route should give status code 200', async() => {
at Object.close (tests/supertest.test.js:7:15)
Did I invoke them wrong or am I missing something somewhere?
+1 i could use help with this issue also, same env as above and same errors.
@lucianonooijen @danielantelo Sorry about that, I sometimes confuse the express app with the server object.
When supertest is used in an express application not yet bound to a port, it will try to automatically bound it to a random port, holding a reference to the net server internally. To gain control over that process, you may want to try manually listen on a port yourself, so that you can close the connections manually.
let server, agent;
beforeEach((done) => {
server = app.listen(4000, (err) => {
if (err) return done(err);
agent = request.agent(server); // since the application is already listening, it should use the allocated port
done();
});
});
afterEach((done) => {
return server && server.close(done);
});
it('description', async () => {
// use agent instead of manually calling `request(app)` each time
await agent.get('/some-path-relative-to-the-express-app')
});
As an alternative to setting up an agent, you can also use the end
method to force supertest to close the automatic bound connection.
// on each test
it('the description', (done) => {
request(app)
.get('/some-path')
.end(done);
});
However, I am not sure how well this method plays with the promise based interface, because as you can see is expecting a callback.
References: https://github.com/visionmedia/supertest/blob/master/lib/test.js#L54
Hope it helps!
thanks for the reply @jonathansamines , appreciate it! Still having issues tho.
This is my setupTestFrameworkScriptFile:
const request = require('supertest');
const Mockgoose = require('mockgoose').Mockgoose;
const app = require('./src/app');
const db = require('./db');
const fixtures = require('./tests/fixtures');
let server;
beforeEach(async () => {
await db.connect(Mockgoose);
await fixtures();
server = await app.listen(4000);
global.agent = request.agent(server);
});
afterEach(async () => {
await server.close();
await db.disconnect();
});
and then a test in another file:
describe('read', () => {
test('lists all', async () => {
const response = await global.agent.get('/get/all');
expect(response.statusCode).toBe(200);
expect(Array.isArray(response.body)).toBe(true);
expect(response.body.length).toBe(3);
expect(response.body).toMatchSnapshot();
});
});
my tests pass, but jest does not exit due to open handles in global.agent.get('/get/all')
using .end()
on the request does not work when using promises to assert values, i got the error superagent request was sent twice, because both .end() and .then() were called. Never call .end() if you use promises
.
Any ideas what i am missing on the setup before/afters?
Much appreciated!
@danielantelo @lucianonooijen Unless you are promisifying the server methods close
and listen
, that is unlikely to work, because they are expecting a callback and do not return a promise by default. You should either promisify those methods or use a callback based api
@jonathansamines you mean passing the done like in your example?
let server;
beforeEach((done) => {
server = app.listen(4000, (err) => {
if (err) return done(err);
global.agent = request.agent(server);
done();
});
});
afterEach((done) => {
return server && server.close(done);
});
this is not solving it either
@danielantelo What error are you getting while using the callback based api?
got it working with just jest/supertest/mongoose (removing mockgoose). Thanks for the help.
let server;
beforeAll(async (done) => {
await mongoose.connect('mongodb://localhost:27017/testdb');
server = app.listen(4000, () => {
global.agent = request.agent(server);
done();
});
});
afterAll(async () => {
await server.close();
await mongoose.disconnect();
});
Now time to figure out the issue with mockgoose 😂
got it working with just jest/supertest/mongoose (removing mockgoose). Thanks for the help.
let server; beforeAll(async (done) => { await mongoose.connect('mongodb://localhost:27017/testdb'); server = app.listen(4000, () => { global.agent = request.agent(server); done(); }); }); afterAll(async () => { await server.close(); await mongoose.disconnect(); });
Now time to figure out the issue with mockgoose 😂
This fix doesn't work if you have multiple test files. The server doesn't seem to be closing properly and you will get port collisions when trying to start the server in the different test files.
yarn run v1.10.1
$ jest --forceExit
PASS __tests__/routes/reviews.test.js
FAIL __tests__/routes/movies.test.js (6.343s)
● Console
console.error node_modules/jest-jasmine2/build/jasmine/Env.js:157
Unhandled error
console.error node_modules/jest-jasmine2/build/jasmine/Env.js:158
Error: listen EADDRINUSE :::5025
at Object._errnoException (util.js:992:11)
at _exceptionWithHostPort (util.js:1014:20)
at Server.setupListenHandle [as _listen2] (net.js:1355:14)
at listenInCluster (net.js:1396:12)
at Server.listen (net.js:1480:7)
@skykanin you are right, after i got mockgoose working i realised that when i ran multiple files there was still an issue.
With mockgoose i had to close the connection after each test, otherwise would leave open handles on a single file. But doing that on the server just breaks things further.
So think we are now stuck on the same issue.
const request = require('supertest');
const mongoose = require('mongoose');
const Mockgoose = require('mockgoose').Mockgoose;
const app = require('./src/app');
const fixtures = require('./tests/fixtures');
const mockgoose = new Mockgoose(mongoose);
let server;
afterEach(async () => {
await mockgoose.helper.reset();
await fixtures();
});
beforeAll(async done => {
await mockgoose.prepareStorage();
await mongoose.connect('mongodb://testdb');
await fixtures();
server = app.listen(4000, () => {
global.agent = request.agent(server);
done();
});
});
afterAll(async () => {
if (server) {
await server.close();
}
mongoose.connections.forEach(async con => {
await con.close();
});
await mockgoose.mongodHelper.mongoBin.childProcess.kill();
await mockgoose.helper.reset();
await mongoose.disconnect();
});
Jest23 runs the first file, then fails to run any other files and gives lots of async timeout errors.
@danielantelo Sorry, I am not familiar with the mockgoose library. Most I can do, is to ask a couple of questions:
Is mongoose.connections.forEach
method an special kind of object which properly deals with async/await? If not, I would rather do something like:
await Promise.all(mongoose.connections.map(con => con.close()))
Here I am assumming con.close
is a promise returning function, otherwise, you would probably have to wrap it with a promise.
Also, is server.close
a promise aware function? (it is natively not) If not, you will have to wrap it so that it properly awaits for the connection to close.
Are you using Apollo Engine by any chance? I was able to solve this problem by disabling the engine in Apollo Server when testing.
I'm not using Apollo Engine and I still get the same error. end()
helps, but it forces me to use the uglier async handling style. Interestingly, the open handle is detected only for one post()
request, out of many other requests I have in tests.
Node version: 11.0.0 npm version: 6.5.0
package.json
{
"name": "authentication",
"version": "1.0.0",
"description": "",
"main": "server.js",
"scripts": {
"lint": "eslint --fix server.js src/**",
"start": "node server.js",
"start:dev": "nodemon server.js",
"test": "jest --forceExit --detectOpenHandles"
},
"husky": {
"hooks": {
"post-pull": "npm install",
"pre-commit": "npm run lint && npm test"
}
},
"keywords": [],
"author": "",
"license": "ISC",
"dependencies": {
"bcrypt": "^3.0.3",
"body-parser": "^1.18.3",
"dotenv": "^6.2.0",
"express": "^4.16.4",
"jsonwebtoken": "^8.4.0",
"lodash": "^4.17.11",
"morgan": "^1.9.1"
},
"devDependencies": {
"eslint": "^5.11.1",
"eslint-plugin-node": "^8.0.1",
"eslint-plugin-security": "^1.4.0",
"husky": "^1.3.1",
"jest": "^23.6.0",
"nodemon": "^1.18.9",
"prettier": "^1.15.3",
"supertest": "^3.3.0"
}
}
test task in package.json
"scripts": {
"test": "jest --forceExit --detectOpenHandles"
},
app.test.js
const request = require('supertest');
const app = require('../app');
describe('App Request', () => {
it('should responds with 404', () => {
request(app)
.get('/')
.expect('Content-Type', /json/)
.expect(404)
.end((error, response) => {
if (error) {
return error;
}
return response;
});
});
});
> jest --forceExit --detectOpenHandles
PASS src/test/app.test.js
App Request
✓ should responds with 404 (19ms)
Test Suites: 1 passed, 1 total
Tests: 1 passed, 1 total
Snapshots: 0 total
Time: 1.891s
Ran all test suites.
Jest has detected the following 1 open handle potentially keeping Jest from exiting:
● TCPSERVERWRAP
5 | it('should responds with 404', () => {
6 | request(app)
> 7 | .get('/')
| ^
8 | .expect('Content-Type', /json/)
9 | .expect(404)
10 | .end((error, response) => {
at Test.Object.<anonymous>.Test.serverAddress (node_modules/supertest/lib/test.js:59:33)
at new Test (node_modules/supertest/lib/test.js:36:12)
at Object.obj.(anonymous function) [as get] (node_modules/supertest/index.js:25:14)
at Object.get (src/test/app.test.js:7:8)
@AlexisNava
Try this in your app.test
const request = require('supertest');
const app = require('../app');
describe('App Request', () => {
test('should responds with 404', async (done) => {
const result = await request(app).get('/');
expect(result.status).toBe(404);
done();
});
});
I am experiencing the same error. I suppose a solution for the time being is to use --forceExit with Jest.
I found out that if you are using process.env in any test it also won't exit.
I'd chased this down, just to find out that it was a simple forgetfulness on my part. Our application fell back on the database that wasn't set up on CI or stubbed, causing a long wait that manifested in TCPSERVERWRAP
.
tl;dr Check your application for other reasons that your request could hang
i also have the same problem, need help, did some solved it ?
For me this did the trick
afterAll(async () => {
await new Promise(resolve => setTimeout(() => resolve(), 500)); // avoid jest open handle error
});
Seems like it needs some more ticks to close that handle.
@yss14 much thanks, it really works for me. anyone have this issue can try @yss14 's code
For me this did the trick
afterAll(async () => { await new Promise(resolve => setTimeout(() => resolve(), 500)); // avoid jest open handle error });
Seems like it needs some more ticks to close that handle.
@yss14 Thanks for your help, It solved the issue for me too, which I was chasing for couple of days but can you elaborate why this resolve the issues?
i"m also interested in knowing how it solves the issue.
@sharmaar12 @angristan I don't know the reason for sure, because I'm not familiar with the supertest code base. I guess that during the shutdown of supertest, somewhere the thread is not waiting for an async system operation to finish. Thus, the operation is delegated to the system but the execution is not blocked or does not wait for the async operation to finish. In the meantime, your test case execution finishes, jest is shuting down, and complains about an open handle.
If I'm wrong, please someone corrects me :)
For me this did the trick
afterAll(async () => { await new Promise(resolve => setTimeout(() => resolve(), 500)); // avoid jest open handle error });
Seems like it needs some more ticks to close that handle.
This didn't helped me. But I manged to force Jest to exit with this flag:
--forceExit
However I am still getting the warring that Jest detected an open handle.
Were any other solutions found for this issue? I have a similar problem. When I run my test suite normally, I get the warning that Jest did not exit one second after completion of the test run.
Calling with --detectLeaks
finds a leak, and calling with --detectOpenHandles
shows the same issue - TCPSERVERWRAP.
If I add @yss14 's code in the afterAll
hook, something peculiar happens - running Jest normally still shows the open handle issue. Running with the --detectLeaks
flag still shows a leak. Running with --detectOpenHandles
shows nothing.
I'm testing a middleware function, and my suite looks like this:
// Hooks - Before All
beforeAll(async () => {
await configureDatabase();
app.use(auth).get('/', (req, res) => res.send());
});
// Hooks - After All
afterAll(async () => {
await new Promise(resolve => setTimeout(() => resolve(), 500)); // avoid jest open handle error
});
describe('Express Auth Middleware', () => {
test('Should return 401 with an invalid token', async () => {
await request(app)
.get('/')
.set('Authorization', 'Bearer 123')
.send()
.expect(401);
});
test('Should return 401 without an Authorization Header', async () => {
await request(app)
.get('/')
.send()
.expect(401);
});
test('Should return 200 with a valid token', async () => {
await request(app)
.get('/')
.set('Authorization', `Bearer ${userOneToken}`)
.send()
.expect(200);
});
});
Binding to a port and closing manully does not work, nor does getting rid of async/await
, and instead passing done
into end
(i.e passing not calling, so .end(done);
.
Does anyone have any ideas? This is a serious issue because it seems that every test suite that relies on Express/Supertest is leaking memory, eventually to the point that my tests terminate with a JavaScript heap out of memory
error.
Thank you.
More detailed information for my above comment can be found in this SO question: https://stackoverflow.com/questions/56027190/jest-memory-leak-testing-express-middleware
@lucianonooijen @danielantelo @marcospgp Did you ever find a solution other than --forceExit
and the solution provided above? Thanks.
@JamieCorkhill have you tried any of the suggestions stated at this comment?
If any of those worked for you, it is very likely that the actual issue is somewhere else in your application. I could help with that, but I'll need an isolated reproduction example to take a look at.
@skykanin you are right, after i got mockgoose working i realised that when i ran multiple files there was still an issue.
With mockgoose i had to close the connection after each test, otherwise would leave open handles on a single file. But doing that on the server just breaks things further.
So think we are now stuck on the same issue.
const request = require('supertest'); const mongoose = require('mongoose'); const Mockgoose = require('mockgoose').Mockgoose; const app = require('./src/app'); const fixtures = require('./tests/fixtures'); const mockgoose = new Mockgoose(mongoose); let server; afterEach(async () => { await mockgoose.helper.reset(); await fixtures(); }); beforeAll(async done => { await mockgoose.prepareStorage(); await mongoose.connect('mongodb://testdb'); await fixtures(); server = app.listen(4000, () => { global.agent = request.agent(server); done(); }); }); afterAll(async () => { if (server) { await server.close(); } mongoose.connections.forEach(async con => { await con.close(); }); await mockgoose.mongodHelper.mongoBin.childProcess.kill(); await mockgoose.helper.reset(); await mongoose.disconnect(); });
Jest23 runs the first file, then fails to run any other files and gives lots of async timeout errors.
Sorry for the super late response, but you can apply the same trick as supertest does to bound your server to a random port.
server.listen(0);
Of course, you can still get port collisions, but they are very unlikely.
@jonathansamines I did try solution proposed in the comment you linked to, but I recieve the same response:
node_modules/env-cmd/bin/env-cmd.js config/test.env node_modules/jest/bin/jest.js auth.test.js
PASS __tests__/middleware/auth.test.js
Express Auth Middleware
√ Should return 401 with an invalid token (27ms)
√ Should return 401 without an Authorization Header (4ms)
√ Should return 200 with a valid token (9ms)
Test Suites: 1 passed, 1 total
Tests: 3 passed, 3 total
Snapshots: 0 total
Time: 3.246s, estimated 5s
Ran all test suites matching /auth.test.js/i.
Jest did not exit one second after the test run has completed.
This usually means that there are asynchronous operations that weren't stopped in your tests. Consider running Jest with `--detectOpenHandles` to troubleshoot this issue.
Running with --detectOpenHandles
does not find anything.
The auth.test.js
file contains only three tests:
const request = require('supertest');
const app = require('./../../src/app');
// Database Fixtures
const { userOneToken, configureDatabase } = require('./../fixtures/db');
// Testing
const auth = require('./../../src/middleware/auth');
let server, agent;
// Hooks - Before All
beforeAll(async (done) => {
await configureDatabase();
server = app.listen(4000, err => {
if (err) return done(err);
agent = request.agent(server);
app.use(auth).get('/', (req, res) => res.send());
done();
});
});
// Hooks - After All
afterAll(done => {
return server && server.close(done);
});
describe('Express Auth Middleware', () => {
test('Should return 401 with an invalid token', async () => {
await agent
.get('/')
.set('Authorization', 'Bearer 123')
.send()
.expect(401);
});
test('Should return 401 without an Authorization Header', async () => {
await agent
.get('/')
.send()
.expect(401);
});
test('Should return 200 with a valid token', async () => {
await agent
.get('/')
.set('Authorization', `Bearer ${userOneToken}`)
.send()
.expect(200);
});
});
I can try to create an isolated repro example, but you'll need a local MongoDB Server running.
Thank you.
@JamieCorkhill that works for me
@jonathansamines Great. Thanks for your time. I'll start building it now, and you'll be able to run the server as an npm
script with environment variables pre-loaded.
@jonathansamines Alright. Repro complete. Please bear with me if this does not suit your needs. It is my first time creating an isolated reproduction example.
The only script you can run is npm test
, which will initialize the application with the required environment variables. That also means that firebase-admin
will work because it's functioning of the mock and not expecting an API key.
A local MongoDB server will have to be running. I don't know if you are on Windows, but for ease, I have provided a .bat
file. You just have to edit that file to include the path to mongod.exe
and the --dbpath
flag. That can be run with npm dev-db-server
.
Here is the repository:
https://github.com/JamieCorkhill/Jest-Memory-Leak-Repro
Please let me know if there is anything else you need. I appreciate your time.
Hi @JamieCorkhill I finally got some time to take a look. After some debugging, I noticed there was an open mongodb connection, which was never closed.
See a couple of changes I made to your example: https://github.com/JamieCorkhill/Jest-Memory-Leak-Repro/compare/master...jonathansamines:master
But, in summary:
request
directly.@jonathansamines Sorry for the late response. I did merge your changes and take a brief look, but I've been busy lately and have not yet had a chance to test if that fixes the memory leaks.
I'll get back to you later today.
Thank you for your help, Jamie
@jonathansamines I just tested your changes and it worked fantastically without memory leaks.
I did have one question, though.
In auth.test.js
, in the beforeAll
hook, you have this:
// Hooks - Before All
beforeAll(async () => {
connection = await connectionFactory();
await configureDatabase();
app.use(auth).get('/', (req, res) => res.send());
new Promise((resolve, reject) => {
server = app.listen(4000, (err) => {
if (err) return reject(err);
resolve();
});
});
agent = request.agent(server);
});
Do you not have to await
the promise to settle, as seen below, or do you just promisify the call to server.listen()
so it works with the async callback to beforeAll
?
// Hooks - Before All
beforeAll(async () => {
connection = await connectionFactory();
await configureDatabase();
app.use(auth).get('/', (req, res) => res.send());
await new Promise((resolve, reject) => {
server = app.listen(4000, (err) => {
if (err) return reject(err);
resolve();
});
});
agent = request.agent(server);
});
Thank you, Jamie
@JamieCorkhill Yes please, await gor that one as well. I just forgot to await for it.
@jonathansamines Thank you.
@jonathansamines And I just wanted to make sure you know that I really appreciate your help and time. I have many test suites, and they've been taking a long time to compete, as well as all new test cases throwing a JavaScript heap out of memory
error. I expect that will all be fixed now considering the heap should stop being bloated by open connections.
With the memory leak issue hindering me, I considered moving on with building the rest of the API without code coverage, so thank you very much for saving my TDD endeavor.
This is working for us to get around this issue. Seems like something doesn't shutdown in the same tick as it should.
afterAll(done => {
setImmediate(done);
});
Thanks, @re5et I'll try it out.
--maxWorkers=1 fixed all my issues
"scripts": {
"test": "jest --watchAll --forceExit --detectOpenHandles --verbose --coverage --maxWorkers=1",
Start Server
beforeEach( ()=> {
server = require('../../../index');
Close Sever
afterEach( async () => {
await server.close();
My issue was actually being caused by express-session
middleware. I suspect something abnormal was happening with supertest requests, quick workaround was to include this in any tests using supertest
jest.mock('express-session', () =>
(options: SessionOptions) =>
(req: Request, res: Response, next: NextFunction) => {
next();
});
@cif The issue is not usually caused by express-session
itself, but rather by the particular store used. I usually rely on the default MemoryStore
on tests, if it is really hard to properly shutdown the upstream connection to the store.
@jonathansamines thanks for clarification there! We are indeed using a database store for sessions since we are using multiple instances of the service fronted by a load balancer (no sticky session)
Hi, I'm also facing same issue, this is my test code
const request = require('supertest');
const app = require('../server');
describe('Hello', () => {
test('succeeds with greetings', async (done) => {
const res = await get('/api/hello')
.expect(200);
expect(res.body.greeting).toBe('hello API');
done();
});
});
function get(url) {
const httpRequest = request(app).get(url);
httpRequest.set('Accept', 'application/json')
httpRequest.set('Origin', 'http://localhost:57305')
return httpRequest;
}
And specifying a port while running my app like
var listener = app.listen(57305, function () {
console.log('Your app is listening on port ' + listener.address().port);
});
all tests are getting passed but throws
--forceExit
does solve my problem but still throws warning as discussed above and I'm trying to integrate travisCI for the first time but build is not getting complete.
repo
going to run my build with --forceExit
for now
Thank you
I use --forceExit combined with --detectOpenHandles flags to be sure that this doesn't occur.
I found out that if you are using process.env in any test it also won't exit.
WHAT?! that seems super weird -- do we know why?
Error:
Env:
Testcode:
Full console output: