diff --git a/go.mod b/go.mod index 0b417c5..74d66ae 100644 --- a/go.mod +++ b/go.mod @@ -6,7 +6,6 @@ require ( cloud.google.com/go v0.43.0 github.com/alecthomas/chroma v0.6.6 github.com/fatih/color v1.7.0 - github.com/google/go-cmp v0.3.2-0.20190829225427-b1c9c4891a65 github.com/mattn/go-colorable v0.1.2 // indirect github.com/mattn/go-isatty v0.0.9 // indirect go.opencensus.io v0.22.1 diff --git a/go.sum b/go.sum index f21f294..3ecd103 100644 --- a/go.sum +++ b/go.sum @@ -48,8 +48,6 @@ github.com/google/btree v1.0.0/go.mod h1:lNA+9X1NB3Zf8V7Ke586lFgjr2dZNuvo3lPJSGZ github.com/google/go-cmp v0.2.0/go.mod h1:oXzfMopK8JAjlY9xF4vHSVASa0yLyX7SntLO5aqRK0M= github.com/google/go-cmp v0.3.0 h1:crn/baboCvb5fXaQ0IJ1SGTsTVrWpDsCWC8EGETZijY= github.com/google/go-cmp v0.3.0/go.mod h1:8QqcDgzrUqlUb/G2PQTWiueGozuR1884gddMywk6iLU= -github.com/google/go-cmp v0.3.2-0.20190829225427-b1c9c4891a65 h1:B3yqxlLHBEoav+FDQM8ph7IIRA6AhQ70w119k3hoT2Y= -github.com/google/go-cmp v0.3.2-0.20190829225427-b1c9c4891a65/go.mod h1:8QqcDgzrUqlUb/G2PQTWiueGozuR1884gddMywk6iLU= github.com/google/martian v2.1.0+incompatible h1:/CP5g8u/VJHijgedC/Legn3BAbAaWPgecwXBIDzw5no= github.com/google/martian v2.1.0+incompatible/go.mod h1:9I4somxYTbIHy5NJKHRl3wXiIaQGbYVAs8BPL6v8lEs= github.com/google/pprof v0.0.0-20181206194817-3ea8567a2e57/go.mod h1:zfwlbNMJ+OItoe0UupaVj+oy1omPYYDuagoSzA8v9mc= diff --git a/internal/assert/assert.go b/internal/assert/assert.go index 4ac73a2..3f920c8 100644 --- a/internal/assert/assert.go +++ b/internal/assert/assert.go @@ -10,17 +10,16 @@ import ( // Equal asserts exp == act. func Equal(t testing.TB, exp, act interface{}, name string) { t.Helper() - diff := CmpDiff(exp, act) - if diff != "" { - t.Fatalf("unexpected %v: %v", name, diff) + if !reflect.DeepEqual(exp, act) { + t.Fatalf("unexpected %v: exp: %q but got %q", name, exp, act) } } // NotEqual asserts exp != act. func NotEqual(t testing.TB, exp, act interface{}, name string) { t.Helper() - if CmpDiff(exp, act) == "" { - t.Fatalf("expected different %v: %+v", name, act) + if reflect.DeepEqual(exp, act) { + t.Fatalf("expected different %v: %q", name, act) } } diff --git a/internal/assert/cmp.go b/internal/assert/cmp.go deleted file mode 100644 index c3768be..0000000 --- a/internal/assert/cmp.go +++ /dev/null @@ -1,54 +0,0 @@ -package assert - -import ( - "reflect" - - "github.com/google/go-cmp/cmp" -) - -// CmpDiff compares exp to act and returns a human readable string representing their diff. -// https://github.com/google/go-cmp/issues/40#issuecomment-328615283 -func CmpDiff(exp, act interface{}) string { - return cmp.Diff(exp, act, deepAllowUnexported(exp, act)) -} - -func deepAllowUnexported(vs ...interface{}) cmp.Option { - m := make(map[reflect.Type]struct{}) - for _, v := range vs { - structTypes(reflect.ValueOf(v), m) - } - var typs []interface{} - for t := range m { - typs = append(typs, reflect.New(t).Elem().Interface()) - } - return cmp.AllowUnexported(typs...) -} - -func structTypes(v reflect.Value, m map[reflect.Type]struct{}) { - if !v.IsValid() { - return - } - switch v.Kind() { - case reflect.Ptr: - if !v.IsNil() { - structTypes(v.Elem(), m) - } - case reflect.Interface: - if !v.IsNil() { - structTypes(v.Elem(), m) - } - case reflect.Slice, reflect.Array: - for i := 0; i < v.Len(); i++ { - structTypes(v.Index(i), m) - } - case reflect.Map: - for _, k := range v.MapKeys() { - structTypes(v.MapIndex(k), m) - } - case reflect.Struct: - m[v.Type()] = struct{}{} - for i := 0; i < v.NumField(); i++ { - structTypes(v.Field(i), m) - } - } -} diff --git a/internal/entryhuman/entry_test.go b/internal/entryhuman/entry_test.go index 0246ddb..b476870 100644 --- a/internal/entryhuman/entry_test.go +++ b/internal/entryhuman/entry_test.go @@ -47,6 +47,18 @@ func TestEntry(t *testing.T) { line2`) }) + t.Run("multilineField", func(t *testing.T) { + t.Parallel() + + test(t, slog.SinkEntry{ + Message: "msg", + Level: slog.LevelInfo, + Fields: slog.M(slog.F("field", "line1\nline2")), + }, `0001-01-01 00:00:00.000 [INFO] <.:0> msg ... +"field": line1 +line2`) + }) + t.Run("named", func(t *testing.T) { t.Parallel() diff --git a/map.go b/map.go index 6627f40..c810d60 100644 --- a/map.go +++ b/map.go @@ -27,17 +27,23 @@ var _ json.Marshaler = Map(nil) // // Every field value is encoded with the following process: // -// 1. slog.Value is handled to allow any type to replace its representation for logging. +// 1. slog.Value is checked to allow any type to replace its representation for logging. +// +// 2. json.Marshaller is handled. // // 2. xerrors.Formatter is handled. // -// 3. Protobufs are handled with json.Marshal. +// 3. Protobufs are encoded with json.Marshal. +// +// 4. error and fmt.Stringer are used if possible. // -// 4. error and fmt.Stringer are handled. +// 5. slices and arrays go through the encode function for every element. // -// 5. slices and arrays are handled to go through the encode function for every value. +// 6. If the value is a struct without exported fields or a type that +// cannot be encoded with json.Marshal (like channels) then +// fmt.Sprintf("%+v") is used. // -// 6. json.Marshal is invoked as the default case. +// 8. json.Marshal(v) is used for all other values. func (m Map) MarshalJSON() ([]byte, error) { b := &bytes.Buffer{} b.WriteByte('{') @@ -56,8 +62,11 @@ func (m Map) MarshalJSON() ([]byte, error) { return b.Bytes(), nil } -// ForceJSON ensures the value is logged via json.Marshal even -// if it implements fmt.Stringer or error. +// ForceJSON ensures the value is logged via json.Marshal. +// +// Use it when implementing SlogValue to ensure a structured +// representation of a struct if you know it's capable even +// when it implements fmt.Stringer or error. func ForceJSON(v interface{}) interface{} { return jsonVal{v: v} } @@ -66,13 +75,6 @@ type jsonVal struct { v interface{} } -var _ json.Marshaler = jsonVal{} - -// MarshalJSON implements json.Marshaler. -func (v jsonVal) MarshalJSON() ([]byte, error) { - return json.Marshal(v.v) -} - func marshalList(rv reflect.Value) []byte { b := &bytes.Buffer{} b.WriteByte('[') @@ -93,6 +95,10 @@ func encode(v interface{}) []byte { switch v := v.(type) { case Value: return encode(v.SlogValue()) + case json.Marshaler: + return encodeJSON(v) + case jsonVal: + return encodeJSON(v.v) case xerrors.Formatter: return encode(errorChain(v)) case interface { @@ -103,28 +109,47 @@ func encode(v interface{}) []byte { return encode(fmt.Sprint(v)) default: rv := reflect.Indirect(reflect.ValueOf(v)) - if rv.IsValid() { - switch rv.Type().Kind() { - case reflect.Slice: - if rv.IsNil() { - break - } - fallthrough - case reflect.Array: + if !rv.IsValid() { + return encodeJSON(v) + } + + switch rv.Type().Kind() { + case reflect.Slice: + if !rv.IsNil() { return marshalList(rv) } - } + case reflect.Array: + return marshalList(rv) + case reflect.Struct: + typ := rv.Type() + for i := 0; i < rv.NumField(); i++ { + // Found an exported field. + if typ.Field(i).PkgPath == "" { + return encodeJSON(v) + } + } - b, err := json.Marshal(v) - if err != nil { - return encode(M( - Error(xerrors.Errorf("failed to marshal to JSON: %w", err)), - F("type", reflect.TypeOf(v)), - F("value", fmt.Sprintf("%+v", v)), - )) + return encodeJSON(fmt.Sprintf("%+v", v)) + case reflect.Chan, reflect.Complex64, reflect.Complex128, reflect.Func: + // These types cannot be directly encoded with json.Marshal. + // See https://golang.org/pkg/encoding/json/#Marshal + return encodeJSON(fmt.Sprintf("%+v", v)) } - return b + + return encodeJSON(v) + } +} + +func encodeJSON(v interface{}) []byte { + b, err := json.Marshal(v) + if err != nil { + return encode(M( + Error(xerrors.Errorf("failed to marshal to JSON: %w", err)), + F("type", reflect.TypeOf(v)), + F("value", fmt.Sprintf("%+v", v)), + )) } + return b } func errorChain(f xerrors.Formatter) []interface{} { diff --git a/map_test.go b/map_test.go index 57f44f0..b02793e 100644 --- a/map_test.go +++ b/map_test.go @@ -3,11 +3,11 @@ package slog_test import ( "bytes" "encoding/json" - "fmt" "io" "runtime" "strings" "testing" + "time" "golang.org/x/xerrors" @@ -86,19 +86,19 @@ func TestMap(t *testing.T) { mapTestFile := strings.Replace(mapTestFile, "_test", "", 1) test(t, slog.M( - slog.F("meow", indentJSON), + slog.F("meow", slog.ForceJSON(complex(10, 10))), ), `{ "meow": { "error": [ { "msg": "failed to marshal to JSON", - "fun": "cdr.dev/slog.encode", - "loc": "`+mapTestFile+`:121" + "fun": "cdr.dev/slog.encodeJSON", + "loc": "`+mapTestFile+`:147" }, - "json: unsupported type: func(*testing.T, string) string" + "json: unsupported type: complex128" ], - "type": "func(*testing.T, string) string", - "value": "`+fmt.Sprint(interface{}(indentJSON))+`" + "type": "complex128", + "value": "(10+10i)" } }`) }) @@ -145,6 +145,24 @@ func TestMap(t *testing.T) { }`) }) + t.Run("array", func(t *testing.T) { + t.Parallel() + + test(t, slog.M( + slog.F("meow", [3]string{ + "1", + "2", + "3", + }), + ), `{ + "meow": [ + "1", + "2", + "3" + ] + }`) + }) + t.Run("forceJSON", func(t *testing.T) { t.Parallel() @@ -174,6 +192,54 @@ func TestMap(t *testing.T) { "slice": null }`) }) + + t.Run("nil", func(t *testing.T) { + t.Parallel() + + test(t, slog.M( + slog.F("val", nil), + ), `{ + "val": null + }`) + }) + + t.Run("json.Marshaler", func(t *testing.T) { + t.Parallel() + + test(t, slog.M( + slog.F("val", time.Date(2000, 02, 05, 4, 4, 4, 0, time.UTC)), + ), `{ + "val": "2000-02-05T04:04:04Z" + }`) + }) + + t.Run("complex", func(t *testing.T) { + t.Parallel() + + test(t, slog.M( + slog.F("val", complex(10, 10)), + ), `{ + "val": "(10+10i)" + }`) + }) + + t.Run("privateStruct", func(t *testing.T) { + t.Parallel() + + test(t, slog.M( + slog.F("val", struct { + meow string + bar int + far uint + }{ + meow: "hi", + bar: 23, + far: 600, + }), + ), `{ + "val": "{meow:hi bar:23 far:600}" + }`) + }) } type meow struct { diff --git a/sloggers/slogtest/assert/assert.go b/sloggers/slogtest/assert/assert.go index 7d95e59..b1ac367 100644 --- a/sloggers/slogtest/assert/assert.go +++ b/sloggers/slogtest/assert/assert.go @@ -1,11 +1,16 @@ // Package assert is a helper package for test assertions. +// +// On failure, every assertion will fatal the test. +// +// The name parameter is available in each assertion for easier debugging. package assert // import "cdr.dev/slog/sloggers/slogtest/assert" import ( + "errors" + "reflect" "testing" "cdr.dev/slog" - "cdr.dev/slog/internal/assert" "cdr.dev/slog/sloggers/slogtest" ) @@ -13,19 +18,24 @@ import ( // // If they are not equal, it will fatal the test with a diff of the // two objects. +// +// If act or exp is an error it will be unwrapped. func Equal(t testing.TB, exp, act interface{}, name string) { slog.Helper() - if diff := assert.CmpDiff(exp, act); diff != "" { + + exp = unwrapErr(exp) + act = unwrapErr(act) + + if !reflect.DeepEqual(exp, act) { slogtest.Fatal(t, "unexpected value", slog.F("name", name), - slog.F("diff", diff), + slog.F("exp", exp), + slog.F("act", act), ) } } // Success asserts err == nil. -// -// If err isn't nil, it will fatal the test with the error. func Success(t testing.TB, err error, name string) { slog.Helper() if err != nil { @@ -37,9 +47,30 @@ func Success(t testing.TB, err error, name string) { } // True asserts act == true. -// -// If act isn't true, it will fatal the test. func True(t testing.TB, act bool, name string) { slog.Helper() Equal(t, true, act, name) } + +// Error asserts err != nil. +func Error(t testing.TB, err error, name string) { + slog.Helper() + if err == nil { + slogtest.Fatal(t, "expected error", + slog.F("name", name), + ) + } +} + +func unwrapErr(v interface{}) interface{} { + err, ok := v.(error) + if !ok { + return v + } + uerr := errors.Unwrap(err) + for uerr != nil { + err = uerr + uerr = errors.Unwrap(uerr) + } + return err +} diff --git a/sloggers/slogtest/assert/assert_test.go b/sloggers/slogtest/assert/assert_test.go index 8b8255d..6008294 100644 --- a/sloggers/slogtest/assert/assert_test.go +++ b/sloggers/slogtest/assert/assert_test.go @@ -1,6 +1,7 @@ package assert_test import ( + "fmt" "io" "testing" @@ -21,6 +22,19 @@ func TestEqual(t *testing.T) { assert.Equal(tb, 3, 4, "meow") } +func TestEqual_error(t *testing.T) { + t.Parallel() + + tb := &fakeTB{} + assert.Equal(tb, io.EOF, fmt.Errorf("failed: %w", io.EOF), "meow") + + defer func() { + recover() + simpleassert.Equal(t, 1, tb.fatals, "fatals") + }() + assert.Equal(tb, io.ErrClosedPipe, fmt.Errorf("failed: %w", io.EOF), "meow") +} + func TestSuccess(t *testing.T) { t.Parallel() @@ -47,6 +61,19 @@ func TestTrue(t *testing.T) { assert.True(tb, false, "meow") } +func TestError(t *testing.T) { + t.Parallel() + + tb := &fakeTB{} + assert.Error(tb, io.EOF, "meow") + + defer func() { + recover() + simpleassert.Equal(t, 1, tb.fatals, "fatals") + }() + assert.Error(tb, nil, "meow") +} + type fakeTB struct { testing.TB @@ -64,5 +91,5 @@ func (tb *fakeTB) Error(v ...interface{}) { func (tb *fakeTB) Fatal(v ...interface{}) { tb.fatals++ - panic("") + panic(fmt.Sprint(v...)) }