From 5cd83174d516edb6798955c9b7ff494c0aa7dc8f Mon Sep 17 00:00:00 2001 From: Suyash Date: Mon, 6 Mar 2017 20:47:43 +0530 Subject: [PATCH 1/3] writer: add OpenWriteSyncers Allowing functions to take interfaces as parameters allows for better customization. For example, to have a writer that also counts the number of writes specifically made to its instance ``` type MyWriter struct { *os.File logcount int } func (w *MyWriter) Write(data []byte) (int, error) { w.logcount++ return w.File.Write(data) } func (w *MyWriter) Sync() error { return w.File.Sync() } ... func main() { f, err := os.Create(...) if err != nil { log.Fatal(err) } defer f.Close() w := &MyWriter{f, 0} ws, err := zap.OpenWriteSyncers(os.Stdout, zapcore.AddSync(w)) if err != nil { log.Fatal(err) } core := zapcore.NewCore( zapcore.NewConsoleEncoder(zap.NewProductionEncoderConfig()), ws, zapcore.DebugLevel) logger := zap.New(core) ... fmt.Printf("logged %v times to MyWriter", w.logcount) } ``` --- writer.go | 22 +++++++++++++++------- writer_test.go | 25 +++++++++++++++++++++++++ zapcore/write_syncer.go | 3 +++ zapcore/write_syncer_test.go | 6 ++++++ 4 files changed, 49 insertions(+), 7 deletions(-) diff --git a/writer.go b/writer.go index 67b478c50..81bb7fd88 100644 --- a/writer.go +++ b/writer.go @@ -36,15 +36,14 @@ import ( // Passing no paths returns a no-op WriteSyncer. The special paths "stdout" and // "stderr" are interpreted as os.Stdout and os.Stderr, respectively. func Open(paths ...string) (zapcore.WriteSyncer, func(), error) { - if len(paths) == 0 { - return zapcore.AddSync(ioutil.Discard), func() {}, nil - } - writers, close, err := open(paths) - if len(writers) == 1 { - return zapcore.Lock(writers[0]), close, err + + if err != nil { + return nil, nil, err } - return zapcore.Lock(zapcore.NewMultiWriteSyncer(writers...)), close, err + + writer, err := OpenWriteSyncers(writers...) + return writer, close, err } func open(paths []string) ([]zapcore.WriteSyncer, func(), error) { @@ -76,3 +75,12 @@ func open(paths []string) ([]zapcore.WriteSyncer, func(), error) { } return writers, close, errs.AsError() } + +// OpenWriteSyncers combines the passed set of WriteSyncer objects into a locked +// WriteSyncer. It returns any error encountered. +func OpenWriteSyncers(writers ...zapcore.WriteSyncer) (zapcore.WriteSyncer, error) { + if len(writers) == 0 { + return zapcore.AddSync(ioutil.Discard), nil + } + return zapcore.Lock(zapcore.NewMultiWriteSyncer(writers...)), nil +} diff --git a/writer_test.go b/writer_test.go index 0890f8eb5..335501ae5 100644 --- a/writer_test.go +++ b/writer_test.go @@ -81,3 +81,28 @@ func TestOpen(t *testing.T) { assert.Equal(t, tt.filenames, names, "Opened unexpected files given paths %v.", tt.paths) } } + +type testWriter struct { + expected []byte + t testing.TB +} + +func (w *testWriter) Write(actual []byte) (int, error) { + assert.Equal(w.t, w.expected, actual, "expected writer to write %v, wrote %v", string(w.expected), string(actual)) + return 0, nil +} + +func (w *testWriter) Sync() error { + return nil +} + +func TestOpenWriteSyncers(t *testing.T) { + tw := &testWriter{[]byte("test"), t} + + w, err := OpenWriteSyncers(tw) + if err != nil { + require.NoError(t, err, "OpenWriters Failed.") + } + + w.Write([]byte("test")) +} diff --git a/zapcore/write_syncer.go b/zapcore/write_syncer.go index d2292cb02..2a9237640 100644 --- a/zapcore/write_syncer.go +++ b/zapcore/write_syncer.go @@ -88,6 +88,9 @@ type multiWriteSyncer []WriteSyncer // NewMultiWriteSyncer creates a WriteSyncer that duplicates its writes // and sync calls, much like io.MultiWriter. func NewMultiWriteSyncer(ws ...WriteSyncer) WriteSyncer { + if len(ws) == 1 { + return ws[0] + } // Copy to protect against https://github.com/golang/go/issues/7809 return multiWriteSyncer(append([]WriteSyncer(nil), ws...)) } diff --git a/zapcore/write_syncer_test.go b/zapcore/write_syncer_test.go index 386f13013..766653c3e 100644 --- a/zapcore/write_syncer_test.go +++ b/zapcore/write_syncer_test.go @@ -65,6 +65,12 @@ func TestAddSyncWriter(t *testing.T) { assert.NoError(t, ws.Sync(), "Unexpected error calling a no-op Sync method.") } +func TestNewMultiWriteSyncerWorksForSingleWriter(t *testing.T) { + w := AddSync(&bytes.Buffer{}) + ws := NewMultiWriteSyncer(w) + assert.Equal(t, w, ws, "Expected NewMultiWriteSyncer to return the same WriteSyncer object for a single argument.") +} + func TestMultiWriteSyncerWritesBoth(t *testing.T) { first := &bytes.Buffer{} second := &bytes.Buffer{} From a592c8c328dfa125ad248df5a10560f88bea90ae Mon Sep 17 00:00:00 2001 From: Suyash Date: Wed, 8 Mar 2017 23:21:01 +0530 Subject: [PATCH 2/3] rename to CombineWriteSyncers --- writer.go | 15 +++++++-------- writer_test.go | 15 +++++---------- zapcore/write_syncer_test.go | 6 +++++- 3 files changed, 17 insertions(+), 19 deletions(-) diff --git a/writer.go b/writer.go index 81bb7fd88..121db9826 100644 --- a/writer.go +++ b/writer.go @@ -37,13 +37,12 @@ import ( // "stderr" are interpreted as os.Stdout and os.Stderr, respectively. func Open(paths ...string) (zapcore.WriteSyncer, func(), error) { writers, close, err := open(paths) - if err != nil { return nil, nil, err } - writer, err := OpenWriteSyncers(writers...) - return writer, close, err + writer := CombineWriteSyncers(writers...) + return writer, close, nil } func open(paths []string) ([]zapcore.WriteSyncer, func(), error) { @@ -76,11 +75,11 @@ func open(paths []string) ([]zapcore.WriteSyncer, func(), error) { return writers, close, errs.AsError() } -// OpenWriteSyncers combines the passed set of WriteSyncer objects into a locked -// WriteSyncer. It returns any error encountered. -func OpenWriteSyncers(writers ...zapcore.WriteSyncer) (zapcore.WriteSyncer, error) { +// CombineWriteSyncers combines the passed set of WriteSyncer objects into a +// locked WriteSyncer. +func CombineWriteSyncers(writers ...zapcore.WriteSyncer) zapcore.WriteSyncer { if len(writers) == 0 { - return zapcore.AddSync(ioutil.Discard), nil + return zapcore.AddSync(ioutil.Discard) } - return zapcore.Lock(zapcore.NewMultiWriteSyncer(writers...)), nil + return zapcore.Lock(zapcore.NewMultiWriteSyncer(writers...)) } diff --git a/writer_test.go b/writer_test.go index 335501ae5..426f3666f 100644 --- a/writer_test.go +++ b/writer_test.go @@ -83,13 +83,13 @@ func TestOpen(t *testing.T) { } type testWriter struct { - expected []byte + expected string t testing.TB } func (w *testWriter) Write(actual []byte) (int, error) { - assert.Equal(w.t, w.expected, actual, "expected writer to write %v, wrote %v", string(w.expected), string(actual)) - return 0, nil + assert.Equal(w.t, []byte(w.expected), actual, "Unexpected write error.") + return len(actual), nil } func (w *testWriter) Sync() error { @@ -97,12 +97,7 @@ func (w *testWriter) Sync() error { } func TestOpenWriteSyncers(t *testing.T) { - tw := &testWriter{[]byte("test"), t} - - w, err := OpenWriteSyncers(tw) - if err != nil { - require.NoError(t, err, "OpenWriters Failed.") - } - + tw := &testWriter{"test", t} + w := CombineWriteSyncers(tw) w.Write([]byte("test")) } diff --git a/zapcore/write_syncer_test.go b/zapcore/write_syncer_test.go index 766653c3e..43c28cef1 100644 --- a/zapcore/write_syncer_test.go +++ b/zapcore/write_syncer_test.go @@ -66,9 +66,13 @@ func TestAddSyncWriter(t *testing.T) { } func TestNewMultiWriteSyncerWorksForSingleWriter(t *testing.T) { - w := AddSync(&bytes.Buffer{}) + w := &testutils.Buffer{} + ws := NewMultiWriteSyncer(w) assert.Equal(t, w, ws, "Expected NewMultiWriteSyncer to return the same WriteSyncer object for a single argument.") + + ws.Sync() + assert.True(t, w.Called(), "Expected Sync to be called on the created WriteSyncer") } func TestMultiWriteSyncerWritesBoth(t *testing.T) { From 88322a50174f6d00d4089a4e4c86329d6cb8cdf2 Mon Sep 17 00:00:00 2001 From: Suyash Date: Wed, 8 Mar 2017 23:23:30 +0530 Subject: [PATCH 3/3] fix Test Function name --- writer_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/writer_test.go b/writer_test.go index 426f3666f..9cc3e775d 100644 --- a/writer_test.go +++ b/writer_test.go @@ -96,7 +96,7 @@ func (w *testWriter) Sync() error { return nil } -func TestOpenWriteSyncers(t *testing.T) { +func TestCombineWriteSyncers(t *testing.T) { tw := &testWriter{"test", t} w := CombineWriteSyncers(tw) w.Write([]byte("test"))