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.

Sign up for GitHub

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

Jump to bottom

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

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

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

simon3z opened this issue Mar 5, 2015 · 8 comments
Labels
FrozenDueToAge

Comments

@simon3z
Copy link

simon3z 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
Copy link
Member

minux 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 as completed Mar 5, 2015
@simon3z
Copy link
Author

simon3z commented Mar 5, 2015

@minux then this looks like a DeepEqual bug

@minux
Copy link
Member

minux commented Mar 5, 2015 via email

@simon3z
Copy link
Author

simon3z commented Mar 5, 2015

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

@cespare
Copy link
Contributor

cespare 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
Copy link
Author

simon3z 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
Copy link
Contributor

cespare 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
Copy link

@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.

@cespare cespare mentioned this issue Aug 13, 2015
Closed
@golang golang locked and limited conversation to collaborators Jun 25, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@simon3z @minux @cespare @rjeczalik @gopherbot