-
Notifications
You must be signed in to change notification settings - Fork 2.1k
fix: cli prompt termination exit code #4888
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| package builder | ||
|
|
||
| import ( | ||
| "context" | ||
|
|
||
| "github.com/docker/docker/api/types" | ||
| "github.com/docker/docker/client" | ||
| ) | ||
|
|
||
| type fakeClient struct { | ||
| client.Client | ||
| builderPruneFunc func(ctx context.Context, opts types.BuildCachePruneOptions) (*types.BuildCachePruneReport, error) | ||
| } | ||
|
|
||
| func (c *fakeClient) BuildCachePrune(ctx context.Context, opts types.BuildCachePruneOptions) (*types.BuildCachePruneReport, error) { | ||
| if c.builderPruneFunc != nil { | ||
| return c.builderPruneFunc(ctx, opts) | ||
| } | ||
| return nil, nil | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| package builder | ||
|
|
||
| import ( | ||
| "context" | ||
| "errors" | ||
| "testing" | ||
|
|
||
| "github.com/docker/cli/cli/command" | ||
| "github.com/docker/cli/internal/test" | ||
| "github.com/docker/docker/api/types" | ||
| "gotest.tools/v3/assert" | ||
| ) | ||
|
|
||
| func TestBuilderPromptTermination(t *testing.T) { | ||
| ctx, cancel := context.WithCancel(context.Background()) | ||
| t.Cleanup(cancel) | ||
|
|
||
| cli := test.NewFakeCli(&fakeClient{ | ||
| builderPruneFunc: func(ctx context.Context, opts types.BuildCachePruneOptions) (*types.BuildCachePruneReport, error) { | ||
| return nil, errors.New("fakeClient builderPruneFunc should not be called") | ||
| }, | ||
| }) | ||
| cmd := NewPruneCommand(cli) | ||
| test.TerminatePrompt(ctx, t, cmd, cli, func(t *testing.T, err error) { | ||
| t.Helper() | ||
| assert.ErrorIs(t, err, command.ErrPromptTerminated) | ||
| }) | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| package container | ||
|
|
||
| import ( | ||
| "context" | ||
| "testing" | ||
|
|
||
| "github.com/docker/cli/cli/command" | ||
| "github.com/docker/cli/internal/test" | ||
| "github.com/docker/docker/api/types" | ||
| "github.com/docker/docker/api/types/filters" | ||
| "github.com/pkg/errors" | ||
| "gotest.tools/v3/assert" | ||
| ) | ||
|
|
||
| func TestContainerPrunePromptTermination(t *testing.T) { | ||
| ctx, cancel := context.WithCancel(context.Background()) | ||
| t.Cleanup(cancel) | ||
|
|
||
| cli := test.NewFakeCli(&fakeClient{ | ||
| containerPruneFunc: func(ctx context.Context, pruneFilters filters.Args) (types.ContainersPruneReport, error) { | ||
| return types.ContainersPruneReport{}, errors.New("fakeClient containerPruneFunc should not be called") | ||
| }, | ||
| }) | ||
| cmd := NewPruneCommand(cli) | ||
| test.TerminatePrompt(ctx, t, cmd, cli, func(t *testing.T, err error) { | ||
| t.Helper() | ||
| assert.ErrorIs(t, err, command.ErrPromptTerminated) | ||
| }) | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| package network | ||
|
|
||
| import ( | ||
| "context" | ||
| "testing" | ||
|
|
||
| "github.com/docker/cli/cli/command" | ||
| "github.com/docker/cli/internal/test" | ||
| "github.com/docker/docker/api/types" | ||
| "github.com/docker/docker/api/types/filters" | ||
| "github.com/pkg/errors" | ||
| "gotest.tools/v3/assert" | ||
| ) | ||
|
|
||
| func TestNetworkPrunePromptTermination(t *testing.T) { | ||
| ctx, cancel := context.WithCancel(context.Background()) | ||
| t.Cleanup(cancel) | ||
|
|
||
| cli := test.NewFakeCli(&fakeClient{ | ||
| networkPruneFunc: func(ctx context.Context, pruneFilters filters.Args) (types.NetworksPruneReport, error) { | ||
| return types.NetworksPruneReport{}, errors.New("fakeClient networkPruneFunc should not be called") | ||
| }, | ||
| }) | ||
| cmd := NewPruneCommand(cli) | ||
| test.TerminatePrompt(ctx, t, cmd, cli, func(t *testing.T, err error) { | ||
| t.Helper() | ||
| assert.ErrorIs(t, err, command.ErrPromptTerminated) | ||
| }) | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| Upgrading plugin foo/bar from localhost:5000/foo/bar:v0.1.0 to localhost:5000/foo/bar:v1.0.0 | ||
| Plugin images do not match, are you sure? [y/N] | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing a newline at the end here? (The other new golden files have them 🤔)
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @Benehiko could we fix this?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. hmm, i'm not sure why this is. The file is generated by the
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Right, this is likely due to us missing a newline when printing things somewhere |
||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
here the context isn't apparent. Either we use the top level dockerCli context or create a new context within the function. The moby project does not support passing context on this function call https://github.com/moby/moby/blob/d19d98b136f6a7e80029b65c5ae8886eacdf43d7/api/types/client.go#L292
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FWIW; if we would benefit from passing contexts, we should consider updating the moby code. For a lot of code in moby,
contextdidn't exist yet when it was written, and we started passing through contexts more, but there's still many areas where we didn't.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll take a look at also adding context in the moby api client. It will help greatly with otel and just task cleanup in general.