From b8cddc63add5704c0ae434040ed490b97bb1b28b Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sun, 13 Oct 2024 17:47:07 +0200 Subject: [PATCH 1/9] completion: ContainerNames: don't panic on nil filter Signed-off-by: Sebastiaan van Stijn --- cli/command/completion/functions.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/command/completion/functions.go b/cli/command/completion/functions.go index b217ec54b05d..deda08e7ba92 100644 --- a/cli/command/completion/functions.go +++ b/cli/command/completion/functions.go @@ -59,7 +59,7 @@ func ContainerNames(dockerCLI APIClientProvider, all bool, filters ...func(conta for _, ctr := range list { skip := false for _, fn := range filters { - if !fn(ctr) { + if fn != nil && !fn(ctr) { skip = true break } From 8f2e5662e7e4d74f5cc33887749f9378287df627 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sun, 13 Oct 2024 17:48:49 +0200 Subject: [PATCH 2/9] completion: add test for EnvVarNames Signed-off-by: Sebastiaan van Stijn --- cli/command/completion/functions_test.go | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/cli/command/completion/functions_test.go b/cli/command/completion/functions_test.go index a4bb6f543857..817aef27a61b 100644 --- a/cli/command/completion/functions_test.go +++ b/cli/command/completion/functions_test.go @@ -1,13 +1,28 @@ package completion import ( + "sort" "testing" "github.com/spf13/cobra" "gotest.tools/v3/assert" is "gotest.tools/v3/assert/cmp" + "gotest.tools/v3/env" ) +func TestCompleteEnvVarNames(t *testing.T) { + env.PatchAll(t, map[string]string{ + "ENV_A": "hello-a", + "ENV_B": "hello-b", + }) + values, directives := EnvVarNames(nil, nil, "") + assert.Check(t, is.Equal(directives&cobra.ShellCompDirectiveNoFileComp, cobra.ShellCompDirectiveNoFileComp), "Should not perform file completion") + + sort.Strings(values) + expected := []string{"ENV_A", "ENV_B"} + assert.Check(t, is.DeepEqual(values, expected)) +} + func TestCompletePlatforms(t *testing.T) { values, directives := Platforms(nil, nil, "") assert.Check(t, is.Equal(directives&cobra.ShellCompDirectiveNoFileComp, cobra.ShellCompDirectiveNoFileComp), "Should not perform file completion") From a5ca5b33f11eb19111a29c425c9a75f50c1aefd5 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sun, 13 Oct 2024 17:52:50 +0200 Subject: [PATCH 3/9] completion: add test for FileNames Signed-off-by: Sebastiaan van Stijn --- cli/command/completion/functions_test.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/cli/command/completion/functions_test.go b/cli/command/completion/functions_test.go index 817aef27a61b..d915ccdc362c 100644 --- a/cli/command/completion/functions_test.go +++ b/cli/command/completion/functions_test.go @@ -23,6 +23,12 @@ func TestCompleteEnvVarNames(t *testing.T) { assert.Check(t, is.DeepEqual(values, expected)) } +func TestCompleteFileNames(t *testing.T) { + values, directives := FileNames(nil, nil, "") + assert.Check(t, is.Equal(directives, cobra.ShellCompDirectiveDefault)) + assert.Check(t, is.Len(values, 0)) +} + func TestCompletePlatforms(t *testing.T) { values, directives := Platforms(nil, nil, "") assert.Check(t, is.Equal(directives&cobra.ShellCompDirectiveNoFileComp, cobra.ShellCompDirectiveNoFileComp), "Should not perform file completion") From 51713196c9d5312cf98f59ff0a73452dda5049e2 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sun, 13 Oct 2024 17:50:29 +0200 Subject: [PATCH 4/9] completion: add test for FromList Signed-off-by: Sebastiaan van Stijn --- cli/command/completion/functions_test.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/cli/command/completion/functions_test.go b/cli/command/completion/functions_test.go index d915ccdc362c..7a98e2d3c954 100644 --- a/cli/command/completion/functions_test.go +++ b/cli/command/completion/functions_test.go @@ -29,6 +29,14 @@ func TestCompleteFileNames(t *testing.T) { assert.Check(t, is.Len(values, 0)) } +func TestCompleteFromList(t *testing.T) { + expected := []string{"one", "two", "three"} + + values, directives := FromList(expected...)(nil, nil, "") + assert.Check(t, is.Equal(directives&cobra.ShellCompDirectiveNoFileComp, cobra.ShellCompDirectiveNoFileComp), "Should not perform file completion") + assert.Check(t, is.DeepEqual(values, expected)) +} + func TestCompletePlatforms(t *testing.T) { values, directives := Platforms(nil, nil, "") assert.Check(t, is.Equal(directives&cobra.ShellCompDirectiveNoFileComp, cobra.ShellCompDirectiveNoFileComp), "Should not perform file completion") From be197da6b848c4f3efbaf6bcdde5f50868acbb6c Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sun, 13 Oct 2024 17:54:40 +0200 Subject: [PATCH 5/9] completion: add test for NoComplete Signed-off-by: Sebastiaan van Stijn --- cli/command/completion/functions_test.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/cli/command/completion/functions_test.go b/cli/command/completion/functions_test.go index 7a98e2d3c954..f5227539302f 100644 --- a/cli/command/completion/functions_test.go +++ b/cli/command/completion/functions_test.go @@ -37,6 +37,12 @@ func TestCompleteFromList(t *testing.T) { assert.Check(t, is.DeepEqual(values, expected)) } +func TestCompleteNoComplete(t *testing.T) { + values, directives := NoComplete(nil, nil, "") + assert.Check(t, is.Equal(directives, cobra.ShellCompDirectiveNoFileComp)) + assert.Check(t, is.Len(values, 0)) +} + func TestCompletePlatforms(t *testing.T) { values, directives := Platforms(nil, nil, "") assert.Check(t, is.Equal(directives&cobra.ShellCompDirectiveNoFileComp, cobra.ShellCompDirectiveNoFileComp), "Should not perform file completion") From f3b4094eb05815083ba0a7e7e064dfccd05bd091 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sun, 13 Oct 2024 17:57:46 +0200 Subject: [PATCH 6/9] completion: add test for ContainerNames Signed-off-by: Sebastiaan van Stijn --- cli/command/completion/functions_test.go | 135 +++++++++++++++++++++++ 1 file changed, 135 insertions(+) diff --git a/cli/command/completion/functions_test.go b/cli/command/completion/functions_test.go index f5227539302f..6f39d8ade191 100644 --- a/cli/command/completion/functions_test.go +++ b/cli/command/completion/functions_test.go @@ -1,15 +1,150 @@ package completion import ( + "context" + "errors" "sort" "testing" + "github.com/docker/docker/api/types/container" + "github.com/docker/docker/api/types/filters" + "github.com/docker/docker/client" + "github.com/google/go-cmp/cmp/cmpopts" "github.com/spf13/cobra" "gotest.tools/v3/assert" is "gotest.tools/v3/assert/cmp" "gotest.tools/v3/env" ) +type fakeCLI struct { + *fakeClient +} + +// Client implements [APIClientProvider]. +func (c fakeCLI) Client() client.APIClient { + return c.fakeClient +} + +type fakeClient struct { + client.Client + containerListFunc func(options container.ListOptions) ([]container.Summary, error) +} + +func (c *fakeClient) ContainerList(_ context.Context, options container.ListOptions) ([]container.Summary, error) { + if c.containerListFunc != nil { + return c.containerListFunc(options) + } + return []container.Summary{}, nil +} + +func TestCompleteContainerNames(t *testing.T) { + tests := []struct { + doc string + showAll, showIDs bool + filters []func(container.Summary) bool + containers []container.Summary + expOut []string + expOpts container.ListOptions + expDirective cobra.ShellCompDirective + }{ + { + doc: "no results", + expDirective: cobra.ShellCompDirectiveNoFileComp, + }, + { + doc: "all containers", + showAll: true, + containers: []container.Summary{ + {ID: "id-c", State: "running", Names: []string{"/container-c", "/container-c/link-b"}}, + {ID: "id-b", State: "created", Names: []string{"/container-b"}}, + {ID: "id-a", State: "exited", Names: []string{"/container-a"}}, + }, + expOut: []string{"container-c", "container-c/link-b", "container-b", "container-a"}, + expOpts: container.ListOptions{All: true}, + expDirective: cobra.ShellCompDirectiveNoFileComp, + }, + { + doc: "all containers with ids", + showAll: true, + showIDs: true, + containers: []container.Summary{ + {ID: "id-c", State: "running", Names: []string{"/container-c", "/container-c/link-b"}}, + {ID: "id-b", State: "created", Names: []string{"/container-b"}}, + {ID: "id-a", State: "exited", Names: []string{"/container-a"}}, + }, + expOut: []string{"id-c", "container-c", "container-c/link-b", "id-b", "container-b", "id-a", "container-a"}, + expOpts: container.ListOptions{All: true}, + expDirective: cobra.ShellCompDirectiveNoFileComp, + }, + { + doc: "only running containers", + showAll: false, + containers: []container.Summary{ + {ID: "id-c", State: "running", Names: []string{"/container-c", "/container-c/link-b"}}, + }, + expOut: []string{"container-c", "container-c/link-b"}, + expDirective: cobra.ShellCompDirectiveNoFileComp, + }, + { + doc: "with filter", + showAll: true, + filters: []func(container.Summary) bool{ + func(container container.Summary) bool { return container.State == "created" }, + }, + containers: []container.Summary{ + {ID: "id-c", State: "running", Names: []string{"/container-c", "/container-c/link-b"}}, + {ID: "id-b", State: "created", Names: []string{"/container-b"}}, + {ID: "id-a", State: "exited", Names: []string{"/container-a"}}, + }, + expOut: []string{"container-b"}, + expOpts: container.ListOptions{All: true}, + expDirective: cobra.ShellCompDirectiveNoFileComp, + }, + { + doc: "multiple filters", + showAll: true, + filters: []func(container.Summary) bool{ + func(container container.Summary) bool { return container.ID == "id-a" }, + func(container container.Summary) bool { return container.State == "created" }, + }, + containers: []container.Summary{ + {ID: "id-c", State: "running", Names: []string{"/container-c", "/container-c/link-b"}}, + {ID: "id-b", State: "created", Names: []string{"/container-b"}}, + {ID: "id-a", State: "created", Names: []string{"/container-a"}}, + }, + expOut: []string{"container-a"}, + expOpts: container.ListOptions{All: true}, + expDirective: cobra.ShellCompDirectiveNoFileComp, + }, + { + doc: "with error", + expDirective: cobra.ShellCompDirectiveError, + }, + } + + for _, tc := range tests { + tc := tc + t.Run(tc.doc, func(t *testing.T) { + if tc.showIDs { + t.Setenv("DOCKER_COMPLETION_SHOW_CONTAINER_IDS", "yes") + } + comp := ContainerNames(fakeCLI{&fakeClient{ + containerListFunc: func(opts container.ListOptions) ([]container.Summary, error) { + assert.Check(t, is.DeepEqual(opts, tc.expOpts, cmpopts.IgnoreUnexported(container.ListOptions{}, filters.Args{}))) + if tc.expDirective == cobra.ShellCompDirectiveError { + return nil, errors.New("some error occurred") + } + return tc.containers, nil + }, + }}, tc.showAll, tc.filters...) + + containers, directives := comp(&cobra.Command{}, nil, "") + assert.Check(t, is.Equal(directives&tc.expDirective, tc.expDirective)) + assert.Check(t, is.DeepEqual(containers, tc.expOut)) + }) + } +} + func TestCompleteEnvVarNames(t *testing.T) { env.PatchAll(t, map[string]string{ "ENV_A": "hello-a", From ab418a38d826d7137802840d2b406eead9d82cdb Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sun, 13 Oct 2024 18:00:20 +0200 Subject: [PATCH 7/9] completion: add test for ImageNames Signed-off-by: Sebastiaan van Stijn --- cli/command/completion/functions_test.go | 55 ++++++++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/cli/command/completion/functions_test.go b/cli/command/completion/functions_test.go index 6f39d8ade191..9de5d73cdd97 100644 --- a/cli/command/completion/functions_test.go +++ b/cli/command/completion/functions_test.go @@ -8,6 +8,7 @@ import ( "github.com/docker/docker/api/types/container" "github.com/docker/docker/api/types/filters" + "github.com/docker/docker/api/types/image" "github.com/docker/docker/client" "github.com/google/go-cmp/cmp/cmpopts" "github.com/spf13/cobra" @@ -28,6 +29,7 @@ func (c fakeCLI) Client() client.APIClient { type fakeClient struct { client.Client containerListFunc func(options container.ListOptions) ([]container.Summary, error) + imageListFunc func(options image.ListOptions) ([]image.Summary, error) } func (c *fakeClient) ContainerList(_ context.Context, options container.ListOptions) ([]container.Summary, error) { @@ -37,6 +39,13 @@ func (c *fakeClient) ContainerList(_ context.Context, options container.ListOpti return []container.Summary{}, nil } +func (c *fakeClient) ImageList(_ context.Context, options image.ListOptions) ([]image.Summary, error) { + if c.imageListFunc != nil { + return c.imageListFunc(options) + } + return []image.Summary{}, nil +} + func TestCompleteContainerNames(t *testing.T) { tests := []struct { doc string @@ -172,6 +181,52 @@ func TestCompleteFromList(t *testing.T) { assert.Check(t, is.DeepEqual(values, expected)) } +func TestCompleteImageNames(t *testing.T) { + tests := []struct { + doc string + images []image.Summary + expOut []string + expDirective cobra.ShellCompDirective + }{ + { + doc: "no results", + expDirective: cobra.ShellCompDirectiveNoFileComp, + }, + { + doc: "with results", + images: []image.Summary{ + {RepoTags: []string{"image-c:latest", "image-c:other"}}, + {RepoTags: []string{"image-b:latest", "image-b:other"}}, + {RepoTags: []string{"image-a:latest", "image-a:other"}}, + }, + expOut: []string{"image-c:latest", "image-c:other", "image-b:latest", "image-b:other", "image-a:latest", "image-a:other"}, + expDirective: cobra.ShellCompDirectiveNoFileComp, + }, + { + doc: "with error", + expDirective: cobra.ShellCompDirectiveError, + }, + } + + for _, tc := range tests { + tc := tc + t.Run(tc.doc, func(t *testing.T) { + comp := ImageNames(fakeCLI{&fakeClient{ + imageListFunc: func(options image.ListOptions) ([]image.Summary, error) { + if tc.expDirective == cobra.ShellCompDirectiveError { + return nil, errors.New("some error occurred") + } + return tc.images, nil + }, + }}) + + volumes, directives := comp(&cobra.Command{}, nil, "") + assert.Check(t, is.Equal(directives&tc.expDirective, tc.expDirective)) + assert.Check(t, is.DeepEqual(volumes, tc.expOut)) + }) + } +} + func TestCompleteNoComplete(t *testing.T) { values, directives := NoComplete(nil, nil, "") assert.Check(t, is.Equal(directives, cobra.ShellCompDirectiveNoFileComp)) From 302d73f990f7ddba8ac5c61d84cd000dcf6c802c Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sun, 13 Oct 2024 18:02:35 +0200 Subject: [PATCH 8/9] completion: add test for NetworkNames Signed-off-by: Sebastiaan van Stijn --- cli/command/completion/functions_test.go | 55 ++++++++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/cli/command/completion/functions_test.go b/cli/command/completion/functions_test.go index 9de5d73cdd97..a47bd0b236bf 100644 --- a/cli/command/completion/functions_test.go +++ b/cli/command/completion/functions_test.go @@ -9,6 +9,7 @@ import ( "github.com/docker/docker/api/types/container" "github.com/docker/docker/api/types/filters" "github.com/docker/docker/api/types/image" + "github.com/docker/docker/api/types/network" "github.com/docker/docker/client" "github.com/google/go-cmp/cmp/cmpopts" "github.com/spf13/cobra" @@ -30,6 +31,7 @@ type fakeClient struct { client.Client containerListFunc func(options container.ListOptions) ([]container.Summary, error) imageListFunc func(options image.ListOptions) ([]image.Summary, error) + networkListFunc func(ctx context.Context, options network.ListOptions) ([]network.Summary, error) } func (c *fakeClient) ContainerList(_ context.Context, options container.ListOptions) ([]container.Summary, error) { @@ -46,6 +48,13 @@ func (c *fakeClient) ImageList(_ context.Context, options image.ListOptions) ([] return []image.Summary{}, nil } +func (c *fakeClient) NetworkList(ctx context.Context, options network.ListOptions) ([]network.Summary, error) { + if c.networkListFunc != nil { + return c.networkListFunc(ctx, options) + } + return []network.Inspect{}, nil +} + func TestCompleteContainerNames(t *testing.T) { tests := []struct { doc string @@ -227,6 +236,52 @@ func TestCompleteImageNames(t *testing.T) { } } +func TestCompleteNetworkNames(t *testing.T) { + tests := []struct { + doc string + networks []network.Summary + expOut []string + expDirective cobra.ShellCompDirective + }{ + { + doc: "no results", + expDirective: cobra.ShellCompDirectiveNoFileComp, + }, + { + doc: "with results", + networks: []network.Summary{ + {ID: "nw-c", Name: "network-c"}, + {ID: "nw-b", Name: "network-b"}, + {ID: "nw-a", Name: "network-a"}, + }, + expOut: []string{"network-c", "network-b", "network-a"}, + expDirective: cobra.ShellCompDirectiveNoFileComp, + }, + { + doc: "with error", + expDirective: cobra.ShellCompDirectiveError, + }, + } + + for _, tc := range tests { + tc := tc + t.Run(tc.doc, func(t *testing.T) { + comp := NetworkNames(fakeCLI{&fakeClient{ + networkListFunc: func(ctx context.Context, options network.ListOptions) ([]network.Summary, error) { + if tc.expDirective == cobra.ShellCompDirectiveError { + return nil, errors.New("some error occurred") + } + return tc.networks, nil + }, + }}) + + volumes, directives := comp(&cobra.Command{}, nil, "") + assert.Check(t, is.Equal(directives&tc.expDirective, tc.expDirective)) + assert.Check(t, is.DeepEqual(volumes, tc.expOut)) + }) + } +} + func TestCompleteNoComplete(t *testing.T) { values, directives := NoComplete(nil, nil, "") assert.Check(t, is.Equal(directives, cobra.ShellCompDirectiveNoFileComp)) From e1c472a436067150ff035f33fc49b943b363c216 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sun, 13 Oct 2024 18:03:32 +0200 Subject: [PATCH 9/9] completion: add test for VolumeNames Signed-off-by: Sebastiaan van Stijn --- cli/command/completion/functions_test.go | 55 ++++++++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/cli/command/completion/functions_test.go b/cli/command/completion/functions_test.go index a47bd0b236bf..aacd9769e19e 100644 --- a/cli/command/completion/functions_test.go +++ b/cli/command/completion/functions_test.go @@ -10,6 +10,7 @@ import ( "github.com/docker/docker/api/types/filters" "github.com/docker/docker/api/types/image" "github.com/docker/docker/api/types/network" + "github.com/docker/docker/api/types/volume" "github.com/docker/docker/client" "github.com/google/go-cmp/cmp/cmpopts" "github.com/spf13/cobra" @@ -32,6 +33,7 @@ type fakeClient struct { containerListFunc func(options container.ListOptions) ([]container.Summary, error) imageListFunc func(options image.ListOptions) ([]image.Summary, error) networkListFunc func(ctx context.Context, options network.ListOptions) ([]network.Summary, error) + volumeListFunc func(filter filters.Args) (volume.ListResponse, error) } func (c *fakeClient) ContainerList(_ context.Context, options container.ListOptions) ([]container.Summary, error) { @@ -55,6 +57,13 @@ func (c *fakeClient) NetworkList(ctx context.Context, options network.ListOption return []network.Inspect{}, nil } +func (c *fakeClient) VolumeList(_ context.Context, options volume.ListOptions) (volume.ListResponse, error) { + if c.volumeListFunc != nil { + return c.volumeListFunc(options.Filters) + } + return volume.ListResponse{}, nil +} + func TestCompleteContainerNames(t *testing.T) { tests := []struct { doc string @@ -293,3 +302,49 @@ func TestCompletePlatforms(t *testing.T) { assert.Check(t, is.Equal(directives&cobra.ShellCompDirectiveNoFileComp, cobra.ShellCompDirectiveNoFileComp), "Should not perform file completion") assert.Check(t, is.DeepEqual(values, commonPlatforms)) } + +func TestCompleteVolumeNames(t *testing.T) { + tests := []struct { + doc string + volumes []*volume.Volume + expOut []string + expDirective cobra.ShellCompDirective + }{ + { + doc: "no results", + expDirective: cobra.ShellCompDirectiveNoFileComp, + }, + { + doc: "with results", + volumes: []*volume.Volume{ + {Name: "volume-c"}, + {Name: "volume-b"}, + {Name: "volume-a"}, + }, + expOut: []string{"volume-c", "volume-b", "volume-a"}, + expDirective: cobra.ShellCompDirectiveNoFileComp, + }, + { + doc: "with error", + expDirective: cobra.ShellCompDirectiveError, + }, + } + + for _, tc := range tests { + tc := tc + t.Run(tc.doc, func(t *testing.T) { + comp := VolumeNames(fakeCLI{&fakeClient{ + volumeListFunc: func(filter filters.Args) (volume.ListResponse, error) { + if tc.expDirective == cobra.ShellCompDirectiveError { + return volume.ListResponse{}, errors.New("some error occurred") + } + return volume.ListResponse{Volumes: tc.volumes}, nil + }, + }}) + + volumes, directives := comp(&cobra.Command{}, nil, "") + assert.Check(t, is.Equal(directives&tc.expDirective, tc.expDirective)) + assert.Check(t, is.DeepEqual(volumes, tc.expOut)) + }) + } +}