IMPROVEMENT-35: Improve application logging (#46)

* IMPROVEMENT-35: Cover main app functions with log statments
* IMPROVEMENT-35: Update unit tests

cmd/goto/main.go

@@ -41,7 +41,7 @@ func main() {
 
 	// Check if "ssh" utility is in application path
 	if err := utils.CheckAppInstalled("ssh"); err != nil {
-		log.Fatalf("ssh utility is not installed or cannot be found in the executable path: %v", err)
+		log.Fatalf("[MAIN] ssh utility is not installed or cannot be found in the executable path: %v", err)
 	}
 
 	commandLineParams := config.User{}
@@ -56,62 +56,68 @@ func main() {
 	// Get application home folder path
 	commandLineParams.AppHome, err = utils.AppDir(appName, commandLineParams.AppHome)
 	if err != nil {
-		log.Fatalf("Can't get application home folder: %v", err)
+		log.Fatalf("[MAIN] Can't get application home folder: %v", err)
 	}
 
 	// Create application folder
 	if err = utils.CreateAppDirIfNotExists(commandLineParams.AppHome); err != nil {
-		log.Fatalf("Can't create application home folder: %v", err)
+		log.Fatalf("[MAIN] Can't create application home folder: %v", err)
 	}
 
 	// Create application logger
 	lg, err := logger.New(commandLineParams.AppHome, commandLineParams.LogLevel)
 	if err != nil {
-		log.Fatalf("Can't create log file: %v", err)
+		log.Fatalf("[MAIN] Can't create log file: %v", err)
 	}
 
 	// Create application configuration and set application home folder
 	appConfig := config.Merge(environmentParams, commandLineParams, &lg)
 
 	// If "-v" parameter provided, display application version configuration and exit
 	if displayApplicationDetailsAndExit {
+		lg.Debug("[MAIN] Display application version")
 		version.Print()
 		fmt.Println()
 		appConfig.Print()
 
+		lg.Debug("[MAIN] Exit application")
 		os.Exit(0)
 	}
 
 	// Logger created. Immediately print application version
-	lg.Info("Start application")
-	lg.Info("Version:    %s", version.Number())
-	lg.Info("Commit:     %s", version.CommitHash())
-	lg.Info("Branch:     %s", version.BuildBranch())
-	lg.Info("Build date: %s", version.BuildDate())
+	lg.Info("[MAIN] Start application")
+	lg.Info("[MAIN] Version:    %s", version.Number())
+	lg.Info("[MAIN] Commit:     %s", version.CommitHash())
+	lg.Info("[MAIN] Branch:     %s", version.BuildBranch())
+	lg.Info("[MAIN] Build date: %s", version.BuildDate())
 
 	ctx := context.Background()
 	application := config.NewApplication(ctx, appConfig, &lg)
 
 	hostStorage, err := storage.Get(ctx, application)
 	if err != nil {
-		lg.Error("Error running application: %v", err)
+		lg.Error("[MAIN] Error running application: %v", err)
 		os.Exit(1)
 	}
 
 	appState := state.Get(application.Config.AppHome, &lg)
 	uiComponent := ui.NewMainModel(ctx, hostStorage, appState, &lg)
 	p := tea.NewProgram(&uiComponent, tea.WithAltScreen())
 
+	lg.Debug("[MAIN] Start user interface")
 	if _, err = p.Run(); err != nil {
-		lg.Error("Error running application: %v", err)
+		lg.Error("[MAIN] Error starting user interface: %v", err)
 		os.Exit(1)
 	}
 
-	lg.Debug("Save application state")
+	// Quit signal should be intercepted on the UI level, however it will require an
+	// additional switch-case block with an appropriate checks. Leaving this message here.
+	lg.Debug("[MAIN] Receive quit signal")
+	lg.Debug("[MAIN] Save application state")
 	err = appState.Persist()
 	if err != nil {
-		lg.Error("Can't save application state before closing %v", err)
+		lg.Error("[MAIN] Can't save application state before closing %v", err)
 	}
 
-	lg.Info("Close the application")
+	lg.Info("[MAIN] Close the application")
 }

internal/config/config.go

@@ -8,6 +8,7 @@ import (
 
 type iLogger interface {
 	Debug(format string, args ...any)
+	Info(format string, args ...any)
 	Error(format string, args ...any)
 	Close()
 }
@@ -31,12 +32,12 @@ func Merge(envParams, cmdParams User, logger iLogger) User {
 	if len(cmdParams.AppHome) > 0 {
 		envParams.AppHome = cmdParams.AppHome
 	}
-	logger.Debug("Set application home folder to %s\n", envParams.AppHome)
+	logger.Debug("[CONFIG] Set application home folder to %s\n", envParams.AppHome)
 
 	if len(cmdParams.LogLevel) > 0 {
 		envParams.LogLevel = cmdParams.LogLevel
 	}
-	logger.Debug("Set application log level to %s\n", envParams.LogLevel)
+	logger.Debug("[CONFIG] Set application log level to %s\n", envParams.LogLevel)
 
 	return envParams
 }

internal/mock/logger.go

@@ -1,14 +1,29 @@
 package mock
 
-type MockLogger struct{}
+import "fmt"
+
+type MockLogger struct {
+	Logs []string
+}
+
+func (ml *MockLogger) print(format string, args ...interface{}) {
+	logMessage := format
+	if len(args) > 0 {
+		logMessage = fmt.Sprintf(format, args...)
+	}
+	ml.Logs = append(ml.Logs, logMessage)
+}
 
 func (l *MockLogger) Debug(format string, args ...any) {
+	l.print(format, args...)
 }
 
 func (l *MockLogger) Info(format string, args ...any) {
+	l.print(format, args...)
 }
 
 func (l *MockLogger) Error(format string, args ...any) {
+	l.print(format, args...)
 }
 
 func (l *MockLogger) Close() {

internal/mock/storage.go

@@ -10,9 +10,12 @@ import (
 
 func NewMockStorage(shouldFail bool) *mockStorage {
 	hosts := []model.Host{
-		model.NewHost(0, "Mock Host", "", "localhost", "root", "id_rsa", "2222"),
-		model.NewHost(0, "Mock Host", "", "localhost", "root", "id_rsa", "2222"),
-		model.NewHost(0, "Mock Host", "", "localhost", "root", "id_rsa", "2222"),
+		// Yaml storage specific: if host has id which is equal to "0"
+		// that means that this host doesn't yet exist. It's a hack,
+		// but simplifies the application. That's why we cound hosts from "1"
+		model.NewHost(1, "Mock Host 1", "", "localhost", "root", "id_rsa", "2222"),
+		model.NewHost(2, "Mock Host 2", "", "localhost", "root", "id_rsa", "2222"),
+		model.NewHost(3, "Mock Host 3", "", "localhost", "root", "id_rsa", "2222"),
 	}
 
 	return &mockStorage{
@@ -43,7 +46,7 @@ func (ms *mockStorage) Get(hostID int) (model.Host, error) {
 		return model.Host{}, errors.New("mock error")
 	}
 
-	return model.Host{}, nil
+	return ms.Hosts[hostID], nil
 }
 
 // GetAll implements storage.HostStorage.
@@ -61,5 +64,7 @@ func (ms *mockStorage) Save(m model.Host) (model.Host, error) {
 		return m, errors.New("mock error")
 	}
 
+	ms.Hosts = append(ms.Hosts, m)
+
 	return m, nil
 }

internal/state/state.go

@@ -28,6 +28,8 @@ var (
 
 type iLogger interface {
 	Debug(format string, args ...any)
+	Info(format string, args ...any)
+	Error(format string, args ...any)
 }
 
 // ApplicationState stores application state.
@@ -43,30 +45,32 @@ type ApplicationState struct {
 
 // Get - reads application stat from disk.
 func Get(appHomePath string, lg iLogger) *ApplicationState {
+	lg.Debug("[APPSTATE] Get application state")
 	once.Do(func() {
 		appState = &ApplicationState{
 			appStateFilePath: path.Join(appHomePath, stateFile),
 			logger:           lg,
 		}
 
 		// If we cannot read previously created application state, that's fine - we can continue execution.
+		lg.Debug("[APPSTATE] Application state is not ready, restore from file")
 		_ = appState.readFromFile()
 	})
 
 	return appState
 }
 
 func (as *ApplicationState) readFromFile() error {
-	as.logger.Debug("Read application state from %s\n", as.appStateFilePath)
+	as.logger.Debug("[APPSTATE] Read application state from: %s", as.appStateFilePath)
 	fileData, err := os.ReadFile(as.appStateFilePath)
 	if err != nil {
-		as.logger.Debug("Can't read application state %v\n", err)
+		as.logger.Info("[APPSTATE] Can't read application state loaded from file %v", err)
 		return err
 	}
 
 	err = yaml.Unmarshal(fileData, as)
 	if err != nil {
-		as.logger.Debug("Can't read parse application state %v\n", err)
+		as.logger.Error("[APPSTATE] Can't parse application state loaded from file %v", err)
 		return err
 	}
 
@@ -75,13 +79,16 @@ func (as *ApplicationState) readFromFile() error {
 
 // Persist saves app state to disk.
 func (as *ApplicationState) Persist() error {
+	as.logger.Debug("[APPSTATE] Persist application state to file: %s", as.appStateFilePath)
 	result, err := yaml.Marshal(as)
 	if err != nil {
+		as.logger.Error("[APPSTATE] Cannot marshall application state. %v", err)
 		return err
 	}
 
 	err = os.WriteFile(as.appStateFilePath, result, 0o600)
 	if err != nil {
+		as.logger.Error("[APPSTATE] Cannot save application state. %v", err)
 		return err
 	}
 

internal/state/state_test.go

@@ -2,30 +2,18 @@
 package state
 
 import (
-	"fmt"
 	"os"
 	"path"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
 	"gopkg.in/yaml.v2"
-)
-
-// MockLogger implements the iLogger interface for testing.
-type MockLogger struct {
-	Logs []string
-}
 
-func (ml *MockLogger) Debug(format string, args ...interface{}) {
-	logMessage := format
-	if len(args) > 0 {
-		logMessage = fmt.Sprintf(format, args...)
-	}
-	ml.Logs = append(ml.Logs, logMessage)
-}
+	"github.com/grafviktor/goto/internal/mock"
+)
 
 // That's a wrapper function for state.Get which is required to overcome sync.Once restrictions
-func stateGet(tempDir string, mockLogger *MockLogger) *ApplicationState {
+func stateGet(tempDir string, mockLogger *mock.MockLogger) *ApplicationState {
 	appState := Get(tempDir, mockLogger)
 
 	// We need this hack because state.Get function utilizes `sync.once`. That means, if all unit tests
@@ -46,7 +34,7 @@ func Test_GetApplicationState(t *testing.T) {
 	defer os.RemoveAll(tempDir)
 
 	// Create a mock logger for testing
-	mockLogger := &MockLogger{}
+	mockLogger := &mock.MockLogger{}
 
 	// Call the Get function with the temporary directory and mock logger
 	appState := stateGet(tempDir, mockLogger)
@@ -55,8 +43,8 @@ func Test_GetApplicationState(t *testing.T) {
 	assert.NotNil(t, appState)
 
 	// Ensure that the logger was called during the initialization.
-	// The first line always contains "Read application state from"
-	assert.Contains(t, mockLogger.Logs[0], "Read application state from")
+	// The first line always contains "Get application state"
+	assert.Contains(t, mockLogger.Logs[0], "Get application state")
 }
 
 // Test persisting app state
@@ -69,7 +57,7 @@ func Test_PersistApplicationState(t *testing.T) {
 	defer os.RemoveAll(tempDir)
 
 	// Create a mock logger for testing
-	mockLogger := &MockLogger{}
+	mockLogger := &mock.MockLogger{}
 
 	// Call the Get function with the temporary directory and mock logger
 	appState := stateGet(tempDir, mockLogger)