Launching a server on-the-fly can intermittently cause failures due to asynchronous startup

[davidje13]

Server.listen is asynchronous with a callback parameter, but this line invokes it as a synchronous operation (or fire-and-forget).

This is usually fine, as the server address is set immediately and by the time a request first reaches the server, it has usually started up. But it is a race condition, and I have noticed tests in my project occasionally fail because of it.

The fix is to start the server in advance, for example:

describe('whatever', () => {
  let server;

  beforeEach((done) => {
    server = app.listen(0, done); // expressjs (syntax is similar for raw http.Server)
  });

  afterEach(() => {
    server.close();
  });

  it('something', async () => {
    await request(server)
      .get('/woo')
      .expect(200);
  });
});

This also allows closing the server at the end; something which supertest does not do if it launches a server on request (see e.g. #489 and #437)

This issue can be worked around by adding special logic to internally wait for the listen to succeed before sending any requests, but this will add a fair amount of complexity to the code. I suggest the better fix is to drop this feature and encourage all users of the library to write tests as shown above; with an externally-managed server lifecycle. The user can wait until it has fully started up and is aware of when the server can be shutdown; supertest doesn't have all this information so shouldn't try to half-way infer it.