From 8db55d0bebf4d6ba6071dd94d390f218aae2f42f 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. Added a gotest.tools test for root.go. --- docs/cmd/kn_revision.md | 4 ++ docs/cmd/kn_service.md | 4 ++ go.mod | 4 +- go.sum | 4 ++ pkg/kn/core/root.go | 28 +++++++++- pkg/kn/core/root_test.go | 110 +++++++++++++++++++++++++++++++++++++++ 6 files changed, 152 insertions(+), 2 deletions(-) create mode 100644 pkg/kn/core/root_test.go diff --git a/docs/cmd/kn_revision.md b/docs/cmd/kn_revision.md index 01f393e23d..452fdf3125 100644 --- a/docs/cmd/kn_revision.md +++ b/docs/cmd/kn_revision.md @@ -6,6 +6,10 @@ Revision command group Revision command group +``` +kn revision [flags] +``` + ### Options ``` diff --git a/docs/cmd/kn_service.md b/docs/cmd/kn_service.md index 85cd324ac9..5b4a36f95c 100644 --- a/docs/cmd/kn_service.md +++ b/docs/cmd/kn_service.md @@ -6,6 +6,10 @@ Service command group Service command group +``` +kn service [flags] +``` + ### Options ``` diff --git a/go.mod b/go.mod index c12adfafc1..df346b07ff 100644 --- a/go.mod +++ b/go.mod @@ -22,11 +22,12 @@ require ( github.com/knative/build v0.6.0 // indirect github.com/knative/pkg v0.0.0-20190518173526-34792a92cec2 github.com/knative/serving v0.6.0 - github.com/knative/test-infra v0.0.0-20190624052103-517330119155 + github.com/knative/test-infra v0.0.0-20190702025031-91d37e4abc30 github.com/mattbaird/jsonpatch v0.0.0-20171005235357-81af80346b1a // indirect github.com/mitchellh/go-homedir v1.1.0 github.com/modern-go/reflect2 v1.0.1 // indirect github.com/peterbourgon/diskv v2.0.1+incompatible // indirect + github.com/pkg/errors v0.8.1 // indirect github.com/spf13/cobra v0.0.3 github.com/spf13/pflag v1.0.3 github.com/spf13/viper v1.3.1 @@ -39,6 +40,7 @@ require ( golang.org/x/time v0.0.0-20190308202827-9d24e82272b4 // indirect golang.org/x/tools v0.0.0-20190628034336-212fb13d595e // indirect gopkg.in/inf.v0 v0.9.1 // indirect + gotest.tools v2.2.0+incompatible k8s.io/api v0.0.0-20190226173710-145d52631d00 k8s.io/apimachinery v0.0.0-20190221084156-01f179d85dbc k8s.io/cli-runtime v0.0.0-20190325194458-f2b4781c3ae1 diff --git a/go.sum b/go.sum index 69c1cb777e..7e1831f302 100644 --- a/go.sum +++ b/go.sum @@ -159,6 +159,8 @@ github.com/knative/test-infra v0.0.0-20190531180034-a3c073a2fea1 h1:Y2QunZIzGuyv github.com/knative/test-infra v0.0.0-20190531180034-a3c073a2fea1/go.mod h1:l77IWBscEV5T4sYb64/9iwRCVY4UXEIqMcAppsblHW4= github.com/knative/test-infra v0.0.0-20190624052103-517330119155 h1:ceTQvUNU19Vc13TlSNo6u0F0pHak9wcGJQ+gv0X4KlE= github.com/knative/test-infra v0.0.0-20190624052103-517330119155/go.mod h1:l77IWBscEV5T4sYb64/9iwRCVY4UXEIqMcAppsblHW4= +github.com/knative/test-infra v0.0.0-20190702025031-91d37e4abc30 h1:giPqryO6DJHa/b4eHxhOOcr2KiEs3g6k6IVsPTMdKZM= +github.com/knative/test-infra v0.0.0-20190702025031-91d37e4abc30/go.mod h1:l77IWBscEV5T4sYb64/9iwRCVY4UXEIqMcAppsblHW4= github.com/konsorten/go-windows-terminal-sequences v1.0.1/go.mod h1:T0+1ngSBFLxvqU3pZ+m/2kptfBszLMUkC4ZK/EgS/cQ= github.com/kr/logfmt v0.0.0-20140226030751-b84e30acd515/go.mod h1:+0opPa2QZZtGFBFZlji/RkVcI2GknAs/DXo4wKdlNEc= github.com/kr/pretty v0.1.0 h1:L/CwN0zerZDmRFUapSPitk6f+Q3+0za1rQkzVuMiMFI= @@ -362,6 +364,8 @@ gopkg.in/yaml.v2 v2.0.0-20170812160011-eb3733d160e7/go.mod h1:JAlM8MvJe8wmxCU4Bl gopkg.in/yaml.v2 v2.2.1/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v2 v2.2.2 h1:ZCJp+EgiOT7lHqUV2J862kp8Qj64Jo6az82+3Td9dZw= gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= +gotest.tools v2.2.0+incompatible h1:VsBPFP1AI068pPrMxtb/S8Zkgf9xEmTLJjfM+P5UIEo= +gotest.tools v2.2.0+incompatible/go.mod h1:DsYFclhRJ6vuDpmuTbkuFWG+y2sxOXAzmJt81HFBacw= honnef.co/go/tools v0.0.0-20180728063816-88497007e858/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4= honnef.co/go/tools v0.0.0-20190102054323-c2f93a96b099/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4= honnef.co/go/tools v0.0.0-20190106161140-3f1c8253044a/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4= diff --git a/pkg/kn/core/root.go b/pkg/kn/core/root.go index 7522af0255..733486d36a 100644 --- a/pkg/kn/core/root.go +++ b/pkg/kn/core/root.go @@ -15,6 +15,7 @@ package core import ( + "errors" "flag" "fmt" "os" @@ -30,7 +31,7 @@ import ( _ "k8s.io/client-go/plugin/pkg/client/auth/oidc" ) -// rootCmd represents the base command when called without any subcommands +// NewKnCommand creates new rootCmd represents the base command when called without any subcommands func NewKnCommand(params ...commands.KnParams) *cobra.Command { var p *commands.KnParams if len(params) == 0 { @@ -71,11 +72,36 @@ 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() + fmt.Println() + + 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 sub-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 != "" { diff --git a/pkg/kn/core/root_test.go b/pkg/kn/core/root_test.go new file mode 100644 index 0000000000..32bf6fd073 --- /dev/null +++ b/pkg/kn/core/root_test.go @@ -0,0 +1,110 @@ +// Copyright © 2019 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 core + +import ( + "strings" + "testing" + + "github.com/knative/client/pkg/kn/commands" + "github.com/spf13/cobra" + "gotest.tools/assert" +) + +func TestNewKnCommand(t *testing.T) { + var rootCmd *cobra.Command + + setup := func() { + rootCmd = NewKnCommand(commands.KnParams{}) + } + + setup() + + t.Run("returns a valid root command", func(t *testing.T) { + assert.Assert(t, rootCmd != nil) + + assert.Equal(t, rootCmd.Name(), "kn") + assert.Equal(t, rootCmd.Short, "Knative client") + assert.Assert(t, strings.Contains(rootCmd.Long, "Manage your Knative building blocks:")) + + assert.Assert(t, rootCmd.DisableAutoGenTag) + assert.Assert(t, rootCmd.SilenceUsage) + assert.Assert(t, rootCmd.SilenceErrors) + + assert.Assert(t, rootCmd.RunE == nil) + }) + + t.Run("sets the output params", func(t *testing.T) { + assert.Assert(t, rootCmd.OutOrStdout() != nil) + }) + + t.Run("sets the config and kubeconfig global flags", func(t *testing.T) { + assert.Assert(t, rootCmd.PersistentFlags().Lookup("config") != nil) + assert.Assert(t, rootCmd.PersistentFlags().Lookup("kubeconfig") != nil) + }) + + t.Run("adds the top level commands: version and completion", func(t *testing.T) { + checkCommand(t, "version", rootCmd) + checkCommand(t, "completion", rootCmd) + }) + + t.Run("adds the top level group commands", func(t *testing.T) { + checkCommandGroup(t, "service", rootCmd) + checkCommandGroup(t, "revision", rootCmd) + }) +} + +func TestEmptyAndUnknownSubCommands(t *testing.T) { + var rootCmd, fakeCmd, fakeSubCmd *cobra.Command + + setup := func() { + rootCmd = NewKnCommand(commands.KnParams{}) + fakeCmd = &cobra.Command{ + Use: "fake-cmd-name", + } + fakeSubCmd = &cobra.Command{ + Use: "fake-sub-cmd-name", + } + fakeCmd.AddCommand(fakeSubCmd) + rootCmd.AddCommand(fakeCmd) + + assert.Assert(t, fakeCmd.RunE == nil) + assert.Assert(t, fakeSubCmd.RunE == nil) + } + + setup() + + t.Run("deals with empty and unknown sub-commands for all group commands", func(t *testing.T) { + EmptyAndUnknownSubCommands(rootCmd) + checkCommand(t, "fake-sub-cmd-name", fakeCmd) + checkCommandGroup(t, "fake-cmd-name", rootCmd) + }) +} + +// Private + +func checkCommand(t *testing.T, name string, rootCmd *cobra.Command) { + cmd, _, err := rootCmd.Find([]string{"version"}) + assert.Assert(t, err == nil) + assert.Assert(t, cmd != nil) +} + +func checkCommandGroup(t *testing.T, name string, rootCmd *cobra.Command) { + cmd, _, err := rootCmd.Find([]string{name}) + assert.Assert(t, err == nil) + assert.Assert(t, cmd != nil) + assert.Assert(t, cmd.RunE != nil) + assert.Assert(t, cmd.HasSubCommands()) +}