Skip to content
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

Make tests use OS-assigned ports instead of a manual specified port #193

Open
3 tasks
kstrafe opened this issue Jun 14, 2019 · 5 comments
Open
3 tasks

Make tests use OS-assigned ports instead of a manual specified port #193

kstrafe opened this issue Jun 14, 2019 · 5 comments
Labels
difficulty: easy enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed

Comments

@kstrafe
Copy link
Contributor

kstrafe commented Jun 14, 2019

Task Description

Hard-coding test ports are error-prone. We should use the OS loopback interface to find any available port instead of manually ensuring that ports on each test are different.

In rust we are able to use 127.0.0.1:0, next the OS will asignt a free port.

Task TODO

  • Check if loopback address can be used inside tests.
  • Inventarize all places where ports are bein hardcoded, report them here
  • Change all places to the dynamic port where possible.
@TimonPost
Copy link
Owner

Agree, we did do that, however, when I did this in the past I got some 'address is already in use errors', not sure why it did not resolve the port automatically.

@kstrafe
Copy link
Contributor Author

kstrafe commented Jun 15, 2019

I'm not sure how you implemented it but might be a TOCTOU error. (I can't find the use of loopbacks in the git log)

@TimonPost
Copy link
Owner

I guess there is one way to find out, just change the tests and see what it does.

@kstrafe
Copy link
Contributor Author

kstrafe commented Jun 17, 2019

Will wait for #196 to merge

@TimonPost TimonPost added difficulty: easy enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed labels Jul 6, 2019
@TimonPost TimonPost changed the title tests: Use loopback to find any available port Make tests use OS-assigned port instead of manual given port Jul 6, 2019
@TimonPost TimonPost changed the title Make tests use OS-assigned port instead of manual given port Make tests use OS-assigned ports instead of a manual specified port Jul 6, 2019
@maniyar1
Copy link

Is this still something that should happen? I went through and patched all the tests I could find. There is one test in connection_manager.rs I omitted because it used FakeSocket. All the tests seem to run on my machine, if this vaguely looks ok I can submit a PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
difficulty: easy enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants