Refactor to allow testing; write first tests. (#10)

* Refactor commands to allow insertion of fakes for testing

* Pin client-go to version that matches signatures other libs use

* Add tests for service list

* actually add test file

* Review comments: Fatal instead of Error/return, Split instead of read to newline

cmd/kn/main.go

@@ -28,7 +28,7 @@ func init() {
 func main() {
 	err := commands.NewKnCommand().Execute()
 	if err != nil {
-		fmt.Println(os.Stderr, err)
+		fmt.Fprintln(os.Stderr, err)
 		os.Exit(1)
 	}
 }

go.mod

@@ -7,6 +7,7 @@ require (
 	github.com/ghodss/yaml v1.0.0 // indirect
 	github.com/go-openapi/spec v0.18.0 // indirect
 	github.com/gogo/protobuf v1.2.0 // indirect
+	github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b // indirect
 	github.com/golang/protobuf v1.2.0 // indirect
 	github.com/google/btree v0.0.0-20180813153112-4030bb1f1f0c // indirect
 	github.com/google/go-cmp v0.2.0 // indirect
@@ -43,7 +44,7 @@ require (
 	k8s.io/api v0.0.0-20181221193117-173ce66c1e39 // indirect
 	k8s.io/apimachinery v0.0.0-20190104073114-849b284f3b75
 	k8s.io/cli-runtime v0.0.0-20190107235426-31214e12222d
-	k8s.io/client-go v10.0.0+incompatible
+	k8s.io/client-go v2.0.0-alpha.0.0.20181015214059-cbd9965a0e71+incompatible
 	k8s.io/klog v0.1.0 // indirect
 	k8s.io/kube-openapi v0.0.0-20181114233023-0317810137be // indirect
 	sigs.k8s.io/kustomize v1.0.11 // indirect

go.sum

@@ -29,6 +29,8 @@ github.com/go-openapi/swag v0.17.0 h1:iqrgMg7Q7SvtbWLlltPrkMs0UBJI6oTSs79JFRUi88
 github.com/go-openapi/swag v0.17.0/go.mod h1:AByQ+nYG6gQg71GINrmuDXCPWdL640yX49/kXLo40Tg=
 github.com/gogo/protobuf v1.2.0 h1:xU6/SpYbvkNYiptHJYEDRseDLvYE7wSqhYYNy0QSUzI=
 github.com/gogo/protobuf v1.2.0/go.mod h1:r8qH/GZQm5c6nD/R0oafs1akxWv10x8SbQlK7atdtwQ=
+github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b h1:VKtxabqXZkF25pY9ekfRL6a582T4P37/31XEstQ5p58=
+github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b/go.mod h1:SBH7ygxi8pfUlaOkMMuAQtPIUF8ecWP5IEl/CR7VP2Q=
 github.com/golang/protobuf v1.2.0 h1:P3YflyNX/ehuJFLhxviNdFxQPkGK5cDcApsge1SqnvM=
 github.com/golang/protobuf v1.2.0/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U=
 github.com/google/btree v0.0.0-20180813153112-4030bb1f1f0c h1:964Od4U6p2jUkFxvCydnIczKteheJEzHRToSGK3Bnlw=
@@ -146,6 +148,8 @@ k8s.io/cli-runtime v0.0.0-20181121073402-2f0d1d0a58f2 h1:0tWjdH70/BhNHxQ1cc0DEO6
 k8s.io/cli-runtime v0.0.0-20181121073402-2f0d1d0a58f2/go.mod h1:qWnH3/b8sp/l7EvlDh7ulDU3UWA4P4N1NFbEEP791tM=
 k8s.io/cli-runtime v0.0.0-20190107235426-31214e12222d h1:hdarETxu5sE+zfQ8CPfvWbQzMe1yaJA1XvURyx1u514=
 k8s.io/cli-runtime v0.0.0-20190107235426-31214e12222d/go.mod h1:qWnH3/b8sp/l7EvlDh7ulDU3UWA4P4N1NFbEEP791tM=
+k8s.io/client-go v2.0.0-alpha.0.0.20181015214059-cbd9965a0e71+incompatible h1:kambcoJKwJwrIu5h+566BnlQnJGGUSqI3Tp5x63mWnY=
+k8s.io/client-go v2.0.0-alpha.0.0.20181015214059-cbd9965a0e71+incompatible/go.mod h1:7vJpHMYJwNQCWgzmNV+VYUl1zCObLyodBc8nIyt8L5s=
 k8s.io/client-go v10.0.0+incompatible h1:F1IqCqw7oMBzDkqlcBymRq1450wD0eNqLE9jzUrIi34=
 k8s.io/client-go v10.0.0+incompatible/go.mod h1:7vJpHMYJwNQCWgzmNV+VYUl1zCObLyodBc8nIyt8L5s=
 k8s.io/klog v0.1.0 h1:I5HMfc/DtuVaGR1KPwUrTc476K8NCqNBldC7H4dYEzk=

pkg/kn/commands/revision.go

@@ -18,12 +18,12 @@ import (
 	"github.com/spf13/cobra"
 )
 
-func NewRevisionCommand() *cobra.Command {
+func NewRevisionCommand(p *KnParams) *cobra.Command {
 	revisionCmd := &cobra.Command{
 		Use:   "revision",
 		Short: "Revision command group.",
 	}
 	revisionCmd.PersistentFlags().StringP("namespace", "n", "default", "Namespace to use.")
-	revisionCmd.AddCommand(NewRevisionListCommand())
+	revisionCmd.AddCommand(NewRevisionListCommand(p))
 	return revisionCmd
 }

pkg/kn/commands/revision_list.go

@@ -15,33 +15,24 @@
 package commands
 
 import (
-	"os"
-
-	serving "github.com/knative/serving/pkg/client/clientset/versioned/typed/serving/v1alpha1"
 	"github.com/spf13/cobra"
 	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/cli-runtime/pkg/genericclioptions"
 	_ "k8s.io/client-go/plugin/pkg/client/auth/gcp"
-	"k8s.io/client-go/tools/clientcmd"
 )
 
 var revisionListPrintFlags *genericclioptions.PrintFlags
 
 // listCmd represents the list command
-func NewRevisionListCommand() *cobra.Command {
+func NewRevisionListCommand(p *KnParams) *cobra.Command {
 	revisionListPrintFlags = genericclioptions.NewPrintFlags("").WithDefaultOutput(
 		"jsonpath={range .items[*]}{.metadata.name}{\"\\n\"}{end}")
 	revisionListCmd := &cobra.Command{
 		Use:   "list",
 		Short: "List available revisions.",
 		RunE: func(cmd *cobra.Command, args []string) error {
-			// use the current context in kubeconfig
+			client, err := p.ServingFactory()
-			config, err := clientcmd.BuildConfigFromFlags("", kubeCfgFile)
-			if err != nil {
-				return err
-			}
-			client, err := serving.NewForConfig(config)
 			if err != nil {
 				return err
 			}
@@ -59,7 +50,7 @@ func NewRevisionListCommand() *cobra.Command {
 				Group:   "knative.dev",
 				Version: "v1alpha1",
 				Kind:    "Revision"})
-			err = printer.PrintObj(revision, os.Stdout)
+			err = printer.PrintObj(revision, cmd.OutOrStdout())
 			if err != nil {
 				return err
 			}

pkg/kn/commands/root.go

@@ -16,19 +16,44 @@ package commands
 
 import (
 	"fmt"
+	"io"
 	"os"
 	"path/filepath"
 
+	serving "github.com/knative/serving/pkg/client/clientset/versioned/typed/serving/v1alpha1"
 	homedir "github.com/mitchellh/go-homedir"
 	"github.com/spf13/cobra"
 	"github.com/spf13/viper"
+	"k8s.io/client-go/tools/clientcmd"
 )
 
 var cfgFile string
 var kubeCfgFile string
 
+// Parameters for creating commands. Useful for inserting mocks for testing.
+type KnParams struct {
+	Output         io.Writer
+	ServingFactory func() (serving.ServingV1alpha1Interface, error)
+}
+
+func (c *KnParams) Initialize() {
+	if c.ServingFactory == nil {
+		c.ServingFactory = GetConfig
+	}
+}
+
 // rootCmd represents the base command when called without any subcommands
-func NewKnCommand() *cobra.Command {
+func NewKnCommand(params ...KnParams) *cobra.Command {
+	var p *KnParams
+	if len(params) == 0 {
+		p = &KnParams{}
+	} else if len(params) == 1 {
+		p = &params[0]
+	} else {
+		panic("Too many params objects to NewKnCommand")
+	}
+	p.Initialize()
+
 	rootCmd := &cobra.Command{
 		Use:   "kn",
 		Short: "Knative client.",
@@ -38,10 +63,13 @@ Serving: Manage your services and release new software to them.
 Build: Create builds and keep track of their results.
 Eventing: Manage event subscriptions and channels. Connect up event sources.`,
 	}
+	if p.Output != nil {
+		rootCmd.SetOutput(p.Output)
+	}
 	rootCmd.PersistentFlags().StringVar(&cfgFile, "config", "", "config file (default is $HOME/.kn.yaml)")
 	rootCmd.PersistentFlags().StringVar(&kubeCfgFile, "kubeconfig", "", "kubectl config file (default is $HOME/.kube/config)")
-	rootCmd.AddCommand(NewServiceCommand())
+	rootCmd.AddCommand(NewServiceCommand(p))
-	rootCmd.AddCommand(NewRevisionCommand())
+	rootCmd.AddCommand(NewRevisionCommand(p))
 	return rootCmd
 }
 
@@ -55,7 +83,7 @@ func initKubeConfig() {
 	if kubeCfgFile == "" {
 		home, err := homedir.Dir()
 		if err != nil {
-			fmt.Println(err)
+			fmt.Fprintln(os.Stderr, err)
 			os.Exit(1)
 		}
 		kubeCfgFile = filepath.Join(home, ".kube", "config")
@@ -71,7 +99,7 @@ func initConfig() {
 		// Find home directory.
 		home, err := homedir.Dir()
 		if err != nil {
-			fmt.Println(err)
+			fmt.Fprintln(os.Stderr, err)
 			os.Exit(1)
 		}
 
@@ -84,6 +112,18 @@ func initConfig() {
 
 	// If a config file is found, read it in.
 	if err := viper.ReadInConfig(); err == nil {
-		fmt.Println("Using config file:", viper.ConfigFileUsed())
+		fmt.Fprintln(os.Stderr, "Using config file:", viper.ConfigFileUsed())
 	}
 }
+
+func GetConfig() (serving.ServingV1alpha1Interface, error) {
+	config, err := clientcmd.BuildConfigFromFlags("", kubeCfgFile)
+	if err != nil {
+		return nil, err
+	}
+	client, err := serving.NewForConfig(config)
+	if err != nil {
+		return nil, err
+	}
+	return client, nil
+}

pkg/kn/commands/service.go

@@ -18,12 +18,12 @@ import (
 	"github.com/spf13/cobra"
 )
 
-func NewServiceCommand() *cobra.Command {
+func NewServiceCommand(p *KnParams) *cobra.Command {
 	serviceCmd := &cobra.Command{
 		Use:   "service",
 		Short: "Service command group.",
 	}
 	serviceCmd.PersistentFlags().StringP("namespace", "n", "default", "Namespace to use.")
-	serviceCmd.AddCommand(NewServiceListCommand())
+	serviceCmd.AddCommand(NewServiceListCommand(p))
 	return serviceCmd
 }

pkg/kn/commands/service_list.go

@@ -15,21 +15,17 @@
 package commands
 
 import (
-	"os"
-
-	serving "github.com/knative/serving/pkg/client/clientset/versioned/typed/serving/v1alpha1"
 	"github.com/spf13/cobra"
 	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/cli-runtime/pkg/genericclioptions"
 	_ "k8s.io/client-go/plugin/pkg/client/auth/gcp"
-	"k8s.io/client-go/tools/clientcmd"
 )
 
 var serviceListPrintFlags *genericclioptions.PrintFlags
 
 // listCmd represents the list command
-func NewServiceListCommand() *cobra.Command {
+func NewServiceListCommand(p *KnParams) *cobra.Command {
 
 	serviceListPrintFlags := genericclioptions.NewPrintFlags("").WithDefaultOutput(
 		"jsonpath={range .items[*]}{.metadata.name}{\"\\n\"}{end}")
@@ -37,12 +33,7 @@ func NewServiceListCommand() *cobra.Command {
 		Use:   "list",
 		Short: "List available services.",
 		RunE: func(cmd *cobra.Command, args []string) error {
-			// use the current context in kubeconfig
+			client, err := p.ServingFactory()
-			config, err := clientcmd.BuildConfigFromFlags("", kubeCfgFile)
-			if err != nil {
-				return err
-			}
-			client, err := serving.NewForConfig(config)
 			if err != nil {
 				return err
 			}
@@ -60,7 +51,7 @@ func NewServiceListCommand() *cobra.Command {
 				Group:   "knative.dev",
 				Version: "v1alpha1",
 				Kind:    "Service"})
-			err = printer.PrintObj(service, os.Stdout)
+			err = printer.PrintObj(service, cmd.OutOrStdout())
 			if err != nil {
 				return err
 			}

pkg/kn/commands/service_list_test.go

@@ -0,0 +1,105 @@
+// 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 commands
+
+import (
+	"bytes"
+	"strings"
+	"testing"
+
+	"github.com/knative/serving/pkg/apis/serving/v1alpha1"
+	serving "github.com/knative/serving/pkg/client/clientset/versioned/typed/serving/v1alpha1"
+	"github.com/knative/serving/pkg/client/clientset/versioned/typed/serving/v1alpha1/fake"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
+	client_testing "k8s.io/client-go/testing"
+)
+
+func fakeList(args []string, response *v1alpha1.ServiceList) (action client_testing.Action, output []string, err error) {
+	buf := new(bytes.Buffer)
+	fakeServing := &fake.FakeServingV1alpha1{&client_testing.Fake{}}
+	cmd := NewKnCommand(KnParams{
+		Output:         buf,
+		ServingFactory: func() (serving.ServingV1alpha1Interface, error) { return fakeServing, nil },
+	})
+	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 TestListEmpty(t *testing.T) {
+	action, output, err := fakeList([]string{"service", "list"}, &v1alpha1.ServiceList{})
+	if err != nil {
+		t.Error(err)
+		return
+	}
+	for _, s := range output {
+		if s != "" {
+			t.Errorf("Bad output line %v", s)
+		}
+	}
+	if action == nil {
+		t.Errorf("No action")
+	} else if !action.Matches("list", "services") {
+		t.Errorf("Bad action %v", action)
+	}
+}
+
+var serviceType = metav1.TypeMeta{
+	Kind:       "service",
+	APIVersion: "serving.knative.dev/v1alpha1",
+}
+
+func TestListDefaultOutput(t *testing.T) {
+	action, output, err := fakeList([]string{"service", "list"}, &v1alpha1.ServiceList{
+		Items: []v1alpha1.Service{
+			v1alpha1.Service{
+				TypeMeta: serviceType,
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "foo",
+				},
+			},
+			v1alpha1.Service{
+				TypeMeta: serviceType,
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "bar",
+				},
+			},
+		},
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	expected := []string{"foo", "bar", ""}
+	for i, s := range output {
+		if s != expected[i] {
+			t.Errorf("Bad output line %v expected %v", s, expected[i])
+		}
+	}
+	if action == nil {
+		t.Errorf("No action")
+	} else if !action.Matches("list", "services") {
+		t.Errorf("Bad action %v", action)
+	}
+}