Http Server not closing after tests

[martypdx]

Hey I've been using chai-http for testing for quite some time and I'm 95% sure that it would correctly allow mocha to exist after the tests are run. Now though, it just causes mocha to hang after the tests. If I pass in a server (instead of a listening function) and explicitly close, it terminates correctly.

Below are steps for workaround I had to use to get it to work:

const chaiHttp = require('chai-http');
const http = require('http'); // (1) add a require for http
const assert = chai.assert;
chai.use(chaiHttp);

describe.only('http app', () => {

    // (2) create the server and pass that to chai.request
    const server = http.createServer(app);
    const request = chai.request(server);

    // (3) add an after that closes the server
    after(done => server.close(done));

    // (4) profit by having npm test terminate correctly!
    it('GET /greeting/name responds with greeting', done => {

[liesislukas]

in test:

  before(async () => {
    process.env.PORT = await portfinder.getPortPromise({port: 10002});
    app = require('./../../../app');
  });

  after(async () => {
    require('./../../../app').stop();
  });

and in app:

...

let port = process.env.PORT || 8080;
let server = app.listen(port, () => {
  console.log('App listening on port %s, in environment %s!', port, _.toUpper(process.env.NODE_ENV || ''));
});

function stop() {
  server.close();
}

module.exports = server;
module.exports.stop = stop;

[cklanac]

@martypdx Mocha 4.0 changed the default behavior.
"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"

See mochajs/mocha#2879

So, if you decide to upgrade to Mocha 4.x you'll need to add the --exit flag mocha --exit. Or manually close the server as @liesislukas suggested above.

...
"scripts": {
  "start": "node server.js",
  "test": "mocha --exit"
},
...

[keithamus]

Thanks for the issue @martypdx and thanks for the responses @liesislukas and @cklanac.

I think all of the work-arounds seem valid; just depends on which one you'd be happiest with in your test suite.

Would anyone like to file a PR improving our documentation around this? Listing the alternative ideas? I'd be happy to merge such a PR!

[martypdx]

Thanks for responses, makes sense.

I think one thing that might help from an API perspective is to expose the server as a prop on the request when using the listening function overload, or at least offer close method.

To effectively deal with this issue, you have to run your own http.createServer(app) in order to maintain a handle. I tend to create a single request via a dedicated module (side question: is there any reason not to share the superagent request across tests?), so this isn't that big a deal.

Generally though, having access to the server seems logical:

const chai = require('chai');
const chaiHttp = require('chai-http');
chai.use(chaiHttp);

const app = require('../../lib/app');
const request = chai.request(app);

after(() => request.server.close());

[keithamus]

Yeah, actually this makes a lot of sense. I worry though - what of the ergonomics when using a non-express app, what will request.server be then? null?

[barraponto]

@martypdx if you want to share the agent, use const agent = chai.request.agent(app). It is useful when you want to keep the cookies between the requests.

[barraponto]

@martypdx @keithamus we could have something like request.server(server => server.close()).

[keithamus]

I see little point in making it a callback - as then we'd simply offer no assurance it got called, and no way to verify at runtime if it was doable. I suppose having request.server == null in other cases can at least be checked.

I'd welcome a PR which adds request.server alongside some tests, and documentation within the README.

[martypdx]

@keithamus:

I worry though - what of the ergonomics when using a non-express app, what will request.server be then? null?

Do you mean a string hostname like chai.request('http://localhost:3000')? The app param is a handler at the node http server level, nothing express specific.

I did notice that the chai.request.agent(...) method already stores the app argument as this.app, which includes functions converted to httpServer. Perhaps a better approach would be to add .app to the returned request object and the documentation story is that .app is whatever got passed in as an argument, unless it was a function it which case .app is the created httpServer.

Whether .app or adding a specific .server, you know chai http audience better than I do. Just good to make .request() and request.agent() the same.

Happy to help with PR

@barraponto:

want to share the agent, use const agent = chai.request.agent(app). It is useful when you want to keep the cookies between the requests.

Cookies? Aren't those just used by ads, invasive trackers, and tedious oauth calls? 😉

[keithamus]

Do you mean a string hostname like chai.request('http://localhost:3000')? The app param is a handler at the node http server level, nothing express specific.

Sorry, yes, I meant strings.

[kswope]

@martypdx 'workaround' seems to me to be not a workaround but the proper way.

The change to mocha exposed a flaw in this lib's api, although it seems convenient, having request start a listening server by itself and then relying on mocha to murder it when its no longer wanted is a bad plan.

I think just changing the documentation, and maybe deprecating passing a requestListener function has my vote. Somehow exposing the internally started http.Server, otherwise returning a null if an an address had been passed to request smells funny to me.

With those changes, if I understand the code correctly, the only reason the http.Server is being passed in is to build the address string that would have been passed in.

So the docs would read something like:

chai.request() takes a URL

chai.request('http://localhost:3000')

If you are running a express server, as a convenience, you can pass in the server itself and the URL is obtained automatically from server.address().address and server.address().port ( hence doesn't work with koa or hapi )

chai.request(server)

[garrettlove8]

For anyone having an issue using CircleCi where the build won't complete this solves it