Fix returning the namespace for listing resources (#88)

* Fix returning the namespace for listing resources

 Fixes #86

* Adds license to pkg/kn/commands/namespaced.go

* Adds tests for namespace flags and retrieval

* Renames test function

pkg/kn/commands/namespaced.go

@@ -1,3 +1,17 @@
+// 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 commands
 
 import (
@@ -31,19 +45,19 @@ func AddNamespaceFlags(flags *pflag.FlagSet, allowAll bool) {
 // GetNamespace returns namespace from command specified by flag
 func GetNamespace(cmd *cobra.Command) (string, error) {
 	namespace := cmd.Flag("namespace").Value.String()
-	if cmd.Flags().Lookup("all-namespaces") == nil {
-		if namespace == "" {
-			return defaultNamespace, nil
+	// check value of all-namepace only if its defined
+	if cmd.Flags().Lookup("all-namespaces") != nil {
+		all, err := cmd.Flags().GetBool("all-namespaces")
+		if err != nil {
+			return "", err
+		} else if all { // if all-namespaces=True
+			// namespace = "" <-- all-namespaces representation
+			return "", nil
 		}
-		return namespace, nil
 	}
-
-	all, err := cmd.Flags().GetBool("all-namespaces")
-	if err != nil {
-		return "", err
-	}
-	if all {
-		namespace = ""
+	// if all-namepaces=False or namespace not given, use default namespace
+	if namespace == "" {
+		namespace = defaultNamespace
 	}
 	return namespace, nil
 }

pkg/kn/commands/namespaced_test.go

@@ -0,0 +1,122 @@
+// 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 commands
+
+import (
+	"github.com/spf13/cobra"
+	"testing"
+)
+
+// testCommandGenerator generates a test cobra command
+func testCommandGenerator(allNamespaceFlag bool) *cobra.Command {
+	var testCmd = &cobra.Command{
+		Use:   "kn",
+		Short: "Namespace test kn command",
+		Run:   func(cmd *cobra.Command, args []string) {},
+	}
+	AddNamespaceFlags(testCmd.Flags(), allNamespaceFlag)
+	return testCmd
+}
+
+// test by setting some namespace
+func TestGetNamespaceSample(t *testing.T) {
+	testCmd := testCommandGenerator(true)
+	expectedNamespace := "test1"
+	testCmd.SetArgs([]string{"--namespace", expectedNamespace})
+	testCmd.Execute()
+	actualNamespace, err := GetNamespace(testCmd)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if actualNamespace != expectedNamespace {
+		t.Fatalf("Incorrect namespace retrieved: %v, expected: %v", actualNamespace, expectedNamespace)
+	}
+}
+
+// test without setting any namespace
+func TestGetNamespaceDefault(t *testing.T) {
+	testCmd := testCommandGenerator(true)
+	expectedNamespace := "default"
+	testCmd.Execute()
+	actualNamespace, err := GetNamespace(testCmd)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if actualNamespace != expectedNamespace {
+		t.Fatalf("Incorrect namespace retrieved: %v, expected: %v", actualNamespace, expectedNamespace)
+	}
+}
+
+// test with all-namespaces flag set with sample namespace
+// all-namespaces flag takes the precendence
+func TestGetNamespaceAllNamespacesSet(t *testing.T) {
+	testCmd := testCommandGenerator(true)
+	expectedNamespace := ""
+	sampleNamespace := "test1"
+	testCmd.SetArgs([]string{"--namespace", sampleNamespace, "--all-namespaces"})
+	testCmd.Execute()
+	actualNamespace, err := GetNamespace(testCmd)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if actualNamespace != expectedNamespace {
+		t.Fatalf("Incorrect namespace retrieved: %v, expected: %v", actualNamespace, expectedNamespace)
+	}
+}
+
+// test with all-namespace flag set without any namespace flag set
+// all-namespace flag takes precendence
+func TestGetNamespaceDefaultAllNamespacesUnset(t *testing.T) {
+	testCmd := testCommandGenerator(true)
+	expectedNamespace := ""
+	testCmd.SetArgs([]string{"--all-namespaces"})
+	testCmd.Execute()
+	actualNamespace, err := GetNamespace(testCmd)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if actualNamespace != expectedNamespace {
+		t.Fatalf("Incorrect namespace retrieved: %v, expected: %v", actualNamespace, expectedNamespace)
+	}
+}
+
+// test with all-namespaces flag not defined for command
+func TestGetNamespaceAllNamespacesNotDefined(t *testing.T) {
+	testCmd := testCommandGenerator(false)
+	expectedNamespace := "test1"
+	testCmd.SetArgs([]string{"--namespace", expectedNamespace})
+	testCmd.Execute()
+	actualNamespace, err := GetNamespace(testCmd)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if actualNamespace != expectedNamespace {
+		t.Fatalf("Incorrect namespace retrieved: %v, expected: %v", actualNamespace, expectedNamespace)
+	}
+}
+
+// test with all-namespace flag not defined and no namespace given
+func TestGetNamespaceDefaultAllNamespacesNotDefined(t *testing.T) {
+	testCmd := testCommandGenerator(false)
+	expectedNamespace := "default"
+	testCmd.Execute()
+	actualNamespace, err := GetNamespace(testCmd)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if actualNamespace != expectedNamespace {
+		t.Fatalf("Incorrect namespace retrieved: %v, expected: %v", actualNamespace, expectedNamespace)
+	}
+}