Failed comparison of identical time variables

[wolfgangmeyers]

When I try comparing two time variables in a test:

	It("Should show identical times as equal", func() {
		now := time.Now()
		data, _ := now.MarshalJSON()
		copy := time.Time{}
		copy.UnmarshalJSON(data)
		Expect(now).To(Equal(copy))
	})

It fails with output like this:

  Expected
      <time.Time>: 2018-02-05T14:03:52.391613-08:00
  to equal
      <time.Time>: 2018-02-05T14:03:52.391613-08:00

I think that the Equal matcher should identify the two time variables as being equal.

[nodo]

Thanks @wolfgangmeyers !

I was able to reproduce the problem. I have also tried to add some Printf:

fmt.Printf("%v\n", now)                                                                                                                             
fmt.Printf("%v\n", copy)

And this is the result:

2018-02-07 09:20:36.429395819 +0000 GMT
2018-02-07 09:20:36.429395819 +0000 UTC

It seems that time.Location for the two times is indeed different. There is also an issue open in golang which seems relevant (golang/go#10089).

We could potentially catch that in Equal and instead of calling reflect.DeepEqual use https://golang.org/pkg/time/#Time.Equal .

[williammartin]

Cool investigation @nodo. I sort of wonder whether there's use in having a separate set of time related matchers. I can definitely see how it would be nice to have BeAfter, BeBefore etc etc. Just food for thought.

[onsi]

Yes this bites folks a lot. The issue, as you've observed @nodo, is a mismatch in timezones. This could be solved with a time matcher (I believe the little known BeTemporally has the right behavior?). Updating Equal to do type-checking and use Time.Equal would help as well.....

...but the real pain point is when you have mismatched time zones for time objects buried inside a struct. A recent example of this bit a team at Pivotal where a struct contained a field of type Time and the deserialized JSON actual did not match the hand-constructed expected. Without rewriting reflect.DeepEqual to type-check for Time and use Time.Equal I'm not sure we can solve for this case which is, often, even more confusing that the example in this issue.

Any thoughts on what we can do here?

[wolfgangmeyers]

In the example code I pasted, I believe the timezones are pacific for both the original and copy.

[onsi]

@wolfgangmeyers you might need to try fmt.Printf("%v vs %v\n", now, copy)

[nodo]

We could do something similar to what I proposed here #161 (comment) and just change the message to make it less confusing.

For instance here: https://github.com/onsi/gomega/blob/master/format/format.go#L197 we could catch the case when the value is a time and print the location as well. The code would be something like:

default:
  t, ok := object.(time.Time)
  if !ok {
    return fmt.Sprintf("%T", object)
  }
  return fmt.Sprintf("%T | Location: %s", t, t.Location())

and the message would be something like:

  Expected
      <time.Time | Location: Local>: 2018-02-11T20:52:22.519981Z
  to equal
      <time.Time | Location: UTC>: 2018-02-11T20:52:22.519981Z

It's a bit of a hack, but I couldn't think of any other alternative. WDYT?

Unfortunately, this approach does not completely solves the case of complex struct with times nested inside. That requires a more complex workaround.

[wolfgangmeyers]

So I have an update. I got an error again when checking two time variables that should have been identical. When serialized with json and deserialized, at first they appeared to be identical (from the error output), but when printing them out with log.Printf I noticed that the first value (from time.Now()) has an additional "monotonic" component. The timezones were identical in the output, so I don't think they are causing the problem:

2018-03-07 10:29:36.418397 -0800 PST m=+0.533406547
2018-03-07 10:29:36.418397 -0800 PST

I think the monotonic portion - which according to the go docs, is only used for debugging - is causing the difference. Could the matcher be updated to ignore the monotonic component?

The docs describing this can be found here: https://github.com/golang/go/blob/master/src/time/format.go#L431

[nodo]

Sorry for the delay @wolfgangmeyers. Would BeTemporally (https://onsi.github.io/gomega/#betemporallycomparator-string-compareto-timetime-threshold-timeduration) work for you?

It seems that handling this in Equal requires either to write our own version ofDeepEqual or provide a better error message.

Any opinions on that?

[wolfgangmeyers]

I think that works for me. I know I would have seen this if I had read completely through the docs, but it would be nice if it were more obvious. Thanks for pointing it out! 😄

[korya]

Still an issue in v1.5.0 and go1.12.6.

[A87P]

FTW

fmt.Println(t1)
fmt.Println(t2)
fmt.Println(t1.Sub(t2) == 0)

2020-12-03 01:12:31.1229002 -0500 EST m=+0.000460501
2020-12-03 01:12:31.1229002 -0500 EST
true

[sedyh]

I manage to reproduce that.
https://go.dev/play/p/JEywXBnRRb8

Its probably because gomega doesn't call time.Equal().
As I can see there is BeTemporally matcher, but you can't compare slices this way.
The strangest thing that I can call Local() or UTC() on each of times and gomega equal will work after that.
I still think this is a bug.

[YuviGold]

I manage to reproduce that. https://go.dev/play/p/JEywXBnRRb8

Its probably because gomega doesn't call time.Equal(). As I can see there is BeTemporally matcher, but you can't compare slices this way. The strangest thing that I can call Local() or UTC() on each of times and gomega equal will work after that. I still think this is a bug.

I agree this issue should be reopened.
I just encountered #216 (comment) suggestion using https://github.com/google/go-cmp . Your example works with it https://go.dev/play/p/kFKCsf7dccR