Skip to content
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

EncoderConfig and AtomicLevel validation added #791

Merged
merged 9 commits into from Mar 4, 2020
9 changes: 9 additions & 0 deletions config.go
Expand Up @@ -21,6 +21,7 @@
package zap

import (
"fmt"
"sort"
"time"

Expand Down Expand Up @@ -174,6 +175,10 @@ func (cfg Config) Build(opts ...Option) (*Logger, error) {
return nil, err
}

if cfg.Level == AtomicLevel{} {
return nil, fmt.Errorf("missing Level")
}

log := New(
zapcore.NewCore(enc, sink, cfg.Level),
cfg.buildOptions(errSink)...,
Expand Down Expand Up @@ -239,5 +244,9 @@ func (cfg Config) openSinks() (zapcore.WriteSyncer, zapcore.WriteSyncer, error)
}

func (cfg Config) buildEncoder() (zapcore.Encoder, error) {
if cfg.EncoderConfig.TimeKey != "" && cfg.EncoderConfig.EncodeTime == nil {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that newEncoder is the ultimate constructor for zapcore.Encoder given
an EncoderConfig. We should maybe move this check to the newEncoder function,
leaving buildEncoder unchanged.

return nil, fmt.Errorf("missing EncodeTime in EncoderConfig")
}

return newEncoder(cfg.Encoding, cfg.EncoderConfig)
}
37 changes: 37 additions & 0 deletions config_test.go
Expand Up @@ -27,6 +27,7 @@ import (

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"go.uber.org/zap/zapcore"
)

func TestConfig(t *testing.T) {
Expand Down Expand Up @@ -106,3 +107,39 @@ func TestConfigWithInvalidPaths(t *testing.T) {
})
}
}

func TestConfigWithMissingAttributes(t *testing.T) {
tests := []struct {
desc string
cfg Config
expectErr string
}{
{
desc: "missing_level",
abhinav marked this conversation as resolved.
Show resolved Hide resolved
cfg: Config{
Encoding: "json",
},
expectErr: "missing Level",
},
{
desc: "missing_encoder_time_in_encoder_config",
abhinav marked this conversation as resolved.
Show resolved Hide resolved
cfg: Config{
Level: NewAtomicLevelAt(zapcore.InfoLevel),
Encoding: "json",
EncoderConfig: zapcore.EncoderConfig{
MessageKey: "msg",
TimeKey: "ts",
},
},
expectErr: "missing EncodeTime in EncoderConfig",
},
}

for _, tt := range tests {
t.Run(tt.desc, func(t *testing.T) {
cfg := tt.cfg
_, err := cfg.Build()
abhinav marked this conversation as resolved.
Show resolved Hide resolved
assert.Equal(t, err.Error(), tt.expectErr)
abhinav marked this conversation as resolved.
Show resolved Hide resolved
})
}
}