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: Equal does not consider Zone information in time.Time #1536

Closed
brackendawson opened this issue Feb 18, 2024 · 0 comments · Fixed by #1537
Closed

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

brackendawson opened this issue Feb 18, 2024 · 0 comments · Fixed by #1537
Labels
bug pkg-assert Change related to package testify/assert

Comments

@brackendawson
Copy link
Collaborator

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug pkg-assert Change related to package testify/assert
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants