Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion cli/command/network/remove.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package network
import (
"context"
"fmt"
"strconv"

"github.com/docker/cli/cli"
"github.com/docker/cli/cli/command"
Expand Down Expand Up @@ -68,7 +69,7 @@ func runRemove(ctx context.Context, dockerCLI command.Cli, networks []string, op
}

if status != 0 {
return cli.StatusError{StatusCode: status}
return cli.StatusError{StatusCode: status, Status: "exit status " + strconv.Itoa(status)}
}
return nil
}
8 changes: 3 additions & 5 deletions cli/error.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,5 @@
package cli

import (
"strconv"
)

// StatusError reports an unsuccessful exit by a command.
type StatusError struct {
Cause error
Expand All @@ -21,7 +17,9 @@ func (e StatusError) Error() string {
if e.Cause != nil {
return e.Cause.Error()
}
return "exit status " + strconv.Itoa(e.StatusCode)
// we don't want to set a default message here,
// some commands might want to be explicit about the error message
return ""
}

func (e StatusError) Unwrap() error {
Expand Down
4 changes: 3 additions & 1 deletion cmd/docker/docker.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,9 @@ func main() {
}

if err != nil && !errdefs.IsCancelled(err) {
_, _ = fmt.Fprintln(os.Stderr, err)
if err.Error() != "" {
_, _ = fmt.Fprintln(os.Stderr, err)
}
os.Exit(getExitCode(err))
}
}
Expand Down
5 changes: 1 addition & 4 deletions e2e/cli-plugins/socket_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,6 @@ func TestPluginSocketBackwardsCompatible(t *testing.T) {
assert.Assert(t, errors.As(err, &exitError))
assert.Check(t, exitError.Exited())
assert.Check(t, is.Equal(exitError.ExitCode(), 1))
assert.Check(t, is.ErrorContains(err, "exit status 1"))

// the plugin process does not receive a SIGINT and does
// the CLI cannot cancel it over the socket, so it kills
Expand Down Expand Up @@ -199,11 +198,10 @@ func TestPluginSocketCommunication(t *testing.T) {
assert.Assert(t, errors.As(err, &exitError))
assert.Check(t, exitError.Exited())
assert.Check(t, is.Equal(exitError.ExitCode(), 2))
assert.Check(t, is.ErrorContains(err, "exit status 2"))

// the plugin does not get signalled, but it does get its
// context canceled by the CLI through the socket
const expected = "test-socket: exiting after context was done\nexit status 2"
const expected = "test-socket: exiting after context was done"
actual := strings.TrimSpace(string(out))
assert.Check(t, is.Equal(actual, expected))
})
Expand Down Expand Up @@ -238,7 +236,6 @@ func TestPluginSocketCommunication(t *testing.T) {
assert.Assert(t, errors.As(err, &exitError))
assert.Check(t, exitError.Exited())
assert.Check(t, is.Equal(exitError.ExitCode(), 1))
assert.Check(t, is.ErrorContains(err, "exit status 1"))

// the plugin process does not receive a SIGINT and does
// not exit after having it's context canceled, so the CLI
Expand Down
1 change: 0 additions & 1 deletion e2e/container/attach_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,5 +58,4 @@ func TestAttachInterrupt(t *testing.T) {
// the CLI should exit with 33 (the SIGINT was forwarded to the container), and the
// CLI process waited for the container exit and properly captured/set the exit code
assert.Equal(t, c.ProcessState.ExitCode(), 33)
assert.Equal(t, d.String(), "exit status 33\n")
}
1 change: 0 additions & 1 deletion e2e/container/run_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,6 @@ func TestRunAttach(t *testing.T) {
}

assert.Equal(t, c.ProcessState.ExitCode(), 7)
assert.Check(t, is.Contains(d.String(), "exit status 7"))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess if we want to be strict in our tests, we could consider checking for the error message to be empty (with a comment);

assert.Check(t, is.Equal(err.Error(), ""), "should not print our own error message, as the container's output is the error message")

Not urgent for how, but something we could consider

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Derp; meant checking the d.String() for what we expect it to output (for that we could add a echo "something failed" to the container's command.

(but again, probably fine to look at in a follow up)

})
}
}
Expand Down
Loading