Skip to content

Commit

Permalink
fixes(issue #111) generically for all command groups
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
maximilien committed Jul 2, 2019
1 parent 9400919 commit 8db55d0
Show file tree
Hide file tree
Showing 6 changed files with 152 additions and 2 deletions.
4 changes: 4 additions & 0 deletions docs/cmd/kn_revision.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@ Revision command group

Revision command group

```
kn revision [flags]
```

### Options

```
Expand Down
4 changes: 4 additions & 0 deletions docs/cmd/kn_service.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@ Service command group

Service command group

```
kn service [flags]
```

### Options

```
Expand Down
4 changes: 3 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
4 changes: 4 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down Expand Up @@ -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=
Expand Down
28 changes: 27 additions & 1 deletion pkg/kn/core/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package core

import (
"errors"
"flag"
"fmt"
"os"
Expand All @@ -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 {
Expand Down Expand Up @@ -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 != "" {
Expand Down
110 changes: 110 additions & 0 deletions pkg/kn/core/root_test.go
Original file line number Diff line number Diff line change
@@ -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())
}

0 comments on commit 8db55d0

Please sign in to comment.