Logs validation on root level (#946)

* refactor: simplify exit code handling in log utils

* feat: add logs configuration validation in root command

* enhance: improve logger tests with comprehensive validation cases

cmd/root.go

@@ -39,6 +39,29 @@ var RootCmd = &cobra.Command{
 			cmd.SilenceUsage = true
 			cmd.SilenceErrors = true
 		}
+
+		logsLevel, _ := cmd.Flags().GetString("logs-level")
+		logsFile, _ := cmd.Flags().GetString("logs-file")
+
+		errorConfig := schema.AtmosConfiguration{
+			Logs: schema.Logs{
+				Level: logsLevel,
+				File:  logsFile,
+			},
+		}
+
+		configAndStacksInfo := schema.ConfigAndStacksInfo{
+			LogsLevel: logsLevel,
+			LogsFile:  logsFile,
+		}
+
+		// Only validate the config, don't store it yet since commands may need to add more info
+		_, err := cfg.InitCliConfig(configAndStacksInfo, false)
+		if err != nil {
+			if !errors.Is(err, cfg.NotFound) {
+				u.LogErrorAndExit(errorConfig, err)
+			}
+		}
 	},
 	Run: func(cmd *cobra.Command, args []string) {
 		// Check Atmos configuration

pkg/logger/logger_test.go

@@ -65,25 +65,32 @@ func TestNewLoggerFromCliConfig(t *testing.T) {
 
 func TestParseLogLevel(t *testing.T) {
 	tests := []struct {
-		input    string
-		expected LogLevel
-		err      bool
+		name        string
+		input       string
+		expected    LogLevel
+		expectError bool
 	}{
-		{"Trace", LogLevelTrace, false},
-		{"Debug", LogLevelDebug, false},
-		{"Info", LogLevelInfo, false},
-		{"Warning", LogLevelWarning, false},
-		{"Invalid", LogLevelInfo, true},
+		{"Empty string returns Info", "", LogLevelInfo, false},
+		{"Valid Trace level", "Trace", LogLevelTrace, false},
+		{"Valid Debug level", "Debug", LogLevelDebug, false},
+		{"Valid Info level", "Info", LogLevelInfo, false},
+		{"Valid Warning level", "Warning", LogLevelWarning, false},
+		{"Valid Off level", "Off", LogLevelOff, false},
+		{"Invalid lowercase level", "trace", "", true},
+		{"Invalid mixed case level", "TrAcE", "", true},
+		{"Invalid level", "InvalidLevel", "", true},
+		{"Invalid empty spaces", "  ", "", true},
+		{"Invalid special characters", "Debug!", "", true},
 	}
 
 	for _, test := range tests {
-		t.Run(fmt.Sprintf("input=%s", test.input), func(t *testing.T) {
-			logLevel, err := ParseLogLevel(test.input)
-			if test.err {
+		t.Run(test.name, func(t *testing.T) {
+			level, err := ParseLogLevel(test.input)
+			if test.expectError {
 				assert.Error(t, err)
 			} else {
 				assert.NoError(t, err)
-				assert.Equal(t, test.expected, logLevel)
+				assert.Equal(t, test.expected, level)
 			}
 		})
 	}
@@ -168,3 +175,165 @@ func TestLogger_SetLogLevel(t *testing.T) {
 	assert.NoError(t, err)
 	assert.Equal(t, LogLevelDebug, logger.LogLevel)
 }
+
+func TestLogger_isLevelEnabled(t *testing.T) {
+	tests := []struct {
+		name          string
+		currentLevel  LogLevel
+		checkLevel    LogLevel
+		expectEnabled bool
+	}{
+		{"Trace enables all levels", LogLevelTrace, LogLevelTrace, true},
+		{"Trace enables Debug", LogLevelTrace, LogLevelDebug, true},
+		{"Trace enables Info", LogLevelTrace, LogLevelInfo, true},
+		{"Trace enables Warning", LogLevelTrace, LogLevelWarning, true},
+		{"Debug disables Trace", LogLevelDebug, LogLevelTrace, false},
+		{"Debug enables Debug", LogLevelDebug, LogLevelDebug, true},
+		{"Debug enables Info", LogLevelDebug, LogLevelInfo, true},
+		{"Debug enables Warning", LogLevelDebug, LogLevelWarning, true},
+		{"Info disables Trace", LogLevelInfo, LogLevelTrace, false},
+		{"Info disables Debug", LogLevelInfo, LogLevelDebug, false},
+		{"Info enables Info", LogLevelInfo, LogLevelInfo, true},
+		{"Info enables Warning", LogLevelInfo, LogLevelWarning, true},
+		{"Warning disables Trace", LogLevelWarning, LogLevelTrace, false},
+		{"Warning disables Debug", LogLevelWarning, LogLevelDebug, false},
+		{"Warning disables Info", LogLevelWarning, LogLevelInfo, false},
+		{"Warning enables Warning", LogLevelWarning, LogLevelWarning, true},
+		{"Off disables all levels", LogLevelOff, LogLevelTrace, false},
+		{"Off disables Debug", LogLevelOff, LogLevelDebug, false},
+		{"Off disables Info", LogLevelOff, LogLevelInfo, false},
+		{"Off disables Warning", LogLevelOff, LogLevelWarning, false},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			logger := &Logger{LogLevel: test.currentLevel}
+			enabled := logger.isLevelEnabled(test.checkLevel)
+			assert.Equal(t, test.expectEnabled, enabled)
+		})
+	}
+}
+
+func TestLogger_LogMethods(t *testing.T) {
+	tests := []struct {
+		name         string
+		loggerLevel  LogLevel
+		message      string
+		expectOutput bool
+		logFunc      func(*Logger, string)
+	}{
+		{"Trace logs when level is Trace", LogLevelTrace, "trace message", true, (*Logger).Trace},
+		{"Trace doesn't log when level is Debug", LogLevelDebug, "trace message", false, (*Logger).Trace},
+		{"Debug logs when level is Trace", LogLevelTrace, "debug message", true, (*Logger).Debug},
+		{"Debug logs when level is Debug", LogLevelDebug, "debug message", true, (*Logger).Debug},
+		{"Debug doesn't log when level is Info", LogLevelInfo, "debug message", false, (*Logger).Debug},
+		{"Info logs when level is Trace", LogLevelTrace, "info message", true, (*Logger).Info},
+		{"Info logs when level is Debug", LogLevelDebug, "info message", true, (*Logger).Info},
+		{"Info logs when level is Info", LogLevelInfo, "info message", true, (*Logger).Info},
+		{"Info doesn't log when level is Warning", LogLevelWarning, "info message", false, (*Logger).Info},
+		{"Warning logs when level is Trace", LogLevelTrace, "warning message", true, (*Logger).Warning},
+		{"Warning logs when level is Warning", LogLevelWarning, "warning message", true, (*Logger).Warning},
+		{"Nothing logs when level is Off", LogLevelOff, "any message", false, (*Logger).Info},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			// Pipe to capture output
+			r, w, _ := os.Pipe()
+			oldStdout := os.Stdout
+			os.Stdout = w
+
+			// Channel to capture output
+			outC := make(chan string)
+			go func() {
+				var buf bytes.Buffer
+				io.Copy(&buf, r)
+				outC <- buf.String()
+			}()
+
+			logger, _ := NewLogger(test.loggerLevel, "/dev/stdout")
+			test.logFunc(logger, test.message)
+
+			// Close the writer and restore stdout
+			w.Close()
+			os.Stdout = oldStdout
+
+			// Read the output
+			output := <-outC
+
+			if test.expectOutput {
+				assert.Contains(t, output, test.message)
+			} else {
+				assert.Empty(t, output)
+			}
+		})
+	}
+}
+
+func TestLoggerFromCliConfig(t *testing.T) {
+	tests := []struct {
+		name        string
+		config      schema.AtmosConfiguration
+		expectError bool
+	}{
+		{
+			name: "Valid config with Info level",
+			config: schema.AtmosConfiguration{
+				Logs: schema.Logs{
+					Level: "Info",
+					File:  "/dev/stdout",
+				},
+			},
+			expectError: false,
+		},
+		{
+			name: "Valid config with Trace level",
+			config: schema.AtmosConfiguration{
+				Logs: schema.Logs{
+					Level: "Trace",
+					File:  "/dev/stdout",
+				},
+			},
+			expectError: false,
+		},
+		{
+			name: "Invalid log level",
+			config: schema.AtmosConfiguration{
+				Logs: schema.Logs{
+					Level: "Invalid",
+					File:  "/dev/stdout",
+				},
+			},
+			expectError: true,
+		},
+		{
+			name: "Empty log level defaults to Info",
+			config: schema.AtmosConfiguration{
+				Logs: schema.Logs{
+					Level: "",
+					File:  "/dev/stdout",
+				},
+			},
+			expectError: false,
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			logger, err := NewLoggerFromCliConfig(test.config)
+			if test.expectError {
+				assert.Error(t, err)
+				assert.Nil(t, logger)
+			} else {
+				assert.NoError(t, err)
+				assert.NotNil(t, logger)
+				if test.config.Logs.Level == "" {
+					assert.Equal(t, LogLevelInfo, logger.LogLevel)
+				} else {
+					assert.Equal(t, LogLevel(test.config.Logs.Level), logger.LogLevel)
+				}
+				assert.Equal(t, test.config.Logs.File, logger.File)
+			}
+		})
+	}
+}

pkg/utils/log_utils.go

@@ -37,8 +37,7 @@ func LogErrorAndExit(atmosConfig schema.AtmosConfiguration, err error) {
 		// Find the executed command's exit code from the error
 		var exitError *exec.ExitError
 		if errors.As(err, &exitError) {
-			exitCode := exitError.ExitCode()
-			os.Exit(exitCode)
+			os.Exit(exitError.ExitCode())
 		}
 
 		os.Exit(1)