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

time: Time Zero MarshalJSON / UnmarshalJSON produces a different object #10089

Closed
simon3z opened this issue Mar 5, 2015 · 8 comments

Comments

Projects
None yet
5 participants
@simon3z
Copy link

commented Mar 5, 2015

It seems that marshaling and unmarshaling a zero time produces an object that is different from the original:

func TestMarshalUnmarshalJSON(t *testing.T) {
        var expected, parsed Time

        body, err := expected.MarshalJSON()
        if err != nil {
                t.Errorf("json.Marshal error = %v", err)
        }

        err = parsed.UnmarshalJSON(body)
        if err != nil {
                t.Errorf("json.Unmarshal error = %v", err)
        }

        if !reflect.DeepEqual(expected, parsed) {
                t.Errorf("json error, expected = %#v, parsed = %#v", expected, parsed)
        }
}

--- FAIL: TestMarshalUnmarshalJSON (0.00 seconds)
        time_test.go:807: json error, expected = time.Time{sec:0, nsec:0x0, loc:(*time.Location)(nil)}, parsed = time.Time{sec:0, nsec:0x0, loc:(*time.Location)(0x6cd2a0)}
@minux

This comment has been minimized.

Copy link
Member

commented Mar 5, 2015

Parsing a time will get a non-zero location. This is working as intended. You shouldn't use == to compare time.Time's.

@minux minux closed this Mar 5, 2015

@simon3z

This comment has been minimized.

Copy link
Author

commented Mar 5, 2015

@minux then this looks like a DeepEqual bug

@minux

This comment has been minimized.

Copy link
Member

commented Mar 5, 2015

@simon3z

This comment has been minimized.

Copy link
Author

commented Mar 5, 2015

@minux exactly, but as you said, it shouldn't if that's not the way to compare Time

@cespare

This comment has been minimized.

Copy link
Contributor

commented Mar 5, 2015

@simon3z wrong conclusion.

  • DeepEqual uses ==
  • time.Times should not be compared with Equal, not ==
  • These both cannot change because of the Go 1 compatibility promise
  • Therefore DeepEqual should not be used if the structures you're comparing contain time.Times

If you need a DeepEqual that knows about time.Times, you will need to write it.

@simon3z

This comment has been minimized.

Copy link
Author

commented Mar 5, 2015

@cespare probably a better design would be something like:

type compare func(a1, a2 interface{}) int

DeepCompare(a1, a2 interface{}, fn compare)

do you think it could be something I could write/include in the reflect package?

@cespare

This comment has been minimized.

Copy link
Contributor

commented Mar 5, 2015

@simon3z Better deep comparison is interesting, but let's take this to the golang-nuts mailing list.

@mikioh mikioh changed the title Time Zero MarshalJSON / UnmarshalJSON produces a different object time: Time Zero MarshalJSON / UnmarshalJSON produces a different object Mar 5, 2015

@rjeczalik

This comment has been minimized.

Copy link

commented Mar 6, 2015

@simon3z If you really care about comparing times directly (part of larger struct), you can get away by keeping times in the same timezone (like http://play.golang.org/p/GhvxWMgk-x), but it just workarounds the fact they should be compared with http://golang.org/pkg/time/#Time.Equal.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.