Skip to content

Conversation

Kuldeep-Sahoo
Copy link

No description provided.

@UlisesGascon
Copy link
Member

Hey @Kuldeep-Sahoo! Thanks for open a PR to the project 🙌

Can you explain the motivation of the change?

@Kuldeep-Sahoo
Copy link
Author

Hi @UlisesGascon 👋,
The motivation for this change is to ensure that the server does not remain open during tests.

Previously, the code:

const server = app.listen();
server.close(done);

could leave the server open momentarily.

With the change:

const server = app.listen(0, () => server.close(done));

the server is started on an ephemeral port (0) and immediately closed inside the callback, ensuring that all servers are properly closed before moving forward.

This helps avoid dangling server instances during tests. ✅

Copy link
Member

@bjohansebas bjohansebas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey, thanks for your contribution! I don’t really understand why that would be a problem, are you having issues running the tests?

@Kuldeep-Sahoo
Copy link
Author

Hi @bjohansebas 👋, thanks for the feedback!

You're right — in most cases the original code works fine. The motivation for my change is to make the test more deterministic and robust:

With

const server = app.listen();
server.close(done);

there’s a small chance the server might not have fully started listening before server.close() is called. That can leave a brief window where the server is open, or cause flaky behavior in some environments.

By using

const server = app.listen(0, () => server.close(done));

we ensure the server is actually listening on an ephemeral port before we close it. That way the test guarantees all servers are properly started and closed in a controlled way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants