-
Notifications
You must be signed in to change notification settings - Fork 1.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
zaptest: Add testing.TB compatible logger #518
Changes from 8 commits
4f818a2
be2b5ea
a73cb3f
e78537e
9c4600e
316a110
ca6ac7d
660acc6
4de70e3
959d416
063f7a1
8dd84b6
3ebc5fc
cdd3f0b
2b3338c
d7b6a16
d0af2be
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,70 @@ | ||
// Copyright (c) 2017 Uber Technologies, Inc. | ||
// | ||
// Permission is hereby granted, free of charge, to any person obtaining a copy | ||
// of this software and associated documentation files (the "Software"), to deal | ||
// in the Software without restriction, including without limitation the rights | ||
// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell | ||
// copies of the Software, and to permit persons to whom the Software is | ||
// furnished to do so, subject to the following conditions: | ||
// | ||
// The above copyright notice and this permission notice shall be included in | ||
// all copies or substantial portions of the Software. | ||
// | ||
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR | ||
// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, | ||
// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE | ||
// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER | ||
// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, | ||
// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN | ||
// THE SOFTWARE. | ||
|
||
package zaptest | ||
|
||
import ( | ||
"go.uber.org/zap" | ||
"go.uber.org/zap/zapcore" | ||
) | ||
|
||
// NewLogger builds a new Logger that logs all messages to the given | ||
// testing.TB. | ||
// | ||
// Use this with a *testing.T or *testing.B to get logs which get printed only | ||
// if a test fails or if you ran go test -v. | ||
func NewLogger(t TestingT) *zap.Logger { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. let's add support for options, so in future, we can add options for whether to log only on failure, or on failure + in our repos, we'd like a logger that only logs on failure, and is quiet otherwise, even with |
||
return NewLoggerAt(t, zapcore.DebugLevel) | ||
} | ||
|
||
// NewLoggerAt builds a new Logger that logs messages to the given testing.TB | ||
// if the given LevelEnabler allows it. | ||
// | ||
// Use this with a *testing.T or *testing.B to get logs which get printed only | ||
// if a test fails or if you ran go test -v. | ||
func NewLoggerAt(t TestingT, enab zapcore.LevelEnabler) *zap.Logger { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can this just be an option instead to |
||
return zap.New( | ||
zapcore.NewCore( | ||
zapcore.NewConsoleEncoder(zap.NewDevelopmentEncoderConfig()), | ||
testingWriter{t}, | ||
enab, | ||
), | ||
) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Makes sense, although if you end up writing to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Excellent point. It should fail the test. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done in #566 |
||
} | ||
|
||
// testingWriter is a WriteSyncer that writes to the given testing.TB. | ||
type testingWriter struct{ t TestingT } | ||
|
||
func (w testingWriter) Write(p []byte) (n int, err error) { | ||
s := string(p) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would be more efficient to do it after the ending newline character has been stripped. Extra string allocation for each Edit: There is no need to convert to string explicitly at all. Just pass There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good call. Thanks! |
||
|
||
// Strip trailing newline because t.Log always adds one. | ||
if s[len(s)-1] == '\n' { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. optionally: you can use |
||
s = s[:len(s)-1] | ||
} | ||
|
||
// Note: t.Log is safe for concurrent use. | ||
w.t.Logf("%s", s) | ||
return len(p), nil | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This introduces a bug now - it should be returning length of the original There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh, good catch! |
||
} | ||
|
||
func (w testingWriter) Sync() error { | ||
return nil | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,99 @@ | ||
// Copyright (c) 2017 Uber Technologies, Inc. | ||
// | ||
// Permission is hereby granted, free of charge, to any person obtaining a copy | ||
// of this software and associated documentation files (the "Software"), to deal | ||
// in the Software without restriction, including without limitation the rights | ||
// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell | ||
// copies of the Software, and to permit persons to whom the Software is | ||
// furnished to do so, subject to the following conditions: | ||
// | ||
// The above copyright notice and this permission notice shall be included in | ||
// all copies or substantial portions of the Software. | ||
// | ||
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR | ||
// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, | ||
// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE | ||
// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER | ||
// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, | ||
// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN | ||
// THE SOFTWARE. | ||
|
||
package zaptest | ||
|
||
import ( | ||
"errors" | ||
"fmt" | ||
"strings" | ||
"sync" | ||
"testing" | ||
|
||
"go.uber.org/zap" | ||
|
||
"github.com/stretchr/testify/assert" | ||
) | ||
|
||
func TestTestLoggerIncludesDebug(t *testing.T) { | ||
ts := newTestLogSpy(t) | ||
log := NewLogger(ts) | ||
log.Debug("calculating") | ||
log.Info("finished calculating", zap.Int("answer", 42)) | ||
|
||
ts.AssertMessages( | ||
"DEBUG calculating", | ||
`INFO finished calculating {"answer": 42}`, | ||
) | ||
} | ||
|
||
func TestTestLoggerAt(t *testing.T) { | ||
ts := newTestLogSpy(t) | ||
log := NewLoggerAt(ts, zap.WarnLevel) | ||
|
||
log.Info("received work order") | ||
log.Debug("starting work") | ||
log.Warn("work may fail") | ||
log.Error("work failed", zap.Error(errors.New("great sadness"))) | ||
|
||
assert.Panics(t, func() { | ||
log.Panic("failed to do work") | ||
}, "log.Panic should panic") | ||
|
||
ts.AssertMessages( | ||
"WARN work may fail", | ||
`ERROR work failed {"error": "great sadness"}`, | ||
"PANIC failed to do work", | ||
) | ||
} | ||
|
||
// testLogSpy is a testing.TB that captures logged messages. | ||
type testLogSpy struct { | ||
testing.TB | ||
|
||
mu sync.Mutex | ||
Messages []string | ||
} | ||
|
||
func newTestLogSpy(t testing.TB) *testLogSpy { | ||
return &testLogSpy{TB: t} | ||
} | ||
|
||
func (t *testLogSpy) Logf(format string, args ...interface{}) { | ||
// Log messages are in the format, | ||
// | ||
// 2017-10-27T13:03:01.000-0700 DEBUG your message here {data here} | ||
// | ||
// We strip the first part of these messages because we can't really test | ||
// for the timestamp from these tests. | ||
m := fmt.Sprintf(format, args...) | ||
m = m[strings.IndexByte(m, '\t')+1:] | ||
|
||
// t.Log should be thread-safe. | ||
t.mu.Lock() | ||
t.Messages = append(t.Messages, m) | ||
t.mu.Unlock() | ||
|
||
t.TB.Log(args...) | ||
} | ||
|
||
func (t *testLogSpy) AssertMessages(msgs ...string) { | ||
assert.Equal(t, msgs, t.Messages) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,38 @@ | ||
// Copyright (c) 2017 Uber Technologies, Inc. | ||
// | ||
// Permission is hereby granted, free of charge, to any person obtaining a copy | ||
// of this software and associated documentation files (the "Software"), to deal | ||
// in the Software without restriction, including without limitation the rights | ||
// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell | ||
// copies of the Software, and to permit persons to whom the Software is | ||
// furnished to do so, subject to the following conditions: | ||
// | ||
// The above copyright notice and this permission notice shall be included in | ||
// all copies or substantial portions of the Software. | ||
// | ||
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR | ||
// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, | ||
// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE | ||
// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER | ||
// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, | ||
// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN | ||
// THE SOFTWARE. | ||
|
||
package zaptest | ||
|
||
// TestingT is a subset of the API provided by all *testing.T and *testing.B | ||
// objects. | ||
type TestingT interface { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we'll be locked into this interface when we release it, so we might want to include methods that could be useful in future. e.g., if we have an option to keep logs quiet unless the test fails, and we add a not sure if we need anything else ( We could just take a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Happy to take a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This interface is a subset of I'm happy to switch this to a different subset. FWIW, just Depending directly on There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would also like to avoid importing the The only minor downside I see is that such a wide interface with unused functions isn't exactly idiomatic. |
||
// Logs the given message without failing the test. | ||
Logf(string, ...interface{}) | ||
|
||
// Logs the given message and marks the test as failed. | ||
Errorf(string, ...interface{}) | ||
|
||
// Marks the test as failed and stops execution of that test. | ||
FailNow() | ||
} | ||
|
||
// Note: We currently only rely on Logf. We are including Errorf and FailNow | ||
// in the interface in anticipation of future need since we can't extend the | ||
// interface without a breaking change. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
// Copyright (c) 2017 Uber Technologies, Inc. | ||
// | ||
// Permission is hereby granted, free of charge, to any person obtaining a copy | ||
// of this software and associated documentation files (the "Software"), to deal | ||
// in the Software without restriction, including without limitation the rights | ||
// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell | ||
// copies of the Software, and to permit persons to whom the Software is | ||
// furnished to do so, subject to the following conditions: | ||
// | ||
// The above copyright notice and this permission notice shall be included in | ||
// all copies or substantial portions of the Software. | ||
// | ||
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR | ||
// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, | ||
// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE | ||
// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER | ||
// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, | ||
// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN | ||
// THE SOFTWARE. | ||
|
||
package zaptest | ||
|
||
import "testing" | ||
|
||
// Just a compile-time test to ensure that TestingT matches the testing.TB | ||
// interface. We could do this in testingt.go but that would put a dependency | ||
// on the "testing" package from zaptest. | ||
|
||
var _ TestingT = (testing.TB)(nil) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's worth mentioning that the default log level is
DebugLevel
.