stretchr / testify

A toolkit with common assertions and mocks that plays nicely with the standard library
MIT License
23.15k stars 1.59k forks source link

assert: Equal does not consider Zone information in time.Time #1536

Closed brackendawson closed 7 months ago

brackendawson commented 7 months ago

This test should not pass:

package kata_test

import (
    "testing"
    "time"

    "github.com/stretchr/testify/require"
)

func ToSydney(t time.Time) time.Time {
    // zoneSyd, err := time.LoadLocation("Australia/Sydney")
    zoneSyd, err := time.LoadLocation("Europe/London")
    if err != nil {
        panic("Australia is gone")
    }
    return t.In(zoneSyd)
}

func TestToSydney(t *testing.T) {
    zoneSyd, err := time.LoadLocation("Australia/Sydney")
    require.NoError(t, err)
    now := time.Now()
    require.Equal(t, now.In(zoneSyd), ToSydney(now))
}

And on v1.8.4 it correctly fails, but on master it incorrectly passes.

This is a regression introduced by #1464. time.Time.Equal does not mean the two instances are equal, it means they represent the same instant. There is more information contained in a time.Time instance than the instant, this regression makes it impossible to test for state which is a best practice. If the user only cares about the instant represented then WithinDuration(t, expected, actual, 0) is available.