Merge pull request #2599 from jeanlaurent/extract-recording

Remove Logrus follow-up

commands/config.go

@@ -12,7 +12,7 @@ import (
 func cmdConfig(c CommandLine, api libmachine.API) error {
 	// Ensure that log messages always go to stderr when this command is
 	// being run (it is intended to be run in a subshell)
-	log.SetOut(os.Stderr)
+	log.SetOutWriter(os.Stderr)
 
 	if len(c.Args()) != 1 {
 		return ErrExpectedOneMachine

commands/env.go

@@ -50,7 +50,7 @@ func cmdEnv(c CommandLine, api libmachine.API) error {
 
 	// Ensure that log messages always go to stderr when this command is
 	// being run (it is intended to be run in a subshell)
-	log.SetOut(os.Stderr)
+	log.SetOutWriter(os.Stderr)
 
 	if c.Bool("unset") {
 		shellCfg, err = shellCfgUnset(c, api)

libmachine/drivers/plugin/localbinary/plugin_test.go

@@ -78,16 +78,17 @@ func TestLocalBinaryPluginClose(t *testing.T) {
 }
 
 func TestExecServer(t *testing.T) {
-	logReader, logWriter := io.Pipe()
+	logOutReader, logOutWriter := io.Pipe()
+	logErrReader, logErrWriter := io.Pipe()
 
 	log.SetDebug(true)
-	log.SetOut(logWriter)
+	log.SetOutWriter(logOutWriter)
-	log.SetErr(logWriter)
+	log.SetErrWriter(logErrWriter)
 
 	defer func() {
 		log.SetDebug(false)
-		log.SetOut(os.Stdout)
+		log.SetOutWriter(os.Stdout)
-		log.SetErr(os.Stderr)
+		log.SetErrWriter(os.Stderr)
 	}()
 
 	stdoutReader, stdoutWriter := io.Pipe()
@@ -113,7 +114,8 @@ func TestExecServer(t *testing.T) {
 		finalErr <- lbp.execServer()
 	}()
 
-	logScanner := bufio.NewScanner(logReader)
+	logOutScanner := bufio.NewScanner(logOutReader)
+	logErrScanner := bufio.NewScanner(logErrReader)
 
 	// Write the ip address
 	expectedAddr := "127.0.0.1:12345"
@@ -132,8 +134,8 @@ func TestExecServer(t *testing.T) {
 	}
 
 	expectedOut := fmt.Sprintf(pluginOut, machineName, expectedPluginOut)
-	if logScanner.Scan(); logScanner.Text() != expectedOut {
+	if logOutScanner.Scan(); logOutScanner.Text() != expectedOut {
-		t.Fatalf("Output written to log was not what we expected\nexpected: %s\nactual:   %s", expectedOut, logScanner.Text())
+		t.Fatalf("Output written to log was not what we expected\nexpected: %s\nactual:   %s", expectedOut, logOutScanner.Text())
 	}
 
 	// Write a log in stderr
@@ -143,8 +145,8 @@ func TestExecServer(t *testing.T) {
 	}
 
 	expectedErr := fmt.Sprintf(pluginErr, machineName, expectedPluginErr)
-	if logScanner.Scan(); logScanner.Text() != expectedErr {
+	if logErrScanner.Scan(); logErrScanner.Text() != expectedErr {
-		t.Fatalf("Error written to log was not what we expected\nexpected: %s\nactual:   %s", expectedErr, logScanner.Text())
+		t.Fatalf("Error written to log was not what we expected\nexpected: %s\nactual:   %s", expectedErr, logErrScanner.Text())
 	}
 
 	lbp.Close()

libmachine/log/fmt_machine_logger.go

@@ -4,25 +4,22 @@ import (
 	"fmt"
 	"io"
 	"os"
-	"sync"
 )
 
 type FmtMachineLogger struct {
-	out         io.Writer
+	outWriter io.Writer
-	err         io.Writer
+	errWriter io.Writer
-	debug       bool
+	debug     bool
-	historyLock *sync.Mutex
+	history   *HistoryRecorder
-	history     []string
 }
 
 // NewFmtMachineLogger creates a MachineLogger implementation used by the drivers
 func NewFmtMachineLogger() MachineLogger {
 	return &FmtMachineLogger{
-		out:         os.Stdout,
+		outWriter: os.Stdout,
-		err:         os.Stderr,
+		errWriter: os.Stderr,
-		debug:       false,
+		debug:     false,
-		historyLock: &sync.Mutex{},
+		history:   NewHistoryRecorder(),
-		history:     []string{},
 	}
 }
 
@@ -30,82 +27,70 @@ func (ml *FmtMachineLogger) SetDebug(debug bool) {
 	ml.debug = debug
 }
 
-func (ml *FmtMachineLogger) SetOut(out io.Writer) {
+func (ml *FmtMachineLogger) SetOutWriter(out io.Writer) {
-	ml.out = out
+	ml.outWriter = out
 }
 
-func (ml *FmtMachineLogger) SetErr(err io.Writer) {
+func (ml *FmtMachineLogger) SetErrWriter(err io.Writer) {
-	ml.err = err
+	ml.errWriter = err
 }
 
 func (ml *FmtMachineLogger) Debug(args ...interface{}) {
-	ml.record(args...)
+	ml.history.Record(args...)
 	if ml.debug {
-		fmt.Fprintln(ml.err, args...)
+		fmt.Fprintln(ml.errWriter, args...)
 	}
 }
 
 func (ml *FmtMachineLogger) Debugf(fmtString string, args ...interface{}) {
-	ml.recordf(fmtString, args...)
+	ml.history.Recordf(fmtString, args...)
 	if ml.debug {
-		fmt.Fprintf(ml.err, fmtString+"\n", args...)
+		fmt.Fprintf(ml.errWriter, fmtString+"\n", args...)
 	}
 }
 
 func (ml *FmtMachineLogger) Error(args ...interface{}) {
-	ml.record(args...)
+	ml.history.Record(args...)
-	fmt.Fprintln(ml.err, args...)
+	fmt.Fprintln(ml.errWriter, args...)
 }
 
 func (ml *FmtMachineLogger) Errorf(fmtString string, args ...interface{}) {
-	ml.recordf(fmtString, args...)
+	ml.history.Recordf(fmtString, args...)
-	fmt.Fprintf(ml.err, fmtString+"\n", args...)
+	fmt.Fprintf(ml.errWriter, fmtString+"\n", args...)
 }
 
 func (ml *FmtMachineLogger) Info(args ...interface{}) {
-	ml.record(args...)
+	ml.history.Record(args...)
-	fmt.Fprintln(ml.out, args...)
+	fmt.Fprintln(ml.outWriter, args...)
 }
 
 func (ml *FmtMachineLogger) Infof(fmtString string, args ...interface{}) {
-	ml.recordf(fmtString, args...)
+	ml.history.Recordf(fmtString, args...)
-	fmt.Fprintf(ml.out, fmtString+"\n", args...)
+	fmt.Fprintf(ml.outWriter, fmtString+"\n", args...)
 }
 
 func (ml *FmtMachineLogger) Fatal(args ...interface{}) {
-	ml.record(args...)
+	ml.history.Record(args...)
-	fmt.Fprintln(ml.err, args...)
+	fmt.Fprintln(ml.errWriter, args...)
 	os.Exit(1)
 }
 
 func (ml *FmtMachineLogger) Fatalf(fmtString string, args ...interface{}) {
-	ml.recordf(fmtString, args...)
+	ml.history.Recordf(fmtString, args...)
-	fmt.Fprintf(ml.err, fmtString+"\n", args...)
+	fmt.Fprintf(ml.errWriter, fmtString+"\n", args...)
 	os.Exit(1)
 }
 
 func (ml *FmtMachineLogger) Warn(args ...interface{}) {
-	ml.record(args...)
+	ml.history.Record(args...)
-	fmt.Fprintln(ml.out, args...)
+	fmt.Fprintln(ml.outWriter, args...)
 }
 
 func (ml *FmtMachineLogger) Warnf(fmtString string, args ...interface{}) {
-	ml.recordf(fmtString, args...)
+	ml.history.Recordf(fmtString, args...)
-	fmt.Fprintf(ml.out, fmtString+"\n", args...)
+	fmt.Fprintf(ml.outWriter, fmtString+"\n", args...)
 }
 
 func (ml *FmtMachineLogger) History() []string {
-	return ml.history
+	return ml.history.records
-}
-
-func (ml *FmtMachineLogger) record(args ...interface{}) {
-	ml.historyLock.Lock()
-	defer ml.historyLock.Unlock()
-	ml.history = append(ml.history, fmt.Sprint(args...))
-}
-
-func (ml *FmtMachineLogger) recordf(fmtString string, args ...interface{}) {
-	ml.historyLock.Lock()
-	defer ml.historyLock.Unlock()
-	ml.history = append(ml.history, fmt.Sprintf(fmtString, args...))
 }

libmachine/log/fmt_machine_logger_test.go

@@ -12,7 +12,7 @@ import (
 func captureOutput(testLogger MachineLogger, lambda func()) string {
 	pipeReader, pipeWriter := io.Pipe()
 	scanner := bufio.NewScanner(pipeReader)
-	testLogger.SetOut(pipeWriter)
+	testLogger.SetOutWriter(pipeWriter)
 	go lambda()
 	scanner.Scan()
 	return scanner.Text()
@@ -21,7 +21,7 @@ func captureOutput(testLogger MachineLogger, lambda func()) string {
 func captureError(testLogger MachineLogger, lambda func()) string {
 	pipeReader, pipeWriter := io.Pipe()
 	scanner := bufio.NewScanner(pipeReader)
-	testLogger.SetErr(pipeWriter)
+	testLogger.SetErrWriter(pipeWriter)
 	go lambda()
 	scanner.Scan()
 	return scanner.Text()
@@ -42,14 +42,14 @@ func TestSetDebugToFalse(t *testing.T) {
 
 func TestSetOut(t *testing.T) {
 	testLogger := NewFmtMachineLogger().(*FmtMachineLogger)
-	testLogger.SetOut(ioutil.Discard)
+	testLogger.SetOutWriter(ioutil.Discard)
-	assert.Equal(t, ioutil.Discard, testLogger.out)
+	assert.Equal(t, ioutil.Discard, testLogger.outWriter)
 }
 
 func TestSetErr(t *testing.T) {
 	testLogger := NewFmtMachineLogger().(*FmtMachineLogger)
-	testLogger.SetErr(ioutil.Discard)
+	testLogger.SetErrWriter(ioutil.Discard)
-	assert.Equal(t, ioutil.Discard, testLogger.err)
+	assert.Equal(t, ioutil.Discard, testLogger.errWriter)
 }
 
 func TestDebug(t *testing.T) {

libmachine/log/history_recorder.go

@@ -0,0 +1,34 @@
+package log
+
+import (
+	"fmt"
+	"sync"
+)
+
+type HistoryRecorder struct {
+	lock    *sync.Mutex
+	records []string
+}
+
+func NewHistoryRecorder() *HistoryRecorder {
+	return &HistoryRecorder{
+		lock:    &sync.Mutex{},
+		records: []string{},
+	}
+}
+
+func (ml *HistoryRecorder) History() []string {
+	return ml.records
+}
+
+func (ml *HistoryRecorder) Record(args ...interface{}) {
+	ml.lock.Lock()
+	defer ml.lock.Unlock()
+	ml.records = append(ml.records, fmt.Sprint(args...))
+}
+
+func (ml *HistoryRecorder) Recordf(fmtString string, args ...interface{}) {
+	ml.lock.Lock()
+	defer ml.lock.Unlock()
+	ml.records = append(ml.records, fmt.Sprintf(fmtString, args...))
+}

libmachine/log/history_recorder_test.go

@@ -0,0 +1,21 @@
+package log
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestRecording(t *testing.T) {
+	recorder := NewHistoryRecorder()
+	recorder.Record("foo")
+	recorder.Record("bar")
+	recorder.Record("qix")
+	assert.Equal(t, recorder.History(), []string{"foo", "bar", "qix"})
+}
+
+func TestFormattedRecording(t *testing.T) {
+	recorder := NewHistoryRecorder()
+	recorder.Recordf("%s, %s and %s", "foo", "bar", "qix")
+	assert.Equal(t, recorder.History()[0], "foo, bar and qix")
+}

libmachine/log/log.go

@@ -69,12 +69,12 @@ func SetDebug(debug bool) {
 	logger.SetDebug(debug)
 }
 
-func SetOut(out io.Writer) {
+func SetOutWriter(out io.Writer) {
-	logger.SetOut(out)
+	logger.SetOutWriter(out)
 }
 
-func SetErr(err io.Writer) {
+func SetErrWriter(err io.Writer) {
-	logger.SetErr(err)
+	logger.SetErrWriter(err)
 }
 
 func History() []string {

libmachine/log/machine_logger.go

@@ -5,8 +5,8 @@ import "io"
 type MachineLogger interface {
 	SetDebug(debug bool)
 
-	SetOut(io.Writer)
+	SetOutWriter(io.Writer)
-	SetErr(io.Writer)
+	SetErrWriter(io.Writer)
 
 	Debug(args ...interface{})
 	Debugf(fmtString string, args ...interface{})