-
-
Notifications
You must be signed in to change notification settings - Fork 347
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
feat: write listen files with actual address #1607
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.
I would also like to do this for all other services, so we can use this as a test playground for the feature.
github.com/pelletier/go-toml v1.9.5 | ||
github.com/phayes/freeport v0.0.0-20220201140144-74d24b5ae9f5 |
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.
No more racy freeport 🎉
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.
Looks very nice! I just have some minor remarks ;)
|
||
type GetAddr = func(testing.TB, string) (host string, port string, fullAddr string) | ||
|
||
func UseDynamicPorts(ctx context.Context, t testing.TB, r Registry) GetAddr { |
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.
Could we just return a struct with all the values here instead of a closure?
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.
No not really. The test has to do these steps in this order:
- create the registry
- call
UseDynamicPorts
to set the config values - start the server(s); potentially only some and not all
- use the closure to retrieve the actual address the server is listening on
What we do with this change is let the OS choose a free port when the server starts listening (using port 0
). We can only get the actual port number after the server was started, therefore this step comes later.
The problem with freeport
is that their approach is to open a socket on port 0
, get the actual port number, and then close it again. This results in racy behavior where a parallel test gets the same port assigned, because the socket is closed and then reopened.
b0bb1f2
to
ba8c4db
Compare
It is now possible to set a
write_listen_file
URL in the config for each port. The server will write the actual listen address to that file, so it is now possible to easily use port0
.