From fd2b68cbe6e2fcb7797d1a40a38e645c16a587fd Mon Sep 17 00:00:00 2001 From: Techassi Date: Thu, 13 Jan 2022 15:27:27 +0100 Subject: [PATCH 1/5] Add LevelFromString func This adds the public LevelFromString func which enables the user to construct a log level based on ASCII text input --- zapcore/level.go | 31 ++++++++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/zapcore/level.go b/zapcore/level.go index e575c9f43..2872e985d 100644 --- a/zapcore/level.go +++ b/zapcore/level.go @@ -26,7 +26,10 @@ import ( "fmt" ) -var errUnmarshalNilLevel = errors.New("can't unmarshal a nil *Level") +var ( + errUnmarshalNilLevel = errors.New("can't unmarshal a nil *Level") + errInvalidLevel = errors.New("invalid level") +) // A Level is a logging priority. Higher levels are more important. type Level int8 @@ -55,6 +58,32 @@ const ( _maxLevel = FatalLevel ) +// LevelFromString returns a level based on the lower-case or all-caps ASCII +// representation of the log level. +// +// This is particularly useful when dealing with text input to configure log +// levels. +func LevelFromString(text string) (Level, error) { + switch text { + case "debug", "DEBUG": + return DebugLevel, nil + case "info", "INFO", "": // make the zero value useful + return InfoLevel, nil + case "warn", "WARN": + return WarnLevel, nil + case "error", "ERROR": + return ErrorLevel, nil + case "dpanic", "DPANIC": + return DPanicLevel, nil + case "panic", "PANIC": + return PanicLevel, nil + case "fatal", "FATAL": + return FatalLevel, nil + default: + return -2, errInvalidLevel + } +} + // String returns a lower-case ASCII representation of the log level. func (l Level) String() string { switch l { From d34f022b9805b9a86f43a018e129cde9ac74384b Mon Sep 17 00:00:00 2001 From: Techassi Date: Thu, 13 Jan 2022 19:04:22 +0100 Subject: [PATCH 2/5] Adjust UnmarshalLevel func --- zapcore/level.go | 34 ++++++++++------------------------ 1 file changed, 10 insertions(+), 24 deletions(-) diff --git a/zapcore/level.go b/zapcore/level.go index 2872e985d..3f1a1b468 100644 --- a/zapcore/level.go +++ b/zapcore/level.go @@ -26,10 +26,7 @@ import ( "fmt" ) -var ( - errUnmarshalNilLevel = errors.New("can't unmarshal a nil *Level") - errInvalidLevel = errors.New("invalid level") -) +var errUnmarshalNilLevel = errors.New("can't unmarshal a nil *Level") // A Level is a logging priority. Higher levels are more important. type Level int8 @@ -58,30 +55,19 @@ const ( _maxLevel = FatalLevel ) -// LevelFromString returns a level based on the lower-case or all-caps ASCII -// representation of the log level. +// UnmarshalLevel unmarshals a level based on the lower-case or all-caps ASCII +// representation of the log level. If the provided ASCII representation is +// invalid an error is returned. // // This is particularly useful when dealing with text input to configure log // levels. -func LevelFromString(text string) (Level, error) { - switch text { - case "debug", "DEBUG": - return DebugLevel, nil - case "info", "INFO", "": // make the zero value useful - return InfoLevel, nil - case "warn", "WARN": - return WarnLevel, nil - case "error", "ERROR": - return ErrorLevel, nil - case "dpanic", "DPANIC": - return DPanicLevel, nil - case "panic", "PANIC": - return PanicLevel, nil - case "fatal", "FATAL": - return FatalLevel, nil - default: - return -2, errInvalidLevel +func UnmarshalLevel(text string) (Level, error) { + level := DebugLevel + err := level.UnmarshalText([]byte(text)) + if err != nil { + return level, err } + return level, nil } // String returns a lower-case ASCII representation of the log level. From 647e596b218d2b77b986f83432d96e91ff1646ef Mon Sep 17 00:00:00 2001 From: Abhinav Gupta Date: Thu, 13 Jan 2022 12:05:25 -0800 Subject: [PATCH 3/5] Apply suggestions from code review --- zapcore/level.go | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/zapcore/level.go b/zapcore/level.go index 3f1a1b468..56e88dc0c 100644 --- a/zapcore/level.go +++ b/zapcore/level.go @@ -55,19 +55,16 @@ const ( _maxLevel = FatalLevel ) -// UnmarshalLevel unmarshals a level based on the lower-case or all-caps ASCII +// ParseLevel parses a level based on the lower-case or all-caps ASCII // representation of the log level. If the provided ASCII representation is // invalid an error is returned. // // This is particularly useful when dealing with text input to configure log // levels. -func UnmarshalLevel(text string) (Level, error) { - level := DebugLevel +func ParseLevel(text string) (Level, error) { + var level Level err := level.UnmarshalText([]byte(text)) - if err != nil { - return level, err - } - return level, nil + return level, err } // String returns a lower-case ASCII representation of the log level. From 6dca8e8709e41f3dfc1e3e67802b2fa520ea0cec Mon Sep 17 00:00:00 2001 From: Techassi Date: Thu, 13 Jan 2022 21:20:17 +0100 Subject: [PATCH 4/5] Add tests --- zapcore/level_test.go | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/zapcore/level_test.go b/zapcore/level_test.go index 28b75b37f..f4ab216e4 100644 --- a/zapcore/level_test.go +++ b/zapcore/level_test.go @@ -23,6 +23,7 @@ package zapcore import ( "bytes" "flag" + "fmt" "strings" "testing" @@ -76,6 +77,23 @@ func TestLevelText(t *testing.T) { } } +func TestParseLevel(t *testing.T) { + tests := []struct { + text string + level Level + err error + }{ + {"info", InfoLevel, nil}, + {"DEBUG", DebugLevel, nil}, + {"FOO", 0, fmt.Errorf("unrecognized level: \"FOO\"")}, + } + for _, tt := range tests { + parsedLevel, err := ParseLevel(tt.text) + assert.Equal(t, tt.level, parsedLevel) + assert.Equal(t, tt.err, err) + } +} + func TestCapitalLevelsParse(t *testing.T) { tests := []struct { text string From b45b8a99ab66121e3c7dc11dc95ca08d3d6af319 Mon Sep 17 00:00:00 2001 From: Abhinav Gupta Date: Thu, 13 Jan 2022 12:29:56 -0800 Subject: [PATCH 5/5] test: Minor nits - We can use backticks to avoid having to escape quotes - Prefer to have the error and non-error paths branch so that we're not asserting the value of level in the error case (since that's specifically not part of the contract) --- zapcore/level_test.go | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/zapcore/level_test.go b/zapcore/level_test.go index f4ab216e4..c4a120538 100644 --- a/zapcore/level_test.go +++ b/zapcore/level_test.go @@ -23,7 +23,6 @@ package zapcore import ( "bytes" "flag" - "fmt" "strings" "testing" @@ -81,16 +80,21 @@ func TestParseLevel(t *testing.T) { tests := []struct { text string level Level - err error + err string }{ - {"info", InfoLevel, nil}, - {"DEBUG", DebugLevel, nil}, - {"FOO", 0, fmt.Errorf("unrecognized level: \"FOO\"")}, + {"info", InfoLevel, ""}, + {"DEBUG", DebugLevel, ""}, + {"FOO", 0, `unrecognized level: "FOO"`}, } for _, tt := range tests { parsedLevel, err := ParseLevel(tt.text) - assert.Equal(t, tt.level, parsedLevel) - assert.Equal(t, tt.err, err) + if len(tt.err) == 0 { + assert.NoError(t, err) + assert.Equal(t, tt.level, parsedLevel) + } else { + assert.Error(t, err) + assert.Contains(t, err.Error(), tt.err) + } } }