-
-
Notifications
You must be signed in to change notification settings - Fork 522
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
fix(redis): port race #2694
fix(redis): port race #2694
Conversation
Fix race condition on port availability by switching to port instead of log check.
✅ Deploy Preview for testcontainers-go ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
@@ -54,7 +54,7 @@ func Run(ctx context.Context, img string, opts ...testcontainers.ContainerCustom | |||
req := testcontainers.ContainerRequest{ | |||
Image: img, | |||
ExposedPorts: []string{"6379/tcp"}, | |||
WaitingFor: wait.ForLog("* Ready to accept connections"), | |||
WaitingFor: wait.ForListeningPort("6379/tcp"), |
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 haven't seen any issue with this wait strategy, which is widely use here and in the java module.
Could you bring more light about the motivation for the change?
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.
With all the iterations on the test suite I've had a few cases where test failed due to redis not accept incoming requests.
I didn't dig in fully as with this change, I never saw a reoccurrence. It could be redis reports the log entry early?
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.
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.
Sure will keep an eye for a failure.
Closing until we get another reproduction of this issue. |
Fix race condition on port availability by switching to port instead of log check.