feat(wait): add human-readable String() methods to all wait strategies #119
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add String() method to all wait strategy types to provide human-readable
descriptions that are displayed in container lifecycle logs. This improves
the user experience by showing clear wait conditions instead of raw struct
output.
Changes:
- Add String() method to HTTPStrategy, HealthStrategy, LogStrategy,
HostPortStrategy, FileStrategy, ExecStrategy, ExitStrategy,
waitForSQL, MultiStrategy, NopStrategy, and TLSStrategy
- Update lifecycle logging to use strategy String() output
- Each strategy provides contextual information (ports, paths, conditions)
Example output changes:
Before: Waiting for: &{timeout:<nil> deadline:0x140001c7190 Strategies:[0x1400032e1d0]}
After: Waiting for container to be ready strategy="HTTP GET request on port 8080 path /health"
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <[email protected]>
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR implements the String() method (fmt.Stringer interface) for all wait strategy types to provide human-readable descriptions. This enables better logging when containers are waiting to become ready.
- Adds
String()method to all wait strategy implementations (TLSStrategy, SQLStrategy, NopStrategy, LogStrategy, HTTPStrategy, HostPortStrategy, HealthStrategy, FileStrategy, ExitStrategy, ExecStrategy, MultiStrategy) - Updates the lifecycle hook to use the String() method for enhanced logging output
- Each implementation provides contextual information about what the strategy is waiting for
Reviewed Changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| container/wait/tls.go | Adds String() method returning description of TLS certificates being waited for |
| container/wait/sql.go | Adds String() method showing SQL port, driver, and optional custom query |
| container/wait/nop.go | Adds String() method returning generic "custom wait condition" description |
| container/wait/log.go | Adds String() method showing log message/pattern and occurrence count |
| container/wait/http.go | Adds String() method describing HTTP/HTTPS request details |
| container/wait/host_port.go | Adds String() method explaining port listening behavior |
| container/wait/health.go | Adds String() method for health check strategy |
| container/wait/file.go | Adds String() method describing file existence and optional matching condition |
| container/wait/exit.go | Adds String() method for exit strategy |
| container/wait/exec.go | Adds String() method showing exec command being waited for |
| container/wait/all.go | Adds String() method handling multi-strategy composition with proper aggregation |
| container/lifecycle.create.go | Updates readiness hook to use String() method for improved logging |
Comments suppressed due to low confidence (1)
container/wait/http.go:1
- When Path is empty (the default), the output will show
path \"\"which may be confusing. Consider adding a conditional check: ifws.Path == \"\", use "path /" or omit the path segment entirely.
package wait
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Use switch statement instead of if-else chain in HostPortStrategy - Quote path in HTTPStrategy output for clarity - Use Port.Port() method instead of string cast for nat.Port 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Only show command name and argument count instead of full command with arguments, which may contain passwords, tokens, or other credentials. Example: "exec command \"mysql\" with 4 argument(s)" instead of ["mysql", "-u", "root", "-pSecretPass", "mydb"] 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Contributor
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Always include "all of:" prefix even when there's only one strategy after filtering out nils, to make it clear that a MultiStrategy wrapper is being used. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Use "argument" (singular) when argCount is 1, and "arguments" (plural) otherwise for grammatically correct output. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Contributor
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
mdelapenya
added a commit
to ndeloof/go-sdk
that referenced
this pull request
Nov 6, 2025
* main: feat(wait): add human-readable String() methods to all wait strategies (docker#119) feat(image): display formatted pull progress on Windows terminals (docker#118) chore(release): add Go proxy refresh automation (docker#117) chore(release): bump module versions chore(release): prevent for pushing to personal forks (docker#115) chore(release): add pre-release checks to make the release process more consistent (docker#114) chore(volume): return pointer in FindByID (docker#105) fix(container): proper error message on empty container names (docker#113) fix(container): add to nil modifiers (docker#112) feat(container): add new functional options to add container config, hostConfig and endpointSettings (docker#111) feat(container): configure pull handler at container creation (docker#110)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
String()method to all wait strategy types for human-readable descriptionsChanges
This PR adds
String()methods to all wait strategy implementations:Example Output
Before:
Waiting for container to be ready containerID=0fc8e4de4995 image=nginx:alpine Waiting for: &{timeout: deadline:0x140001c7190 Strategies:[0x1400032e1d0]}
After:
Waiting for container to be ready containerID=0fc8e4de4995 image=nginx:alpine strategy='HTTP GET request on port 8080 path /health'
Test plan
🤖 Generated with Claude Code