Add validation tests for malformed configuration

[sgaunet]

Problem

The validation test suite doesn't cover many malformed configuration scenarios that could cause runtime errors or unexpected behavior.

Current Test Coverage

The existing pkg/config/validation_test.go covers basic validation but misses critical edge cases.

Missing Test Coverage

1. Empty Keywords Array

Current config allows:

log_level:
  detection:
    enabled: true
    keywords:
      error: []  # Empty array!

Expected behavior: Should this be valid or return error?

Test needed:

func TestValidationEmptyKeywords(t *testing.T) {
    cfg := &Config{
        LogLevel: LogLevelConfig{
            Detection: DetectionConfig{
                Enabled: true,
                Keywords: KeywordsConfig{
                    Error: []string{},  // Empty!
                },
            },
        },
    }
    
    err := cfg.Validate()
    // Should this error or just disable detection?
    assert.Error(t, err, "empty keywords should be invalid")
}

2. Keywords with Empty Strings

Malformed config:

log_level:
  detection:
    keywords:
      error: ["ERROR", "", "FATAL"]  # Empty string in middle!

Test needed:

func TestValidationEmptyStringInKeywords(t *testing.T) {
    cfg := &Config{
        LogLevel: LogLevelConfig{
            Detection: DetectionConfig{
                Keywords: KeywordsConfig{
                    Error: []string{"ERROR", "", "FATAL"},
                },
            },
        },
    }
    
    err := cfg.Validate()
    assert.Error(t, err, "empty string keywords should be invalid")
}

3. Invalid Strftime Format Combinations

Edge cases not tested:

prefix:
  timestamp:
    format: "%Q%R%Z"  # %Q is not valid strftime

Test needed:

func TestValidationInvalidStrftimeFormats(t *testing.T) {
    tests := []struct {
        name    string
        format  string
        wantErr bool
    }{
        {"Valid format", "%Y-%m-%d %H:%M:%S", false},
        {"Invalid directive %Q", "%Q", true},
        {"Invalid directive %K", "%K", true},
        {"Mixed valid/invalid", "%Y-%Q-%m", true},
        {"Empty format", "", false},  // Should empty be allowed?
        {"Only text no directives", "PREFIX: ", false},
        {"Unclosed directive", "%", true},
    }
    
    for _, tt := range tests {
        t.Run(tt.name, func(t *testing.T) {
            cfg := &Config{
                Prefix: PrefixConfig{
                    Timestamp: TimestampConfig{
                        Format: tt.format,
                    },
                },
            }
            
            err := cfg.Validate()
            if tt.wantErr {
                assert.Error(t, err)
            } else {
                assert.NoError(t, err)
            }
        })
    }
}

4. Invalid Timezone Values

Malformed config:

prefix:
  timestamp:
    timezone: "Invalid/Timezone"

Test needed:

func TestValidationInvalidTimezone(t *testing.T) {
    tests := []struct {
        timezone string
        wantErr  bool
    }{
        {"UTC", false},
        {"Local", false},
        {"America/New_York", false},
        {"Invalid/Timezone", true},
        {"", false},  // Empty should default to Local
        {"GMT+5", true},  // Not a valid timezone name
    }
    
    for _, tt := range tests {
        cfg := &Config{
            Prefix: PrefixConfig{
                Timestamp: TimestampConfig{
                    Timezone: tt.timezone,
                },
            },
        }
        
        err := cfg.Validate()
        if tt.wantErr {
            assert.Error(t, err)
        }
    }
}

5. Conflicting Configuration

Example: Detection disabled but keywords provided

log_level:
  detection:
    enabled: false
    keywords:  # Should these be ignored? Error? Warning?
      error: ["ERROR"]

Test needed:

func TestValidationConflictingConfig(t *testing.T) {
    cfg := &Config{
        LogLevel: LogLevelConfig{
            Detection: DetectionConfig{
                Enabled: false,
                Keywords: KeywordsConfig{
                    Error: []string{"ERROR"},
                },
            },
        },
    }
    
    // Should this warn or error?
    err := cfg.Validate()
    // Decision: Probably OK (just ignore keywords)
    assert.NoError(t, err)
}

6. Invalid Log Levels

Case sensitivity test:

log_level:
  default: "error"  # lowercase - currently invalid

Test needed:

func TestValidationLogLevelCase(t *testing.T) {
    tests := []struct {
        level   string
        wantErr bool
    }{
        {"ERROR", false},
        {"error", true},   // Currently invalid
        {"Error", true},   // Also invalid
        {"INVALID", true},
        {"", true},
    }
    
    for _, tt := range tests {
        cfg := &Config{
            LogLevel: LogLevelConfig{
                Default: tt.level,
            },
        }
        
        err := cfg.Validate()
        if tt.wantErr {
            assert.Error(t, err)
        }
    }
}

7. Buffer Mode Validation

If buffer modes are implemented (#16):

func TestValidationBufferMode(t *testing.T) {
    tests := []struct {
        mode    string
        wantErr bool
    }{
        {"line", false},
        {"none", false},
        {"full", false},
        {"invalid", true},
        {"", false},  // Should default to "line"
    }
    
    for _, tt := range tests {
        cfg := &Config{
            Output: OutputConfig{
                Buffer: tt.mode,
            },
        }
        
        err := cfg.Validate()
        if tt.wantErr {
            assert.Error(t, err)
        }
    }
}

Implementation Checklist

High Priority:

• Test empty keywords array
• Test keywords with empty strings
• Test invalid strftime formats
• Test invalid timezones
• Test invalid log levels (case sensitivity)

Medium Priority:

• Test conflicting configuration
• Test buffer mode validation (depends on Implement or remove incomplete buffering feature #16)
• Test template validation edge cases
• Test color code validation

Nice to Have:

• Property-based testing for config validation
• Fuzz testing for config parsing

Error Message Quality

When adding these tests, also verify error messages are helpful:

// Bad error message:
err := fmt.Errorf("invalid config")

// Good error message:
err := fmt.Errorf("invalid log level keyword: empty string at index 1 in error keywords (valid: non-empty strings)")

Table-Driven Test Structure

Organize tests using table-driven approach:

func TestConfigValidation(t *testing.T) {
    tests := []struct {
        name    string
        cfg     *Config
        wantErr bool
        errMsg  string  // Optional: verify specific error message
    }{
        {
            name: "empty keywords array",
            cfg: &Config{...},
            wantErr: true,
            errMsg: "empty keywords",
        },
        // ... more cases
    }
    
    for _, tt := range tests {
        t.Run(tt.name, func(t *testing.T) {
            err := tt.cfg.Validate()
            if tt.wantErr {
                assert.Error(t, err)
                if tt.errMsg != "" {
                    assert.Contains(t, err.Error(), tt.errMsg)
                }
            } else {
                assert.NoError(t, err)
            }
        })
    }
}

Coverage Goals

Target coverage for validation.go:

• Current: ~70% (estimate)
• Target: >90%
• Focus: Edge cases and error paths

# Check current coverage
go test -cover ./pkg/config

# Generate coverage report
go test -coverprofile=coverage.out ./pkg/config
go tool cover -html=coverage.out

Related Issues

• Refactor redundant validation logic in config package #11 - Refactor validation logic (cleanup while adding tests)
• Implement or remove incomplete buffering feature #16 - Buffer mode implementation (add validation tests)
• Enhance strftime timestamp format validation #20 - Timestamp validation (enhance strftime tests)

Benefits

Immediate:

• Catch malformed config before runtime
• Better error messages for users
• More robust validation

Long-term:

• Safer config changes (tests catch regressions)
• Documentation via tests (show what's valid)
• Confidence in validation logic