make logger locking optional, fix for logger tests

core/engine.go

@@ -147,15 +147,8 @@ func (e *Engine) JobManager() *JobManager {
 	return e.jobManager
 }
 
-// Logger returns current logger in goroutine-safe way
+// Logger returns current logger
 func (e *Engine) Logger() LoggerInterface {
-	e.mutex.RLock()
-	defer e.mutex.RUnlock()
-	return e.logger
-}
-
-// UnsafeLogger returns current logger in goroutine-unsafe way. Using this method you can cause race condition.
-func (e *Engine) UnsafeLogger() LoggerInterface {
 	return e.logger
 }
 

core/engine_test.go

@@ -187,17 +187,6 @@ func (e *EngineTest) Test_WithFilesystemSessions() {
 	assert.NotNil(e.T(), e.engine.Sessions)
 }
 
-func (e *EngineTest) Test_UnsafeLogger() {
-	origLogger := e.engine.logger
-	defer func() {
-		e.engine.logger = origLogger
-	}()
-	e.engine.logger = nil
-	assert.Nil(e.T(), e.engine.UnsafeLogger())
-	e.engine.logger = &Logger{}
-	assert.IsType(e.T(), &Logger{}, e.engine.UnsafeLogger())
-}
-
 func (e *EngineTest) Test_SetLogger() {
 	origLogger := e.engine.logger
 	defer func() {

core/logger.go

@@ -28,10 +28,10 @@ type LoggerInterface interface {
 }
 
 // Logger component. Uses github.com/op/go-logging under the hood.
-// This logger utilises sync.RWMutex functionality in order to avoid race conditions (in some cases it is useful).
+// This logger can prevent any write operations (disabled by default, use .Exclusive() method to enable)
 type Logger struct {
 	logger *logging.Logger
-	mutex  sync.RWMutex
+	mutex  *sync.RWMutex
 }
 
 // NewLogger will create new goroutine-safe logger with specified formatter.
@@ -62,114 +62,137 @@ func DefaultLogFormatter() logging.Formatter {
 	)
 }
 
+// Exclusive makes logger goroutine-safe
+func (l *Logger) Exclusive() *Logger {
+	if l.mutex == nil {
+		l.mutex = &sync.RWMutex{}
+	}
+
+	return l
+}
+
+// lock locks logger
+func (l *Logger) lock() {
+	if l.mutex != nil {
+		l.mutex.Lock()
+	}
+}
+
+// unlock unlocks logger
+func (l *Logger) unlock() {
+	if l.mutex != nil {
+		l.mutex.Unlock()
+	}
+}
+
 // Fatal is equivalent to l.Critical(fmt.Sprint()) followed by a call to os.Exit(1).
 func (l *Logger) Fatal(args ...interface{}) {
-	l.mutex.Lock()
+	l.lock()
-	defer l.mutex.Unlock()
+	defer l.unlock()
 	l.logger.Fatal(args...)
 }
 
 // Fatalf is equivalent to l.Critical followed by a call to os.Exit(1).
 func (l *Logger) Fatalf(format string, args ...interface{}) {
-	l.mutex.Lock()
+	l.lock()
-	defer l.mutex.Unlock()
+	defer l.unlock()
 	l.logger.Fatalf(format, args...)
 }
 
 // Panic is equivalent to l.Critical(fmt.Sprint()) followed by a call to panic().
 func (l *Logger) Panic(args ...interface{}) {
-	l.mutex.Lock()
+	l.lock()
-	defer l.mutex.Unlock()
+	defer l.unlock()
 	l.logger.Panic(args...)
 }
 
 // Panicf is equivalent to l.Critical followed by a call to panic().
 func (l *Logger) Panicf(format string, args ...interface{}) {
-	l.mutex.Lock()
+	l.lock()
-	defer l.mutex.Unlock()
+	defer l.unlock()
 	l.logger.Panicf(format, args...)
 }
 
 // Critical logs a message using CRITICAL as log level.
 func (l *Logger) Critical(args ...interface{}) {
-	l.mutex.Lock()
+	l.lock()
-	defer l.mutex.Unlock()
+	defer l.unlock()
 	l.logger.Critical(args...)
 }
 
 // Criticalf logs a message using CRITICAL as log level.
 func (l *Logger) Criticalf(format string, args ...interface{}) {
-	l.mutex.Lock()
+	l.lock()
-	defer l.mutex.Unlock()
+	defer l.unlock()
 	l.logger.Criticalf(format, args...)
 }
 
 // Error logs a message using ERROR as log level.
 func (l *Logger) Error(args ...interface{}) {
-	l.mutex.Lock()
+	l.lock()
-	defer l.mutex.Unlock()
+	defer l.unlock()
 	l.logger.Error(args...)
 }
 
 // Errorf logs a message using ERROR as log level.
 func (l *Logger) Errorf(format string, args ...interface{}) {
-	l.mutex.Lock()
+	l.lock()
-	defer l.mutex.Unlock()
+	defer l.unlock()
 	l.logger.Errorf(format, args...)
 }
 
 // Warning logs a message using WARNING as log level.
 func (l *Logger) Warning(args ...interface{}) {
-	l.mutex.Lock()
+	l.lock()
-	defer l.mutex.Unlock()
+	defer l.unlock()
 	l.logger.Warning(args...)
 }
 
 // Warningf logs a message using WARNING as log level.
 func (l *Logger) Warningf(format string, args ...interface{}) {
-	l.mutex.Lock()
+	l.lock()
-	defer l.mutex.Unlock()
+	defer l.unlock()
 	l.logger.Warningf(format, args...)
 }
 
 // Notice logs a message using NOTICE as log level.
 func (l *Logger) Notice(args ...interface{}) {
-	l.mutex.Lock()
+	l.lock()
-	defer l.mutex.Unlock()
+	defer l.unlock()
 	l.logger.Notice(args...)
 }
 
 // Noticef logs a message using NOTICE as log level.
 func (l *Logger) Noticef(format string, args ...interface{}) {
-	l.mutex.Lock()
+	l.lock()
-	defer l.mutex.Unlock()
+	defer l.unlock()
 	l.logger.Noticef(format, args...)
 }
 
 // Info logs a message using INFO as log level.
 func (l *Logger) Info(args ...interface{}) {
-	l.mutex.Lock()
+	l.lock()
-	defer l.mutex.Unlock()
+	defer l.unlock()
 	l.logger.Info(args...)
 }
 
 // Infof logs a message using INFO as log level.
 func (l *Logger) Infof(format string, args ...interface{}) {
-	l.mutex.Lock()
+	l.lock()
-	defer l.mutex.Unlock()
+	defer l.unlock()
 	l.logger.Infof(format, args...)
 }
 
 // Debug logs a message using DEBUG as log level.
 func (l *Logger) Debug(args ...interface{}) {
-	l.mutex.Lock()
+	l.lock()
-	defer l.mutex.Unlock()
+	defer l.unlock()
 	l.logger.Debug(args...)
 }
 
 // Debugf logs a message using DEBUG as log level.
 func (l *Logger) Debugf(format string, args ...interface{}) {
-	l.mutex.Lock()
+	l.lock()
-	defer l.mutex.Unlock()
+	defer l.unlock()
 	l.logger.Debugf(format, args...)
 }

core/logger_test.go

@@ -31,6 +31,10 @@ func Test_Logger(t *testing.T) {
 	suite.Run(t, new(LoggerTest))
 }
 
+func (t *LoggerTest) SetupSuite() {
+	t.logger = NewLogger("code", logging.DEBUG, DefaultLogFormatter()).Exclusive()
+}
+
 // TODO Cover Fatal and Fatalf (implementation below is no-op)
 // func (t *LoggerTest) Test_Fatal() {
 // 	if os.Getenv("FLAG") == "1" {
@@ -64,7 +68,7 @@ func (t *LoggerTest) Test_Panicf() {
 
 func (t *LoggerTest) Test_Critical() {
 	defer func() {
-		if v := recover(); v == nil {
+		if v := recover(); v != nil {
 			t.T().Fatal(v)
 		}
 	}()
@@ -73,7 +77,7 @@ func (t *LoggerTest) Test_Critical() {
 
 func (t *LoggerTest) Test_Criticalf() {
 	defer func() {
-		if v := recover(); v == nil {
+		if v := recover(); v != nil {
 			t.T().Fatal(v)
 		}
 	}()
@@ -82,7 +86,7 @@ func (t *LoggerTest) Test_Criticalf() {
 
 func (t *LoggerTest) Test_Warning() {
 	defer func() {
-		if v := recover(); v == nil {
+		if v := recover(); v != nil {
 			t.T().Fatal(v)
 		}
 	}()
@@ -91,7 +95,7 @@ func (t *LoggerTest) Test_Warning() {
 
 func (t *LoggerTest) Test_Notice() {
 	defer func() {
-		if v := recover(); v == nil {
+		if v := recover(); v != nil {
 			t.T().Fatal(v)
 		}
 	}()
@@ -100,7 +104,7 @@ func (t *LoggerTest) Test_Notice() {
 
 func (t *LoggerTest) Test_Info() {
 	defer func() {
-		if v := recover(); v == nil {
+		if v := recover(); v != nil {
 			t.T().Fatal(v)
 		}
 	}()
@@ -109,7 +113,7 @@ func (t *LoggerTest) Test_Info() {
 
 func (t *LoggerTest) Test_Debug() {
 	defer func() {
-		if v := recover(); v == nil {
+		if v := recover(); v != nil {
 			t.T().Fatal(v)
 		}
 	}()