Skip to content

Commit

Permalink
Fixed: testcases. service-name is now used (#59)
Browse files Browse the repository at this point in the history
  • Loading branch information
satyamsi committed May 9, 2019
1 parent 69caf6d commit ed3b344
Show file tree
Hide file tree
Showing 2 changed files with 94 additions and 54 deletions.
48 changes: 19 additions & 29 deletions logutils/log.go
Expand Up @@ -79,7 +79,10 @@ func newLogger(serviceName string, level string, format string, file string, fil
// Set the logger
config.Level = levelToZapLevel(level)

logger, err := initLogger(w, config)
logger, err := config.Build()
if w != nil {
logger, err = config.Build(SetOutput(w, config))
}
if err != nil {
panic(err)
}
Expand Down Expand Up @@ -197,31 +200,20 @@ func getEncoder(c zap.Config) (zapcore.Encoder, error) {
}
}

// initLogger constructs the logger from the options
func initLogger(w zapcore.WriteSyncer, conf zap.Config) (*zap.Logger, error) {

// SetOutput returns the zap option with the new sync writer
func SetOutput(w zapcore.WriteSyncer, conf zap.Config) zap.Option {
enc, err := getEncoder(conf)
if err != nil {
return nil, err
}

core := zapcore.NewCore(enc, zapcore.Lock(os.Stderr), conf.Level)

if w != nil {
core = zapcore.NewTee(
zapcore.NewCore(enc, w, conf.Level),
core,
)
panic("unknown encoding")
}

return zap.New(core), nil
return zap.WrapCore(func(zapcore.Core) zapcore.Core {
return zapcore.NewCore(enc, w, conf.Level)
})
}

// handleOutputFile handles options in log configs to redirect to file
func handleOutputFile(config *zap.Config, file string, fileOnly bool) (zapcore.WriteSyncer, error) {

var w zapcore.WriteSyncer

if file == "" {
return nil, nil
}
Expand All @@ -232,21 +224,19 @@ func handleOutputFile(config *zap.Config, file string, fileOnly bool) (zapcore.W
}
}

if file != "" {
w = zapcore.AddSync(&lumberjack.Logger{
Filename: file,
MaxSize: logFileSizeDefault,
MaxBackups: logFileNumBackups,
MaxAge: logFileAge,
Compress: true,
})
}
var w zapcore.WriteSyncer
w = zapcore.AddSync(&lumberjack.Logger{
Filename: file,
MaxSize: logFileSizeDefault,
MaxBackups: logFileNumBackups,
MaxAge: logFileAge,
})

if fileOnly {
config.OutputPaths = []string{file}
} else {
config.OutputPaths = append(config.OutputPaths, file)
return w, nil
}

config.OutputPaths = append(config.OutputPaths, file)
return w, nil
}
100 changes: 75 additions & 25 deletions logutils/log_test.go
Expand Up @@ -21,7 +21,7 @@ import (
"testing"
"time"

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

Expand Down Expand Up @@ -59,22 +59,34 @@ func captureOutAndErr(f func()) (o, e string) {
go func() {
var buf bytes.Buffer
wg.Done()
io.Copy(&buf, oreader) // nolint
_, err := io.Copy(&buf, oreader)
if err != nil {
panic(err)
}
oout <- buf.String()
}()
go func() {
var buf bytes.Buffer
wg.Done()
io.Copy(&buf, ereader) // nolint
_, err := io.Copy(&buf, ereader)
if err != nil {
panic(err)
}
eout <- buf.String()
}()
wg.Wait()

// Execute function
f()

owriter.Close() // nolint
ewriter.Close() // nolint
err = owriter.Close()
if err != nil {
panic(err)
}
err = ewriter.Close()
if err != nil {
panic(err)
}
// Return captures
return <-oout, <-eout
}
Expand All @@ -83,6 +95,7 @@ func TestConfigureWithOptions(t *testing.T) {
type args struct {
level string
format string
service string
file string
fileOnly bool
prettyTimestamp bool
Expand All @@ -99,29 +112,58 @@ func TestConfigureWithOptions(t *testing.T) {
"info",
"json",
"",
"",
false,
false,
},
iterations: 20,
iterations: 2,
want: "",
},
{
name: "file only",
args: args{
"info",
"json",
"",
"/tmp/some-log-file",
true,
false,
},
iterations: 20,
iterations: 3,
want: "",
},
{
name: "service name on json",
args: args{
"info",
"stackdriver",
"some-bizzare-bad-named-service-1",
"",
false,
false,
},
iterations: 1,
want: "",
},
{
name: "service name on stackdriver",
args: args{
"info",
"json",
"some-bizzare-bad-named-service-2",
"",
false,
false,
},
iterations: 1,
want: "",
},
{
name: "tee file logging and stdout with wraps in files",
args: args{
"info",
"json",
"",
"/tmp/some-tee-log-file",
false,
false,
Expand All @@ -134,9 +176,13 @@ func TestConfigureWithOptions(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {

minBytesPrinted := 0
ro, _ := captureOutAndErr(func() {
ro, re := captureOutAndErr(func() {

ConfigureWithOptions(tt.args.level, tt.args.format, tt.args.file, tt.args.fileOnly, tt.args.prettyTimestamp)
if tt.args.service != "" {
ConfigureWithName(tt.args.service, tt.args.level, tt.args.format)
} else {
ConfigureWithOptions(tt.args.level, tt.args.format, tt.args.file, tt.args.fileOnly, tt.args.prettyTimestamp)
}

for i := 0; i < tt.iterations; i++ {
buf := fmt.Sprintf("%80d - hello", i)
Expand All @@ -145,42 +191,46 @@ func TestConfigureWithOptions(t *testing.T) {
}
})

// On fileOnly case, nothing is expected on stderr as well.
// validate nothing is printed on stdout
assertions.ShouldEqual(0, len(ro))
if tt.args.fileOnly {
minBytesPrinted = 0
// validate nothing is printed on stderr in fileOnly case
assertions.ShouldEqual(0, len(re))
} else {
// validate we have printed more than minBytesPrinted on stderr
assertions.ShouldBeLessThan(minBytesPrinted, len(re))
}

// validate nothing is printed on stdout
assert.Equal(t, 0, len(ro))
// validate we have printed more than minBytesPrinted on stderr
// assert.LessOrEqual(t, minBytesPrinted, len(re))
if tt.args.service != "" {
assertions.ShouldContainSubstring(re, tt.args.service)
}

if tt.args.file != "" {

// numFilesExpected := 1
// if minBytesPrinted > logFileSizeDefault*1024*1024 {
// numFilesExpected += logFileNumBackups
// }
numFilesExpected := 1
if minBytesPrinted > logFileSizeDefault*1024*1024 {
numFilesExpected += logFileNumBackups
}

// Wait for one second as file may not have been deleted.
time.Sleep(time.Second)

files, err := filepath.Glob(tt.args.file + "*")
assert.ObjectsAreEqual(err, nil)
assertions.ShouldBeNil(err)

// logging to files tests wraparound. we should have
// assert.Equal(t, numFilesExpected, len(files))
assertions.ShouldEqual(numFilesExpected, len(files))

for _, f := range files {

_, err := os.Stat(f)
assert.ObjectsAreEqual(err, nil)
fi, err := os.Stat(f)
assertions.ShouldBeNil(err)

// If we printed into a file, the file should be less than wrapped size.
// assert.Less(t, fi.Size(), int64(logFileSizeDefault*1024*1024))
assertions.ShouldBeLessThan(t, fi.Size(), int64(logFileSizeDefault*1024*1024))

err = os.Remove(f)
assert.ObjectsAreEqual(err, nil)
assertions.ShouldBeNil(err)
}
}
})
Expand Down

0 comments on commit ed3b344

Please sign in to comment.