Skip to content

Commit 8175489

Browse files
committed
test: Improve pkg/detector test coverage from 69.6% to 84.2%
- Fix integration tests by handling both ReloadCoordinator and detector event naming - Add webhook content comparison in exportersEqual (fixes config change detection bug) - Add comprehensive statistics tests for GetTotalExports, GetExportSuccessRate, GetDeduplicationRate - Add MonitorHandle tests for GetConfig, IsRunning, and Stop methods - Add ProblemDetector.Run() test coverage - Replace deprecated ioutil functions with os package equivalents - All tests pass with race detection enabled Task: #3829
1 parent 1988139 commit 8175489

File tree

4 files changed

+604
-19
lines changed

4 files changed

+604
-19
lines changed
Lines changed: 304 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,304 @@
1+
package detector
2+
3+
import (
4+
"context"
5+
"sync"
6+
"testing"
7+
"time"
8+
9+
"github.com/supporttools/node-doctor/pkg/types"
10+
)
11+
12+
// TestMonitorHandle_GetConfig tests the GetConfig method of MonitorHandle.
13+
func TestMonitorHandle_GetConfig(t *testing.T) {
14+
config := types.MonitorConfig{
15+
Name: "test-monitor",
16+
Type: "test",
17+
Enabled: true,
18+
Interval: 10 * time.Second,
19+
Timeout: 5 * time.Second,
20+
Config: map[string]interface{}{
21+
"key": "value",
22+
},
23+
}
24+
25+
ctx, cancel := context.WithCancel(context.Background())
26+
defer cancel()
27+
28+
mock := NewMockMonitor(config.Name)
29+
statusCh := make(chan *types.Status)
30+
31+
mh := &MonitorHandle{
32+
monitor: mock,
33+
config: config,
34+
statusCh: statusCh,
35+
cancelFunc: cancel,
36+
wg: &sync.WaitGroup{},
37+
ctx: ctx,
38+
stopped: false,
39+
}
40+
41+
// Test GetConfig returns the correct config
42+
got := mh.GetConfig()
43+
44+
if got.Name != config.Name {
45+
t.Errorf("GetConfig().Name = %s, want %s", got.Name, config.Name)
46+
}
47+
if got.Type != config.Type {
48+
t.Errorf("GetConfig().Type = %s, want %s", got.Type, config.Type)
49+
}
50+
if got.Enabled != config.Enabled {
51+
t.Errorf("GetConfig().Enabled = %v, want %v", got.Enabled, config.Enabled)
52+
}
53+
if got.Interval != config.Interval {
54+
t.Errorf("GetConfig().Interval = %v, want %v", got.Interval, config.Interval)
55+
}
56+
if got.Timeout != config.Timeout {
57+
t.Errorf("GetConfig().Timeout = %v, want %v", got.Timeout, config.Timeout)
58+
}
59+
}
60+
61+
// TestMonitorHandle_IsRunning tests the IsRunning method of MonitorHandle.
62+
func TestMonitorHandle_IsRunning(t *testing.T) {
63+
tests := []struct {
64+
name string
65+
stopped bool
66+
want bool
67+
}{
68+
{
69+
name: "running when not stopped",
70+
stopped: false,
71+
want: true,
72+
},
73+
{
74+
name: "not running when stopped",
75+
stopped: true,
76+
want: false,
77+
},
78+
}
79+
80+
for _, tt := range tests {
81+
t.Run(tt.name, func(t *testing.T) {
82+
ctx, cancel := context.WithCancel(context.Background())
83+
defer cancel()
84+
85+
mock := NewMockMonitor("test")
86+
statusCh := make(chan *types.Status)
87+
88+
mh := &MonitorHandle{
89+
monitor: mock,
90+
config: types.MonitorConfig{Name: "test"},
91+
statusCh: statusCh,
92+
cancelFunc: cancel,
93+
wg: &sync.WaitGroup{},
94+
ctx: ctx,
95+
stopped: tt.stopped,
96+
}
97+
98+
got := mh.IsRunning()
99+
if got != tt.want {
100+
t.Errorf("IsRunning() = %v, want %v", got, tt.want)
101+
}
102+
})
103+
}
104+
}
105+
106+
// TestMonitorHandle_IsRunning_ConcurrentAccess tests thread safety of IsRunning.
107+
func TestMonitorHandle_IsRunning_ConcurrentAccess(t *testing.T) {
108+
ctx, cancel := context.WithCancel(context.Background())
109+
defer cancel()
110+
111+
mock := NewMockMonitor("test")
112+
statusCh := make(chan *types.Status)
113+
114+
mh := &MonitorHandle{
115+
monitor: mock,
116+
config: types.MonitorConfig{Name: "test"},
117+
statusCh: statusCh,
118+
cancelFunc: cancel,
119+
wg: &sync.WaitGroup{},
120+
ctx: ctx,
121+
stopped: false,
122+
}
123+
124+
var wg sync.WaitGroup
125+
const goroutines = 10
126+
const iterations = 100
127+
128+
// Start multiple goroutines calling IsRunning
129+
for i := 0; i < goroutines; i++ {
130+
wg.Add(1)
131+
go func() {
132+
defer wg.Done()
133+
for j := 0; j < iterations; j++ {
134+
_ = mh.IsRunning()
135+
}
136+
}()
137+
}
138+
139+
// Stop the monitor midway through
140+
go func() {
141+
time.Sleep(5 * time.Millisecond)
142+
mh.mu.Lock()
143+
mh.stopped = true
144+
mh.mu.Unlock()
145+
}()
146+
147+
wg.Wait()
148+
// Test passes if no race conditions or panics occurred
149+
}
150+
151+
// TestMonitorHandle_GetName tests the GetName method of MonitorHandle.
152+
func TestMonitorHandle_GetName(t *testing.T) {
153+
tests := []struct {
154+
name string
155+
monitorName string
156+
expectedName string
157+
}{
158+
{
159+
name: "simple name",
160+
monitorName: "test-monitor",
161+
expectedName: "test-monitor",
162+
},
163+
{
164+
name: "name with spaces",
165+
monitorName: "my test monitor",
166+
expectedName: "my test monitor",
167+
},
168+
{
169+
name: "empty name",
170+
monitorName: "",
171+
expectedName: "",
172+
},
173+
}
174+
175+
for _, tt := range tests {
176+
t.Run(tt.name, func(t *testing.T) {
177+
ctx, cancel := context.WithCancel(context.Background())
178+
defer cancel()
179+
180+
mh := &MonitorHandle{
181+
monitor: NewMockMonitor(tt.monitorName),
182+
config: types.MonitorConfig{Name: tt.monitorName},
183+
statusCh: make(chan *types.Status),
184+
cancelFunc: cancel,
185+
wg: &sync.WaitGroup{},
186+
ctx: ctx,
187+
stopped: false,
188+
}
189+
190+
got := mh.GetName()
191+
if got != tt.expectedName {
192+
t.Errorf("GetName() = %s, want %s", got, tt.expectedName)
193+
}
194+
})
195+
}
196+
}
197+
198+
// TestProblemDetector_Run tests the Run method which is an alias for Start.
199+
func TestProblemDetector_Run(t *testing.T) {
200+
helper := NewTestHelper()
201+
config := helper.CreateTestConfig()
202+
factory := NewMockMonitorFactory()
203+
204+
detector, err := NewProblemDetector(
205+
config,
206+
[]types.Monitor{NewMockMonitor("test")},
207+
[]types.Exporter{NewMockExporter("test")},
208+
"/tmp/test-config.yaml",
209+
factory,
210+
)
211+
if err != nil {
212+
t.Fatalf("NewProblemDetector() error = %v", err)
213+
}
214+
defer detector.Stop()
215+
216+
// Test that Run() starts the detector (same as Start())
217+
err = detector.Run()
218+
if err != nil {
219+
t.Errorf("Run() error = %v", err)
220+
}
221+
222+
// Verify detector is running
223+
if !detector.IsRunning() {
224+
t.Error("Run() should have started the detector")
225+
}
226+
227+
// Test that calling Run() again returns an error (same as Start())
228+
err = detector.Run()
229+
if err == nil {
230+
t.Error("Run() should error when detector is already running")
231+
}
232+
}
233+
234+
// TestMonitorHandle_Stop tests the Stop method of MonitorHandle.
235+
func TestMonitorHandle_Stop(t *testing.T) {
236+
ctx, cancel := context.WithCancel(context.Background())
237+
238+
mock := NewMockMonitor("test")
239+
statusCh := make(chan *types.Status)
240+
241+
mh := &MonitorHandle{
242+
monitor: mock,
243+
config: types.MonitorConfig{Name: "test"},
244+
statusCh: statusCh,
245+
cancelFunc: cancel,
246+
wg: &sync.WaitGroup{},
247+
ctx: ctx,
248+
stopped: false,
249+
}
250+
251+
// Stop should succeed the first time
252+
err := mh.Stop()
253+
if err != nil {
254+
t.Errorf("Stop() error = %v", err)
255+
}
256+
257+
// Verify IsRunning returns false after stop
258+
if mh.IsRunning() {
259+
t.Error("IsRunning() should return false after Stop()")
260+
}
261+
262+
// Stop should be idempotent
263+
err = mh.Stop()
264+
if err != nil {
265+
t.Errorf("Stop() should be idempotent, got error = %v", err)
266+
}
267+
}
268+
269+
// TestMonitorHandle_StopConcurrent tests concurrent Stop calls.
270+
func TestMonitorHandle_StopConcurrent(t *testing.T) {
271+
ctx, cancel := context.WithCancel(context.Background())
272+
273+
mock := NewMockMonitor("test")
274+
statusCh := make(chan *types.Status)
275+
276+
mh := &MonitorHandle{
277+
monitor: mock,
278+
config: types.MonitorConfig{Name: "test"},
279+
statusCh: statusCh,
280+
cancelFunc: cancel,
281+
wg: &sync.WaitGroup{},
282+
ctx: ctx,
283+
stopped: false,
284+
}
285+
286+
var wg sync.WaitGroup
287+
const goroutines = 10
288+
289+
// Start multiple goroutines calling Stop concurrently
290+
for i := 0; i < goroutines; i++ {
291+
wg.Add(1)
292+
go func() {
293+
defer wg.Done()
294+
_ = mh.Stop()
295+
}()
296+
}
297+
298+
wg.Wait()
299+
300+
// Verify monitor is stopped
301+
if mh.IsRunning() {
302+
t.Error("Monitor should be stopped after concurrent Stop calls")
303+
}
304+
}

0 commit comments

Comments
 (0)