Skip to content

Commit

Permalink
Merge pull request #13 from neilotoole/equivalence-tests
Browse files Browse the repository at this point in the history
Encoder chnaged to output time.Duration as int64 a la stdlib
  • Loading branch information
neilotoole committed Oct 4, 2021
2 parents cb33465 + 427ebc7 commit a8b485b
Show file tree
Hide file tree
Showing 6 changed files with 52 additions and 26 deletions.
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,8 @@ Again, trust these benchmarks at your peril. Create your own benchmarks for your
the `segmentio` codebase at the time of forking. Thus, the linter report may not be of great use.
In an ideal world, the `jsoncolor` functionality would be ported to a more recent (and better-linted)
version of the `segementio` codebase.
- The `segmentio` encoder (at least as of `v0.1.14`) encodes `time.Duration` as string, while `stdlib` outputs the `int64`.
This package follows `stdlib`.

## Acknowledgments

Expand Down
21 changes: 10 additions & 11 deletions benchmark_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import (
)

func BenchmarkEncode(b *testing.B) {
recs := makeBenchmarkRecords(b)
recs := makeRecords(b, 10000)

benchmarks := []struct {
name string
Expand Down Expand Up @@ -59,35 +59,34 @@ func BenchmarkEncode(b *testing.B) {
}
}

func makeBenchmarkRecords(b *testing.B) [][]interface{} {
const maxRecs = 10000
recs := make([][]interface{}, 0, maxRecs)
func makeRecords(tb testing.TB, n int) [][]interface{} {
recs := make([][]interface{}, 0, n)

// add a bunch of data from a file, just to make the recs bigger
data, err := ioutil.ReadFile("testdata/sakila_actor.json")
if err != nil {
b.Fatal(err)
tb.Fatal(err)
}

x := new(interface{})
if err = stdj.Unmarshal(data, x); err != nil {
b.Fatal(err)
f := new(interface{})
if err = stdj.Unmarshal(data, f); err != nil {
tb.Fatal(err)
}

type someStruct struct {
i int64
a string
x interface{}
f interface{} // x holds JSON loaded from file
}

for i := 0; i < maxRecs; i++ {
for i := 0; i < n; i++ {
rec := []interface{}{
int(1),
int64(2),
float32(2.71),
float64(3.14),
"hello world",
someStruct{i: 8, a: "goodbye world", x: x},
someStruct{i: 8, a: "goodbye world", f: f},
map[string]interface{}{"a": 9, "b": "ca va"},
true,
false,
Expand Down
31 changes: 20 additions & 11 deletions encode.go
Original file line number Diff line number Diff line change
Expand Up @@ -328,19 +328,28 @@ func (e encoder) doEncodeBytes(b []byte, p unsafe.Pointer) ([]byte, error) {
}

func (e encoder) encodeDuration(b []byte, p unsafe.Pointer) ([]byte, error) {
if e.clrs == nil {
b = append(b, '"')
b = appendDuration(b, *(*time.Duration)(p))
b = append(b, '"')
return b, nil
}
// NOTE: The segmentj encoder does special handling for time.Duration (converts to string).
// The stdlib encoder does not. It just outputs the int64 value.
// We choose to follow the stdlib pattern, for fuller compatibility.

b = append(b, e.clrs.Time...)
b = append(b, '"')
b = appendDuration(b, *(*time.Duration)(p))
b = append(b, '"')
b = append(b, ansiReset...)
b = e.clrs.appendInt64(b, int64(*(*time.Duration)(p)))
return b, nil

// NOTE: if we were to follow the segmentj pattern, we'd execute the code below.
//if e.clrs == nil {
// b = append(b, '"')
//
// b = appendDuration(b, *(*time.Duration)(p))
// b = append(b, '"')
// return b, nil
//}
//
//b = append(b, e.clrs.Time...)
//b = append(b, '"')
//b = appendDuration(b, *(*time.Duration)(p))
//b = append(b, '"')
//b = append(b, ansiReset...)
//return b, nil
}

func (e encoder) encodeTime(b []byte, p unsafe.Pointer) ([]byte, error) {
Expand Down
4 changes: 1 addition & 3 deletions json.go
Original file line number Diff line number Diff line change
Expand Up @@ -373,9 +373,7 @@ func (dec *Decoder) InputOffset() int64 {

// Encoder is documented at https://golang.org/pkg/encoding/json/#Encoder
type Encoder struct {
writer io.Writer
// prefix string
// indent string
writer io.Writer
buffer *bytes.Buffer
err error
flags AppendFlags
Expand Down
1 change: 1 addition & 0 deletions json_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -394,6 +394,7 @@ func TestCodec(t *testing.T) {
// values. Therefore, plugging durations into TestCodec would cause fail since
// it checks equality on the marshaled strings from the two libraries.
func TestCodecDuration(t *testing.T) {
t.Skip("Skipping because neilotoole/jsoncolor follows stdlib (encode to int64) rather than segmentj (encode to string)")
for _, v1 := range durationTestValues {
t.Run(testName(v1), func(t *testing.T) {
v2 := newValue(v1)
Expand Down
19 changes: 18 additions & 1 deletion jsoncolor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -455,7 +455,7 @@ func TestEncode_BigStruct(t *testing.T) {
// has a fast path).
//
// NOTE: Currently the encoder is broken wrt colors enabled
// for non-string map keys.
// for non-string map keys, though that is kinda JSON-illegal anyway.
func TestEncode_Map_Not_StringInterface(t *testing.T) {
buf := &bytes.Buffer{}
enc := jsoncolor.NewEncoder(buf)
Expand Down Expand Up @@ -556,3 +556,20 @@ func newSmallStruct() SmallStruct {
FString: "hello",
}
}

func TestEquivalence(t *testing.T) {
rec := makeRecords(t, 1)[0]

bufStdj := &bytes.Buffer{}
err := stdjson.NewEncoder(bufStdj).Encode(rec)
require.NoError(t, err)

bufSegmentj := &bytes.Buffer{}
err = json.NewEncoder(bufSegmentj).Encode(rec)
require.NoError(t, err)
require.NotEqual(t, bufStdj.String(), bufSegmentj.String(), "segmentj encodes time.Duration to string; stdlib does not")

bufJ := &bytes.Buffer{}
err = jsoncolor.NewEncoder(bufJ).Encode(rec)
require.Equal(t, bufStdj.String(), bufJ.String())
}

0 comments on commit a8b485b

Please sign in to comment.