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

afterAll fails to close Express server and leaves open handles #9227

Closed
johncmunson opened this issue Nov 23, 2019 · 6 comments
Closed

afterAll fails to close Express server and leaves open handles #9227

johncmunson opened this issue Nov 23, 2019 · 6 comments
Labels

Comments

@johncmunson
Copy link

🐛 Bug Report

Jest fails to close the Express server in the afterAll hook and leaves an open handle. After running into this issue with both supertest and axiosist, and then trying to roll my own solution, I'm starting to think this is actually an issue with Jest.

To Reproduce

  1. Clone this repo
  2. Checkout the jest-github-issue branch
  3. Run npm install
  4. Create .env file containing NODE_ENV=test
  5. Run npm test

Expected behavior

Jest should close the Express server in the afterAll hook and not leave any open handles

Link to repl or repo (highly encouraged)

https://github.com/johncmunson/todo-api
Be sure to checkout the jest-github-issue branch

envinfo

System:
    OS: macOS Mojave 10.14.6
    CPU: (8) x64 Intel(R) Core(TM) i7-4870HQ CPU @ 2.50GHz
  Binaries:
    Node: 10.16.3 - ~/n/bin/node
    Yarn: 1.19.1 - /usr/local/bin/yarn
    npm: 6.9.0 - ~/n/bin/npm
  npmPackages:
    jest: ^24.9.0 => 24.9.0
@johncmunson
Copy link
Author

johncmunson commented Nov 23, 2019

After following @SimenB's advice here, I converted my beforeAll and afterAll hooks to use done instead of being async. His advice did not work, and the issue is not resolved. Jest still leaves open handles.

const { createServer } = require('http')
let axios = require('axios')
const getPort = require('get-port')
const { app } = require('../../index')
const { Todo } = require('../../models')

let server

beforeAll((done) => {
  getPort()
    .then(port => {
      axios = axios.create({ baseURL: `http://localhost:${port}` })
      server = createServer(app)
      server.listen(port, done)
    })
})

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

describe('GET /todos', () => {
  it('returns a list of todos', async () => {
    const { data: todos } = await axios.get('/todos')
    todos.forEach(todo => {
      expect(() => Todo.fromJson(todo)).not.toThrow()
    })
  })
})

@SimenB
Copy link
Member

SimenB commented Nov 23, 2019

This is because knex has a connection pool. This diff will exit the process correctly, but probably also make it so you can't reuse your db in other tests. You should startup and close a db connection in every test, not have a shared singleton.

diff --git i/tests/integration/todos.test.js w/tests/integration/todos.test.js
index d5e79d5..a37475e 100644
--- i/tests/integration/todos.test.js
+++ w/tests/integration/todos.test.js
@@ -2,6 +2,7 @@ let axios = require('axios')
 const getPort = require('get-port')
 const { app } = require('../../index')
 const { Todo } = require('../../models')
+const knex = require('../../db')
 
 let server
 
@@ -13,6 +14,7 @@ beforeAll(async () => {
 
 afterAll(() => {
   server.close()
+  knex.destroy()
 })
 
 describe('GET /todos', () => {

Note that you should probably wait for both close and destroy to invoke their callbacks.

We have a guide for mongodb - I'm sure knex is similar enough that it can be applied https://jestjs.io/docs/en/mongodb

EDIT: That guide is just a preset, not as general as I thought it was. There is an example in this repo though, which has all the code for it: https://github.com/facebook/jest/tree/77c3ceedfd61ddc841e11fec7b76e540924d3e60/examples/mongodb. Maybe use that as basis to create a similar preset for knex? Dunno 🙂

@SimenB SimenB closed this as completed Nov 23, 2019
@johncmunson
Copy link
Author

johncmunson commented Nov 23, 2019

@SimenB thank you for making me aware of the testEnvironment config property. Perhaps drawing more attention to this in the docs would be helpful, because unintuitively it would seem that in many cases globalSetup and globalTeardown are not sufficient on their own. I will make another comment with my solution once I've got it working. Perhaps it can be the basis for a new knex example repo in the docs, similar to the mongo example.

Update:
As promised, here is my new and improved Jest setup.
https://github.com/johncmunson/todo-api/tree/jest-github-issue-resolved

@SimenB
Copy link
Member

SimenB commented Nov 23, 2019

Yeah, sounds good! Happy to take any other doc updates to make it clearer as well

@johncmunson
Copy link
Author

On a sidenote, jest-circus looks absolutely fascinating. The ability to act on any one of these Jest events looks very powerful!

@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 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

2 participants