Add a test for Development option #46
Conversation
|
Rather than this, I'd rather take the There is another option, which is kind of hacky, but could work, which is to:
I think the first option is probably cleaner though |
|
Sure, works for me. |
| prod := NewJSON().(*jsonLogger) | ||
| require.False(t, prod.development, "Expected default logger to be in production mode.") | ||
|
|
||
| dev := NewJSON(Development()).(*jsonLogger) |
There was a problem hiding this comment.
where does Development() come from? testify?
There was a problem hiding this comment.
It's an option you can pass when creating a logger:
https://godoc.org/github.com/uber-common/zap#Development
We're using it to support the idea of DFatal which is an error log in production mode, but a fatal in test mode.
| @@ -35,6 +36,20 @@ func opts(opts ...Option) []Option { | |||
| return opts | |||
| } | |||
|
|
|||
| type stubbedExit struct { | |||
| Status int | |||
There was a problem hiding this comment.
to distinguish vs being called with Exit(0) vs not being called, can we either use a separate bool to track whether it was called, or make Status a pointer.
There was a problem hiding this comment.
Good point, I completely overlooked that.
| withJSONLogger(t, nil, func(jl *jsonLogger, output func() []string) { | ||
| jl.Log(Fatal, "foo") | ||
| assertMessage(t, "fatal", "foo", output()[0]) | ||
| assert.Equal(t, 1, *stub.Status, "Expected to call os.Exit with status 1.") |
There was a problem hiding this comment.
optional nit: if exit isn't called, rather than failing with a nice error, it'll cause a panic. we could do
if assert.NotNil(t, stub.Status, "Expected a call to os.Exit") {
assert.Equal(t, 1, *stub.Status, "os.Exit got unexpected exit code")
}(i also try to not mention the specific value we expect in the message, since Equal will print the value it expected vs what it got already, and if we change it, it means less places to change / accidentally miss)
|
lgtm with minor nit |
Fixes #55.