Testify assert.Equal between two times doesn't work same as (time.Time).Equal.

[gfelixc]

Motivation

Before using testify I used to test my time.Time objects like this:

var (
    expected time.Time
    got time.Time
)

// ... 

if !expected.Equal(got) {
    t.Error(...)
}

Once I started using testify, my intuition was replace the above test with

var (
    expected time.Time
    got time.Time
)

// ... 

assert.Equal(t, expected, got)

Which mostly worked fine.... until not. I notice that two objects with different locations but representing same time instant were considered as not equal... I stopped using assert.Equal to compare objects time.Time but instead, using

assert.True(t, expected.Equal(got))

This way to use the library feels tricky, uncomfortable and lose the idiomatic meaningful. I don't want to assert a boolean, but two time.Time objects instead 😔

I propose to use built-in Equal for time.Time objects.

Considering this case could be translatable to any object with a method <T>.Equal(<T>) bool. Maybe checking existence of such method and using it, is a good approach.

Thanks for your consideration

[gfelixc]

Is possible to do this repo eligible for https://hacktoberfest.digitalocean.com/ ?? @glesica @boyan-soubachov @mvdkleijn

[nicholas-eden]

There was a similar PR merged a while back, but the change seems to have disappeared. #979

[ArnoSen]

I can confirm this. I have no problem with creating a PR for this but then the desired behavior needs to be clear.

IMHO there are 2 use cases that need to be addresed:

• the remarks on the time.Time object when it comes to comparison:
  Note that the Go == operator compares not just the time instant but also the Location and the monotonic clock reading. Therefore, Time values should not be used as map or database keys without first guaranteeing that the identical Location has been set for all values, which can be achieved through use of the UTC or Local method, and that the monotonic clock reading has been stripped by setting t = t.Round(0). In general, prefer t.Equal(u) to t == u, since t.Equal uses the most accurate comparison available and correctly handles the case when only one of its arguments has a monotonic clock reading.
• how to deal with time objects in different time zones but referring to the same time. I ran into this today. An example:

        t.Run("EqualTime", func(t *testing.T) {
                t1 := time.Date(2021, 11, 22, 10, 0, 0, 0, time.UTC)
                
                loc, err := time.LoadLocation("Europe/Amsterdam")
                require.NoError(t, err)
                t2 := time.Date(2021, 11, 22, 11, 0, 0, 0, loc)
                
                assert.Equal(t, t1, t2) // fails 
                assert.EqualValues(t, t1, t2) // fails 
                assert.True(t, t1.Equal(t2)) // passes
                
        })

Ideally you can define the behavior you want. At this moment assert is setup as a singleton so it will require some work to define the time comparison 'style' as an option (e.g. a := NewAssert(USE_TIME_EQUAL_METHOD), a.Equal(t, time1, time2) ) so maybe it is better to discriminate using methods. Then I would propose that methods like EqualValues do a comparison using the (time.Time).Equal method.

[brackendawson]

I agree that the assert.Equal function should be preserved as a pure struct comparison in case anyone's tests depend on that.

Is using the (time.Time).Equal method inside assert.EqualValues not ultimately equivalent to assert.WithinDuration with a zero duration?

func TestTime(t *testing.T) {
	start := time.Now()
	assert.True(t, start.Equal(start))
	assert.WithinDuration(t, start, start, 0)
}

[brackendawson]

I think this is answered, and is related to #1204