ronantakizawa/codereview-bench huggingface dataset:
Repository: uber-go/zap
File: zapcore/console_encoder_test.go (Go)

Code before:
// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
// THE SOFTWARE.
package zapcore_test

import (
	"github.com/stretchr/testify/assert"
	. "go.uber.org/zap/zapcore"
	"testing"
)

var (
	testEntry = Entry{
		LoggerName: "main",
		Level:      InfoLevel,
		Message:    `hello`,
		Time:       _epoch,
		Stack:      "fake-stack",
		Caller:     EntryCaller{Defined: true, File: "foo.go", Line: 42},
	}
)

func TestConsoleSeparator(t *testing.T) {

	tests := []struct {
		desc            string
		cfg             EncoderConfig
		expectedConsole string
	}{
		{
			desc:            "space console separator",
			cfg:             encoderTestEncoderConfig(' '),
			expectedConsole: "0 info main foo.go:42 hello\nfake-stack\n",
		},
		{
			desc:            "default console separator",
			cfg:             testEncoderConfig(),
			expectedConsole: "0\tinfo\tmain\tfoo.go:42\thello\nfake-stack\n",
		},
		{
			desc:            "default console separator",
			cfg:             encoderTestEncoderConfig('\t'),
			expectedConsole: "0\tinfo\tmain\tfoo.go:42\thello\nfake-stack\n",
		},
		{
			desc:            "dash console separator",
			cfg:             encoderTestEncoderConfig('-'),
			expectedConsole: "0-info-main-foo.go:42-hello\nfake-stack\n",
		},
	}

	for i, tt := range tests {

Review comment (suggestion):
I think this can be simplified slightly by changing this field to a `separator byte`.

```
tests := []struct{
	desc        string
	separator   byte
	wantConsole string
}{
	...
}
```

Omit the field from the struct for the default test case and set up the
configuration for the encoder in the test itself.

```
cfg := testEncoderConfig()
cfg.ConsoleSeparator = tt.separator
```

Code after:
// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
// THE SOFTWARE.
package zapcore_test

import (
	"testing"

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

var (
	testEntry = Entry{
		LoggerName: "main",
		Level:      InfoLevel,
		Message:    `hello`,
		Time:       _epoch,
		Stack:      "fake-stack",
		Caller:     EntryCaller{Defined: true, File: "foo.go", Line: 42},
	}
)

func TestConsoleSeparator(t *testing.T) {
	tests := []struct {
		desc        string
		separator   string
		wantConsole string
	}{
		{
			desc:        "space console separator",
			separator:   " ",
			wantConsole: "0 info main foo.go:42 hello\nfake-stack\n",
		},
		{
			desc:        "default console separator",
			separator:   "",
			wantConsole: "0\tinfo\tmain\tfoo.go:42\thello\nfake-stack\n",
		},
		{
			desc:        "tag console separator",
			separator:   "\t",
			wantConsole: "0\tinfo\tmain\tfoo.go:42\thello\nfake-stack\n",
		},
		{
			desc:        "dash console separator",
			separator:   "--",
			wantConsole: "0--info--main--foo.go:42--hello\nfake-stack\n",
		},
	}

	for _, tt := range tests {
Repository: uber-go/zap
File: zapcore/console_encoder_test.go (Go)

Code before:
		expectedConsole string
	}{
		{
			desc:            "space console separator",
			cfg:             encoderTestEncoderConfig(' '),
			expectedConsole: "0 info main foo.go:42 hello\nfake-stack\n",
		},
		{
			desc:            "default console separator",
			cfg:             testEncoderConfig(),
			expectedConsole: "0\tinfo\tmain\tfoo.go:42\thello\nfake-stack\n",
		},
		{
			desc:            "default console separator",
			cfg:             encoderTestEncoderConfig('\t'),
			expectedConsole: "0\tinfo\tmain\tfoo.go:42\thello\nfake-stack\n",
		},
		{
			desc:            "dash console separator",
			cfg:             encoderTestEncoderConfig('-'),
			expectedConsole: "0-info-main-foo.go:42-hello\nfake-stack\n",
		},
	}

	for i, tt := range tests {
		console := NewConsoleEncoder(tt.cfg)
		entry := testEntry
		consoleOut, consoleErr := console.EncodeEntry(entry, nil)
		if assert.NoError(t, consoleErr, "Unexpected error console-encoding entry in case #%d.", i) {
			assert.Equal(
				t,
				tt.expectedConsole,
				consoleOut.String(),
				"Unexpected console output: expected to %v.", tt.desc,
			)
		}
	}
}

func encoderTestEncoderConfig(separator byte) EncoderConfig {
	testEncoder := testEncoderConfig()
	testEncoder.ConsoleSeparator = separator
	return testEncoder
}


Review comment (suggestion):
We should use subtests here for better error messages.

```
for _, tt := range tests {
	t.Run(tt.desc, func(t *testing.T) {
		...
	})
}
```


Then the individual `assert` statements don't need to include the test case
description or index.

Code after:
		wantConsole string
	}{
		{
			desc:        "space console separator",
			separator:   " ",
			wantConsole: "0 info main foo.go:42 hello\nfake-stack\n",
		},
		{
			desc:        "default console separator",
			separator:   "",
			wantConsole: "0\tinfo\tmain\tfoo.go:42\thello\nfake-stack\n",
		},
		{
			desc:        "tag console separator",
			separator:   "\t",
			wantConsole: "0\tinfo\tmain\tfoo.go:42\thello\nfake-stack\n",
		},
		{
			desc:        "dash console separator",
			separator:   "--",
			wantConsole: "0--info--main--foo.go:42--hello\nfake-stack\n",
		},
	}

	for _, tt := range tests {
		console := NewConsoleEncoder(encoderTestEncoderConfig(tt.separator))
		t.Run(tt.desc, func(t *testing.T) {
			entry := testEntry
			consoleOut, err := console.EncodeEntry(entry, nil)
			if !assert.NoError(t, err) {
				return
			}
			assert.Equal(
				t,
				tt.wantConsole,
				consoleOut.String(),
				"Unexpected console output",
			)
		})

	}
}

func encoderTestEncoderConfig(separator string) EncoderConfig {
	testEncoder := testEncoderConfig()
	testEncoder.ConsoleSeparator = separator
	return testEncoder
}
Repository: uber-go/zap
File: zapcore/console_encoder_test.go (Go)

Code before:
		{
			desc:            "space console separator",
			cfg:             encoderTestEncoderConfig(' '),
			expectedConsole: "0 info main foo.go:42 hello\nfake-stack\n",
		},
		{
			desc:            "default console separator",
			cfg:             testEncoderConfig(),
			expectedConsole: "0\tinfo\tmain\tfoo.go:42\thello\nfake-stack\n",
		},
		{
			desc:            "default console separator",
			cfg:             encoderTestEncoderConfig('\t'),
			expectedConsole: "0\tinfo\tmain\tfoo.go:42\thello\nfake-stack\n",
		},
		{
			desc:            "dash console separator",
			cfg:             encoderTestEncoderConfig('-'),
			expectedConsole: "0-info-main-foo.go:42-hello\nfake-stack\n",
		},
	}

	for i, tt := range tests {
		console := NewConsoleEncoder(tt.cfg)
		entry := testEntry
		consoleOut, consoleErr := console.EncodeEntry(entry, nil)
		if assert.NoError(t, consoleErr, "Unexpected error console-encoding entry in case #%d.", i) {
			assert.Equal(
				t,
				tt.expectedConsole,
				consoleOut.String(),
				"Unexpected console output: expected to %v.", tt.desc,
			)
		}
	}
}

func encoderTestEncoderConfig(separator byte) EncoderConfig {
	testEncoder := testEncoderConfig()
	testEncoder.ConsoleSeparator = separator
	return testEncoder
}


Review comment (suggestion):
The error should be called just `err` unless there's a need to maintain a
reference to an existing `err` that's already in scope.

Code after:
		{
			desc:        "space console separator",
			separator:   " ",
			wantConsole: "0 info main foo.go:42 hello\nfake-stack\n",
		},
		{
			desc:        "default console separator",
			separator:   "",
			wantConsole: "0\tinfo\tmain\tfoo.go:42\thello\nfake-stack\n",
		},
		{
			desc:        "tag console separator",
			separator:   "\t",
			wantConsole: "0\tinfo\tmain\tfoo.go:42\thello\nfake-stack\n",
		},
		{
			desc:        "dash console separator",
			separator:   "--",
			wantConsole: "0--info--main--foo.go:42--hello\nfake-stack\n",
		},
	}

	for _, tt := range tests {
		console := NewConsoleEncoder(encoderTestEncoderConfig(tt.separator))
		t.Run(tt.desc, func(t *testing.T) {
			entry := testEntry
			consoleOut, err := console.EncodeEntry(entry, nil)
			if !assert.NoError(t, err) {
				return
			}
			assert.Equal(
				t,
				tt.wantConsole,
				consoleOut.String(),
				"Unexpected console output",
			)
		})

	}
}

func encoderTestEncoderConfig(separator string) EncoderConfig {
	testEncoder := testEncoderConfig()
	testEncoder.ConsoleSeparator = separator
	return testEncoder
}
Repository: uber-go/zap
File: zapcore/console_encoder_test.go (Go)

Code before:
			separator:   " ",
			wantConsole: "0 info main foo.go:42 hello\nfake-stack\n",
		},
		{
			desc:        "default console separator",
			separator:   "",
			wantConsole: "0\tinfo\tmain\tfoo.go:42\thello\nfake-stack\n",
		},
		{
			desc:        "tag console separator",
			separator:   "\t",
			wantConsole: "0\tinfo\tmain\tfoo.go:42\thello\nfake-stack\n",
		},
		{
			desc:        "dash console separator",
			separator:   "-",
			wantConsole: "0-info-main-foo.go:42-hello\nfake-stack\n",
		},
	}

	for _, tt := range tests {
		console := NewConsoleEncoder(encoderTestEncoderConfig(tt.separator))
		t.Run(tt.desc, func(t *testing.T) {
			entry := testEntry
			consoleOut, err := console.EncodeEntry(entry, nil)
			assert.Nil(t, err)
			assert.Equal(
				t,
				tt.wantConsole,
				consoleOut.String(),
				"Unexpected console output: expected to %v.", tt.desc,
			)
		})

	}
}

func encoderTestEncoderConfig(separator string) EncoderConfig {
	testEncoder := testEncoderConfig()
	testEncoder.ConsoleSeparator = separator
	return testEncoder
}


Review comment (suggestion):
We should use `assert.NoError` here, and if it is non-nil, skip the following assert.Equal.

```suggestion
			if !assert.NoError(t, err) {
				return
			}
```

Code after:
			separator:   " ",
			wantConsole: "0 info main foo.go:42 hello\nfake-stack\n",
		},
		{
			desc:        "default console separator",
			separator:   "",
			wantConsole: "0\tinfo\tmain\tfoo.go:42\thello\nfake-stack\n",
		},
		{
			desc:        "tag console separator",
			separator:   "\t",
			wantConsole: "0\tinfo\tmain\tfoo.go:42\thello\nfake-stack\n",
		},
		{
			desc:        "dash console separator",
			separator:   "--",
			wantConsole: "0--info--main--foo.go:42--hello\nfake-stack\n",
		},
	}

	for _, tt := range tests {
		console := NewConsoleEncoder(encoderTestEncoderConfig(tt.separator))
		t.Run(tt.desc, func(t *testing.T) {
			entry := testEntry
			consoleOut, err := console.EncodeEntry(entry, nil)
			if !assert.NoError(t, err) {
				return
			}
			assert.Equal(
				t,
				tt.wantConsole,
				consoleOut.String(),
				"Unexpected console output",
			)
		})

	}
}

func encoderTestEncoderConfig(separator string) EncoderConfig {
	testEncoder := testEncoderConfig()
	testEncoder.ConsoleSeparator = separator
	return testEncoder
}
Repository: uber-go/zap
File: zapcore/console_encoder_test.go (Go)

Code before:
			separator:   "",
			wantConsole: "0\tinfo\tmain\tfoo.go:42\thello\nfake-stack\n",
		},
		{
			desc:        "tag console separator",
			separator:   "\t",
			wantConsole: "0\tinfo\tmain\tfoo.go:42\thello\nfake-stack\n",
		},
		{
			desc:        "dash console separator",
			separator:   "-",
			wantConsole: "0-info-main-foo.go:42-hello\nfake-stack\n",
		},
	}

	for _, tt := range tests {
		console := NewConsoleEncoder(encoderTestEncoderConfig(tt.separator))
		t.Run(tt.desc, func(t *testing.T) {
			entry := testEntry
			consoleOut, err := console.EncodeEntry(entry, nil)
			assert.Nil(t, err)
			assert.Equal(
				t,
				tt.wantConsole,
				consoleOut.String(),
				"Unexpected console output: expected to %v.", tt.desc,
			)
		})

	}
}

func encoderTestEncoderConfig(separator string) EncoderConfig {
	testEncoder := testEncoderConfig()
	testEncoder.ConsoleSeparator = separator
	return testEncoder
}


Review comment (suggestion):
minor: since tt.desc is part of the test name, we don't need it in the error message again.
```suggestion
				"Unexpected console output",
```

Code after:
			separator:   "",
			wantConsole: "0\tinfo\tmain\tfoo.go:42\thello\nfake-stack\n",
		},
		{
			desc:        "tag console separator",
			separator:   "\t",
			wantConsole: "0\tinfo\tmain\tfoo.go:42\thello\nfake-stack\n",
		},
		{
			desc:        "dash console separator",
			separator:   "--",
			wantConsole: "0--info--main--foo.go:42--hello\nfake-stack\n",
		},
	}

	for _, tt := range tests {
		console := NewConsoleEncoder(encoderTestEncoderConfig(tt.separator))
		t.Run(tt.desc, func(t *testing.T) {
			entry := testEntry
			consoleOut, err := console.EncodeEntry(entry, nil)
			if !assert.NoError(t, err) {
				return
			}
			assert.Equal(
				t,
				tt.wantConsole,
				consoleOut.String(),
				"Unexpected console output",
			)
		})

	}
}

func encoderTestEncoderConfig(separator string) EncoderConfig {
	testEncoder := testEncoderConfig()
	testEncoder.ConsoleSeparator = separator
	return testEncoder
}
Repository: uber-go/zap
File: stacktrace_ext_test.go (Go)

Code before:
	return filepath.Base(file)
}

func setupSymlink(t *testing.T, src, dst string) {
	// Make sure the destination directory exists.
	os.MkdirAll(filepath.Dir(dst), 0777)

	// Get absolute path of the source for the symlink, otherwise we can create a symlink
	// that uses relative paths.
	srcAbs, err := filepath.Abs(src)
	require.NoError(t, err, "Failed to get absolute path")

	require.NoError(t, os.Symlink(srcAbs, dst), "Failed to set up symlink")
}

type dependency struct {
	ImportPath string `json:"Path"` // import path of the dependency
	Dir        string `json:"Dir"`  // location on disk
}

// Downloads all dependencies for the current Go module and reports their
// module paths and locations on disk.
func downloadDependencies(t *testing.T) []dependency {
	cmd := exec.Command("go", "mod", "download", "-json")

	stdout, err := cmd.StdoutPipe()
	require.NoError(t, err, "Unable to create stdout pipe")
	require.NoError(t, cmd.Start(), "Failed to start 'go mod download'")
	defer func() {
		assert.NoError(t, cmd.Wait(), "'go mod download' failed")
	}()

	dec := json.NewDecoder(stdout)

	var deps []dependency
	for dec.More() {
		var d dependency
		require.NoError(t, dec.Decode(&d), "Failed to decode dependency")
		deps = append(deps, d)
	}

	return deps
}

// Creates a copy of the current OS environment variables, adding the
// following entries to it. Existing instances of the provided entries will be
// removed.
func envWith(t *testing.T, entries ...string) []string {
	keys := make(map[string]struct{})
	for _, e := range entries {
		i := strings.IndexByte(e, '=')

Review comment (refactor):
is streaming worth it here, suggest simplifying to:
```
stdout, err := cmd.Output()
require.NoError(t, err, ...)

dec := json.NewDecoder(bytes.NewReader(stdout))
```

Code after:
	return filepath.Base(file)
}

func setupSymlink(t *testing.T, src, dst string) {
	// Make sure the destination directory exists.
	os.MkdirAll(filepath.Dir(dst), 0777)

	// Get absolute path of the source for the symlink, otherwise we can create a symlink
	// that uses relative paths.
	srcAbs, err := filepath.Abs(src)
	require.NoError(t, err, "Failed to get absolute path")

	require.NoError(t, os.Symlink(srcAbs, dst), "Failed to set up symlink")
}

type dependency struct {
	ImportPath string `json:"Path"` // import path of the dependency
	Dir        string `json:"Dir"`  // location on disk
}

// Downloads all dependencies for the current Go module and reports their
// module paths and locations on disk.
func downloadDependencies(t *testing.T) []dependency {
	cmd := exec.Command("go", "mod", "download", "-json")

	stdout, err := cmd.Output()
	require.NoError(t, err, "Failed to run 'go mod download'")

	var deps []dependency
	dec := json.NewDecoder(bytes.NewBuffer(stdout))
	for dec.More() {
		var d dependency
		require.NoError(t, dec.Decode(&d), "Failed to decode dependency")
		deps = append(deps, d)
	}

	return deps
}
Repository: uber-go/zap
File: zapcore/json_encoder.go (Go)

Code before:
}

func (enc *jsonEncoder) AddInt64(key string, val int64) {
	enc.addKey(key)
	enc.AppendInt64(val)
}

func (enc *jsonEncoder) resetReflectBuf() {
	if enc.reflectBuf == nil {
		enc.reflectBuf = bufferpool.Get()
		enc.reflectEnc = json.NewEncoder(enc.reflectBuf)

		// For consistency with our custom JSON encoder.
		enc.reflectEnc.SetEscapeHTML(false)
	} else {
		enc.reflectBuf.Reset()
	}
}

var nullLiteralBytes = []byte("null")

func (enc *jsonEncoder) AddReflected(key string, obj interface{}) error {
	// Only invoke the standard JSON encoder if there is actually something to
	// encode; otherwise write JSON null literal directly. For now, we only
	// apply this optimization here, but not in AppendReflected. We could apply
	// it there as well if needed (some discussion in
	// https://github.com/uber-go/zap/issues/630 indicates that it could be
	// valuable in some cases) but the primary known benefit at the moment is
	// from primitive callers (see https://github.com/uber-go/zap/issues/753)
	valueBytes := nullLiteralBytes
	if obj != nil {
		enc.resetReflectBuf()
		if err := enc.reflectEnc.Encode(obj); err != nil {
			return err
		}
		enc.reflectBuf.TrimNewline()
		valueBytes = enc.reflectBuf.Bytes()
	}
	enc.addKey(key)
	_, err := enc.buf.Write(valueBytes)
	return err
}

func (enc *jsonEncoder) OpenNamespace(key string) {
	enc.addKey(key)
	enc.buf.AppendByte('{')
	enc.openNamespaces++
}

func (enc *jsonEncoder) AddString(key, val string) {
	enc.addKey(key)

Review comment (suggestion):
Since we're treating this as an optimization, I'm in favor of doin it to AppendReflected too.
It already supports `nil` entries inside slices. https://play.golang.org/p/bZb6t58qhpy

Code after:
}

func (enc *jsonEncoder) AddInt64(key string, val int64) {
	enc.addKey(key)
	enc.AppendInt64(val)
}

func (enc *jsonEncoder) resetReflectBuf() {
	if enc.reflectBuf == nil {
		enc.reflectBuf = bufferpool.Get()
		enc.reflectEnc = json.NewEncoder(enc.reflectBuf)

		// For consistency with our custom JSON encoder.
		enc.reflectEnc.SetEscapeHTML(false)
	} else {
		enc.reflectBuf.Reset()
	}
}

var nullLiteralBytes = []byte("null")

// Only invoke the standard JSON encoder if there is actually something to
// encode; otherwise write JSON null literal directly.
func (enc *jsonEncoder) encodeReflected(obj interface{}) ([]byte, error) {
	if obj == nil {
		return nullLiteralBytes, nil
	}
	enc.resetReflectBuf()
	if err := enc.reflectEnc.Encode(obj); err != nil {
		return nil, err
	}
	enc.reflectBuf.TrimNewline()
	return enc.reflectBuf.Bytes(), nil
}

func (enc *jsonEncoder) AddReflected(key string, obj interface{}) error {
	valueBytes, err := enc.encodeReflected(obj)
	if err != nil {
		return err
	}
	enc.addKey(key)
	_, err = enc.buf.Write(valueBytes)
	return err
}

func (enc *jsonEncoder) OpenNamespace(key string) {
	enc.addKey(key)
	enc.buf.AppendByte('{')
	enc.openNamespaces++
}
Repository: uber-go/zap
File: config.go (Go)

Code before:
// Stacktraces are automatically included on logs of WarnLevel and above.
func NewDevelopmentConfig() Config {
	return Config{
		Level:            NewAtomicLevelAt(DebugLevel),
		Development:      true,
		Encoding:         "console",
		EncoderConfig:    NewDevelopmentEncoderConfig(),
		OutputPaths:      []string{"stderr"},
		ErrorOutputPaths: []string{"stderr"},
	}
}

// Build constructs a logger from the Config and Options.
func (cfg Config) Build(opts ...Option) (*Logger, error) {
	enc, err := cfg.buildEncoder()
	if err != nil {
		return nil, err
	}

	sink, errSink, err := cfg.openSinks()
	if err != nil {
		return nil, err
	}

	if cfg.Level == (AtomicLevel{}) {
		cfg.Level = NewAtomicLevelAt(InfoLevel)
		if cfg.Development {
			cfg.Level = NewAtomicLevelAt(DebugLevel)
		}
	}

	log := New(
		zapcore.NewCore(enc, sink, cfg.Level),
		cfg.buildOptions(errSink)...,
	)
	if len(opts) > 0 {
		log = log.WithOptions(opts...)
	}
	return log, nil
}

func (cfg Config) buildOptions(errSink zapcore.WriteSyncer) []Option {
	opts := []Option{ErrorOutput(errSink)}

	if cfg.Development {
		opts = append(opts, Development())
	}

	if !cfg.DisableCaller {
		opts = append(opts, AddCaller())
	}

Review comment (suggestion):
The parentheses are unnecessary.

```suggestion
	if cfg.Level == AtomicLevel{} {
```

Code after:
// console encoder, writes to standard error, and disables sampling.
// Stacktraces are automatically included on logs of WarnLevel and above.
func NewDevelopmentConfig() Config {
	return Config{
		Level:            NewAtomicLevelAt(DebugLevel),
		Development:      true,
		Encoding:         "console",
		EncoderConfig:    NewDevelopmentEncoderConfig(),
		OutputPaths:      []string{"stderr"},
		ErrorOutputPaths: []string{"stderr"},
	}
}

// Build constructs a logger from the Config and Options.
func (cfg Config) Build(opts ...Option) (*Logger, error) {
	enc, err := cfg.buildEncoder()
	if err != nil {
		return nil, err
	}

	sink, errSink, err := cfg.openSinks()
	if err != nil {
		return nil, err
	}

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

	log := New(
		zapcore.NewCore(enc, sink, cfg.Level),
		cfg.buildOptions(errSink)...,
	)
	if len(opts) > 0 {
		log = log.WithOptions(opts...)
	}
	return log, nil
}

func (cfg Config) buildOptions(errSink zapcore.WriteSyncer) []Option {
	opts := []Option{ErrorOutput(errSink)}

	if cfg.Development {
		opts = append(opts, Development())
	}

	if !cfg.DisableCaller {
		opts = append(opts, AddCaller())
	}

	stackLevel := ErrorLevel
Repository: uber-go/zap
File: config_test.go (Go)

Code before:
		{
			desc: "missing_level",
			cfg: Config{
				Encoding: "json",
			},
			expectErr: "missing Level",
		},
		{
			desc: "missing_encoder_time_in_encoder_config",
			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()
			assert.Equal(t, err.Error(), tt.expectErr)
		})
	}
}


Review comment (suggestion):
Testify assertions are in the order: expected, then actual.

```suggestion
			assert.Equal(t, tt.expectErr, err.Error())
```

Code after:
		{
			desc: "missing level",
			cfg: Config{
				Encoding: "json",
			},
			expectErr: "missing Level",
		},
		{
			desc: "missing encoder time in encoder config",
			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()
			require.Error(t, err)
			assert.Equal(t, tt.expectErr, err.Error())
		})
	}
}
Repository: uber-go/zap
File: config_test.go (Go)

Code before:
	}{
		{
			desc: "missing_level",
			cfg: Config{
				Encoding: "json",
			},
			expectErr: "missing Level",
		},
		{
			desc: "missing_encoder_time_in_encoder_config",
			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()
			assert.Equal(t, err.Error(), tt.expectErr)
		})
	}
}


Review comment (bug):
err.Error on the following line will panic if we don't first verify that the error is non-nil.
We should use require.Error here.

```suggestion
			_, err := cfg.Build()
			require.Error(t, err)
```

Code after:
	}{
		{
			desc: "missing level",
			cfg: Config{
				Encoding: "json",
			},
			expectErr: "missing Level",
		},
		{
			desc: "missing encoder time in encoder config",
			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()
			require.Error(t, err)
			assert.Equal(t, tt.expectErr, err.Error())
		})
	}
}