Make zaptest.NewTestingWriter public

[dimmo]

Add more flexibility in configuring zap logger for tests.

The default zapcore.Core, which is created in zaptest.NewLogger() may not be suitable for all use-cases.

func NewLogger(t TestingT, opts ...LoggerOption) *zap.Logger {
...
	return zap.New(
		zapcore.NewCore(
			zapcore.NewConsoleEncoder(zap.NewDevelopmentEncoderConfig()),
			writer,
			cfg.Level,
		),
		zapOptions...,
	)

E.g., we may need custom encoder or encoder config.

This PR allows us to do such customization:

writer := zaptest.NewTestingWriter(t)
core := zapcore.NewCore(encoder, writer, level)
logger := zap.New(core, zap.AddCaller())

[CLAassistant]

All committers have signed the CLA.

[r-hang]

As an alternative to making newTestingWriter public would it help to configure zaptest.NewLogger?

func TestConfigure(t *testing.T) {
	opt := zap.WrapCore(func(zapcore.Core) zapcore.Core {
		return zapcore.NewNopCore()
	})
	logger := zaptest.NewLogger(t, zaptest.WrapOptions(opt))
	logger.Info("a")
}

[dimmo]

@r-hang , thanks for your review.
But i can't see the way it helps.

In the NewLogger function, we have,

func NewLogger(t TestingT, opts ...LoggerOption) *zap.Logger {
...
	return zap.New(
		zapcore.NewCore(
			zapcore.NewConsoleEncoder(zap.NewDevelopmentEncoderConfig()),
			writer,    // <<< we need this writer
			cfg.Level,
		),
		zapOptions...,   // <<< but here whole core will be replaced by zap.WrapCore()
	)
}

zap.WrapCore() just replaces the whole log.core value, and trows away writer, too.
So, we can't combine our custom encoder with a writer which writes to *testing.T.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (d27427d) 98.28% compared to head (8dcabdb) 98.42%.
Report is 1 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1399      +/-   ##
==========================================
+ Coverage   98.28%   98.42%   +0.14%     
==========================================
  Files          53       53              
  Lines        3495     3495              
==========================================
+ Hits         3435     3440       +5     
+ Misses         50       46       -4     
+ Partials       10        9       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[r-hang]

@dimmo I think I understand. Just to confirm my understanding you could replace the core or constructor a new logger today if you duplicated and hand wrote an implementation for the testingWriter but since we already have one you want to reuse that one.

I ran CI and see a go lint failure. It should be green after updating the exported comment.

[dimmo]

you could replace the core or constructor a new logger today if you duplicated and hand wrote an implementation for the testingWriter but since we already have one you want to reuse that one.

Exactly! Right now I temporarily copied implementation of newTestingWriter(),

	encoder := NewConsoleEncoder(encoderConfig, cfg.vars) // our custom encoder
	level := zap.NewAtomicLevelAt(zapcore.DebugLevel)

	var core zapcore.Core
	if cfg.developMode {
		var writer zapcore.WriteSyncer
		if cfg.t == nil {
			writer = zapcore.Lock(os.Stderr)
		} else {
			writer = newTestingWriter(cfg.t)  // this is a copy!
		}

		if term.IsTerminal(int(os.Stderr.Fd())) {
			core = NewColoredCore(encoder, writer, level)
		} else {
			core = zapcore.NewCore(encoder, writer, level)
		}
	} else {
		core = newJournaldCore(encoder, level)
	}

	logger := zap.New(core, zap.AddCaller())

I ran CI and see a go lint failure. It should be green after updating the exported comment.

Thank you.