[WIP] Browser Conversions Unit Tests #649
Conversation
| @@ -173,7 +173,12 @@ func (o *OrderEvent) UnmarshalJSON(data []byte) error { | |||
| } | |||
|
|
|||
| func (o *OrderEvent) fromOrderEventJSON(orderEventJSON orderEventJSON) error { | |||
| o.Timestamp = orderEventJSON.Timestamp | |||
| layout := "2006-01-02 15:04:05 -0700 MST" | |||
There was a problem hiding this comment.
I think you can just use time.RFC3339. That's the format used in JSON and supported by both Go and JavaScript.
There was a problem hiding this comment.
Okay, good to know. I'll try that
| @@ -142,7 +142,7 @@ type OrderEvent struct { | |||
| } | |||
|
|
|||
| type orderEventJSON struct { | |||
| Timestamp time.Time `json:"timestamp"` | |||
| Timestamp string `json:"timestamp"` | |||
There was a problem hiding this comment.
In any Go type this should probably still be time.Time. Was there a reason to make this change?
There was a problem hiding this comment.
Yep, I made this change so that we could unmarshal the json from stringifying an OrderEvent. We can't convert OrderEvent to a JSValue without changing this to a string, so I also thought that it would make sense for this to be consistent.
There was a problem hiding this comment.
This type is supposed to help with marshaling to and from JSON, in which case time.Time works perfectly fine. It's only converting to and from JSValue that causes problems. If we're using this type for JSValue conversions, we should rename it.
| eventObject := event.JSValue() | ||
| eventString := stringify(eventObject) | ||
| recoveredEvent := &OrderEvent{} | ||
| err := recoveredEvent.UnmarshalJSON([]byte(eventString)) |
There was a problem hiding this comment.
I actually think these tests are pretty close to what we want to do. If possible I want to avoid using UnmarshalJSON here since it is a confounding factor that could cover up some bugs. For example, in the UnmarshalJSON method we might do some conversions or handle empty/null values differently than in the actual flow from JavaScript to Go.
| ) | ||
|
|
||
| func TestConvertConfigEmpty(t *testing.T) { | ||
| data, err := json.Marshal(struct { |
There was a problem hiding this comment.
This type of JSON marshaling makes me a lot more comfortable. I'm not sure it's 100% needed, but at least it's a "dumb" conversion and doesn't go through any of our other methods that might do additional conversion.
|
@jalextowle is this superseded by #694? Maybe we should close it? |
No description provided.