From 89aec1a70ea6fb25cd0a4722e60c478091a5e813 Mon Sep 17 00:00:00 2001 From: Doug Davis Date: Tue, 6 Aug 2019 07:31:39 -0700 Subject: [PATCH] Don't show warnings that are not bad conditions Don't show warnings when running `kn plugin list`: - a dir in your plug-in path doesn't exist - e.g. right now it shows it for a missing ~/.kn/plugins - which is a valid state for someone who just downloaded `kn` but doesn't have, or need, a .kn dir - there are no plugins at all - that's a valid state and doesn't deserve a warning To me warning are things people need to be aware of (or things they need to take action to fix). Neither of those case apply here. Instead they user will just be annoyed by this: ``` $ kn plugin list Unable read directory '/root/.kn/plugins' from your plugins path: open /root/.kn/plugins: no such file or directory. Skipping... warning: unable to find any kn plugins in your plugin path: '[/root/.kn/plugins]' ``` When they should see no output at all. Or, if someone wants to add it we coudl output `no plugins found` but not call it a "warning" since it's not. Signed-off-by: Doug Davis --- pkg/kn/commands/plugin/plugin_list.go | 14 +++++---- pkg/kn/commands/plugin/plugin_list_test.go | 36 +++------------------- 2 files changed, 12 insertions(+), 38 deletions(-) diff --git a/pkg/kn/commands/plugin/plugin_list.go b/pkg/kn/commands/plugin/plugin_list.go index b8823c9d66..00dec31172 100644 --- a/pkg/kn/commands/plugin/plugin_list.go +++ b/pkg/kn/commands/plugin/plugin_list.go @@ -110,7 +110,6 @@ func (o *PluginFlags) complete(cmd *cobra.Command) error { } func (o *PluginFlags) run() error { - pluginsFound := false isFirstFile := true pluginErrors := []error{} pluginWarnings := 0 @@ -120,7 +119,15 @@ func (o *PluginFlags) run() error { continue } + // Treat a missing dir as just something we skip - meaning + // it's ok to include a path that might not exist yet or you + // don't have access to - so just skip it + if _, err := os.Stat(dir); os.IsNotExist(err) { + continue + } + files, err := ioutil.ReadDir(dir) + // However, error reading a dir that does exist is an issue if err != nil { if _, ok := err.(*os.PathError); ok { fmt.Fprintf(o.ErrOut, "Unable read directory '%s' from your plugins path: %v. Skipping...", dir, err) @@ -143,7 +150,6 @@ func (o *PluginFlags) run() error { fmt.Fprintf(o.ErrOut, "The following compatible plugins are available, using options:\n") fmt.Fprintf(o.ErrOut, " - plugins dir: '%s'\n", commands.Cfg.PluginsDir) fmt.Fprintf(o.ErrOut, " - lookup plugins in path: '%t'\n\n", commands.Cfg.LookupPluginsInPath) - pluginsFound = true isFirstFile = false } @@ -162,10 +168,6 @@ func (o *PluginFlags) run() error { } } - if !pluginsFound { - pluginErrors = append(pluginErrors, fmt.Errorf("warning: unable to find any kn plugins in your plugin path: '%s'", o.PluginPaths)) - } - if pluginWarnings > 0 { if pluginWarnings == 1 { pluginErrors = append(pluginErrors, fmt.Errorf("error: one plugin warning was found")) diff --git a/pkg/kn/commands/plugin/plugin_list_test.go b/pkg/kn/commands/plugin/plugin_list_test.go index 5c16dad3d0..f132653d7b 100644 --- a/pkg/kn/commands/plugin/plugin_list_test.go +++ b/pkg/kn/commands/plugin/plugin_list_test.go @@ -73,24 +73,6 @@ func TestPluginList(t *testing.T) { t.Run("when --lookup-plugins-in-path is true", func(t *testing.T) { var pluginPath string - beforeEach := func(t *testing.T) { - err = os.Setenv("PATH", tmpPathDir) - assert.Assert(t, err == nil) - } - - t.Run("no plugins installed", func(t *testing.T) { - setup(t) - defer cleanup(t) - beforeEach(t) - - t.Run("warns user that no plugins found", func(t *testing.T) { - rootCmd.SetArgs([]string{"plugin", "list", "--lookup-plugins-in-path=true", pluginsDirFlag}) - err = rootCmd.Execute() - assert.Assert(t, err != nil) - assert.Assert(t, strings.Contains(err.Error(), "warning: unable to find any kn plugins in your plugin path:")) - }) - }) - t.Run("plugins installed", func(t *testing.T) { t.Run("with valid plugin in $PATH", func(t *testing.T) { beforeEach := func(t *testing.T) { @@ -113,21 +95,19 @@ func TestPluginList(t *testing.T) { }) }) - t.Run("with non-executable plugin", func(t *testing.T) { + t.Run("with missing plugin path", func(t *testing.T) { beforeEach := func(t *testing.T) { - pluginPath = CreateTestPluginInPath(t, KnTestPluginName, KnTestPluginScript, FileModeReadable, tmpPathDir) - assert.Assert(t, pluginPath != "") + pluginPath = "/tmp/does-not-exist" } - t.Run("warns user plugin invalid", func(t *testing.T) { + t.Run("don't warns user of missing path", func(t *testing.T) { setup(t) defer cleanup(t) beforeEach(t) rootCmd.SetArgs([]string{"plugin", "list", "--lookup-plugins-in-path=true", pluginsDirFlag}) err = rootCmd.Execute() - assert.Assert(t, err != nil) - assert.Assert(t, strings.Contains(err.Error(), "warning: unable to find any kn plugins in your plugin path:")) + assert.Assert(t, err == nil) }) }) @@ -228,13 +208,5 @@ func TestPluginList(t *testing.T) { assert.Assert(t, err == nil) }) - t.Run("no plugins installed", func(t *testing.T) { - setup(t) - defer cleanup(t) - - rootCmd.SetArgs([]string{"plugin", "list", pluginsDirFlag}) - err = rootCmd.Execute() - assert.Assert(t, err != nil) - }) }) }