-
-
Notifications
You must be signed in to change notification settings - Fork 16.8k
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
use random port in test so it won't fail on already listening #5162
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
I think this is the only test that given a specific port is actually listens on that, but the test never asserts it, so probably fine to make it |
Any blocker from merging this? |
Sorry @rluvaton no, I had just left a general comment, not a review change request. I will get it merged shortly, just wanted to give it at least like 24-48 hours for comments and stuff. |
@dougwilson I updated with the requested test, can you re-review? |
Your new tests will fail, because older Node.js versions don't support |
thanks, removed the use of Promises 😢 |
Any thoughts or maybe a rerun so we can merge this |
Ping |
f5bd4de
to
98f3ef4
Compare
In my machine, the docker desktop is using port 9999, so replacing it to 0 so it won't fail...