From bd7b457d9c0cd377ad90a32ede4512ee276a3427 Mon Sep 17 00:00:00 2001 From: Daniel Smith Date: Thu, 8 Jan 2015 17:34:53 -0800 Subject: [PATCH] Fix error messages; add tests --- pkg/api/helpers.go | 21 +++++++++++ pkg/api/helpers_test.go | 71 +++++++++++++++++++++++++++++++++++ pkg/api/v1beta1/conversion.go | 24 +++++++++--- pkg/api/v1beta2/conversion.go | 24 +++++++++--- 4 files changed, 130 insertions(+), 10 deletions(-) create mode 100644 pkg/api/helpers_test.go diff --git a/pkg/api/helpers.go b/pkg/api/helpers.go index 5ee4fa1b010..75ceff3a7cf 100644 --- a/pkg/api/helpers.go +++ b/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 diff --git a/pkg/api/helpers_test.go b/pkg/api/helpers_test.go new file mode 100644 index 00000000000..5d15ced93c0 --- /dev/null +++ b/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) + } + } +} diff --git a/pkg/api/v1beta1/conversion.go b/pkg/api/v1beta1/conversion.go index 3248d31d4d8..9a0535c1fcc 100644 --- a/pkg/api/v1beta1/conversion.go +++ b/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) diff --git a/pkg/api/v1beta2/conversion.go b/pkg/api/v1beta2/conversion.go index 00d8cfb313a..9a316b425e9 100644 --- a/pkg/api/v1beta2/conversion.go +++ b/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)