Skip to content

Commit

Permalink
Improve test coverage for Unmarshal
Browse files Browse the repository at this point in the history
Also fix bugs in fuzzy JSON unmarshaling where the JSON literals null,
false, and true, were being treated as string literals, not JSON
literals.
  • Loading branch information
vcschapp committed Aug 22, 2021
1 parent cd3751c commit 588f893
Show file tree
Hide file tree
Showing 4 changed files with 423 additions and 75 deletions.
9 changes: 4 additions & 5 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,10 @@ Incite! - CloudWatch Insights queries made (very) easy

TODO list in order:

1. Finish other TODO test cases in unmarshal_test.go.
2. Remove hint support as it overcomplicates to little benefit.
3. Finish any other lingering TODO or FIXME.
4. Audit the logging code to ensure we log enough worthwhile status info.
5. Write README.
1. Remove hint support as it overcomplicates to little benefit.
2. Audit the logging code to ensure we log enough worthwhile status info.
3. To behave like encoding/json, make Unmarshal keep trying after encountering error.
4. Write README.



Expand Down
12 changes: 6 additions & 6 deletions readall_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ func TestReadAll(t *testing.T) {

m.AssertExpectations(t)
assert.NoError(t, err)
assert.Equal(t, []Result{r("@ptr", "0")}, rs)
assert.Equal(t, []Result{{{"@ptr", "0"}}}, rs)
})

t.Run("NothingHappened", func(t *testing.T) {
Expand Down Expand Up @@ -99,7 +99,7 @@ func TestReadAll(t *testing.T) {
assert.NoError(t, err)
assert.NotEmpty(t, rs)
for i := range rs {
assert.Equal(t, r("@ptr", strconv.Itoa(i)), rs[i], "mismatch at index %d", i)
assert.Equal(t, Result{{"@ptr", strconv.Itoa(i)}}, rs[i], "mismatch at index %d", i)
}
})

Expand All @@ -117,7 +117,7 @@ func TestReadAll(t *testing.T) {
assert.NoError(t, err)
assert.NotEmpty(t, rs)
for i := range rs {
assert.Equal(t, r("@ptr", strconv.Itoa(i)), rs[i], "mismatch at index %d", i)
assert.Equal(t, Result{{"@ptr", strconv.Itoa(i)}}, rs[i], "mismatch at index %d", i)
}
})

Expand All @@ -140,9 +140,9 @@ func TestReadAll(t *testing.T) {
assert.NotEmpty(t, rs)
assert.Len(t, rs, len(expectedData1)+1)
for i := range expectedData1 {
assert.Equal(t, r("@ptr", strconv.Itoa(i)), rs[i], "mismatch at index %d", i)
assert.Equal(t, Result{{"@ptr", strconv.Itoa(i)}}, rs[i], "mismatch at index %d", i)
}
assert.Equal(t, r("@ptr", "0"), rs[len(expectedData1)], "mismatch at index %d", len(expectedData1))
assert.Equal(t, Result{{"@ptr", "0"}}, rs[len(expectedData1)], "mismatch at index %d", len(expectedData1))
})
})
}
Expand Down Expand Up @@ -184,7 +184,7 @@ func fillResultSlice(expected *[]Result, lenFunc lenFunc) func(args mock.Argumen
buffer := args[0].([]Result)
*expected = make([]Result, lenFunc(len(buffer)))
for i := range *expected {
ri := r("@ptr", strconv.Itoa(i))
ri := Result{{"@ptr", strconv.Itoa(i)}}
buffer[i] = ri
(*expected)[i] = ri
}
Expand Down
26 changes: 14 additions & 12 deletions unmarshal.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,8 @@ import (
// ResultField field named in the tag. Unmarshaling of the field value
// is done according to additional rules discussed below. If the tag is
// "-" the field is ignored. If the field type does not ultimately
// target a struct field unmarshalable type, an InvalidUnmarshalError is
// returned.
// target a struct field unmarshallable type, an InvalidUnmarshalError
// is returned.
//
// • A struct field with a "json" tag receives the the value of the
// ResultField field named in the tag using the json.Unmarshal function
Expand All @@ -62,9 +62,9 @@ import (
// • A struct field with no "incite" or "json" tag receives the value
// of the ResultField field sharing the same case-sensitive name as the
// struct field, but only if the field type ultimately targets a
// struct field unmarshablable type. Otherwise the field is ignored.
// struct field unmarshallable type. Otherwise the field is ignored.
//
// The following types are considered struct field unmarshalable types:
// The following types are considered struct field unmarshallable types:
//
// bool
// int, int8, int16, int32, int64
Expand All @@ -74,7 +74,7 @@ import (
// Any map, struct, slice, or array type
//
// A struct field targeting interface{} or any map, struct, slice, or
// array type is assumed to contain valid JSON and unmarshaled using
// array type is assumed to contain valid JSON and unmarshalled using
// json.Unmarshal. Any other field is decoded from its string
// representation using the intuitive approach. As a special case, if
// a CloudWatch Logs timestamp field (@timestamp or @ingestionTime) is
Expand Down Expand Up @@ -450,7 +450,7 @@ func decodeColToInt(s *decodeState) error {
if err != nil {
return s.wrap(err)
} else if reflect.Zero(valueType).OverflowInt(n) {
return s.wrap(overflow(reflect.ValueOf(n), valueType))
return s.wrap(overflow(src, valueType))
}
s.dst.Set(reflect.ValueOf(n).Convert(valueType))
return nil
Expand All @@ -465,7 +465,7 @@ func decodeColToUint(s *decodeState) error {
if err != nil {
return s.wrap(err)
} else if reflect.Zero(valueType).OverflowUint(n) {
return s.wrap(overflow(reflect.ValueOf(n), valueType))
return s.wrap(overflow(src, valueType))
}
s.dst.Set(reflect.ValueOf(n).Convert(valueType))
return nil
Expand All @@ -478,7 +478,7 @@ func decodeColToFloat(s *decodeState) error {
if err != nil {
return s.wrap(err)
} else if reflect.Zero(valueType).OverflowFloat(n) {
return s.wrap(overflow(reflect.ValueOf(n), valueType))
return s.wrap(overflow(src, valueType))
}
s.dst.Set(reflect.ValueOf(n).Convert(valueType))
return nil
Expand Down Expand Up @@ -523,13 +523,15 @@ For:
switch c {
case '\t', '\n', '\r', ' ': // Might be JSON, keep skipping whitespace to find out.
break
case '{', '[', '"', '-', '0', '1', '2', '3', '4', '5', '6', '7', '8', '9': // Might be JSON, try to unpack it.
case '{', '[', '"', '-', '0', '1', '2', '3', '4', '5', '6', '7', '8', '9', 'n', 't', 'f': // Might be JSON, try to unpack it.
var i interface{}
err := json.Unmarshal([]byte(src), &i)
if err != nil {
break For
}
s.dst.Set(reflect.ValueOf(i))
if i != nil {
s.dst.Set(reflect.ValueOf(i))
}
return nil
default: // Definitely not JSON.
break For
Expand Down Expand Up @@ -612,6 +614,6 @@ func (e *UnmarshalResultFieldValueError) Error() string {
e.ResultIndex, e.FieldIndex, e.Field, e.Value, e.Cause.Error())
}

func overflow(v reflect.Value, t reflect.Type) error {
return fmt.Errorf("%v overflows %s", v, t)
func overflow(v string, t reflect.Type) error {
return fmt.Errorf("%s overflows %s", v, t)
}
Loading

0 comments on commit 588f893

Please sign in to comment.