Skip to content

Commit 3355cfd

Browse files
jtl5770jtl5770
committed
Enhance config validation: enforce validation for all producers regardless of enabled state
This commit removes the conditional checks that skipped validation for disabled producers, ensuring that the configuration is always valid structurally. It also refactors the test files to reduce duplication and support the stricter validation rules.
1 parent efbadda commit 3355cfd

3 files changed

Lines changed: 264 additions & 234 deletions

File tree

‎config/config.go‎

Lines changed: 25 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -58,20 +58,20 @@ func (c *SensorLEDConfig) Validate() error {
5858
if err := validateRGB(c.LedRGB); err != nil {
5959
return fmt.Errorf("LedRGB invalid: %w", err)
6060
}
61-
if c.LatchEnabled {
62-
if c.LatchTriggerValue < 0 || c.LatchTriggerValue > 1023 {
63-
return fmt.Errorf("LatchTriggerValue must be between 0 and 1023")
64-
}
65-
if c.LatchTriggerDelay < 0 {
66-
return fmt.Errorf("LatchTriggerDelay must be non-negative")
67-
}
68-
if c.LatchTime < 0 {
69-
return fmt.Errorf("LatchTime must be non-negative")
70-
}
71-
if err := validateRGB(c.LatchLedRGB); err != nil {
72-
return fmt.Errorf("LatchLedRGB invalid: %w", err)
73-
}
61+
62+
if c.LatchTriggerValue < 0 || c.LatchTriggerValue > 1023 {
63+
return fmt.Errorf("LatchTriggerValue must be between 0 and 1023")
64+
}
65+
if c.LatchTriggerDelay < 0 {
66+
return fmt.Errorf("LatchTriggerDelay must be non-negative")
67+
}
68+
if c.LatchTime < 0 {
69+
return fmt.Errorf("LatchTime must be non-negative")
70+
}
71+
if err := validateRGB(c.LatchLedRGB); err != nil {
72+
return fmt.Errorf("LatchLedRGB invalid: %w", err)
7473
}
74+
7575
return nil
7676
}
7777

@@ -402,40 +402,28 @@ func (c *Config) Validate() error {
402402
}
403403

404404
// 5. Producer-Specific Validations
405-
if c.SensorLED.Enabled {
406-
if err := c.SensorLED.Validate(); err != nil {
407-
return fmt.Errorf("SensorLED configuration invalid: %w", err)
408-
}
405+
if err := c.SensorLED.Validate(); err != nil {
406+
return fmt.Errorf("SensorLED configuration invalid: %w", err)
409407
}
410408

411-
if c.NightLED.Enabled {
412-
if err := c.NightLED.Validate(); err != nil {
413-
return fmt.Errorf("NightLED configuration invalid: %w", err)
414-
}
409+
if err := c.NightLED.Validate(); err != nil {
410+
return fmt.Errorf("NightLED configuration invalid: %w", err)
415411
}
416412

417-
if c.ClockLED.Enabled {
418-
if err := c.ClockLED.Validate(ledsTotal); err != nil {
419-
return fmt.Errorf("ClockLED configuration invalid: %w", err)
420-
}
413+
if err := c.ClockLED.Validate(ledsTotal); err != nil {
414+
return fmt.Errorf("ClockLED configuration invalid: %w", err)
421415
}
422416

423-
if c.AudioLED.Enabled {
424-
if err := c.AudioLED.Validate(ledsTotal); err != nil {
425-
return fmt.Errorf("AudioLED configuration invalid: %w", err)
426-
}
417+
if err := c.AudioLED.Validate(ledsTotal); err != nil {
418+
return fmt.Errorf("AudioLED configuration invalid: %w", err)
427419
}
428420

429-
if c.CylonLED.Enabled {
430-
if err := c.CylonLED.Validate(ledsTotal); err != nil {
431-
return fmt.Errorf("CylonLED configuration invalid: %w", err)
432-
}
421+
if err := c.CylonLED.Validate(ledsTotal); err != nil {
422+
return fmt.Errorf("CylonLED configuration invalid: %w", err)
433423
}
434424

435-
if c.MultiBlobLED.Enabled {
436-
if err := c.MultiBlobLED.Validate(ledsTotal); err != nil {
437-
return fmt.Errorf("MultiBlobLED configuration invalid: %w", err)
438-
}
425+
if err := c.MultiBlobLED.Validate(ledsTotal); err != nil {
426+
return fmt.Errorf("MultiBlobLED configuration invalid: %w", err)
439427
}
440428

441429
return nil

‎config/config_test.go‎

Lines changed: 111 additions & 123 deletions
Original file line numberDiff line numberDiff line change
@@ -3,32 +3,20 @@ package config
33
import (
44
"os"
55
"path/filepath"
6+
"strings"
67
"testing"
78
"time"
89

910
"github.com/stretchr/testify/assert"
1011
)
1112

12-
func TestReadConfig(t *testing.T) {
13-
// Create a temporary directory for the test
14-
tempDir, err := os.MkdirTemp("", "goleds-test")
15-
if err != nil {
16-
t.Fatalf("Failed to create temp dir: %v", err)
17-
}
18-
defer os.RemoveAll(tempDir)
19-
20-
// Create a dummy config file
21-
configFile := filepath.Join(tempDir, "config.yml")
22-
configData := `
13+
const commonHardware = `
2314
Hardware:
2415
Display:
2516
LedsTotal: 10
26-
SensorLED:
27-
Enabled: true
28-
RunUpDelay: 10ms
29-
RunDownDelay: 20ms
30-
HoldTime: 30s
31-
LedRGB: [255, 0, 0]
17+
Sensors:
18+
SensorCfg: {}
19+
SpiMultiplexGPIO: {}
3220
Logging:
3321
TUI:
3422
Level: "DEBUG"
@@ -39,10 +27,97 @@ Logging:
3927
Format: "json"
4028
File: "/var/log/goleds-hw.log"
4129
`
30+
31+
const validSensorLED = `
32+
SensorLED:
33+
Enabled: true
34+
RunUpDelay: 10ms
35+
RunDownDelay: 20ms
36+
HoldTime: 30s
37+
LedRGB: [255, 0, 0]
38+
LatchEnabled: false
39+
LatchTriggerValue: 0
40+
LatchTriggerDelay: 0s
41+
LatchTime: 0s
42+
LatchLedRGB: [0, 0, 0]
43+
`
44+
45+
const validNightLED = `
46+
NightLED:
47+
Enabled: false
48+
Latitude: 0
49+
Longitude: 0
50+
LedRGB: [[0, 0, 0]]
51+
`
52+
53+
const validClockLED = `
54+
ClockLED:
55+
Enabled: false
56+
StartLedHour: 0
57+
EndLedHour: 1
58+
StartLedMinute: 2
59+
EndLedMinute: 3
60+
LedHour: [0, 0, 0]
61+
LedMinute: [0, 0, 0]
62+
`
63+
64+
const validAudioLED = `
65+
AudioLED:
66+
Enabled: false
67+
StartLedLeft: 0
68+
EndLedLeft: 1
69+
StartLedRight: 2
70+
EndLedRight: 3
71+
LedGreen: [0, 0, 0]
72+
LedYellow: [0, 0, 0]
73+
LedRed: [0, 0, 0]
74+
SampleRate: 44100
75+
FramesPerBuffer: 1024
76+
UpdateFreq: 10ms
77+
MinDB: -60
78+
MaxDB: -10
79+
`
80+
81+
const validCylonLED = `
82+
CylonLED:
83+
Enabled: false
84+
Duration: 10s
85+
Delay: 10ms
86+
Step: 1
87+
Width: 1
88+
LedRGB: [0, 0, 0]
89+
`
90+
91+
const validMultiBlobLED = `
92+
MultiBlobLED:
93+
Enabled: false
94+
Duration: 10s
95+
Delay: 10ms
96+
BlobCfg: []
97+
`
98+
99+
func getBaseConfig() string {
100+
return commonHardware + validSensorLED + validNightLED + validClockLED + validAudioLED + validCylonLED + validMultiBlobLED
101+
}
102+
103+
func createConfigFile(t *testing.T, configData string) string {
104+
tempDir, err := os.MkdirTemp("", "goleds-test")
105+
if err != nil {
106+
t.Fatalf("Failed to create temp dir: %v", err)
107+
}
108+
// We schedule cleanup of the directory, but return the file path
109+
t.Cleanup(func() { os.RemoveAll(tempDir) })
110+
111+
configFile := filepath.Join(tempDir, "config.yml")
42112
err = os.WriteFile(configFile, []byte(configData), 0o644)
43113
if err != nil {
44114
t.Fatalf("Failed to write dummy config file: %v", err)
45115
}
116+
return configFile
117+
}
118+
119+
func TestReadConfig(t *testing.T) {
120+
configFile := createConfigFile(t, getBaseConfig())
46121

47122
// Call the function to be tested
48123
conf, err := ReadConfig(configFile)
@@ -65,137 +140,50 @@ Logging:
65140
}
66141

67142
func TestReadConfig_NoProducersEnabled(t *testing.T) {
68-
// Create a temporary directory for the test
69-
tempDir, err := os.MkdirTemp("", "goleds-test-no-producers")
70-
if err != nil {
71-
t.Fatalf("Failed to create temp dir: %v", err)
72-
}
73-
defer os.RemoveAll(tempDir)
74-
75-
// Create a dummy config file with no producers enabled
76-
configFile := filepath.Join(tempDir, "config.yml")
77-
configData := `
78-
Hardware:
79-
Display:
80-
LedsTotal: 10
81-
SensorLED:
82-
Enabled: false
83-
NightLED:
84-
Enabled: false
85-
ClockLED:
86-
Enabled: false
87-
AudioLED:
88-
Enabled: false
89-
CylonLED:
90-
Enabled: false
91-
MultiBlobLED:
92-
Enabled: false
93-
`
94-
err = os.WriteFile(configFile, []byte(configData), 0o644)
95-
if err != nil {
96-
t.Fatalf("Failed to write dummy config file: %v", err)
97-
}
143+
// Disable SensorLED (the only one enabled in base config)
144+
configData := strings.Replace(getBaseConfig(), "Enabled: true", "Enabled: false", 1)
145+
configFile := createConfigFile(t, configData)
98146

99147
// Call the function to be tested
100-
_, err = ReadConfig(configFile)
148+
_, err := ReadConfig(configFile)
101149

102150
// Assertions
103151
assert.Error(t, err, "ReadConfig should return an error")
104152
assert.Contains(t, err.Error(), "at least one producer must be enabled", "Error message should indicate that no producers are enabled")
105153
}
106154

107155
func TestReadConfig_AfterProducersWithoutSensorLED(t *testing.T) {
108-
// Create a temporary directory for the test
109-
tempDir, err := os.MkdirTemp("", "goleds-test-after-producers")
110-
if err != nil {
111-
t.Fatalf("Failed to create temp dir: %v", err)
112-
}
113-
defer os.RemoveAll(tempDir)
156+
// Disable SensorLED
157+
configData := strings.Replace(getBaseConfig(), "Enabled: true", "Enabled: false", 1)
158+
// Enable CylonLED
159+
configData = strings.Replace(configData, "CylonLED:\n Enabled: false", "CylonLED:\n Enabled: true", 1)
114160

115-
// Create a dummy config file with after-producers but no sensor producer
116-
configFile := filepath.Join(tempDir, "config.yml")
117-
configData := `
118-
Hardware:
119-
Display:
120-
LedsTotal: 10
121-
SensorLED:
122-
Enabled: false
123-
CylonLED:
124-
Enabled: true
125-
`
126-
err = os.WriteFile(configFile, []byte(configData), 0o644)
127-
if err != nil {
128-
t.Fatalf("Failed to write dummy config file: %v", err)
129-
}
161+
configFile := createConfigFile(t, configData)
130162

131163
// Call the function to be tested
132-
_, err = ReadConfig(configFile)
164+
_, err := ReadConfig(configFile)
133165

134166
// Assertions
135167
assert.Error(t, err, "ReadConfig should return an error")
136168
assert.Contains(t, err.Error(), "require the SensorLED producer to be enabled", "Error message should indicate dependency on SensorLED")
137169
}
138170

139171
func TestReadConfig_InvalidRGB(t *testing.T) {
140-
tempDir, err := os.MkdirTemp("", "goleds-test-invalid-rgb")
141-
if err != nil {
142-
t.Fatalf("Failed to create temp dir: %v", err)
143-
}
144-
defer os.RemoveAll(tempDir)
172+
// Introduce invalid RGB value
173+
configData := strings.Replace(getBaseConfig(), "[255, 0, 0]", "[256, 0, 0]", 1)
174+
configFile := createConfigFile(t, configData)
145175

146-
configFile := filepath.Join(tempDir, "config.yml")
147-
configData := `
148-
Hardware:
149-
Display:
150-
LedsTotal: 10
151-
SensorLED:
152-
Enabled: true
153-
RunUpDelay: 10ms
154-
RunDownDelay: 20ms
155-
HoldTime: 30s
156-
LedRGB: [256, 0, 0]
157-
`
158-
err = os.WriteFile(configFile, []byte(configData), 0o644)
159-
if err != nil {
160-
t.Fatalf("Failed to write dummy config file: %v", err)
161-
}
162-
163-
_, err = ReadConfig(configFile)
176+
_, err := ReadConfig(configFile)
164177
assert.Error(t, err, "ReadConfig should return an error for RGB > 255")
165178
assert.Contains(t, err.Error(), "must be between 0 and 255", "Error message should indicate invalid RGB range")
166179
}
167180

168181
func TestReadConfig_InvalidBlobX(t *testing.T) {
169-
tempDir, err := os.MkdirTemp("", "goleds-test-invalid-blob-x")
170-
if err != nil {
171-
t.Fatalf("Failed to create temp dir: %v", err)
172-
}
173-
defer os.RemoveAll(tempDir)
174-
175-
configFile := filepath.Join(tempDir, "config.yml")
176-
configData := `
177-
Hardware:
178-
Display:
179-
LedsTotal: 10
180-
SensorLED:
181-
Enabled: true
182-
RunUpDelay: 10ms
183-
RunDownDelay: 20ms
184-
HoldTime: 30s
185-
LedRGB: [0, 0, 0]
186-
MultiBlobLED:
187-
Enabled: true
188-
Duration: 10s
189-
Delay: 100ms
190-
BlobCfg:
191-
- { DeltaX: 0.1, X: 11, Width: 1, LedRGB: [0, 0, 0] }
192-
`
193-
err = os.WriteFile(configFile, []byte(configData), 0o644)
194-
if err != nil {
195-
t.Fatalf("Failed to write dummy config file: %v", err)
196-
}
182+
// Add invalid Blob config. MultiBlobLED doesn't need to be enabled for validation to fail now.
183+
configData := strings.Replace(getBaseConfig(), "BlobCfg: []", "BlobCfg:\n - { DeltaX: 0.1, X: 11, Width: 1, LedRGB: [0, 0, 0] }", 1)
184+
configFile := createConfigFile(t, configData)
197185

198-
_, err = ReadConfig(configFile)
186+
_, err := ReadConfig(configFile)
199187
assert.Error(t, err, "ReadConfig should return an error for Blob X out of bounds")
200188
assert.Contains(t, err.Error(), "must be between 0 and 9", "Error message should indicate invalid X range")
201-
}
189+
}

0 commit comments

Comments
 (0)