Merge pull request #3351 from lavalamp/fix

Fix error messages; add tests

pkg/api/helpers.go

@@ -17,12 +17,29 @@ limitations under the License.
 package api
 
 import (
+	"reflect"
 	"strings"
 
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api/resource"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/conversion"
+
+	"github.com/davecgh/go-spew/spew"
 )
 
+// Conversion error conveniently packages up errors in conversions.
+type ConversionError struct {
+	In, Out interface{}
+	Message string
+}
+
+// Return a helpful string about the error
+func (c *ConversionError) Error() string {
+	return spew.Sprintf(
+		"Conversion error: %s. (in: %v(%+v) out: %v)",
+		c.Message, reflect.TypeOf(c.In), c.In, reflect.TypeOf(c.Out),
+	)
+}
+
 // Semantic can do semantic deep equality checks for api objects.
 // Example: api.Semantic.DeepEqual(aPod, aPodWithNonNilButEmptyMaps) == true
 var Semantic = conversion.EqualitiesOrDie(
@@ -38,8 +55,12 @@ var Semantic = conversion.EqualitiesOrDie(
 		if b.Amount == nil && a.MilliValue() == 0 {
 			return true
 		}
+		if a.Amount == nil || b.Amount == nil {
+			return false
+		}
 		return a.Amount.Cmp(b.Amount) == 0
 	},
+	pullPoliciesEqual,
 )
 
 // TODO: Address these per #1502

pkg/api/helpers_test.go

@@ -0,0 +1,71 @@
+/*
+Copyright 2015 Google Inc. All rights reserved.
+
+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 api
+
+import (
+	"strings"
+	"testing"
+
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/api/resource"
+
+	"speter.net/go/exp/math/dec/inf"
+)
+
+func TestConversionError(t *testing.T) {
+	var i int
+	var s string
+	i = 3
+	s = "foo"
+	c := ConversionError{
+		In: &i, Out: &s,
+		Message: "Can't make x into y, silly",
+	}
+	var e error
+	e = &c // ensure it implements error
+	msg := e.Error()
+	t.Logf("Message is %v", msg)
+	for _, part := range []string{"3", "int", "string", "Can't"} {
+		if !strings.Contains(msg, part) {
+			t.Errorf("didn't find %v", part)
+		}
+	}
+}
+
+func TestSemantic(t *testing.T) {
+	table := []struct {
+		a, b        interface{}
+		shouldEqual bool
+	}{
+		{resource.MustParse("0"), resource.Quantity{}, true},
+		{resource.Quantity{}, resource.MustParse("0"), true},
+		{resource.Quantity{}, resource.MustParse("1m"), false},
+		{
+			resource.Quantity{inf.NewDec(5, 0), resource.BinarySI},
+			resource.Quantity{inf.NewDec(5, 0), resource.DecimalSI},
+			true,
+		},
+		{resource.MustParse("2m"), resource.MustParse("1m"), false},
+		{PullPolicy("NEVER"), PullPolicy("neveR"), true},
+		{PullPolicy("NEVER"), PullPolicy("neveRi"), false},
+	}
+
+	for index, item := range table {
+		if e, a := item.shouldEqual, Semantic.DeepEqual(item.a, item.b); e != a {
+			t.Errorf("expected %v, got %v.", index, e, a)
+		}
+	}
+}

pkg/api/v1beta1/conversion.go

@@ -17,7 +17,6 @@ limitations under the License.
 package v1beta1
 
 import (
-	"errors"
 	"fmt"
 	"strconv"
 
@@ -234,7 +233,11 @@ func init() {
 			case newer.PodUnknown:
 				*out = PodUnknown
 			default:
-				return errors.New("The string provided is not a valid PodPhase constant value")
+				return &newer.ConversionError{
+					In:      in,
+					Out:     out,
+					Message: "The string provided is not a valid PodPhase constant value",
+				}
 			}
 
 			return nil
@@ -256,7 +259,11 @@ func init() {
 			case PodUnknown:
 				*out = newer.PodUnknown
 			default:
-				return errors.New("The string provided is not a valid PodPhase constant value")
+				return &newer.ConversionError{
+					In:      in,
+					Out:     out,
+					Message: "The string provided is not a valid PodPhase constant value",
+				}
 			}
 			return nil
 		},
@@ -349,7 +356,11 @@ func init() {
 				return err
 			}
 			if in.TemplateRef != nil && in.Template == nil {
-				return errors.New("objects with a template ref cannot be converted to older objects, must populate template")
+				return &newer.ConversionError{
+					In:      in,
+					Out:     out,
+					Message: "objects with a template ref cannot be converted to older objects, must populate template",
+				}
 			}
 			if in.Template != nil {
 				if err := s.Convert(in.Template, &out.PodTemplate, 0); err != nil {
@@ -616,7 +627,10 @@ func init() {
 			for k, v := range *in {
 				fv, err := strconv.ParseFloat(v.String(), 64)
 				if err != nil {
-					return fmt.Errorf("value '%v' of '%v': %v", v, k, err)
+					return &newer.ConversionError{
+						In: in, Out: out,
+						Message: fmt.Sprintf("value '%v' of '%v': %v", v, k, err),
+					}
 				}
 				if k == ResourceCPU {
 					(*out)[newer.ResourceCPU] = *resource.NewMilliQuantity(int64(fv*1000), resource.DecimalSI)

pkg/api/v1beta2/conversion.go

@@ -17,7 +17,6 @@ limitations under the License.
 package v1beta2
 
 import (
-	"errors"
 	"fmt"
 	"strconv"
 
@@ -122,7 +121,11 @@ func init() {
 			case newer.PodUnknown:
 				*out = PodUnknown
 			default:
-				return errors.New("The string provided is not a valid PodPhase constant value")
+				return &newer.ConversionError{
+					In:      in,
+					Out:     out,
+					Message: "The string provided is not a valid PodPhase constant value",
+				}
 			}
 
 			return nil
@@ -144,7 +147,11 @@ func init() {
 			case PodUnknown:
 				*out = newer.PodUnknown
 			default:
-				return errors.New("The string provided is not a valid PodPhase constant value")
+				return &newer.ConversionError{
+					In:      in,
+					Out:     out,
+					Message: "The string provided is not a valid PodPhase constant value",
+				}
 			}
 			return nil
 		},
@@ -237,7 +244,11 @@ func init() {
 				return err
 			}
 			if in.TemplateRef != nil && in.Template == nil {
-				return errors.New("objects with a template ref cannot be converted to older objects, must populate template")
+				return &newer.ConversionError{
+					In:      in,
+					Out:     out,
+					Message: "objects with a template ref cannot be converted to older objects, must populate template",
+				}
 			}
 			if in.Template != nil {
 				if err := s.Convert(in.Template, &out.PodTemplate, 0); err != nil {
@@ -532,7 +543,10 @@ func init() {
 			for k, v := range *in {
 				fv, err := strconv.ParseFloat(v.String(), 64)
 				if err != nil {
-					return fmt.Errorf("value '%v' of '%v': %v", v, k, err)
+					return &newer.ConversionError{
+						In: in, Out: out,
+						Message: fmt.Sprintf("value '%v' of '%v': %v", v, k, err),
+					}
 				}
 				if k == ResourceCPU {
 					(*out)[newer.ResourceCPU] = *resource.NewMilliQuantity(int64(fv*1000), resource.DecimalSI)