feat(container): add name field to Container struct#137
feat(container): add name field to Container struct#137xHozey wants to merge 3 commits intodocker:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adds container name support to the Container abstraction so callers can access the Docker container’s name for debugging/logging.
Changes:
- Add
namefield andName()accessor toContainer. - Populate
Container.nameinFromResponsefrom Docker’sSummary.Names. - Update unit test to assert
Name()is populated.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
container/container.go |
Adds name field + accessor; sets name in FromResponse from Docker list response. |
container/container.run.go |
Sets Container.name on containers created via Run using the definition’s configured name. |
container/container_unit_test.go |
Extends TestFromResponse to include Names and assert ctr.Name(). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| dockerClient: def.dockerClient, | ||
| containerID: resp.ID, | ||
| shortID: resp.ID[:12], | ||
| name: def.name, | ||
| waitingFor: def.waitingFor, |
There was a problem hiding this comment.
Run sets Container.name from def.name, but FromResponse uses the Docker API value (typically "/name"). This makes Container.Name() inconsistent depending on how the container was created/loaded. Consider normalizing the stored name (e.g., always Docker-style with leading /, or always trimmed) so callers don’t have to special-case.
| ctr, err := FromResponse(context.Background(), nil, response) | ||
| require.NoError(t, err) | ||
| require.Equal(t, "1234567890abcdefgh", ctr.ID()) | ||
| require.Equal(t, "1234567890ab", ctr.ShortID()) | ||
| require.Equal(t, "nginx:latest", ctr.Image()) | ||
| require.Equal(t, "/my-container", ctr.Name()) |
There was a problem hiding this comment.
The test now covers the happy path where Names is non-empty, but the new FromResponse logic should also be exercised with an empty/nil Names slice to ensure it doesn’t panic and behaves as intended.
1 participant
What does this PR do?
This PR adds a Name field to the Container struct and updates the FromResponse function to populate it using Names[0] from the Docker API.
A unit test has been changed in container_unit_test.go to assert that the Name field is correctly populated.
Why is it important?
The Name field provides useful metadata for debugging and logging, making it easier to identify containers programmatically.
Related issues
How to test this PR
run TestFromResponse inside container_unit_test.go