Skip to content

Add support for using ephemeral ports #131

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

Merged
merged 2 commits into from
Dec 24, 2016
Merged

Add support for using ephemeral ports #131

merged 2 commits into from
Dec 24, 2016

Conversation

muggenhor
Copy link
Contributor

Ephemeral ports are used when binding to port '0'. The operating system
then automatically selects a (non-zero) port for you that's unused at
the time. This is great to prevent conflicts.

This commit extends the wire server such that it reports the actual port
number we've bound to, even when using ephemeral ports. This is useful
to prevent port conflicts on CI systems running many tests in parallel.

@@ -18,8 +18,10 @@ BOOST=build/examples/Calc/BoostCalculatorSteps
if [ -f $GTEST ]; then
$GTEST >/dev/null &
cucumber examples/Calc
wait
Copy link
Contributor Author

@muggenhor muggenhor Dec 15, 2016

Choose a reason for hiding this comment

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

Without this it's possible for the step definition executable to still be running when cucumber is finished. Keeping the port in use, if timing is working against us just enough then this can result in an inability to open a new listening socket on that port for the BOOST test case below.

Note that a similar race condition still exists here: the step definition executable will not necessarily have opened the TCP socket for listening by the time that cucumber attempts to connect to it. An approach that I'm using in our CI system is to run with --verbose and wait for the Listening on ... message before starting cucumber. That's more difficult to script in a shell script than the Python that I'm using though. (I'm using Python instead of Ruby because most of our CI system is either shell scripts or Python).

Copy link
Member

@paoloambrosio paoloambrosio left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution! Apart from the comment on SocketServer::listenPort() I think it can be merged.

if (ec)
{
return 0;
}
Copy link
Member

Choose a reason for hiding this comment

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

I don't think any part of the software can handle a failure here (return value of 0). Isn't it better to use the other signature that throws boost::system::system_error to make it crash if for some reason the port number cannot be retrieved?

If you agree, remember to change the documentation for the interface as well :-)

P.S. Position of { not consistent with the rest of the code

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@paoloambrosio: ack, it's in with the new push

Ephemeral ports are used when binding to port '0'. The operating system
then automatically selects a (non-zero) port for you that's unused at
the time. This is great to prevent conflicts.

This commit extends the wire server such that it reports the actual port
number we've bound to, even when using ephemeral ports. This is useful
to prevent port conflicts on CI systems running many tests in parallel.
@paoloambrosio paoloambrosio merged commit 843cc3e into cucumber:master Dec 24, 2016
@paoloambrosio
Copy link
Member

One of the Travis builds failed but there is nothing wrong with this PR. The build should go back to green when the next PR is merged.

@muggenhor muggenhor deleted the ephemeral-port-help branch December 24, 2016 22:11
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.

2 participants