From 9828fa255357b745cfcbbe1c32361f30d334041a Mon Sep 17 00:00:00 2001 From: "dr.max" Date: Mon, 1 Jul 2019 16:02:09 +0800 Subject: [PATCH] fixes(issue #111) generically for all command groups Explores all sub-commands from root and adds a RunE for all commands that are groups with child commands and without a RunE. The added RunE will return the correct errors when the command is called with empty sub-command or with an unknown sub-command. It will also print the command help message first. --- pkg/kn/commands/revision/revision.go | 11 ---- pkg/kn/commands/revision/revision_test.go | 66 ----------------------- pkg/kn/commands/service/service.go | 12 ----- pkg/kn/commands/service/service_test.go | 66 ----------------------- pkg/kn/core/root.go | 23 ++++++++ 5 files changed, 23 insertions(+), 155 deletions(-) delete mode 100644 pkg/kn/commands/revision/revision_test.go delete mode 100644 pkg/kn/commands/service/service_test.go diff --git a/pkg/kn/commands/revision/revision.go b/pkg/kn/commands/revision/revision.go index d24fbd3461..087f6c8927 100644 --- a/pkg/kn/commands/revision/revision.go +++ b/pkg/kn/commands/revision/revision.go @@ -15,9 +15,6 @@ package revision import ( - "errors" - "fmt" - "github.com/knative/client/pkg/kn/commands" "github.com/spf13/cobra" ) @@ -26,14 +23,6 @@ func NewRevisionCommand(p *commands.KnParams) *cobra.Command { revisionCmd := &cobra.Command{ Use: "revision", Short: "Revision command group", - RunE: func(cmd *cobra.Command, args []string) error { - cmd.Help() - if len(args) == 0 { - return errors.New("please provide a valid sub-command for \"kn revision\"") - } else { - return errors.New(fmt.Sprintf("unknown command \"%s\" for \"kn revision\"", args[0])) - } - }, } revisionCmd.AddCommand(NewRevisionListCommand(p)) revisionCmd.AddCommand(NewRevisionDescribeCommand(p)) diff --git a/pkg/kn/commands/revision/revision_test.go b/pkg/kn/commands/revision/revision_test.go deleted file mode 100644 index 2572d94ee7..0000000000 --- a/pkg/kn/commands/revision/revision_test.go +++ /dev/null @@ -1,66 +0,0 @@ -// Copyright © 2018 The Knative Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package revision - -import ( - "strings" - "testing" - - "github.com/knative/client/pkg/kn/commands" - v1alpha1 "github.com/knative/serving/pkg/apis/serving/v1alpha1" - "k8s.io/apimachinery/pkg/runtime" - client_testing "k8s.io/client-go/testing" -) - -func fakeRevision(args []string, response *v1alpha1.ServiceList) (action client_testing.Action, output []string, err error) { - knParams := &commands.KnParams{} - cmd, fakeServing, buf := commands.CreateTestKnCommand(NewRevisionCommand(knParams), knParams) - fakeServing.AddReactor("*", "*", - func(a client_testing.Action) (bool, runtime.Object, error) { - action = a - return true, response, nil - }) - cmd.SetArgs(args) - err = cmd.Execute() - if err != nil { - return - } - output = strings.Split(buf.String(), "\n") - return -} - -func TestUnknownSubcommand(t *testing.T) { - _, _, err := fakeRevision([]string{"revision", "unknown"}, &v1alpha1.ServiceList{}) - if err == nil { - t.Error(err) - return - } - - if err.Error() != "unknown command \"unknown\" for \"kn revision\"" { - t.Errorf("Bad error message '%s'", err.Error()) - } -} - -func TestEmptySubcommand(t *testing.T) { - _, _, err := fakeRevision([]string{"revision"}, &v1alpha1.ServiceList{}) - if err == nil { - t.Error(err) - return - } - - if err.Error() != "please provide a valid sub-command for \"kn revision\"" { - t.Errorf("Bad error message '%s'", err.Error()) - } -} diff --git a/pkg/kn/commands/service/service.go b/pkg/kn/commands/service/service.go index 38372ccd21..7a4ff8ef2a 100644 --- a/pkg/kn/commands/service/service.go +++ b/pkg/kn/commands/service/service.go @@ -15,9 +15,6 @@ package service import ( - "errors" - "fmt" - "github.com/knative/client/pkg/kn/commands" "github.com/spf13/cobra" ) @@ -26,15 +23,6 @@ func NewServiceCommand(p *commands.KnParams) *cobra.Command { serviceCmd := &cobra.Command{ Use: "service", Short: "Service command group", - RunE: func(cmd *cobra.Command, args []string) error { - cmd.Help() - if len(args) == 0 { - return errors.New("please provide a valid sub-command for \"kn service\"") - } else { - return errors.New(fmt.Sprintf("unknown command \"%s\" for \"kn service\"", args[0])) - } - return nil - }, } serviceCmd.AddCommand(NewServiceListCommand(p)) serviceCmd.AddCommand(NewServiceDescribeCommand(p)) diff --git a/pkg/kn/commands/service/service_test.go b/pkg/kn/commands/service/service_test.go deleted file mode 100644 index 5c685f1356..0000000000 --- a/pkg/kn/commands/service/service_test.go +++ /dev/null @@ -1,66 +0,0 @@ -// Copyright © 2018 The Knative Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package service - -import ( - "strings" - "testing" - - "github.com/knative/client/pkg/kn/commands" - v1alpha1 "github.com/knative/serving/pkg/apis/serving/v1alpha1" - "k8s.io/apimachinery/pkg/runtime" - client_testing "k8s.io/client-go/testing" -) - -func fakeService(args []string, response *v1alpha1.ServiceList) (action client_testing.Action, output []string, err error) { - knParams := &commands.KnParams{} - cmd, fakeServing, buf := commands.CreateTestKnCommand(NewServiceCommand(knParams), knParams) - fakeServing.AddReactor("*", "*", - func(a client_testing.Action) (bool, runtime.Object, error) { - action = a - return true, response, nil - }) - cmd.SetArgs(args) - err = cmd.Execute() - if err != nil { - return - } - output = strings.Split(buf.String(), "\n") - return -} - -func TestUnknownSubcommand(t *testing.T) { - _, _, err := fakeService([]string{"service", "unknown"}, &v1alpha1.ServiceList{}) - if err == nil { - t.Error(err) - return - } - - if err.Error() != "unknown command \"unknown\" for \"kn service\"" { - t.Errorf("Bad error message '%s'", err.Error()) - } -} - -func TestEmptySubcommand(t *testing.T) { - _, _, err := fakeService([]string{"service"}, &v1alpha1.ServiceList{}) - if err == nil { - t.Error(err) - return - } - - if err.Error() != "please provide a valid sub-command for \"kn service\"" { - t.Errorf("Bad error message '%s'", err.Error()) - } -} diff --git a/pkg/kn/core/root.go b/pkg/kn/core/root.go index 7522af0255..448168fae8 100644 --- a/pkg/kn/core/root.go +++ b/pkg/kn/core/root.go @@ -15,6 +15,7 @@ package core import ( + "errors" "flag" "fmt" "os" @@ -71,11 +72,33 @@ Eventing: Manage event subscriptions and channels. Connect up event sources.`, rootCmd.AddCommand(commands.NewCompletionCommand(p)) rootCmd.AddCommand(commands.NewVersionCommand(p)) + // Deal with empty and unknown sub command groups + emptyAndUnknownSubcommands(rootCmd) + // For glog parse error. flag.CommandLine.Parse([]string{}) return rootCmd } +// emptyAndUnknownSubcommands adds a RunE to all commands that are groups to +// deal with errors when called with empty or unknown sub command +func emptyAndUnknownSubcommands(cmd *cobra.Command) { + for _, childCmd := range cmd.Commands() { + if childCmd.HasSubCommands() && childCmd.RunE == nil { + childCmd.RunE = func(aCmd *cobra.Command, args []string) error { + aCmd.Help() + if len(args) == 0 { + return errors.New(fmt.Sprintf("please provide a valid sub-command for \"kn %s\"", aCmd.Name())) + } else { + return errors.New(fmt.Sprintf("unknown command \"%s\" for \"kn %s\"", args[0], aCmd.Name())) + } + } + // recurse to deal with child commands that are themselves command groups + emptyAndUnknownSubcommands(childCmd) + } + } +} + // initConfig reads in config file and ENV variables if set. func initConfig() { if commands.CfgFile != "" {