From c081abc14506738fcafd0e51067e54ea3229422c Mon Sep 17 00:00:00 2001 From: Roger Peppe Date: Mon, 31 Oct 2016 10:08:41 +0000 Subject: [PATCH] checkers: ignore time zone in DeepEqual This means tests won't need to concern themselves too deeply about the time zone used in times unmarshaled from databases or the network. We also change the printed message to print the time nicely rather than printing the details of the unexported time fields. --- checkers/deepequal.go | 34 ++++++++++++++++++++++++++++++---- checkers/deepequal_test.go | 19 +++++++++++++++---- 2 files changed, 45 insertions(+), 8 deletions(-) diff --git a/checkers/deepequal.go b/checkers/deepequal.go index 5120b8f7b..43567fa8e 100644 --- a/checkers/deepequal.go +++ b/checkers/deepequal.go @@ -10,9 +10,12 @@ package checkers import ( "fmt" "reflect" + "time" "unsafe" ) +var timeType = reflect.TypeOf(time.Time{}) + // During deepValueEqual, must keep track of checks that are // in progress. The comparison algorithm assumes that all // checks in progress are true when it reencounters them. @@ -34,7 +37,17 @@ func (err *mismatchError) Error() string { if path == "" { path = "top level" } - return fmt.Sprintf("mismatch at %s: %s; obtained %#v; expected %#v", path, err.how, interfaceOf(err.v1), interfaceOf(err.v2)) + return fmt.Sprintf("mismatch at %s: %s; obtained %#v; expected %#v", path, err.how, printable(err.v1), printable(err.v2)) +} + +func printable(v reflect.Value) interface{} { + vi := interfaceOf(v) + switch vi := vi.(type) { + case time.Time: + return vi.UTC().Format(time.RFC3339Nano) + default: + return vi + } } // Tests for deep equality using reflected types. The map argument tracks @@ -133,6 +146,15 @@ func deepValueEqual(path string, v1, v2 reflect.Value, visited map[visit]bool, d case reflect.Ptr: return deepValueEqual("(*"+path+")", v1.Elem(), v2.Elem(), visited, depth+1) case reflect.Struct: + if v1.Type() == timeType { + // Special case for time - we ignore the time zone. + t1 := interfaceOf(v1).(time.Time) + t2 := interfaceOf(v2).(time.Time) + if t1.Equal(t2) { + return true, nil + } + return false, errorf("unequal") + } for i, n := 0, v1.NumField(); i < n; i++ { path := path + "." + v1.Type().Field(i).Name if ok, err := deepValueEqual(path, v1.Field(i), v2.Field(i), visited, depth+1); !ok { @@ -214,9 +236,13 @@ func deepValueEqual(path string, v1, v2 reflect.Value, visited map[visit]bool, d // equality. DeepEqual correctly handles recursive types. Functions are // equal only if they are both nil. // -// DeepEqual differs from reflect.DeepEqual in that an empty slice is -// equal to a nil slice. If the two values compare unequal, the -// resulting error holds the first difference encountered. +// DeepEqual differs from reflect.DeepEqual in two ways: +// - an empty slice is considered equal to a nil slice. +// - two time.Time values that represent the same instant +// but with different time zones are considered equal. +// +// If the two values compare unequal, the resulting error holds the +// first difference encountered. func DeepEqual(a1, a2 interface{}) (bool, error) { errorf := func(f string, a ...interface{}) error { return &mismatchError{ diff --git a/checkers/deepequal_test.go b/checkers/deepequal_test.go index 1f4f2acc4..3fbd4d141 100644 --- a/checkers/deepequal_test.go +++ b/checkers/deepequal_test.go @@ -13,6 +13,7 @@ package checkers_test import ( "regexp" "testing" + "time" "github.com/juju/testing/checkers" ) @@ -56,6 +57,9 @@ var deepEqualTests = []DeepEqualTest{ {error(nil), error(nil), true, ""}, {map[int]string{1: "one", 2: "two"}, map[int]string{2: "two", 1: "one"}, true, ""}, {fn1, fn2, true, ""}, + {time.Unix(0, 0), time.Unix(0, 0), true, ""}, + // Same time from different zones (difference from normal DeepEqual) + {time.Unix(0, 0).UTC(), time.Unix(0, 0).In(time.FixedZone("FOO", 60*60)), true, ""}, // Inequalities {1, 2, false, `mismatch at top level: unequal; obtained 1; expected 2`}, @@ -89,6 +93,9 @@ var deepEqualTests = []DeepEqualTest{ {[]int{1, 2, 3}, [3]int{1, 2, 3}, false, `mismatch at top level: type mismatch \[\]int vs \[3\]int; obtained \[\]int\{1, 2, 3\}; expected \[3\]int\{1, 2, 3\}`}, {&[3]interface{}{1, 2, 4}, &[3]interface{}{1, 2, "s"}, false, `mismatch at \(\*\)\[2\]: type mismatch int vs string; obtained 4; expected "s"`}, {Basic{1, 0.5}, NotBasic{1, 0.5}, false, `mismatch at top level: type mismatch checkers_test\.Basic vs checkers_test\.NotBasic; obtained checkers_test\.Basic\{x:1, y:0\.5\}; expected checkers_test\.NotBasic\{x:1, y:0\.5\}`}, + {time.Unix(0, 0).UTC(), time.Unix(0, 0).In(time.FixedZone("FOO", 60*60)).Add(1), false, `mismatch at top level: unequal; obtained "1970-01-01T00:00:00Z"; expected "1970-01-01T00:00:00.000000001Z"`}, + {time.Unix(0, 0).UTC(), time.Unix(0, 0).Add(1), false, `mismatch at top level: unequal; obtained "1970-01-01T00:00:00Z"; expected "1970-01-01T00:00:00.000000001Z"`}, + { map[uint]string{1: "one", 2: "two"}, map[int]string{2: "two", 1: "one"}, @@ -107,10 +114,14 @@ func TestDeepEqual(t *testing.T) { if err != nil { t.Errorf("deepEqual(%v, %v): unexpected error message %q when equal", test.a, test.b, err) } - } else { - if ok, _ := regexp.MatchString(test.msg, err.Error()); !ok { - t.Errorf("deepEqual(%v, %v); unexpected error %q, want %q", test.a, test.b, err.Error(), test.msg) - } + continue + } + if err == nil { + t.Errorf("deepEqual(%v, %v); mismatch but nil error", test.a, test.b) + continue + } + if ok, _ := regexp.MatchString(test.msg, err.Error()); !ok { + t.Errorf("deepEqual(%v, %v); unexpected error %q, want %q", test.a, test.b, err.Error(), test.msg) } } }