Skip to content
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

Don't Display kn --help message with Plugin Errors #910

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
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
13 changes: 11 additions & 2 deletions cmd/kn/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ import (
"knative.dev/client/pkg/kn/root"
)

var pluginErr = false

func init() {
rand.Seed(time.Now().UnixNano())
}
Expand Down Expand Up @@ -79,7 +81,12 @@ func run(args []string) error {
return err
}

return plugin.Execute(argsWithoutCommands(args, plugin.CommandParts()))
err = plugin.Execute(argsWithoutCommands(args, plugin.CommandParts()))
if err != nil {
// Used to not print `--kn help` message with plugin errors
pluginErr = true
danielhelfand marked this conversation as resolved.
Show resolved Hide resolved
}
return err
} else {
// Validate args for root command
err = validateRootCommand(rootCmd)
Expand Down Expand Up @@ -190,7 +197,9 @@ func validateRootCommand(cmd *cobra.Command) error {
// printError prints out any given error
func printError(err error) {
fmt.Fprintf(os.Stderr, "Error: %s\n", cleanupErrorMessage(err.Error()))
fmt.Fprintf(os.Stderr, "Run '%s --help' for usage\n", extractCommandPathFromErrorMessage(err.Error(), os.Args[0]))
if !pluginErr {
fmt.Fprintf(os.Stderr, "Run '%s --help' for usage\n", extractCommandPathFromErrorMessage(err.Error(), os.Args[0]))
}
}

// extractCommandPathFromErrorMessage tries to extract the command name from an error message
Expand Down
60 changes: 52 additions & 8 deletions test/e2e/plugins_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,15 @@ const (
echo "Hello Knative, I'm a Kn plugin"
echo " My plugin file is $0"
echo " I received arguments: $1 $2 $3 $4"`

TestPluginCodeErr string = `#!/bin/bash

exit 1`
)

type pluginTestConfig struct {
knConfigDir, knPluginsDir, knPluginsDir2 string
knConfigPath, knPluginPath, knPluginPath2 string
knConfigDir, knPluginsDir, knPluginsDir2, knPluginsDir3 string
knConfigPath, knPluginPath, knPluginPath2, knPluginPath3 string
}

func (pc *pluginTestConfig) setup() error {
Expand All @@ -64,6 +68,12 @@ func (pc *pluginTestConfig) setup() error {
return err
}

pc.knPluginsDir3 = filepath.Join(pc.knConfigDir, "plugins3")
err = os.MkdirAll(pc.knPluginsDir3, test.FileModeExecutable)
if err != nil {
return err
}

pc.knConfigPath, err = test.CreateFile("config.yaml", "", pc.knConfigDir, test.FileModeReadWrite)
if err != nil {
return err
Expand All @@ -77,6 +87,10 @@ func (pc *pluginTestConfig) setup() error {
if err != nil {
return err
}
pc.knPluginPath3, err = test.CreateFile("kn-hello3e2e", TestPluginCodeErr, pc.knPluginsDir3, test.FileModeExecutable)
if err != nil {
return err
}
return nil
}

Expand All @@ -102,7 +116,7 @@ func TestPluginWithoutLookup(t *testing.T) {
listPlugin(r, knFlags, []string{pc.knPluginPath}, []string{})

t.Log("execute plugin in --plugins-dir")
runPlugin(r, knFlags, "helloe2e", []string{"e2e", "test"}, []string{"Hello Knative, I'm a Kn plugin", "I received arguments", "e2e"})
runPlugin(r, knFlags, "helloe2e", []string{"e2e", "test"}, []string{"Hello Knative, I'm a Kn plugin", "I received arguments", "e2e"}, []string{}, false)

t.Log("does not list any other plugin in $PATH")
listPlugin(r, knFlags, []string{pc.knPluginPath}, []string{pc.knPluginPath2})
Expand Down Expand Up @@ -138,7 +152,7 @@ func TestPluginWithLookup(t *testing.T) {
listPlugin(r, knFlags, []string{pc.knPluginPath}, []string{pc.knPluginPath2})

t.Log("execute plugin in --plugins-dir")
runPlugin(r, knFlags, "helloe2e", []string{}, []string{"Hello Knative, I'm a Kn plugin"})
runPlugin(r, knFlags, "helloe2e", []string{}, []string{"Hello Knative, I'm a Kn plugin"}, []string{}, false)
}

func TestListPluginInPath(t *testing.T) {
Expand Down Expand Up @@ -169,7 +183,26 @@ func TestExecutePluginInPath(t *testing.T) {

t.Log("execute plugin in $PATH")
knFlags := []string{fmt.Sprintf("--plugins-dir=%s", pc.knPluginsDir), "--lookup-plugins=true"}
runPlugin(r, knFlags, "hello2e2e", []string{}, []string{"Hello Knative, I'm a Kn plugin"})
runPlugin(r, knFlags, "hello2e2e", []string{}, []string{"Hello Knative, I'm a Kn plugin"}, []string{}, false)
}

func TestExecutePluginInPathWithError(t *testing.T) {
it, err := test.NewKnTest()
assert.NilError(t, err)

r := test.NewKnRunResultCollector(t, it)
defer r.DumpIfFailed()

pc := pluginTestConfig{}
assert.NilError(t, pc.setup())
oldPath := os.Getenv("PATH")
assert.NilError(t, os.Setenv("PATH", fmt.Sprintf("%s:%s", oldPath, pc.knPluginsDir3)))
defer tearDownWithPath(pc, oldPath)

t.Log("execute plugin in $PATH that returns error")
knFlags := []string{fmt.Sprintf("--plugins-dir=%s", pc.knPluginsDir), "--lookup-plugins=true"}
// Unexpected output []string{"Run", "kn --help", "usage"} to verify no kn --help message for error with plugin
runPlugin(r, knFlags, "hello3e2e", []string{}, []string{"Error: exit status 1"}, []string{"Run", "kn --help", "usage"}, true)
}

// Private
Expand Down Expand Up @@ -206,14 +239,25 @@ func listPlugin(r *test.KnRunResultCollector, knFlags []string, expectedPlugins
}
}

func runPlugin(r *test.KnRunResultCollector, knFlags []string, pluginName string, args []string, expectedOutput []string) {
func runPlugin(r *test.KnRunResultCollector, knFlags []string, pluginName string, args []string, expectedOutput []string, unexpectedOutput []string, expectErr bool) {
knArgs := append([]string{}, knFlags...)
knArgs = append(knArgs, pluginName)
knArgs = append(knArgs, args...)

out := test.Kn{}.Run(knArgs...)
r.AssertNoError(out)
var stdOutOrErr string
if !expectErr {
r.AssertNoError(out)
stdOutOrErr = out.Stdout
} else {
r.AssertError(out)
stdOutOrErr = out.Stderr
}

for _, output := range expectedOutput {
assert.Check(r.T(), util.ContainsAll(out.Stdout, output))
assert.Check(r.T(), util.ContainsAll(stdOutOrErr, output))
}
for _, output := range unexpectedOutput {
assert.Check(r.T(), util.ContainsNone(stdOutOrErr, output))
}
}