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

fix(wait): inconsistent behaviour #2773

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

stevenh
Copy link
Collaborator

@stevenh stevenh commented Sep 8, 2024

Use IsHost method instead of comparison with string.

Remove unnecessary if nil check on slice length.

Remove unnecessary fmt.Sprintf on non format string.

Refactor HostPortStrategy and HTTPStrategy tests to use generated mocks and a builder to increase test coverage and fix the behavioural inconsistencies identified.

Replicate WithForcedIPv4LocalHost from HTTPStrategy to HostPortStrategy to ensure feature parity.

Refactor wait host port lookups to use common functions so that HostPortStrategy and HTTPStrategy don't have hard to find logic differences in edge case handling. This includes consistent target checks, exposed port waiting for specified and unspecified paths.

Improve error checking and wrapping.

Extract wait tests into wait_test package.

Fix ForLog to allow checking across multiple log reads and ensure that the container is valid.

Fix ForLog to return as soon as context is cancelled instead of waiting for poll to expire.

Refactor ForLog to allow line by line checks which is quicker in the common case.

Copy link

netlify bot commented Sep 8, 2024

Deploy Preview for testcontainers-go ready!

Name Link
🔨 Latest commit 326ddfa
🔍 Latest deploy log https://app.netlify.com/sites/testcontainers-go/deploys/66e014eced041800081bd8ea
😎 Deploy Preview https://deploy-preview-2773--testcontainers-go.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@stevenh stevenh force-pushed the fix/consistent-wait-handling branch 4 times, most recently from f9974e1 to 3f1a99e Compare September 9, 2024 16:46
Use IsHost method instead of comparison with string.

Remove unnecessary if nil check on slice length.

Remove unnecessary fmt.Sprintf on non format string.

Refactor HostPortStrategy and HTTPStrategy tests to use generated
mocks and a builder to increase test coverage and fix the
behavioural inconsistencies identified.

Replicate WithForcedIPv4LocalHost from HTTPStrategy to
HostPortStrategy to ensure feature parity.

Refactor wait host port lookups to use common functions so that
HostPortStrategy and HTTPStrategy don't have hard to find logic
differences in edge case handling. This includes consistent
target checks, exposed port waiting for specified and unspecified
paths.

Improve error checking and wrapping.

Extract wait tests into wait_test package.

Fix ForLog to allow checking across multiple log reads and ensure
that the container is valid.

Fix ForLog to return as soon as context is cancelled instead of
waiting for poll to expire.

Refactor ForLog to allow line by line checks which is quicker in
the common case.
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.

1 participant