supertest icon indicating copy to clipboard operation
supertest copied to clipboard

Close server after tests

Open ghost opened this issue 6 years ago • 37 comments

I have koa server with mongo connection, jest as test framework.

const app = new Koa()
...
export default app.listen(PORT, (err) => {
  if (err) console.log(err)

  if (!IS_TEST) {
    console.log(`Server running on port: ${PORT}`)
  }
})

After success finish test or fail server connection steel works, how close koa server connection after test ?

Test example:

import supertest from 'supertest'
import mongoose from 'mongoose'
import server from '../../../app/server'
import User from '../../../app/models/user'

const r = supertest.agent(server.listen())

afterEach(async (done) => {
  await mongoose.connection.db.dropDatabase()
  done()
})

describe('Authorization', () => {
  describe('POST /signup', () => {
    const userData = {
      email: '[email protected]',
      password: 111111,
    }

    test('success create user', (done) => {
      r
        .post(`/api/auth/signup`)
        .send(userData)
        .expect(200)
        .expect({
          data: {
            email: userData.email,
          },
        })
        .end(done)
    })

    test('fail of user create, password required', (done) => {
      const userData = {
        email: '[email protected]',
      }

      r
        .post(`/api/auth/signup`)
        .send(userData)
        .expect(400)
        .expect({
          errors: {
            password: 'Password required',
          },
        })
        .end(done)
    })

    test('fail of user create, user already exist', async (done) => {
      const userData = {
        email: '[email protected]',
        password: 111111,
      }

      await User.create(userData)

      r
        .post(`/api/auth/signup`)
        .send(userData)
        .expect(400)
        .expect({
          errors: {
            email: `User already exist`,
          },
        })
        .end(done)
    })
  })
})

ghost avatar Sep 21 '17 06:09 ghost

same problem with express and the latest version of mocha (only with the latest mocha)--the tests never exit because the server never shuts down.

atomantic avatar Oct 20 '17 19:10 atomantic

Thanks @atomantic if i roll back to 3.5.3 i'm all good.

"mocha": "~3.5.3", 

devpascoe avatar Oct 23 '17 04:10 devpascoe

Running into the same issue here. A connection to a kafka cluster remains alive after tests are done so it never shuts down.

FuzzOli87 avatar Nov 06 '17 21:11 FuzzOli87

This might be caused by https://github.com/visionmedia/supertest/issues/430. If you are calling done() in the .end() callback.

JakubSimandl avatar Nov 10 '17 12:11 JakubSimandl

seems to be an issue with [email protected]

DamianLion avatar Nov 22 '17 15:11 DamianLion

Have you tried mocha --exit?

demisx avatar Nov 27 '17 19:11 demisx

app.listen returns a server instance you need to tear down as the lest test.

also yes newest mocha --exit has changed: https://github.com/visionmedia/supertest/issues/437

michaelBenin avatar Nov 30 '17 22:11 michaelBenin

hate to ask @michaelBenin but do you have a sample on this tear down code block?

devpascoe avatar Dec 01 '17 00:12 devpascoe

never mind, thanks to @demisx and the release notes of mocha 4 this now functions as before with the --exit flag

#2879: By default, Mocha will no longer force the process to exit once all tests complete. This means any test code (or code under test) which would normally prevent node from exiting will do so when run in Mocha. Supply the --exit flag to revert to pre-v4.0.0 behavior https://github.com/mochajs/mocha/issues/2879

devpascoe avatar Dec 01 '17 00:12 devpascoe

I use the following to setup and tear down the server in mocha 4+

const supertest = require('supertest')
const app = require('../../app')

describe('int::app', function(){

  let request = null
  let server = null

  before(function(done){
    server = app.listen(done)
    request = supertest.agent(server)
  })

  after(function(done){
    server.close(done)
  })

  it('should get /api/v1/laps/85299', function(){
    return request.get('/api/v1/laps/85299')
      .expect(200, { data: {} })
  })

})

I haven't checked if there's some way to get a reference to the server(s) superagent sets up.

deployable avatar Dec 01 '17 23:12 deployable

@deployable That works for me but it can take a few seconds for a larger suite to teardown. I'm guessing that it's because supertest is keeping keep-alive connections around? Does anyone have an idea how to force their closure?

timeemit avatar Dec 06 '17 01:12 timeemit

@timeemit I've run into that outside of tests. There are some modules that add connection tracking and can close sockets from that, I'm struggling to recall/find names though.

I think some old old versions of node had bugs where new connections could actually keep the tcp server up too.

deployable avatar Dec 07 '17 23:12 deployable

I just run into the same issue running tape with supertest.agent(app). Tape is waiting for the server to close. I will let you know as soon as I find a solution.

jakubrpawlowski avatar Dec 21 '17 01:12 jakubrpawlowski

@jakubrpawlowski https://github.com/isaacs/server-destroy and How do I shutdown a Node.js http(s) server immediately? and https://github.com/thedillonb/http-shutdown and https://github.com/marten-de-vries/killable

deployable avatar Dec 21 '17 03:12 deployable

Unfortunately even after using both server-destroy and http-shutdown and afterwards waiting for keep-alive to finish the server is still available for more tests (I can still call agent 5 minutes after server should have been shut down and still get reference to the same server and 200 status code back).

jakubrpawlowski avatar Dec 21 '17 04:12 jakubrpawlowski

Okay I figured it out finally. It was Mongoose that was causing the issue in my case. After requiring it in the test file and calling mongoose.connection.close() in the very last test everything seems to be working fine! Wow I can't believe this took me almost 8 hours!

test('teardown', t => {
    app.close()
    mongoose.connection.close()
    t.end() 
})

jakubrpawlowski avatar Dec 21 '17 07:12 jakubrpawlowski

If with mocha 3.5.3 everything works smoothly, should we really encourage developers that are using supertest to add some teardown hacks just to make supertest compatible with the new mocha version or should we aim to fix supertest to work properly again?

Probably the second option right? Just sayin'

robertdumitrescu avatar Dec 21 '17 11:12 robertdumitrescu

Okay I figured it out finally. It was Mongoose that was causing the issue in my case.

Same here! I was closing closing my http server, but wasn't closing my Mongoose connection.

Thanks for the hint @jakubrpawlowski!

mleanos avatar Feb 26 '18 07:02 mleanos

Okay I figured it out finally. It was Mongoose that was causing the issue in my case.

For me it was PostgreSQL, but the same thing. Also, after I closed my database connection within after(), I refactored my test setup back to the original as outlined in the Supertest documentation. (Calling app.get(...).expect(...) directly) instead of the structure mentioned by @deployable and it works. As soon as Mocha finishes, the application exits.

BrianHVB avatar Apr 03 '18 01:04 BrianHVB

@BrianHVB Do you have a quick example of a suite that exits cleanly without that extra server setup? I've had the "hang" occur in apps without a db connection, or something else async hanging around in the background so I thought the cause was down to only supertests setup.

deployable avatar Apr 03 '18 14:04 deployable

It looks like supertest sets up a new server for every call to request(). That server will close when you call .end().

Possibly test's that don't use end() are leaving the server (and mocha) running?

deployable avatar Apr 03 '18 14:04 deployable

I have upgraded the dev dependency for mocha to be the latest, 5.x. I added the --exit to the npm test command, otherwise it has the same issue where mocha doesn't exit after the tests have finished. The issue in with how the test setups are done, all servers that are open aren't closed.

PRs welcome to fix this. A first step might be to break the single monolithic test file into smaller, more manageable test files.

mikelax avatar May 13 '18 21:05 mikelax

Is this fixed? 😅

developer239 avatar Aug 20 '18 15:08 developer239

Unfortunately express still doesn't exit on calling .end(). This is reproducible with a basic setup:

package.json:

{
  "name": "tape-express",
  "version": "1.0.0",
  "description": "",
  "main": "index.js",
  "scripts": {
    "test": "tape index.spec.js"
  },
  "keywords": [],
  "author": "",
  "license": "ISC",
  "dependencies": {
    "express": "^4.16.3",
    "supertest": "^3.3.0",
    "tape": "^4.9.1"
  }
}

index.js:

const
    app = require("express")()
    , port = process.env.PORT || 3301
;

app.set("port", port);

app.get("/test", (req, res, next) => {
    return res.status(200).send("test");
});

app.listen(port, console.log.bind(this, `listening on port ${port}`));

module.exports = app;

index.spec.js:

const
    test = require("tape")
    , supertest = require("supertest")
    , app = require("./index")
;

test("/test should reply with 200", t => {
    supertest(app)
        .get("/test")
        .expect(200)
        .end((err,res) => {
            if(err){
                return t.fail(err)
            }
            t.pass("/test replied with 200");
            t.end()
        })
    ;

    setTimeout(()=>{
        t.fail("failed to shut down express");
        process.exit(1);
    },30000)

});

execute test


> [email protected] test C:\www\testing\tape-express
> tape index.spec.js

listening on port 3301
TAP version 13
# /test should reply with 200
ok 1 /test replied with 200
not ok 2 failed to shut down express
  ---
    operator: fail
    at: Timeout.setTimeout [as _onTimeout] (C:\www\testing\tape-express\index.spec.js:21:11)
    stack: |-
      Error: failed to shut down express
          at Test.assert [as _assert] (C:\www\testing\tape-express\node_modules\tape\lib\test.js:224:54)
          at Test.bound [as _assert] (C:\www\testing\tape-express\node_modules\tape\lib\test.js:76:32)
          at Test.fail (C:\www\testing\tape-express\node_modules\tape\lib\test.js:317:10)
          at Test.bound [as fail] (C:\www\testing\tape-express\node_modules\tape\lib\test.js:76:32)
          at Timeout.setTimeout [as _onTimeout] (C:\www\testing\tape-express\index.spec.js:21:11)
          at ontimeout (timers.js:424:11)
          at tryOnTimeout (timers.js:288:5)
          at listOnTimeout (timers.js:251:5)
          at Timer.processTimers (timers.js:211:10)
  ...
npm ERR! Test failed.  See above for more details.

I added the repro here: https://github.com/kepikoi/supertest-issue-430

kepikoi avatar Oct 01 '18 15:10 kepikoi

I'm seeing this issue too. No matter what I do, I'm unable to get Jest to exit cleanly after making a call with supertest. If I remove the supertest call, it exists cleanly as expected.

bwegrzyn avatar Oct 27 '18 05:10 bwegrzyn

This pr resolved this issue for me. #522

Ohadbasan avatar Nov 20 '18 22:11 Ohadbasan

@kepikoi The problem might be the module.exports exporting the express app itself instead of the result of app.listen()

Either way there are other things at play here - things like database connections. For the time being I'm going to subscribe to this thread and use --forceExit when running Jest.

marcospgp avatar Jan 07 '19 16:01 marcospgp

The problem might be the module.exports exporting the express app itself instead of the result of app.listen()

@marcospgp this doesn't help. Server.listen() returns the Server (here's the code)

codebling avatar Apr 21 '19 00:04 codebling

Okay I figured it out finally. It was Mongoose that was causing the issue in my case. After requiring it in the test file and calling mongoose.connection.close() in the very last test everything seems to be working fine! Wow I can't believe this took me almost 8 hours!

test('teardown', t => {
    app.close()
    mongoose.connection.close()
    t.end() 
})

This works for me! Close a server manually when the tests ended seems to me a good approach! Thanks @jakubrpawlowski

pablolopesk8 avatar Jun 17 '19 14:06 pablolopesk8

It's sad how much developer time these things waste for very little benefit. You guys should just let these things timeout, or force them to shut down. There's no value in getting this to work perfectly when it doesn't out of the box.

And I do understand it's no one's fault - any software will have issues, and open source devs already put a lot of personal time into these projects.

marcospgp avatar Jun 17 '19 15:06 marcospgp

When using koa (v2.7.0), this issue could be resolved by replacing app.listen() to app.callback(). For example,

const supertest = require('supertest')
const { app } = require('../src/app')

const request = supertest(app.callback())

describe('routes', () => {
  it(' GET /', () => {
    return request.get('/').expect(200)
  })
})

jjangga0214 avatar Jul 21 '19 11:07 jjangga0214

It's sad how much developer time these things waste for very little benefit. You guys should just let these things timeout, or force them to shut down. There's no value in getting this to work perfectly when it doesn't out of the box.

In some cases, if the server is not shutting down it's because of database, ftp, etc. connections remaining open, not in the test, but in the actual code; so there is some benefit in tracking down where that is and cleaning them up correctly. I imagine there is some harm in having hundreds or thousands of connections sitting around open when you push to production and every request is opening a new one, for example.

meelash avatar Dec 17 '19 03:12 meelash

I have created an Axios based alternative that doesn’t suffer from this issue.

https://www.npmjs.com/package/axios-test-instance

remcohaszing avatar Mar 19 '20 10:03 remcohaszing

app.close(done) works for me.

// index.js

const app = express();
const server = require('http').Server(app);
.
.
.
server.listen(3000, () => {
});

module.exports = server;

// test.js

const request = require('supertest');
const app = require('../index');

describe('Module', () => {
  after(function (done) {
    app.close(done)
  })

  it('Health Check', (done) => {
    request(app)
      .get('/health-check')
      .expect(200)
      .end((err, res) => {
        if (err) throw done(err);

        done();
      });
  });
});

kdhttps avatar Jun 29 '20 18:06 kdhttps

@kdhttps answer works for me. Be aware that you need to export the listen result, not the app itself. In my case I was using app.listen() directly to start the server, so I did:

const server = app.listen(3000);
module.exports = server;

vccolombo avatar Jul 21 '20 19:07 vccolombo

It looks like supertest sets up a new server for every call to request(). That server will close when you call .end().

Possibly test's that don't use end() are leaving the server (and mocha) running?

@deployable I have found that the server does not close when end is called. It appears to be waiting for the underlying Request's end to be called, which never happens. I am pursuing a workaround though.

ChrisChiasson avatar Jul 18 '22 21:07 ChrisChiasson