Improve unstruct test coverage and error messages.

internal/k8sdeps/kunstruct/kunstruct.go

@@ -20,8 +20,11 @@ package kunstruct
 import (
 	"encoding/json"
 	"fmt"
+	"reflect"
 	"strings"
 
+	"github.com/pkg/errors"
+
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -79,28 +82,31 @@ func (fs *UnstructAdapter) SetMap(m map[string]interface{}) {
 }
 
 // GetFieldValue returns value at the given fieldpath.
-func (fs *UnstructAdapter) GetFieldValue(fieldPath string) (string, error) {
+func (fs *UnstructAdapter) GetFieldValue(path string) (string, error) {
-	return getFieldValue(fs.UnstructuredContent(), strings.Split(fieldPath, "."))
+	s, err := getFieldValue(fs.UnstructuredContent(), strings.Split(path, "."))
+	if err != nil {
+		return "", errors.Wrapf(err, "at path '%s'", path)
+	}
+	return s, nil
 }
 
-func getFieldValue(m map[string]interface{}, pathToField []string) (string, error) {
+func getFieldValue(m map[string]interface{}, path []string) (string, error) {
-	if len(pathToField) == 0 {
+	if len(path) == 0 {
-		return "", fmt.Errorf("field not found")
+		return "", fmt.Errorf("%v not found", path)
 	}
-	if len(pathToField) == 1 {
+	v, ok := m[path[0]]
-		if v, found := m[pathToField[0]]; found {
+	if !ok {
-			if s, ok := v.(string); ok {
+		return "", fmt.Errorf("no field named '%s'", path[0])
-				return s, nil
+	}
-			}
+	if len(path) == 1 {
-			return "", fmt.Errorf("value at fieldpath is not of string type")
+		if s, ok := v.(string); ok {
+			return s, nil
 		}
-		return "", fmt.Errorf("field at given fieldpath does not exist")
+		return "", fmt.Errorf("value at '%v' not a string", path[0])
 	}
-	v := m[pathToField[0]]
+	if deeper, ok := v.(map[string]interface{}); ok {
-	switch typedV := v.(type) {
+		return getFieldValue(deeper, path[1:])
-	case map[string]interface{}:
-		return getFieldValue(typedV, pathToField[1:])
-	default:
-		return "", fmt.Errorf("%#v is not expected to be a primitive type", typedV)
 	}
+	return "", fmt.Errorf("Expected map at %v, but got %s=%v",
+		path[0], reflect.TypeOf(v), v)
 }

internal/k8sdeps/kunstruct/kunstruct_test.go

@@ -35,43 +35,114 @@ func TestGetFieldValue(t *testing.T) {
 				"port": "80",
 			},
 		},
+		"this": map[string]interface{}{
+			"is": map[string]interface{}{
+				"aNumber":    1000,
+				"aNilValue":  nil,
+				"anEmptyMap": map[string]interface{}{},
+				"unrecognizable": testing.InternalExample{
+					Name: "fooBar",
+				},
+			},
+		},
 	})
 
 	tests := []struct {
+		name          string
 		pathToField   string
 		expectedValue string
 		errorExpected bool
+		errorMsg      string
 	}{
 		{
+			name:          "oneField",
 			pathToField:   "Kind",
 			expectedValue: "Service",
 			errorExpected: false,
 		},
 		{
+			name:          "twoFields",
 			pathToField:   "metadata.name",
 			expectedValue: "service-name",
 			errorExpected: false,
 		},
 		{
-			pathToField:   "metadata.non-existing-field",
+			name:          "threeFields",
-			expectedValue: "",
-			errorExpected: true,
-		},
-		{
 			pathToField:   "spec.ports.port",
 			expectedValue: "80",
 			errorExpected: false,
 		},
+		{
+			name:          "empty",
+			pathToField:   "",
+			errorExpected: true,
+			errorMsg:      "at path '': no field named ''",
+		},
+		{
+			name:          "emptyDotEmpty",
+			pathToField:   ".",
+			errorExpected: true,
+			errorMsg:      "at path '.': no field named ''",
+		},
+		{
+			name:          "twoFieldsOneMissing",
+			pathToField:   "metadata.banana",
+			errorExpected: true,
+			errorMsg:      "at path 'metadata.banana': no field named 'banana'",
+		},
+		{
+			name:          "deeperMissingField",
+			pathToField:   "this.is.aDeep.field.that.does.not.exist",
+			errorExpected: true,
+			errorMsg:      "at path 'this.is.aDeep.field.that.does.not.exist': no field named 'aDeep'",
+		},
+		{
+			name:          "emptyMap",
+			pathToField:   "this.is.anEmptyMap",
+			errorExpected: true,
+			errorMsg:      "at path 'this.is.anEmptyMap': value at 'anEmptyMap' not a string",
+		},
+		{
+			name:          "numberAsValue",
+			pathToField:   "this.is.aNumber",
+			errorExpected: true,
+			errorMsg:      "at path 'this.is.aNumber': value at 'aNumber' not a string",
+		},
+		{
+			name:          "nilAsValue",
+			pathToField:   "this.is.aNilValue",
+			errorExpected: true,
+			errorMsg:      "at path 'this.is.aNilValue': value at 'aNilValue' not a string",
+		},
+		{
+			name:          "unrecognizable",
+			pathToField:   "this.is.unrecognizable.Name",
+			errorExpected: true,
+			errorMsg:      "at path 'this.is.unrecognizable.Name': Expected map at unrecognizable, but got testing.InternalExample={fooBar <nil>  false}",
+		},
 	}
 
 	for _, test := range tests {
 		s, err := kunstructured.GetFieldValue(test.pathToField)
-		if test.errorExpected && err == nil {
+		if test.errorExpected {
-			t.Fatalf("should return error, but no error returned")
+			if err == nil {
-		} else {
+				t.Fatalf("%q; path %q - should return error, but no error returned",
-			if test.expectedValue != s {
+					test.name, test.pathToField)
-				t.Fatalf("Got:%s expected:%s", s, test.expectedValue)
 			}
+			if test.errorMsg != err.Error() {
+				t.Fatalf("%q; path %q - expected error: \"%s\", got error: \"%v\"",
+					test.name, test.pathToField, test.errorMsg, err.Error())
+			}
+			continue
 		}
+		if err != nil {
+			t.Fatalf("%q; path %q - unexpected error %v",
+				test.name, test.pathToField, err)
+		}
+		if test.expectedValue != s {
+			t.Fatalf("%q; Got: %s expected: %s",
+				test.name, s, test.expectedValue)
+		}
+
 	}
 }