Show container wait error #755
Merged
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
If container wait has failed, show an error from the engine and return an appropriate exit code. This requires engine changes from moby/moby#34999 Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Codecov Report
@@ Coverage Diff @@
## master #755 +/- ##
==========================================
+ Coverage 50.92% 53.56% +2.63%
==========================================
Files 237 218 -19
Lines 15332 14617 -715
==========================================
+ Hits 7808 7829 +21
+ Misses 7024 6305 -719
+ Partials 500 483 -17 |
Contributor
|
Looks like we are missing unit test coverage for this function. Would you mind writing a test for the error case and the success case? It should be easy to produce an error using the fake client in |
Contributor
Author
|
@dnephin test cases added, PTAL |
vdemeester
approved these changes
Dec 20, 2017
cli/command/container/utils_test.go
Outdated
| } | ||
|
|
||
| client := test.NewFakeCli(&fakeClient{waitFunc: waitFn, Version: api.DefaultVersion}) | ||
| //t.Logf("client version: %v\n", client.ClientVersion()) |
Collaborator
There was a problem hiding this comment.
You can remove that one 👼
dnephin
approved these changes
Dec 20, 2017
Contributor
dnephin
left a comment
dnephin
left a comment
There was a problem hiding this comment.
LGTM (with the commented out line removed)
cli/command/container/utils_test.go
Outdated
| var res container.ContainerWaitOKBody | ||
|
|
||
| go func() { | ||
| if strings.Contains(cid, "exit-code-42") { |
Contributor
There was a problem hiding this comment.
nit: could use a switch/case
switch {
case strings.Contains(cid, "exit-code-42"):
...
}but not a big deal
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Contributor
Author
|
Commits updated as per @vdemeester and @dnephin suggestions; I think it's ready to be merged. |
nobiit
pushed a commit
to nobidev/docker-cli
that referenced
this pull request
Nov 19, 2025
Show container wait error Upstream-commit: 1620538 Component: cli
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
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.
If container wait has failed, show an error from the engine and return an appropriate exit code.
This relies on engine changes from moby/moby#34999