From 346cf3d4f46fe1ccca243633d407dedca9e5b1da Mon Sep 17 00:00:00 2001 From: Brian Pursley Date: Tue, 28 Jan 2020 17:04:31 -0500 Subject: [PATCH 1/4] Ignore empty or blank string in path when listing plugins Kubernetes-commit: f60c0af97710a5dca6f55f3c4a1480412ad50ec8 --- pkg/cmd/plugin/plugin.go | 8 ++++++-- pkg/cmd/plugin/plugin_test.go | 37 +++++++++++++++++++++++------------ 2 files changed, 31 insertions(+), 14 deletions(-) diff --git a/pkg/cmd/plugin/plugin.go b/pkg/cmd/plugin/plugin.go index d578ca08..660fafbf 100644 --- a/pkg/cmd/plugin/plugin.go +++ b/pkg/cmd/plugin/plugin.go @@ -113,10 +113,14 @@ func (o *PluginListOptions) Run() error { pluginWarnings := 0 for _, dir := range uniquePathsList(o.PluginPaths) { + if len(strings.TrimSpace(dir)) == 0 { + continue + } + files, err := ioutil.ReadDir(dir) if err != nil { if _, ok := err.(*os.PathError); ok { - fmt.Fprintf(o.ErrOut, "Unable read directory %q from your PATH: %v. Skipping...", dir, err) + fmt.Fprintf(o.ErrOut, "Unable read directory %q from your PATH: %v. Skipping...\n", dir, err) continue } @@ -133,7 +137,7 @@ func (o *PluginListOptions) Run() error { } if isFirstFile { - fmt.Fprintf(o.ErrOut, "The following compatible plugins are available:\n\n") + fmt.Fprintf(o.Out, "The following compatible plugins are available:\n\n") pluginsFound = true isFirstFile = false } diff --git a/pkg/cmd/plugin/plugin_test.go b/pkg/cmd/plugin/plugin_test.go index 5f6c0d5b..f1f8013c 100644 --- a/pkg/cmd/plugin/plugin_test.go +++ b/pkg/cmd/plugin/plugin_test.go @@ -17,7 +17,6 @@ limitations under the License. package plugin import ( - "bytes" "fmt" "io/ioutil" "os" @@ -98,6 +97,8 @@ func TestPluginPathsAreValid(t *testing.T) { verifier *fakePluginPathVerifier expectVerifyErrors []error expectErr string + expectErrOut string + expectOut string }{ { name: "ensure no plugins found if no files begin with kubectl- prefix", @@ -107,6 +108,7 @@ func TestPluginPathsAreValid(t *testing.T) { return ioutil.TempFile(tempDir, "notkubectl-") }, expectErr: "unable to find any kubectl plugins in your PATH", + expectErrOut: "\n", }, { name: "ensure de-duplicated plugin-paths slice", @@ -115,18 +117,30 @@ func TestPluginPathsAreValid(t *testing.T) { pluginFile: func() (*os.File, error) { return ioutil.TempFile(tempDir, "kubectl-") }, + expectOut: "The following compatible plugins are available:", + }, + { + name: "ensure no errors when empty string or blank path are specified", + pluginPaths: []string{tempDir, "", " "}, + verifier: newFakePluginPathVerifier(), + pluginFile: func() (*os.File, error) { + return ioutil.TempFile(tempDir, "kubectl-") + }, + expectOut: "The following compatible plugins are available:", }, } for _, test := range tc { t.Run(test.name, func(t *testing.T) { - ioStreams, _, _, errOut := genericclioptions.NewTestIOStreams() + ioStreams, _, out, errOut := genericclioptions.NewTestIOStreams() o := &PluginListOptions{ Verifier: test.verifier, IOStreams: ioStreams, PluginPaths: test.pluginPaths, } + o.Out = out + o.ErrOut = errOut // create files if test.pluginFile != nil { @@ -150,18 +164,17 @@ func TestPluginPathsAreValid(t *testing.T) { if err != nil && len(test.expectErr) == 0 { t.Fatalf("unexpected error: %v - %v", err, errOut.String()) } - if err == nil { - return + + if len(test.expectErrOut) == 0 && errOut.Len() > 0 { + t.Fatalf("unexpected error output: expected nothing, but got %v", errOut.String()) + } else if len(test.expectErrOut) > 0 && !strings.Contains(errOut.String(), test.expectErrOut) { + t.Fatalf("unexpected error output: expected to contain %v, but got %v", test.expectErrOut, errOut.String()) } - allErrs := bytes.NewBuffer(errOut.Bytes()) - if _, err := allErrs.WriteString(err.Error()); err != nil { - t.Fatalf("unexpected error: %v", err) - } - if len(test.expectErr) > 0 { - if !strings.Contains(allErrs.String(), test.expectErr) { - t.Fatalf("unexpected error: expected %v, but got %v", test.expectErr, allErrs.String()) - } + if len(test.expectOut) == 0 && out.Len() > 0 { + t.Fatalf("unexpected output: expected nothing, but got %v", out.String()) + } else if len(test.expectOut) > 0 && !strings.Contains(out.String(), test.expectOut) { + t.Fatalf("unexpected output: expected to contain %v, but got %v", test.expectOut, out.String()) } }) } From b69c2a4389395c28dbf95dcf5a48eb72ff33bf6a Mon Sep 17 00:00:00 2001 From: Brian Pursley Date: Tue, 28 Jan 2020 20:51:24 -0500 Subject: [PATCH 2/4] Removed unneeded newline (moved to end of directory not found message) Kubernetes-commit: 48ee18b516b267ba062a1dd00a21e7ae10ecd805 --- pkg/cmd/plugin/plugin.go | 1 - pkg/cmd/plugin/plugin_test.go | 1 - 2 files changed, 2 deletions(-) diff --git a/pkg/cmd/plugin/plugin.go b/pkg/cmd/plugin/plugin.go index 660fafbf..0beba47d 100644 --- a/pkg/cmd/plugin/plugin.go +++ b/pkg/cmd/plugin/plugin.go @@ -169,7 +169,6 @@ func (o *PluginListOptions) Run() error { } } if len(pluginErrors) > 0 { - fmt.Fprintln(o.ErrOut) errs := bytes.NewBuffer(nil) for _, e := range pluginErrors { fmt.Fprintln(errs, e) diff --git a/pkg/cmd/plugin/plugin_test.go b/pkg/cmd/plugin/plugin_test.go index f1f8013c..90dccea2 100644 --- a/pkg/cmd/plugin/plugin_test.go +++ b/pkg/cmd/plugin/plugin_test.go @@ -108,7 +108,6 @@ func TestPluginPathsAreValid(t *testing.T) { return ioutil.TempFile(tempDir, "notkubectl-") }, expectErr: "unable to find any kubectl plugins in your PATH", - expectErrOut: "\n", }, { name: "ensure de-duplicated plugin-paths slice", From e9f1eb93f4eea9eaa5085669fd9a87694d380280 Mon Sep 17 00:00:00 2001 From: Brian Pursley Date: Mon, 3 Feb 2020 12:17:51 -0500 Subject: [PATCH 3/4] Fixed problem in unit test where error expected/actual comparison was not being performed Kubernetes-commit: 97185e97529ef7c006f26bb3190805ad28f15ffe --- pkg/cmd/plugin/plugin_test.go | 41 +++++++++++++++++------------------ 1 file changed, 20 insertions(+), 21 deletions(-) diff --git a/pkg/cmd/plugin/plugin_test.go b/pkg/cmd/plugin/plugin_test.go index 90dccea2..1a1a813e 100644 --- a/pkg/cmd/plugin/plugin_test.go +++ b/pkg/cmd/plugin/plugin_test.go @@ -101,31 +101,31 @@ func TestPluginPathsAreValid(t *testing.T) { expectOut string }{ { - name: "ensure no plugins found if no files begin with kubectl- prefix", - pluginPaths: []string{tempDir}, - verifier: newFakePluginPathVerifier(), - pluginFile: func() (*os.File, error) { + name: "ensure no plugins found if no files begin with kubectl- prefix", + pluginPaths: []string{tempDir}, + verifier: newFakePluginPathVerifier(), + pluginFile: func() (*os.File, error) { return ioutil.TempFile(tempDir, "notkubectl-") }, - expectErr: "unable to find any kubectl plugins in your PATH", + expectErr: "error: unable to find any kubectl plugins in your PATH\n", }, { - name: "ensure de-duplicated plugin-paths slice", - pluginPaths: []string{tempDir, tempDir}, - verifier: newFakePluginPathVerifier(), - pluginFile: func() (*os.File, error) { + name: "ensure de-duplicated plugin-paths slice", + pluginPaths: []string{tempDir, tempDir}, + verifier: newFakePluginPathVerifier(), + pluginFile: func() (*os.File, error) { return ioutil.TempFile(tempDir, "kubectl-") }, - expectOut: "The following compatible plugins are available:", + expectOut: "The following compatible plugins are available:", }, { - name: "ensure no errors when empty string or blank path are specified", - pluginPaths: []string{tempDir, "", " "}, - verifier: newFakePluginPathVerifier(), - pluginFile: func() (*os.File, error) { + name: "ensure no errors when empty string or blank path are specified", + pluginPaths: []string{tempDir, "", " "}, + verifier: newFakePluginPathVerifier(), + pluginFile: func() (*os.File, error) { return ioutil.TempFile(tempDir, "kubectl-") }, - expectOut: "The following compatible plugins are available:", + expectOut: "The following compatible plugins are available:", }, } @@ -138,8 +138,6 @@ func TestPluginPathsAreValid(t *testing.T) { PluginPaths: test.pluginPaths, } - o.Out = out - o.ErrOut = errOut // create files if test.pluginFile != nil { @@ -158,10 +156,11 @@ func TestPluginPathsAreValid(t *testing.T) { err := o.Run() if err == nil && len(test.expectErr) > 0 { - t.Fatalf("unexpected non-error: expecting %v", test.expectErr) - } - if err != nil && len(test.expectErr) == 0 { - t.Fatalf("unexpected error: %v - %v", err, errOut.String()) + t.Fatalf("unexpected non-error: expected %v, but got nothing", test.expectErr) + } else if err != nil && len(test.expectErr) == 0 { + t.Fatalf("unexpected error: expected nothing, but got %v", err.Error()) + } else if err != nil && err.Error() != test.expectErr { + t.Fatalf("unexpected error: expected %v, but got %v", test.expectErr, err.Error()) } if len(test.expectErrOut) == 0 && errOut.Len() > 0 { From 1777c190797fed5f0bbad9df83c0544323e597cc Mon Sep 17 00:00:00 2001 From: Brian Pursley Date: Tue, 4 Feb 2020 10:16:06 -0500 Subject: [PATCH 4/4] Fixed code formatting issues discovered by verify-gofmt Kubernetes-commit: 2d21f16c38e704eaaa42d7d583ddd103c16d0d6b --- pkg/cmd/plugin/plugin_test.go | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/pkg/cmd/plugin/plugin_test.go b/pkg/cmd/plugin/plugin_test.go index 1a1a813e..013fe7f1 100644 --- a/pkg/cmd/plugin/plugin_test.go +++ b/pkg/cmd/plugin/plugin_test.go @@ -101,31 +101,31 @@ func TestPluginPathsAreValid(t *testing.T) { expectOut string }{ { - name: "ensure no plugins found if no files begin with kubectl- prefix", - pluginPaths: []string{tempDir}, - verifier: newFakePluginPathVerifier(), - pluginFile: func() (*os.File, error) { + name: "ensure no plugins found if no files begin with kubectl- prefix", + pluginPaths: []string{tempDir}, + verifier: newFakePluginPathVerifier(), + pluginFile: func() (*os.File, error) { return ioutil.TempFile(tempDir, "notkubectl-") }, - expectErr: "error: unable to find any kubectl plugins in your PATH\n", + expectErr: "error: unable to find any kubectl plugins in your PATH\n", }, { - name: "ensure de-duplicated plugin-paths slice", - pluginPaths: []string{tempDir, tempDir}, - verifier: newFakePluginPathVerifier(), - pluginFile: func() (*os.File, error) { + name: "ensure de-duplicated plugin-paths slice", + pluginPaths: []string{tempDir, tempDir}, + verifier: newFakePluginPathVerifier(), + pluginFile: func() (*os.File, error) { return ioutil.TempFile(tempDir, "kubectl-") }, - expectOut: "The following compatible plugins are available:", + expectOut: "The following compatible plugins are available:", }, { - name: "ensure no errors when empty string or blank path are specified", - pluginPaths: []string{tempDir, "", " "}, - verifier: newFakePluginPathVerifier(), - pluginFile: func() (*os.File, error) { + name: "ensure no errors when empty string or blank path are specified", + pluginPaths: []string{tempDir, "", " "}, + verifier: newFakePluginPathVerifier(), + pluginFile: func() (*os.File, error) { return ioutil.TempFile(tempDir, "kubectl-") }, - expectOut: "The following compatible plugins are available:", + expectOut: "The following compatible plugins are available:", }, }