From 2be620216affd84620a08ee082d6e074d1bb5ca4 Mon Sep 17 00:00:00 2001 From: Albert Salim Date: Sat, 6 Oct 2018 18:08:19 +0800 Subject: [PATCH 1/4] Add option to panic in `test.NewNullLogger` to allow testing of calls to `Fatal*` See #813 --- alt_exit.go | 2 ++ entry.go | 6 +++--- hooks/test/test.go | 14 +++++++++++++- hooks/test/test_test.go | 11 +++++++++++ logger.go | 9 ++++++--- 5 files changed, 35 insertions(+), 7 deletions(-) diff --git a/alt_exit.go b/alt_exit.go index 8af9063..f1bb44c 100644 --- a/alt_exit.go +++ b/alt_exit.go @@ -45,6 +45,8 @@ func runHandlers() { } } +type exitFunc func(int) + // Exit runs all the Logrus atexit handlers and then terminates the program using os.Exit(code) func Exit(code int) { runHandlers() diff --git a/entry.go b/entry.go index ca634a6..225bdb6 100644 --- a/entry.go +++ b/entry.go @@ -198,7 +198,7 @@ func (entry *Entry) Fatal(args ...interface{}) { if entry.Logger.IsLevelEnabled(FatalLevel) { entry.log(FatalLevel, fmt.Sprint(args...)) } - Exit(1) + entry.Logger.Exit(1) } func (entry *Entry) Panic(args ...interface{}) { @@ -246,7 +246,7 @@ func (entry *Entry) Fatalf(format string, args ...interface{}) { if entry.Logger.IsLevelEnabled(FatalLevel) { entry.Fatal(fmt.Sprintf(format, args...)) } - Exit(1) + entry.Logger.Exit(1) } func (entry *Entry) Panicf(format string, args ...interface{}) { @@ -293,7 +293,7 @@ func (entry *Entry) Fatalln(args ...interface{}) { if entry.Logger.IsLevelEnabled(FatalLevel) { entry.Fatal(entry.sprintlnn(args...)) } - Exit(1) + entry.Logger.Exit(1) } func (entry *Entry) Panicln(args ...interface{}) { diff --git a/hooks/test/test.go b/hooks/test/test.go index 234a17d..f84fe80 100644 --- a/hooks/test/test.go +++ b/hooks/test/test.go @@ -39,12 +39,24 @@ func NewLocal(logger *logrus.Logger) *Hook { } +type TestOption func(logger *logrus.Logger) + +func FatalPanics(logger *logrus.Logger) { + logger.Exit = func(code int) { + panic(code) + } +} + // NewNullLogger creates a discarding logger and installs the test hook. -func NewNullLogger() (*logrus.Logger, *Hook) { +func NewNullLogger(options ...TestOption) (*logrus.Logger, *Hook) { logger := logrus.New() logger.Out = ioutil.Discard + for _, option := range options { + option(logger) + } + return logger, NewLocal(logger) } diff --git a/hooks/test/test_test.go b/hooks/test/test_test.go index d6f6d30..692d36a 100644 --- a/hooks/test/test_test.go +++ b/hooks/test/test_test.go @@ -71,3 +71,14 @@ func TestLoggingWithHooksRace(t *testing.T) { entries := hook.AllEntries() assert.Equal(100, len(entries)) } + +func TestFatalWithPanic(t *testing.T) { + assert := assert.New(t) + + logger, hook := NewNullLogger(FatalPanics) + + assert.Nil(hook.LastEntry()) + assert.Equal(0, len(hook.Entries)) + + assert.Panics(func() { logger.Fatal("something went wrong") }) +} diff --git a/logger.go b/logger.go index b67bfcb..188c600 100644 --- a/logger.go +++ b/logger.go @@ -32,6 +32,8 @@ type Logger struct { mu MutexWrap // Reusable empty entry entryPool sync.Pool + // Function to exit the application, defaults to `Exit()` + Exit exitFunc } type MutexWrap struct { @@ -73,6 +75,7 @@ func New() *Logger { Formatter: new(TextFormatter), Hooks: make(LevelHooks), Level: InfoLevel, + Exit: Exit, } } @@ -173,7 +176,7 @@ func (logger *Logger) Fatalf(format string, args ...interface{}) { entry.Fatalf(format, args...) logger.releaseEntry(entry) } - Exit(1) + logger.Exit(1) } func (logger *Logger) Panicf(format string, args ...interface{}) { @@ -236,7 +239,7 @@ func (logger *Logger) Fatal(args ...interface{}) { entry.Fatal(args...) logger.releaseEntry(entry) } - Exit(1) + logger.Exit(1) } func (logger *Logger) Panic(args ...interface{}) { @@ -299,7 +302,7 @@ func (logger *Logger) Fatalln(args ...interface{}) { entry.Fatalln(args...) logger.releaseEntry(entry) } - Exit(1) + logger.Exit(1) } func (logger *Logger) Panicln(args ...interface{}) { From 99bc300c8d60c2f531a4690533f63273e4fbaaf7 Mon Sep 17 00:00:00 2001 From: Albert Salim Date: Wed, 10 Oct 2018 21:54:15 +0800 Subject: [PATCH 2/4] Add a method Exit on Logger that calls `os.Exit` or alternate exit function. This keeps backward compatibility for static declaration of logger that does not specify `ExitFunc` field. --- alt_exit.go | 8 ++++++-- hooks/test/test.go | 14 +------------- hooks/test/test_test.go | 13 +++++++------ logger.go | 14 +++++++++++--- logrus.go | 2 +- 5 files changed, 26 insertions(+), 25 deletions(-) diff --git a/alt_exit.go b/alt_exit.go index f1bb44c..183db77 100644 --- a/alt_exit.go +++ b/alt_exit.go @@ -45,11 +45,15 @@ func runHandlers() { } } -type exitFunc func(int) - // Exit runs all the Logrus atexit handlers and then terminates the program using os.Exit(code) func Exit(code int) { runHandlers() + osExit(code) +} + +type exitFunc func(int) + +func osExit(code int) { os.Exit(code) } diff --git a/hooks/test/test.go b/hooks/test/test.go index f84fe80..234a17d 100644 --- a/hooks/test/test.go +++ b/hooks/test/test.go @@ -39,24 +39,12 @@ func NewLocal(logger *logrus.Logger) *Hook { } -type TestOption func(logger *logrus.Logger) - -func FatalPanics(logger *logrus.Logger) { - logger.Exit = func(code int) { - panic(code) - } -} - // NewNullLogger creates a discarding logger and installs the test hook. -func NewNullLogger(options ...TestOption) (*logrus.Logger, *Hook) { +func NewNullLogger() (*logrus.Logger, *Hook) { logger := logrus.New() logger.Out = ioutil.Discard - for _, option := range options { - option(logger) - } - return logger, NewLocal(logger) } diff --git a/hooks/test/test_test.go b/hooks/test/test_test.go index 692d36a..636bad5 100644 --- a/hooks/test/test_test.go +++ b/hooks/test/test_test.go @@ -72,13 +72,14 @@ func TestLoggingWithHooksRace(t *testing.T) { assert.Equal(100, len(entries)) } -func TestFatalWithPanic(t *testing.T) { +func TestFatalWithAlternateExit(t *testing.T) { assert := assert.New(t) - logger, hook := NewNullLogger(FatalPanics) + logger, hook := NewNullLogger() + logger.ExitFunc = func(code int) {} - assert.Nil(hook.LastEntry()) - assert.Equal(0, len(hook.Entries)) - - assert.Panics(func() { logger.Fatal("something went wrong") }) + logger.Fatal("something went very wrong") + assert.Equal(logrus.FatalLevel, hook.LastEntry().Level) + assert.Equal("something went very wrong", hook.LastEntry().Message) + assert.Equal(1, len(hook.Entries)) } diff --git a/logger.go b/logger.go index 188c600..364819e 100644 --- a/logger.go +++ b/logger.go @@ -32,8 +32,8 @@ type Logger struct { mu MutexWrap // Reusable empty entry entryPool sync.Pool - // Function to exit the application, defaults to `Exit()` - Exit exitFunc + // Function to exit the application, defaults to `osExit()` + ExitFunc exitFunc } type MutexWrap struct { @@ -75,7 +75,7 @@ func New() *Logger { Formatter: new(TextFormatter), Hooks: make(LevelHooks), Level: InfoLevel, - Exit: Exit, + ExitFunc: osExit, } } @@ -313,6 +313,14 @@ func (logger *Logger) Panicln(args ...interface{}) { } } +func (logger *Logger) Exit(code int) { + runHandlers() + if logger.ExitFunc == nil { + logger.ExitFunc = osExit + } + logger.ExitFunc(code) +} + //When file is opened with appending mode, it's safe to //write concurrently to a file (within 4k message on Linux). //In these cases user can choose to disable the lock. diff --git a/logrus.go b/logrus.go index fa0b9de..6fff506 100644 --- a/logrus.go +++ b/logrus.go @@ -69,7 +69,7 @@ const ( // PanicLevel level, highest level of severity. Logs and then calls panic with the // message passed to Debug, Info, ... PanicLevel Level = iota - // FatalLevel level. Logs and then calls `os.Exit(1)`. It will exit even if the + // FatalLevel level. Logs and then calls `logger.Exit(1)`. It will exit even if the // logging level is set to Panic. FatalLevel // ErrorLevel level. Logs. Used for errors that should definitely be noted. From 4346c76f26ec031258a1d8f5f08cfb829f74611e Mon Sep 17 00:00:00 2001 From: Albert Salim Date: Wed, 10 Oct 2018 21:57:58 +0800 Subject: [PATCH 3/4] Remove unnecessary wrapper function on `os.Exit` --- alt_exit.go | 6 ------ logger.go | 6 ++++-- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/alt_exit.go b/alt_exit.go index 183db77..8af9063 100644 --- a/alt_exit.go +++ b/alt_exit.go @@ -48,12 +48,6 @@ func runHandlers() { // Exit runs all the Logrus atexit handlers and then terminates the program using os.Exit(code) func Exit(code int) { runHandlers() - osExit(code) -} - -type exitFunc func(int) - -func osExit(code int) { os.Exit(code) } diff --git a/logger.go b/logger.go index 364819e..21f49ec 100644 --- a/logger.go +++ b/logger.go @@ -36,6 +36,8 @@ type Logger struct { ExitFunc exitFunc } +type exitFunc func(int) + type MutexWrap struct { lock sync.Mutex disabled bool @@ -75,7 +77,7 @@ func New() *Logger { Formatter: new(TextFormatter), Hooks: make(LevelHooks), Level: InfoLevel, - ExitFunc: osExit, + ExitFunc: os.Exit, } } @@ -316,7 +318,7 @@ func (logger *Logger) Panicln(args ...interface{}) { func (logger *Logger) Exit(code int) { runHandlers() if logger.ExitFunc == nil { - logger.ExitFunc = osExit + logger.ExitFunc = os.Exit } logger.ExitFunc(code) } From a13c5db57c07b6f81ed4ad333c219966151ec30a Mon Sep 17 00:00:00 2001 From: Albert Salim Date: Wed, 10 Oct 2018 21:59:03 +0800 Subject: [PATCH 4/4] Fix typo in comment --- logger.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/logger.go b/logger.go index 21f49ec..1c934ed 100644 --- a/logger.go +++ b/logger.go @@ -32,7 +32,7 @@ type Logger struct { mu MutexWrap // Reusable empty entry entryPool sync.Pool - // Function to exit the application, defaults to `osExit()` + // Function to exit the application, defaults to `os.Exit()` ExitFunc exitFunc }