Additional tests, single interface for multiple writers

Signed-off-by: Javier Romero <[email protected]>

internal/app/run.go

@@ -50,8 +50,8 @@ func (i *Image) Run(ctx context.Context, docker *client.Client, ports []string)
 		ctx,
 		docker,
 		ctr.ID,
-		logging.GetOutWriter(i.Logger),
-		logging.GetErrorWriter(i.Logger),
+		logging.GetWriterForLevel(i.Logger, logging.InfoLevel),
+		logging.GetWriterForLevel(i.Logger, logging.ErrorLevel),
 	); err != nil {
 		return errors.Wrap(err, "run container")
 	}

internal/blob/downloader.go

@@ -141,7 +141,7 @@ func (d *downloader) downloadAsStream(ctx context.Context, uri string, etag stri
 
 	if resp.StatusCode >= 200 && resp.StatusCode < 300 {
 		d.logger.Infof("Downloading from %s", style.Symbol(uri))
-		return withProgress(logging.GetOutWriter(d.logger), resp.Body, resp.ContentLength), resp.Header.Get("Etag"), nil
+		return withProgress(logging.GetWriterForLevel(d.logger, logging.InfoLevel), resp.Body, resp.ContentLength), resp.Header.Get("Etag"), nil
 	}
 
 	if resp.StatusCode == 304 {

internal/build/phase.go

@@ -172,8 +172,8 @@ func (p *Phase) Run(ctx context.Context) error {
 		ctx,
 		p.docker,
 		p.ctr.ID,
-		logging.NewPrefixWriter(logging.GetOutWriter(p.logger), p.name),
-		logging.NewPrefixWriter(logging.GetErrorWriter(p.logger), p.name),
+		logging.NewPrefixWriter(logging.GetWriterForLevel(p.logger, logging.InfoLevel), p.name),
+		logging.NewPrefixWriter(logging.GetWriterForLevel(p.logger, logging.ErrorLevel), p.name),
 	)
 }
 

internal/image/fetcher.go

@@ -87,7 +87,7 @@ func (f *Fetcher) pullImage(ctx context.Context, imageID string) error {
 	if err != nil {
 		return err
 	}
-	writer := logging.GetOutWriter(f.logger)
+	writer := logging.GetWriterForLevel(f.logger, logging.InfoLevel)
 	type descriptor interface {
 		Fd() uintptr
 	}

internal/logging/logging.go

@@ -11,6 +11,7 @@ import (
 	"github.com/apex/log"
 
 	"github.com/buildpacks/pack/internal/style"
+	"github.com/buildpacks/pack/logging"
 )
 
 const (
@@ -60,19 +61,23 @@ func NewLogWithWriters(stdout, stderr io.Writer, opts ...func(*LogWithWriters))
 	return lw
 }
 
-func (lw *LogWithWriters) HandleLog(e *log.Entry) error {
-	lw.Lock()
-	defer lw.Unlock()
-
-	if lw.Level > e.Level {
-		return nil
+func (lw *LogWithWriters) WriterForLevel(level logging.Level) io.Writer {
+	if lw.Level > log.Level(level) {
+		return ioutil.Discard
 	}
 
-	writer := lw.out
-	if e.Level == log.ErrorLevel {
-		writer = lw.errOut
+	if level == logging.ErrorLevel {
+		return lw.errOut
 	}
 
+	return lw.out
+}
+
+func (lw *LogWithWriters) HandleLog(e *log.Entry) error {
+	lw.Lock()
+	defer lw.Unlock()
+
+	writer := lw.WriterForLevel(logging.Level(e.Level))
 	if lw.wantTime {
 		ts := lw.clock().Format(timeFmt)
 		_, _ = fmt.Fprint(writer, appendMissingLineFeed(fmt.Sprintf("%s %s%s", ts, formatLevel(e.Level), e.Message)))
@@ -88,20 +93,6 @@ func (lw *LogWithWriters) Writer() io.Writer {
 	return lw.out
 }
 
-// ErrorWriter returns the writer for error messages.
-func (lw *LogWithWriters) ErrorWriter() io.Writer {
-	return lw.errOut
-}
-
-// OutWriter returns the writer for standard messages.
-func (lw *LogWithWriters) OutWriter() io.Writer {
-	if lw.Level >= quietLevel {
-		return ioutil.Discard
-	}
-
-	return lw.out
-}
-
 func (lw *LogWithWriters) WantTime(f bool) {
 	lw.wantTime = f
 }

internal/logging/logging_test.go

@@ -12,6 +12,7 @@ import (
 	"github.com/sclevine/spec"
 
 	ilogging "github.com/buildpacks/pack/internal/logging"
+	"github.com/buildpacks/pack/logging"
 	h "github.com/buildpacks/pack/testhelpers"
 )
 
@@ -32,8 +33,8 @@ func mockStd() (*color.Console, func() string) {
 	}
 }
 
-func TestPackCLILogger(t *testing.T) {
-	spec.Run(t, "PackCLILogger", func(t *testing.T, when spec.G, it spec.S) {
+func TestLogWithWriters(t *testing.T) {
+	spec.Run(t, "LogWithWriters", func(t *testing.T, when spec.G, it spec.S) {
 		var (
 			logger           *ilogging.LogWithWriters
 			outCons, errCons *color.Console
@@ -55,16 +56,43 @@ func TestPackCLILogger(t *testing.T) {
 				h.AssertEq(t, fOut(), "\x1b[94mtest\x1b[0m\n")
 			})
 
+			it("will not log debug messages", func() {
+				logger.Debug("debug_")
+				logger.Debugf("debugf")
+
+				output := fOut()
+				h.AssertNotContains(t, output, "debug_\n")
+				h.AssertNotContains(t, output, "debugf\n")
+			})
+
+			it("logs info and warning messages to standard writer", func() {
+				logger.Info("info_")
+				logger.Infof("infof")
+				logger.Warn("warn_")
+				logger.Warnf("warnf")
+
+				output := fOut()
+				h.AssertContains(t, output, "info_\n")
+				h.AssertContains(t, output, "infof\n")
+				h.AssertContains(t, output, "warn_\n")
+				h.AssertContains(t, output, "warnf\n")
+			})
+
 			it("logs error to error writer", func() {
-				logger.Error("oh no")
+				logger.Error("error_")
+				logger.Errorf("errorf")
 
-				h.AssertContains(t, fErr(), "oh no\n")
+				output := fErr()
+				h.AssertContains(t, output, "error_\n")
+				h.AssertContains(t, output, "errorf\n")
 			})
 
 			it("will return correct writers", func() {
 				h.AssertSameInstance(t, logger.Writer(), outCons)
-				h.AssertSameInstance(t, logger.OutWriter(), outCons)
-				h.AssertSameInstance(t, logger.ErrorWriter(), errCons)
+				h.AssertSameInstance(t, logger.WriterForLevel(logging.DebugLevel), ioutil.Discard)
+				h.AssertSameInstance(t, logger.WriterForLevel(logging.InfoLevel), outCons)
+				h.AssertSameInstance(t, logger.WriterForLevel(logging.WarnLevel), outCons)
+				h.AssertSameInstance(t, logger.WriterForLevel(logging.ErrorLevel), errCons)
 			})
 		})
 
@@ -89,30 +117,83 @@ func TestPackCLILogger(t *testing.T) {
 				logger.WantQuiet(true)
 			})
 
-			it("will not show debug or info messages", func() {
-				logger.Debug("hello")
-				logger.Debugf("there")
-				logger.Info("test")
-
-				h.AssertContains(t, fOut(), "")
+			it("will not log debug or info messages", func() {
+				logger.Debug("debug_")
+				logger.Debugf("debugf")
+				logger.Info("info_")
+				logger.Infof("infof")
+
+				output := fOut()
+				h.AssertNotContains(t, output, "debug_\n")
+				h.AssertNotContains(t, output, "debugf\n")
+				h.AssertNotContains(t, output, "info_\n")
+				h.AssertNotContains(t, output, "infof\n")
 			})
 
 			it("logs warnings to standard writer", func() {
-				logger.Warn("oh no")
+				logger.Warn("warn_")
+				logger.Warnf("warnf")
+
+				output := fOut()
+				h.AssertContains(t, output, "warn_\n")
+				h.AssertContains(t, output, "warnf\n")
+			})
+
+			it("logs error to error writer", func() {
+				logger.Error("error_")
+				logger.Errorf("errorf")
+
+				output := fErr()
+				h.AssertContains(t, output, "error_\n")
+				h.AssertContains(t, output, "errorf\n")
+			})
+
+			it("will return correct writers", func() {
+				h.AssertSameInstance(t, logger.Writer(), outCons)
+				h.AssertSameInstance(t, logger.WriterForLevel(logging.DebugLevel), ioutil.Discard)
+				h.AssertSameInstance(t, logger.WriterForLevel(logging.InfoLevel), ioutil.Discard)
+				h.AssertSameInstance(t, logger.WriterForLevel(logging.WarnLevel), outCons)
+				h.AssertSameInstance(t, logger.WriterForLevel(logging.ErrorLevel), errCons)
+			})
+		})
+
+		when("verbose is set to true", func() {
+			it.Before(func() {
+				logger.WantVerbose(true)
+			})
 
-				h.AssertContains(t, fOut(), "oh no\n")
+			it("all messages are logged", func() {
+				logger.Debug("debug_")
+				logger.Debugf("debugf")
+				logger.Info("info_")
+				logger.Infof("infof")
+				logger.Warn("warn_")
+				logger.Warnf("warnf")
+
+				output := fOut()
+				h.AssertContains(t, output, "debug_")
+				h.AssertContains(t, output, "debugf")
+				h.AssertContains(t, output, "info_")
+				h.AssertContains(t, output, "infof")
+				h.AssertContains(t, output, "warn_")
+				h.AssertContains(t, output, "warnf")
 			})
 
 			it("logs error to error writer", func() {
-				logger.Error("oh no")
+				logger.Error("error_")
+				logger.Errorf("errorf")
 
-				h.AssertContains(t, fErr(), "oh no\n")
+				output := fErr()
+				h.AssertContains(t, output, "error_\n")
+				h.AssertContains(t, output, "errorf\n")
 			})
 
 			it("will return correct writers", func() {
 				h.AssertSameInstance(t, logger.Writer(), outCons)
-				h.AssertSameInstance(t, logger.OutWriter(), ioutil.Discard)
-				h.AssertSameInstance(t, logger.ErrorWriter(), errCons)
+				h.AssertSameInstance(t, logger.WriterForLevel(logging.DebugLevel), outCons)
+				h.AssertSameInstance(t, logger.WriterForLevel(logging.InfoLevel), outCons)
+				h.AssertSameInstance(t, logger.WriterForLevel(logging.WarnLevel), outCons)
+				h.AssertSameInstance(t, logger.WriterForLevel(logging.ErrorLevel), errCons)
 			})
 		})
 

logging/logger.go → logging/logging.go

@@ -8,6 +8,15 @@ import (
 	"github.com/buildpacks/pack/internal/style"
 )
 
+type Level int
+
+const (
+	DebugLevel Level = iota
+	InfoLevel
+	WarnLevel
+	ErrorLevel
+)
+
 // Logger defines behavior required by a logging package used by pack libraries
 type Logger interface {
 	Debug(msg string)
@@ -27,32 +36,20 @@ type Logger interface {
 	IsVerbose() bool
 }
 
-// WithErrorWriter is an optional interface for loggers that want to support a separate writer for errors and standard logging.
-type WithErrorWriter interface {
-	ErrorWriter() io.Writer
+// WithSelectableWriter is an optional interface for loggers that want to support a separate writer per log level.
+type WithSelectableWriter interface {
+	WriterForLevel(level Level) io.Writer
 }
 
-// WithOutWriter is an optional interface what will return a writer that will write raw output if quiet is false.
-type WithOutWriter interface {
-	OutWriter() io.Writer
-}
-
-// GetErrorWriter will return an ErrorWriter, typically stderr if one exists, otherwise the standard logger writer
-// will be returned.
-func GetErrorWriter(l Logger) io.Writer {
-	if er, ok := l.(WithErrorWriter); ok {
-		return er.ErrorWriter()
+// GetWriterForLevel retrieves the appropriate Writer for the log level provided.
+//
+// See WithSelectableWriter
+func GetWriterForLevel(logger Logger, level Level) io.Writer {
+	if er, ok := logger.(WithSelectableWriter); ok {
+		return er.WriterForLevel(level)
 	}
-	return l.Writer()
-}
 
-// GetOutWriter returns a writer
-// See WithOutWriter
-func GetOutWriter(l Logger) io.Writer {
-	if ew, ok := l.(WithOutWriter); ok {
-		return ew.OutWriter()
-	}
-	return l.Writer()
+	return logger.Writer()
 }
 
 // PrefixWriter will prefix writes