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

request free ports from the OS #94

Merged
merged 1 commit into from Mar 30, 2015

Conversation

drewwells
Copy link
Contributor

Tests were hardcoded to ports that may not be available on the system. Some ports were reused in subsequent tests which caused tests to occasionally fail when the system didn't release these ports.

@@ -504,18 +504,20 @@ func SpawnClientServerTest(t *testing.T, host string, client ClientRoutine, serv
endServer := make(chan bool)
listenWait := make(chan bool)

listener, listenErr := net.Listen("tcp", host)
Copy link
Contributor

Choose a reason for hiding this comment

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

Passing in the host as an argument shouldn't be needed any more, just fill this in with "localhost:0"

@dmcgowan
Copy link
Contributor

Yes, this change is needed. Do you think you can make the update I suggested to clean it up. Also please sign the commit according to https://github.com/docker/docker/blob/master/CONTRIBUTING.md#sign-your-work. Then I will get this merged. Thanks!

@drewwells
Copy link
Contributor Author

Definitely already working on it, do I need sign every commit?

@dmcgowan
Copy link
Contributor

Yes add the sign off to every commit

@dmcgowan
Copy link
Contributor

LGTM, just please add sign off to first commit.

passing host string is no longer required

Signed-off-by: Drew Wells <drew.wells00@gmail.com>
@drewwells
Copy link
Contributor Author

Sorry about that, rebased to one commit with sign.

@dmcgowan
Copy link
Contributor

Excellent! thanks LGTM

dmcgowan added a commit that referenced this pull request Mar 30, 2015
@dmcgowan dmcgowan merged commit 3d0b138 into docker:master Mar 30, 2015
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.

None yet

2 participants