Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

assert: allow comparing time.Time #1149

Merged
merged 4 commits into from
Feb 15, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions assert/assertion_compare.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package assert
import (
"fmt"
"reflect"
"time"
)

type CompareType int
Expand Down Expand Up @@ -30,6 +31,8 @@ var (
float64Type = reflect.TypeOf(float64(1))

stringType = reflect.TypeOf("")

timeType = reflect.TypeOf(time.Time{})
)

func compare(obj1, obj2 interface{}, kind reflect.Kind) (CompareType, bool) {
Expand Down Expand Up @@ -299,6 +302,27 @@ func compare(obj1, obj2 interface{}, kind reflect.Kind) (CompareType, bool) {
return compareLess, true
}
}
// Check for known struct types we can check for compare results.
case reflect.Struct:
{
// All structs enter here. We're not interested in most types.
if !canConvert(obj1Value, timeType) {
break
}

// time.Time can compared!
timeObj1, ok := obj1.(time.Time)
if !ok {
timeObj1 = obj1Value.Convert(timeType).Interface().(time.Time)
}
Comment on lines +316 to +317
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we converting to timeType (i.e. time), then interface, then back to time?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Convert() returns a reflect.Value, Interface() takes us out of the reflection wrappers, and then finally we do a type assertion. It looks a bit convoluted, but it's all necessary!


timeObj2, ok := obj2.(time.Time)
if !ok {
timeObj2 = obj2Value.Convert(timeType).Interface().(time.Time)
}

return compare(timeObj1.UnixNano(), timeObj2.UnixNano(), reflect.Int64)
}
}

return compareEqual, false
Expand Down
16 changes: 16 additions & 0 deletions assert/assertion_compare_can_convert.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
//go:build go1.17
// +build go1.17

// TODO: once support for Go 1.16 is dropped, this file can be
// merged/removed with assertion_compare_go1.17_test.go and
// assertion_compare_legacy.go

package assert

import "reflect"

// Wrapper around reflect.Value.CanConvert, for compatability
// reasons.
func canConvert(value reflect.Value, to reflect.Type) bool {
return value.CanConvert(to)
}
54 changes: 54 additions & 0 deletions assert/assertion_compare_go1.17_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
//go:build go1.17
// +build go1.17

// TODO: once support for Go 1.16 is dropped, this file can be
// merged/removed with assertion_compare_can_convert.go and
// assertion_compare_legacy.go

package assert

import (
"reflect"
"testing"
"time"
)

func TestCompare17(t *testing.T) {
type customTime time.Time
for _, currCase := range []struct {
less interface{}
greater interface{}
cType string
}{
{less: time.Now(), greater: time.Now().Add(time.Hour), cType: "time.Time"},
{less: customTime(time.Now()), greater: customTime(time.Now().Add(time.Hour)), cType: "time.Time"},
} {
resLess, isComparable := compare(currCase.less, currCase.greater, reflect.ValueOf(currCase.less).Kind())
if !isComparable {
t.Error("object should be comparable for type " + currCase.cType)
}

if resLess != compareLess {
t.Errorf("object less (%v) should be less than greater (%v) for type "+currCase.cType,
currCase.less, currCase.greater)
}

resGreater, isComparable := compare(currCase.greater, currCase.less, reflect.ValueOf(currCase.less).Kind())
if !isComparable {
t.Error("object are comparable for type " + currCase.cType)
}

if resGreater != compareGreater {
t.Errorf("object greater should be greater than less for type " + currCase.cType)
}

resEqual, isComparable := compare(currCase.less, currCase.less, reflect.ValueOf(currCase.less).Kind())
if !isComparable {
t.Error("object are comparable for type " + currCase.cType)
}

if resEqual != 0 {
t.Errorf("objects should be equal for type " + currCase.cType)
}
}
}
16 changes: 16 additions & 0 deletions assert/assertion_compare_legacy.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
//go:build !go1.17
// +build !go1.17
torkelrogstad marked this conversation as resolved.
Show resolved Hide resolved

// TODO: once support for Go 1.16 is dropped, this file can be
// merged/removed with assertion_compare_go1.17_test.go and
// assertion_compare_can_convert.go

package assert

import "reflect"

// Older versions of Go does not have the reflect.Value.CanConvert
// method.
func canConvert(value reflect.Value, to reflect.Type) bool {
return false
}
3 changes: 2 additions & 1 deletion assert/assertion_compare_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,8 @@ func TestCompare(t *testing.T) {
}

if resLess != compareLess {
t.Errorf("object less should be less than greater for type " + currCase.cType)
t.Errorf("object less (%v) should be less than greater (%v) for type "+currCase.cType,
currCase.less, currCase.greater)
}

resGreater, isComparable := compare(currCase.greater, currCase.less, reflect.ValueOf(currCase.less).Kind())
Expand Down