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

Code before:

	logger.SetLevel(zap.FatalLevel)
	for i, f := range funcs {
		f("ohai")
		if !assert.Equal(t, 0, buf.Len(), "%+v(%d) logged, but shouldn't", f, i) {
			buf.Reset()
		}
	}
	assert.NotPanics(t, func() { logger.Panic("msg") })
}

func TestDebark_Stubs(t *testing.T) {
	logger, _ := newDebark()
	assert.Panics(t, func() { logger.StubTime() })
	assert.NotPanics(t, func() { logger.DFatal("msg") })
}

func TestDebark_zapToBarkFields(t *testing.T) {
	logger, _ := newDebark()
	fields := []zap.Field{
		zap.Bool("a", true),
		zap.Float64("b", float64(0.1)),
		zap.Int("c", 42),
		zap.String("d", "bar"),
		zap.Int64("e", int64(42)),
		zap.Object("f", logger),
		zap.Stringer("g", stringable("g")),
	}
	assert.NotNil(t, logger.With(fields...))

	for _, f := range fields {
		assert.Len(t, zapToBark([]zap.Field{f}).Fields(), 1)
	}
	assert.Len(t, zapToBark(fields).Fields(), len(fields))
}


Review comment (suggestion):
if you set the level to something like `Error`, can you make sure that nothing logged with a lower level logs anything


Code after:
		assert.NotEqual(t, 0, buf.Len())
		buf.Reset()
	}

	logger.SetLevel(zap.PanicLevel)
	for _, l := range levels {
		assert.Nil(t, logger.Check(l, "msg"))
	}

	// We should still panic even if the level isn't enough to log.
	assert.Panics(t, func() {
		logger.SetLevel(zap.FatalLevel)
		logger.Check(zap.PanicLevel, "panic!").Write()
	})
}

func TestDebark_LeveledLogging(t *testing.T) {
	logger, buf := newDebark()
	logger.SetLevel(zap.DebugLevel)
	for _, l := range levels {
		require.Equal(t, 0, buf.Len(), "buffer not zero")
		logger.Log(l, "ohai")
		assert.NotEqual(t, 0, buf.Len(), "%q did not log", l)
		buf.Reset()
	}

	logger.SetLevel(zap.FatalLevel)
	require.Equal(t, 0, buf.Len(), "buffer not zero to begin test")
	for _, l := range append(levels) {
		logger.Log(l, "ohai")
		assert.Equal(t, 0, buf.Len(), "buffer not zero, we should not have logged")
	}

	logger.SetLevel(zap.DebugLevel)
	assert.Panics(t, func() { logger.Log(zap.Level(31337), "") })
	assert.Panics(t, func() { logger.Log(zap.PanicLevel, "") })
}

func TestDebark_Methods(t *testing.T) {
	logger, buf := newDebark()
	logger.SetLevel(zap.DebugLevel)

	funcs := []func(string, ...zap.Field){
		logger.Debug,
		logger.Info,
		logger.Warn,
		logger.Error,
	}

	for i, f := range funcs {
		buf.Reset()
Repository: uber-go/zap
File: zbark/example_test.go (Go)

Code before:
	"github.com/Sirupsen/logrus"
	"github.com/uber-common/bark"
)

func ExampleBarkify() {
	zapLogger := zap.New(zap.NewJSONEncoder(
		zap.NoTime(), // discard timestamps in tests
	))

	// Wrap our structured logger to mimic bark.Logger.
	logger := zbark.Barkify(zapLogger)

	// The wrapped logger has bark's usual fluent API.
	logger.WithField("errors", 0).Infof("%v accepts arbitrary types.", "Bark")

	// Output:
	// {"level":"info","msg":"Bark accepts arbitrary types.","errors":0}
}

func ExampleDebarkify() {
	logrusLogger := logrus.New()
	logrusLogger.Out = os.Stdout
	logrusLogger.Formatter = new(logrus.JSONFormatter)
	logrusLogger.Formatter.(*logrus.JSONFormatter).TimestampFormat = "lies"
	barkLogger := bark.NewLoggerFromLogrus(logrusLogger).WithField("errors", 0)
	logger := zbark.Debarkify(barkLogger, zap.DebugLevel)

	// The wrapped logger has zap's actually fluent API.
	logger.Info("Zap accepts", zap.String("typed", "fields"))

	// Output:
	// {"errors":0,"level":"info","msg":"Zap accepts","time":"lies","typed":"fields"}
}


Review comment (suggestion):
`errors` comes from here and i was just matching the field use above in `ExampleBarkify()` 


Code after:
	"github.com/Sirupsen/logrus"
	"github.com/uber-common/bark"
)

func ExampleBarkify() {
	zapLogger := zap.New(zap.NewJSONEncoder(
		zap.NoTime(), // discard timestamps in tests
	))

	// Wrap our structured logger to mimic bark.Logger.
	logger := zbark.Barkify(zapLogger)

	// The wrapped logger has bark's usual fluent API.
	logger.WithField("errors", 0).Infof("%v accepts arbitrary types.", "Bark")

	// Output:
	// {"level":"info","msg":"Bark accepts arbitrary types.","errors":0}
}

func ExampleDebarkify() {
	logrusLogger := logrus.New()
	logrusLogger.Out = os.Stdout
	logrusLogger.Formatter = &logrus.JSONFormatter{
		TimestampFormat: "lies",
	}
	barkLogger := bark.NewLoggerFromLogrus(logrusLogger).WithField("errors", 0)
	logger := zbark.Debarkify(barkLogger, zap.DebugLevel)

	// The wrapped logger has zap's actually fluent API.
	logger.Info("Zap accepts", zap.String("typed", "fields"))

	// Output:
	// {"errors":0,"level":"info","msg":"Zap accepts","time":"lies","typed":"fields"}
}
Repository: uber-go/zap
File: keyvalue.go (Go)

Code before:
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
// THE SOFTWARE.

package zap

// KeyValue is an encoding-agnostic interface to add structured data to the
// logging context. Like maps, KeyValues aren't safe for concurrent use (though
// typical use shouldn't require locks).
//
// See Marshaler for an example.
type KeyValue interface {
	AddBool(string, bool)
	AddFloat64(string, float64)
	AddInt(string, int)
	AddInt64(string, int64)
	AddMarshaler(string, LogMarshaler) error
	// AddObject uses reflection to serialize arbitrary objects, so it's slow and
	// allocation-heavy. Consider implementing the LogMarshaler interface instead.
	AddObject(string, interface{})
	AddString(string, string)
	Nest(string, func(KeyValue) error) error
}


Review comment (suggestion):
I don't think `f` is a great name here, but I want to remove it very soon anyway (https://github.com/uber-go/zap/issues/87), so didn't spend much time coming up with a better name for it.


Code after:
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
// THE SOFTWARE.

package zap

// KeyValue is an encoding-agnostic interface to add structured data to the
// logging context. Like maps, KeyValues aren't safe for concurrent use (though
// typical use shouldn't require locks).
//
// See Marshaler for an example.
type KeyValue interface {
	AddBool(key string, value bool)
	AddFloat64(key string, value float64)
	AddInt(key string, value int)
	AddInt64(key string, value int64)
	AddMarshaler(key string, marshaler LogMarshaler) error
	// AddObject uses reflection to serialize arbitrary objects, so it's slow and
	// allocation-heavy. Consider implementing the LogMarshaler interface instead.
	AddObject(key string, value interface{})
	AddString(key, value string)
	Nest(key string, f func(KeyValue) error) error
}
Repository: uber-go/zap
File: json_encoder_bench_test.go (Go)

Code before:
import (
	"encoding/json"
	"io/ioutil"
	"testing"
	"time"
)

type logRecord struct {
	Level   string                 `json:"level"`
	Message string                 `json:"msg"`
	Time    time.Time              `json:"ts"`
	Fields  map[string]interface{} `json:"fields"`
}

func BenchmarkJSONEncoderNest(b *testing.B) {
	f := func(kv KeyValue) error {
		kv.AddString("s", "string")
		return nil
	}
	for i := 0; i < b.N; i++ {
		enc := newJSONEncoder()
		enc.Nest("nested", f)
		enc.Free()
	}
}

func BenchmarkZapJSON(b *testing.B) {
	b.RunParallel(func(pb *testing.PB) {
		for pb.Next() {
			enc := newJSONEncoder()
			enc.AddString("str", "foo")
			enc.AddInt("int", 1)
			enc.AddInt64("int64", 1)
			enc.AddFloat64("float64", 1.0)
			enc.AddString("string1", "\n")
			enc.AddString("string2", "💩")
			enc.AddString("string3", "🤔")
			enc.AddString("string4", "🙊")
			enc.AddBool("bool", true)
			enc.WriteMessage(ioutil.Discard, "debug", "fake", time.Unix(0, 0))
			enc.Free()
		}
	})
}

func BenchmarkStandardJSON(b *testing.B) {
	record := logRecord{
		Level:   "debug",
		Message: "fake",
		Time:    time.Unix(0, 0),
		Fields: map[string]interface{}{

Review comment (performance):
Good point, replaced with something that is more realistic by having an inline closure + use of a variable as the parameter (which actually makes it ~10ns slower).


Code after:
import (
	"encoding/json"
	"io/ioutil"
	"testing"
	"time"
)

type logRecord struct {
	Level   string                 `json:"level"`
	Message string                 `json:"msg"`
	Time    time.Time              `json:"ts"`
	Fields  map[string]interface{} `json:"fields"`
}

var s string

func BenchmarkJSONEncoderNest(b *testing.B) {
	for i := 0; i < b.N; i++ {
		enc := newJSONEncoder()
		enc.Nest("nested", func(kv KeyValue) error {
			kv.AddInt("i", i)
			return nil
		})
		enc.Free()
	}
}

func BenchmarkZapJSON(b *testing.B) {
	b.RunParallel(func(pb *testing.PB) {
		for pb.Next() {
			enc := newJSONEncoder()
			enc.AddString("str", "foo")
			enc.AddInt("int", 1)
			enc.AddInt64("int64", 1)
			enc.AddFloat64("float64", 1.0)
			enc.AddString("string1", "\n")
			enc.AddString("string2", "💩")
			enc.AddString("string3", "🤔")
			enc.AddString("string4", "🙊")
			enc.AddBool("bool", true)
			enc.WriteMessage(ioutil.Discard, "debug", "fake", time.Unix(0, 0))
			enc.Free()
		}
	})
}

func BenchmarkStandardJSON(b *testing.B) {
	record := logRecord{
		Level:   "debug",
		Message: "fake",
		Time:    time.Unix(0, 0),
Repository: uber-go/zap
File: json_options.go (Go)

Code before:
package zap

import "time"

// JSONOption is used to set options for a JSON encoder.
type JSONOption interface {
	Apply(*jsonEncoder)
}

// A MessageFormatter defines how to convert a log message into a Field.
type MessageFormatter func(string) Field

// Apply implements JSONOption.
func (mf MessageFormatter) Apply(enc *jsonEncoder) {
	enc.messageF = mf
}

// MessageKey encodes log messages under the provided key.
func MessageKey(key string) MessageFormatter {
	return MessageFormatter(func(msg string) Field {
		return String(key, msg)
	})
}

// NoMessage drops the message field altogether. It's sometimes useful for
// testing.
func NoMessage() MessageFormatter {
	return MessageFormatter(func(_ string) Field {
		return Skip()
	})
}

Review comment (suggestion):
Unexporting `Apply` definitely makes sense...not sure why I exported it to begin with. We should keep `JSONOption` exported, though, since it'll be part of the public `NewJSONEncoder` signature.


Code after:
package zap

import "time"

// JSONOption is used to set options for a JSON encoder.
type JSONOption interface {
	apply(*jsonEncoder)
}

// A MessageFormatter defines how to convert a log message into a Field.
type MessageFormatter func(string) Field

func (mf MessageFormatter) apply(enc *jsonEncoder) {
	enc.messageF = mf
}

// MessageKey encodes log messages under the provided key.
func MessageKey(key string) MessageFormatter {
	return MessageFormatter(func(msg string) Field {
		return String(key, msg)
	})
}

// A TimeFormatter defines how to convert the time of a log entry into a Field.
type TimeFormatter func(time.Time) Field

func (tf TimeFormatter) apply(enc *jsonEncoder) {
	enc.timeF = tf
}

// EpochFormatter uses the Time field (floating-point seconds since epoch) to
Repository: uber-go/zap
File: encoder.go (Go)

Code before:
// in the Software without restriction, including without limitation the rights
// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
// copies of the Software, and to permit persons to whom the Software is
// furnished to do so, subject to the following conditions:
//
// The above copyright notice and this permission notice shall be included in
// all copies or substantial portions of the Software.
//
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
// THE SOFTWARE.

package zap

import (
	"io"
	"time"
)

// encoder is a format-agnostic interface for all log field encoders. It's not
// safe for concurrent use.
type encoder interface {
	KeyValue

	AddFields([]Field)
	Clone() encoder
	Free()
	WriteEntry(io.Writer, string, Level, time.Time) error
}


Review comment (suggestion):
If we are exporting this, let's add comments on the below methods, especially `Clone`/`Free` since we're expecting the implementors to do the pooling.


Code after:
// in the Software without restriction, including without limitation the rights
// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
// copies of the Software, and to permit persons to whom the Software is
// furnished to do so, subject to the following conditions:
//
// The above copyright notice and this permission notice shall be included in
// all copies or substantial portions of the Software.
//
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
// THE SOFTWARE.

package zap

import (
	"io"
	"time"
)

// Encoder is a format-agnostic interface for all log field encoders. It's not
// safe for concurrent use.
type Encoder interface {
	KeyValue

	AddFields([]Field)
	Clone() Encoder
	Free()
	WriteEntry(io.Writer, string, Level, time.Time) error
}
Repository: uber-go/zap
File: tee_test.go (Go)

Code before:

	n, err := ws.Write([]byte("fail"))
	assert.Error(t, err, "Expected error from call to a writer that failed")
	assert.Equal(t, 3, n, "Short writer should drop a byte")
}

func TestTeeSync_PropagatesErrors(t *testing.T) {
	badsink := &unsyncable{}
	badsink.SetError(errors.New("sink is full"))
	ws := Tee(Discard, badsink)

	assert.Error(t, ws.Sync(), "Expected sync error to propagate")
}

func TestTeeSync_NoErrorsOnDiscard(t *testing.T) {
	ws := Tee(Discard)
	assert.NoError(t, ws.Sync(), "Expected error-free sync to /dev/null")
}

func TestMultiError_NilForNoArgs(t *testing.T) {
	assert.NoError(t, newMultiError(), "no errors should cause a nil error")
}

func TestNewMultiError_WrapsStrings(t *testing.T) {
	err := newMultiError(errors.New("battlestar"), errors.New("galactaca"))
	assert.Error(t, err)
	assert.Contains(t, err.Error(), "battlestar")
	assert.Contains(t, err.Error(), "galactaca")
}

type unsyncable struct {
	bytes.Buffer
	spywrite.Syncer
}


Review comment (bug):
should we have a test for `Sync` with multiple writers where the first one fails (similar to the Write)


Code after:

	_, err := ws.Write([]byte("fail"))
	assert.Error(t, err, "Expected error from call to a writer that failed")
	assert.Equal(t, []byte("fail"), second.Bytes(), "Expected second sink to be written after first error")
}

func TestTeeSync_PropagatesErrors(t *testing.T) {
	badsink := &syncSpy{}
	badsink.SetError(errors.New("sink is full"))
	ws := Tee(Discard, badsink)

	assert.Error(t, ws.Sync(), "Expected sync error to propagate")
}

func TestTeeSync_NoErrorsOnDiscard(t *testing.T) {
	ws := Tee(Discard)
	assert.NoError(t, ws.Sync(), "Expected error-free sync to /dev/null")
}

func TestMultiError_WrapsStrings(t *testing.T) {
	err := multiError{errors.New("battlestar"), errors.New("galactaca")}
	assert.Error(t, err)
	assert.Contains(t, err.Error(), "battlestar")
	assert.Contains(t, err.Error(), "galactaca")
}

func TestTeeSync_AllCalled(t *testing.T) {
	failedsink := &syncSpy{}
	second := &syncSpy{}

	failedsink.SetError(errors.New("disposal broken"))
	ws := Tee(failedsink, second)

	assert.Error(t, ws.Sync(), "Expected first sink to fail")
	assert.True(t, second.Called(), "Expected call even with first failure")
}

type syncSpy struct {
	bytes.Buffer
	spywrite.Syncer
}
Repository: uber-go/zap
File: meta.go (Go)

Code before:
// Clone creates a copy of the meta struct. It deep-copies the encoder, but not
// the hooks (since they rarely change).
func (m Meta) Clone() Meta {
	m.Encoder = m.Encoder.Clone()
	return m
}

// Enabled returns true if logging a message at a particular level is enabled.
func (m Meta) Enabled(lvl Level, msg string) bool {
	return lvl >= m.Level()
}

// Check returns a CheckedMessage logging the given message is Enabled, nil
// otherwise.
func (m Meta) Check(log Logger, lvl Level, msg string) *CheckedMessage {
	switch lvl {
	case PanicLevel, FatalLevel:
		// Panic and Fatal should always cause a panic/exit, even if the level
		// is disabled.
		break
	default:
		if !m.Enabled(lvl, msg) {
			return nil
		}
	}
	return NewCheckedMessage(log, lvl, msg)
}


Review comment (suggestion):
Unless there's another use case for it, I'm all for un-exporting `NewCheckedMessage`. Existing callers should all have access to a `Meta`, which they can use instead.


Code after:
// Clone creates a copy of the meta struct. It deep-copies the encoder, but not
// the hooks (since they rarely change).
func (m Meta) Clone() Meta {
	m.Encoder = m.Encoder.Clone()
	return m
}

// Enabled returns true if logging a message at a particular level is enabled.
func (m Meta) Enabled(lvl Level) bool {
	return lvl >= m.Level()
}

// Check returns a CheckedMessage logging the given message is Enabled, nil
// otherwise.
func (m Meta) Check(log Logger, lvl Level, msg string) *CheckedMessage {
	switch lvl {
	case PanicLevel, FatalLevel:
		// Panic and Fatal should always cause a panic/exit, even if the level
		// is disabled.
		break
	default:
		if !m.Enabled(lvl) {
			return nil
		}
	}
	return NewCheckedMessage(log, lvl, msg)
}
Repository: uber-go/zap
File: logger.go (Go)

Code before:
}

func (log *logger) DPanic(msg string, fields ...zapcore.Field) {
	if ce := log.check(DPanicLevel, msg); ce != nil {
		ce.Write(fields...)
	}
}

func (log *logger) Panic(msg string, fields ...zapcore.Field) {
	if ce := log.check(PanicLevel, msg); ce != nil {
		ce.Write(fields...)
	}
}

func (log *logger) Fatal(msg string, fields ...zapcore.Field) {
	if ce := log.check(FatalLevel, msg); ce != nil {
		ce.Write(fields...)
	}
}

// Facility returns the destination that logs entries are written to.
func (log *logger) Facility() zapcore.Facility {
	return log.fac
}

func (log *logger) clone() *logger {
	return &logger{
		fac:         log.fac,
		name:        log.name,
		development: log.development,
		errorOutput: log.errorOutput,
		addCaller:   log.addCaller,
		addStack:    log.addStack,
		callerSkip:  log.callerSkip,
	}
}


Review comment (suggestion):
It looks like this is a straight up copy. We have a couple of other options here that avoid listing every field (and possibly missing out on a new field that's added later):
```go
func (log logger) clone() *logger {
  // Since this is a method on the value, a copy was already created and passed in as log.
  return &log
}
```

```go
func (log *logger) clone() *logger {
  copy := *log
  return &copy
}
```

Code after:
}

func (log *logger) DPanic(msg string, fields ...zapcore.Field) {
	if ce := log.check(DPanicLevel, msg); ce != nil {
		ce.Write(fields...)
	}
}

func (log *logger) Panic(msg string, fields ...zapcore.Field) {
	if ce := log.check(PanicLevel, msg); ce != nil {
		ce.Write(fields...)
	}
}

func (log *logger) Fatal(msg string, fields ...zapcore.Field) {
	if ce := log.check(FatalLevel, msg); ce != nil {
		ce.Write(fields...)
	}
}

// Facility returns the destination that logs entries are written to.
func (log *logger) Facility() zapcore.Facility {
	return log.fac
}

func (log *logger) clone() *logger {
	copy := *log
	return &copy
}
Repository: uber-go/zap
File: flag_test.go (Go)

Code before:
	defer func() { flag.CommandLine = origCommandLine }()

	level := LevelFlag("level", InfoLevel, "")
	tc.run(t, flag.CommandLine, level)
}

func (tc flagTestCase) RunExplicitSet(t testing.TB) {
	var lvl zapcore.Level
	set := flag.NewFlagSet("test", flag.ContinueOnError)
	set.Var(&lvl, "level", "minimum enabled logging level")
	tc.run(t, set, &lvl)
}

func (tc flagTestCase) run(t testing.TB, set *flag.FlagSet, actual *zapcore.Level) {
	err := set.Parse(tc.args)
	if tc.wantErr {
		assert.Error(t, err, "Parse(%v) should fail.", tc.args)
		return
	}
	if assert.NoError(t, err, "Parse(%v) should succeed.", tc.args) {
		assert.Equal(t, tc.wantLevel, *actual, "Level mismatch.")
	}
}

func TestLevelFlag(t *testing.T) {
	tests := []flagTestCase{
		{
			args:      nil,
			wantLevel: zapcore.InfoLevel,
		},
		{
			args:    []string{"--level", "unknown"},
			wantErr: true,
		},
		{
			args:      []string{"--level", "error"},
			wantLevel: zapcore.ErrorLevel,
		},
	}

	for _, tt := range tests {
		tt.RunExplicitSet(t)
		tt.RunImplicitSet(t)
	}
}


Review comment (bug):
It doesn't fit in your current test harness shape, but: test two differently named levels, and that they don't collide (e.g. the avoided pointer sharing in the `LevelFlag` function, I could see that easily getting broken by a refactor).

This is a realistic use case, e.g. varying console vs file logging levels.

Code after:
	defer func() { flag.CommandLine = origCommandLine }()

	level := LevelFlag("level", InfoLevel, "")
	tc.run(t, flag.CommandLine, level)
}

func (tc flagTestCase) runExplicitSet(t testing.TB) {
	var lvl zapcore.Level
	set := flag.NewFlagSet("test", flag.ContinueOnError)
	set.Var(&lvl, "level", "minimum enabled logging level")
	tc.run(t, set, &lvl)
}

func (tc flagTestCase) run(t testing.TB, set *flag.FlagSet, actual *zapcore.Level) {
	err := set.Parse(tc.args)
	if tc.wantErr {
		assert.Error(t, err, "Parse(%v) should fail.", tc.args)
		return
	}
	if assert.NoError(t, err, "Parse(%v) should succeed.", tc.args) {
		assert.Equal(t, tc.wantLevel, *actual, "Level mismatch.")
	}
}

func TestLevelFlag(t *testing.T) {
	tests := []flagTestCase{
		{
			args:      nil,
			wantLevel: zapcore.InfoLevel,
		},
		{
			args:    []string{"--level", "unknown"},
			wantErr: true,
		},
		{
			args:      []string{"--level", "error"},
			wantLevel: zapcore.ErrorLevel,
		},
	}

	for _, tt := range tests {
		tt.runExplicitSet(t)
		tt.runImplicitSet(t)
	}
}

func TestLevelFlagsAreIndependent(t *testing.T) {
	origCommandLine := flag.CommandLine
	flag.CommandLine = flag.NewFlagSet("test", flag.ContinueOnError)
	flag.CommandLine.SetOutput(ioutil.Discard)
	defer func() { flag.CommandLine = origCommandLine }()