-
-
Notifications
You must be signed in to change notification settings - Fork 64
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
fix get-port #52
fix get-port #52
Conversation
What does it fix? Do you intend to finish this PR? |
yes
Looks like
|
Please fill out the pull request description field with what it fixes and how it fixes the problem. |
it's dangerous because some iterations are skipped
await getAvailablePort({port: options.port, host}); // eslint-disable-line no-await-in-loop | ||
} catch (error) { | ||
if (['EADDRNOTAVAIL', 'EINVAL'].includes(error.code)) { | ||
hosts.splice(hosts.indexOf(host), 1); |
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.
This mutates the passed-in array, which can cause problems later on. Copy the array at the top of this function before using it.
const port = await getPort({port: desiredPorts}); | ||
|
||
t.is(port, desiredPorts[2]); | ||
}); |
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.
I would have liked to see more comprehensive testing than a single assertion.
Are you sure that's a |
Bump :) |
get-port doesn't check port availability for all hosts in some environments, so it return port that already in use
i fix that with next steps:
os.networkInterfaces()
EADDRNOTAVAIL
on mac orEINVAL
on linux, so i remove them for all next tests