From 7ea57ce36ae5d9a4816ba3bfeb00fb68a46c2ea3 Mon Sep 17 00:00:00 2001 From: Abhinav Gupta Date: Mon, 28 Jun 2021 11:59:20 -0700 Subject: [PATCH] NopLogger: Fix nil Clock panic In #897, we added a `zap.Clock` option to control the source of time but neglected to set this field on the logger constructed by `zap.NewNop`. This has the effect of panicking the Nop logger with a nil dereference. Fix the nil dereference and add checks for the behavior of the Nop logger. Verified that these are the only instantiations of `Logger` in this package: ``` $ rg '\bLogger\{' *.go logger_test.go 67: for _, logger := range []*Logger{grandparent, parent, child} { logger.go 71: log := &Logger{ 86: return &Logger{ ``` Refs GO-684 --- logger.go | 1 + logger_test.go | 22 +++++++++++++++++++++- 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/logger.go b/logger.go index c6ab4b0ef..f116bd936 100644 --- a/logger.go +++ b/logger.go @@ -87,6 +87,7 @@ func NewNop() *Logger { core: zapcore.NewNopCore(), errorOutput: zapcore.AddSync(ioutil.Discard), addStack: zapcore.FatalLevel + 1, + clock: zapcore.DefaultClock, } } diff --git a/logger_test.go b/logger_test.go index 0fb0600e7..edc7d3dec 100644 --- a/logger_test.go +++ b/logger_test.go @@ -556,7 +556,6 @@ func TestLoggerCustomOnFatal(t *testing.T) { for _, tt := range tests { t.Run(tt.msg, func(t *testing.T) { withLogger(t, InfoLevel, opts(OnFatal(tt.onFatal)), func(logger *Logger, logs *observer.ObservedLogs) { - var finished bool recovered := make(chan interface{}) go func() { @@ -580,6 +579,27 @@ func TestLoggerCustomOnFatal(t *testing.T) { } } +func TestNopLogger(t *testing.T) { + logger := NewNop() + + t.Run("basic levels", func(t *testing.T) { + logger.Debug("foo", String("k", "v")) + logger.Info("bar", Int("x", 42)) + logger.Warn("baz", Strings("ks", []string{"a", "b"})) + logger.Error("qux", Error(errors.New("great sadness"))) + }) + + t.Run("DPanic", func(t *testing.T) { + logger.With(String("component", "whatever")).DPanic("stuff") + }) + + t.Run("Panic", func(t *testing.T) { + assert.Panics(t, func() { + logger.Panic("great sadness") + }, "Nop logger should still cause panics.") + }) +} + func infoLog(logger *Logger, msg string, fields ...Field) { logger.Info(msg, fields...) }