Fix OptPorts to wait for listening ports, not exposed port#40
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adjusts container test configuration so OptPorts waits for each explicitly requested port to be listening (instead of using a generic “exposed port” wait), and updates the related unit test accordingly.
Changes:
- Update
OptPortsto appendwait.ForListeningPortfor every requested port. - Update
Test_Opt_001to cover multiple ports and the corresponding per-port wait strategies.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| pkg/test/opt.go | Changes port option behavior to add a listening-port wait strategy for each requested port. |
| pkg/test/opt_test.go | Updates the option unit test to assert multiple exposed ports and multiple wait strategies. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // OptPorts exposes one or more container ports and waits for each requested port. | ||
| func OptPorts(ports ...string) Opt { | ||
| return func(o *opts) error { | ||
| o.req.ExposedPorts = ports | ||
| o.appendWaitStrategy(wait.ForExposedPort()) | ||
| for _, port := range ports { | ||
| o.appendWaitStrategy(wait.ForListeningPort(nat.Port(port))) | ||
| } | ||
| return nil |
There was a problem hiding this comment.
OptPorts now appends no wait strategy when called with zero ports. Given NewContainer falls back to wait.ForExit() when no strategies are present, an accidental OptPorts() call can cause long-running containers to hang. Consider returning an error when len(ports)==0 (or explicitly documenting/handling the empty case).
This pull request updates how container ports are exposed and waited for in tests. Instead of waiting for just one port, the system now waits for each requested port individually. The related test has been updated to check for multiple ports and their corresponding wait strategies.
Port exposure and waiting improvements:
OptPortsinopt.goto wait for each requested port individually by appending await.ForListeningPortstrategy for every port, instead of a single generic wait.Test_Opt_001test inopt_test.goto use multiple ports ("3389/tcp","389/tcp"), verify that both are exposed, and check that a wait strategy is set up for each port. [1] [2]