refactor: configure mocks and support classes and add unit tests

Author: mlataza

cmd/agent_smith/config.go

@@ -8,9 +8,10 @@ import (
 	"net/http"
 	"os"
 	"runtime"
-	"time"
 
+	"github.com/RewstApp/agent-smith-go/internal/adapters"
 	"github.com/RewstApp/agent-smith-go/internal/agent"
+	"github.com/RewstApp/agent-smith-go/internal/ports"
 	"github.com/RewstApp/agent-smith-go/internal/service"
 	"github.com/RewstApp/agent-smith-go/internal/utils"
 	"github.com/RewstApp/agent-smith-go/internal/version"
@@ -20,7 +21,35 @@ type fetchConfigurationResponse struct {
 	Configuration agent.Device `json:"configuration"`
 }
 
-func runConfig(params *configParams) {
+// ConfigRunner handles configuration with injectable dependencies.
+type ConfigRunner struct {
+	httpClient ports.HTTPClient
+	fs         ports.FileSystem
+	clock      ports.Clock
+}
+
+// NewConfigRunner creates a new ConfigRunner with the provided dependencies.
+func NewConfigRunner(httpClient ports.HTTPClient, fs ports.FileSystem, clock ports.Clock) *ConfigRunner {
+	return &ConfigRunner{
+		httpClient: httpClient,
+		fs:         fs,
+		clock:      clock,
+	}
+}
+
+// DefaultConfigRunner returns a ConfigRunner using real implementations.
+func DefaultConfigRunner() *ConfigRunner {
+	return NewConfigRunner(
+		adapters.NewHTTPClient(),
+		adapters.NewOSFileSystem(),
+		adapters.NewClock(),
+	)
+}
+
+// Default instance for backward compatibility
+var defaultConfigRunner = DefaultConfigRunner()
+
+func (c *ConfigRunner) run(params *configParams) {
 	logger := utils.ConfigureLogger("agent_smith", os.Stdout, utils.Default)
 
 	// Show header
@@ -51,8 +80,7 @@ func runConfig(params *configParams) {
 	req.Header.Set("x-rewst-secret", params.ConfigSecret)
 	req.Header.Set("Content-Type", "application/json")
 
-	client := &http.Client{}
-	res, err := client.Do(req)
+	res, err := c.httpClient.Do(req)
 	if err != nil {
 		logger.Error("Failed to execute http request", "error", err)
 		return
@@ -102,7 +130,7 @@ func runConfig(params *configParams) {
 	// Got configuration
 	logger.Info("Received configuration", "configuration", string(configBytes))
 
-	err = os.WriteFile(configFilePath, configBytes, utils.DefaultFileMod)
+	err = c.fs.WriteFile(configFilePath, configBytes, utils.DefaultFileMod)
 	if err != nil {
 		logger.Error("Failed to save config", "error", err)
 		return
@@ -134,7 +162,7 @@ func runConfig(params *configParams) {
 		// Wait for some time for the service executable to clean up
 		existingService.Close()
 		logger.Info("Waiting for service executable to stop")
-		time.Sleep(serviceExecutableTimeout)
+		c.clock.Sleep(serviceExecutableTimeout)
 	}
 
 	logger.Info("Configuration saved to", "path", configFilePath)
@@ -149,20 +177,20 @@ func runConfig(params *configParams) {
 	}
 
 	// Copy the agent executable
-	execFilePath, err := os.Executable()
+	execFilePath, err := c.fs.Executable()
 	if err != nil {
 		logger.Error("Failed to get executable", "error", err)
 		return
 	}
 
-	execFileBytes, err := os.ReadFile(execFilePath)
+	execFileBytes, err := c.fs.ReadFile(execFilePath)
 	if err != nil {
 		logger.Error("Failed to read executable file", "error", err)
 		return
 	}
 
 	agentExecutablePath := agent.GetAgentExecutablePath(params.OrgId)
-	err = os.WriteFile(agentExecutablePath, execFileBytes, utils.DefaultExecutableFileMod)
+	err = c.fs.WriteFile(agentExecutablePath, execFileBytes, utils.DefaultExecutableFileMod)
 	if err != nil {
 		logger.Error("Failed to create agent executable", "error", err)
 		return
@@ -198,3 +226,7 @@ func runConfig(params *configParams) {
 
 	logger.Info("Service started")
 }
+
+func runConfig(params *configParams) {
+	defaultConfigRunner.run(params)
+}

cmd/agent_smith/config_mock_test.go

@@ -0,0 +1,49 @@
+package main
+
+import (
+	"testing"
+
+	"github.com/RewstApp/agent-smith-go/internal/mocks"
+	"go.uber.org/mock/gomock"
+)
+
+func TestNewConfigRunner(t *testing.T) {
+	ctrl := gomock.NewController(t)
+	defer ctrl.Finish()
+
+	mockHTTP := mocks.NewMockHTTPClient(ctrl)
+	mockFS := mocks.NewMockFileSystem(ctrl)
+	mockClock := mocks.NewMockClock(ctrl)
+
+	runner := NewConfigRunner(mockHTTP, mockFS, mockClock)
+
+	if runner == nil {
+		t.Error("expected non-nil runner")
+	}
+	if runner.httpClient != mockHTTP {
+		t.Error("expected httpClient to be set")
+	}
+	if runner.fs != mockFS {
+		t.Error("expected fs to be set")
+	}
+	if runner.clock != mockClock {
+		t.Error("expected clock to be set")
+	}
+}
+
+func TestDefaultConfigRunner(t *testing.T) {
+	runner := DefaultConfigRunner()
+
+	if runner == nil {
+		t.Error("expected non-nil runner")
+	}
+	if runner.httpClient == nil {
+		t.Error("expected httpClient to be set")
+	}
+	if runner.fs == nil {
+		t.Error("expected fs to be set")
+	}
+	if runner.clock == nil {
+		t.Error("expected clock to be set")
+	}
+}

cmd/agent_smith/main_test.go

@@ -166,3 +166,132 @@ func TestParseUpdateParams(t *testing.T) {
 		}
 	}
 }
+
+func TestParseUpdateParams_InvalidLoggingLevel(t *testing.T) {
+	orgId := "test123"
+	_, err := parseUpdateParams([]string{
+		"--org-id", orgId,
+		"--update",
+		"--logging-level", "invalid-level",
+	})
+
+	if err == nil {
+		t.Error("expected error for invalid logging level")
+	}
+	if !strings.Contains(err.Error(), "invalid logging-level") {
+		t.Errorf("expected 'invalid logging-level' error, got %v", err.Error())
+	}
+}
+
+func TestParseConfigParams_InvalidLoggingLevel(t *testing.T) {
+	orgId := "test123"
+	configUrl := "https://config.url/"
+	configSecret := "secret123"
+
+	_, err := parseConfigParams([]string{
+		"--org-id", orgId,
+		"--config-url", configUrl,
+		"--config-secret", configSecret,
+		"--logging-level", "invalid-level",
+	})
+
+	if err == nil {
+		t.Error("expected error for invalid logging level")
+	}
+	if !strings.Contains(err.Error(), "invalid logging-level") {
+		t.Errorf("expected 'invalid logging-level' error, got %v", err.Error())
+	}
+}
+
+func TestParseConfigParams_AllLoggingLevels(t *testing.T) {
+	orgId := "test123"
+	configUrl := "https://config.url/"
+	configSecret := "secret123"
+
+	// Note: Default is an empty string "", not "default"
+	validLevels := []string{"info", "warn", "error", "off", "debug"}
+
+	for _, level := range validLevels {
+		result, err := parseConfigParams([]string{
+			"--org-id", orgId,
+			"--config-url", configUrl,
+			"--config-secret", configSecret,
+			"--logging-level", level,
+		})
+
+		if err != nil {
+			t.Errorf("logging level '%s' should be valid, got error: %v", level, err)
+		}
+		if result.LoggingLevel != level {
+			t.Errorf("expected logging level '%s', got '%s'", level, result.LoggingLevel)
+		}
+	}
+
+	// Test that default logging level (empty string) works when not specified
+	result, err := parseConfigParams([]string{
+		"--org-id", orgId,
+		"--config-url", configUrl,
+		"--config-secret", configSecret,
+	})
+	if err != nil {
+		t.Errorf("default logging level should work, got error: %v", err)
+	}
+	// Default is "" (empty string)
+	if result.LoggingLevel != "" {
+		t.Errorf("expected empty logging level for default, got '%s'", result.LoggingLevel)
+	}
+}
+
+func TestParseConfigParams_OptionalFlags(t *testing.T) {
+	orgId := "test123"
+	configUrl := "https://config.url/"
+	configSecret := "secret123"
+
+	result, err := parseConfigParams([]string{
+		"--org-id", orgId,
+		"--config-url", configUrl,
+		"--config-secret", configSecret,
+		"--syslog",
+		"--disable-agent-postback",
+		"--no-auto-updates",
+	})
+
+	if err != nil {
+		t.Errorf("expected no error, got %v", err)
+	}
+	if !result.UseSyslog {
+		t.Error("expected UseSyslog true")
+	}
+	if !result.DisableAgentPostback {
+		t.Error("expected DisableAgentPostback true")
+	}
+	if !result.NoAutoUpdates {
+		t.Error("expected NoAutoUpdates true")
+	}
+}
+
+func TestGetAllowedConfigLevelsString(t *testing.T) {
+	result := getAllowedConfigLevelsString(", ")
+
+	// Should contain valid levels but not "default"
+	if strings.Contains(result, "default") {
+		t.Error("result should not contain 'default'")
+	}
+
+	// Should contain other valid levels
+	validLevels := []string{"info", "warn", "error", "off", "debug"}
+	for _, level := range validLevels {
+		if !strings.Contains(result, level) {
+			t.Errorf("result should contain '%s'", level)
+		}
+	}
+}
+
+func TestGetAllowedConfigLevelsString_CustomSeparator(t *testing.T) {
+	result := getAllowedConfigLevelsString("|")
+
+	// Should use the custom separator
+	if !strings.Contains(result, "|") {
+		t.Error("result should use custom separator '|'")
+	}
+}