Skip to content

Commit ea834e1

Browse files
jongiojongio
authored
Merge pull request #250 from jongio/patrol/test-improvements
test: coverage improvements, flaky fix, and dead test cleanup
2 parents 7b48aab + 016033f commit ea834e1

8 files changed

Lines changed: 861 additions & 221 deletions

File tree

‎cli/dashboard/vitest.config.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,12 @@ export default defineConfig({
3232
'**/*.d.ts',
3333
'**/*.config.*',
3434
],
35+
thresholds: {
36+
statements: 40,
37+
branches: 40,
38+
functions: 40,
39+
lines: 40,
40+
},
3541
},
3642
},
3743
resolve: {

‎cli/src/cmd/app/commands/logs_follow_test.go‎

Lines changed: 113 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,22 @@ func newTestExecutor(buf *bytes.Buffer, sigChan chan os.Signal, opts *logsOption
2323
}
2424
}
2525

26+
// waitForOutput polls the buffer until it contains the expected string or times out.
27+
func waitForOutput(buf *bytes.Buffer, contains string, timeout time.Duration) bool {
28+
deadline := time.After(timeout)
29+
for {
30+
select {
31+
case <-deadline:
32+
return false
33+
default:
34+
if strings.Contains(buf.String(), contains) {
35+
return true
36+
}
37+
time.Sleep(5 * time.Millisecond)
38+
}
39+
}
40+
}
41+
2642
func TestLogsExecutor_FollowLogsViaDashboard(t *testing.T) {
2743
t.Run("ping error", func(t *testing.T) {
2844
var buf bytes.Buffer
@@ -60,7 +76,10 @@ func TestLogsExecutor_FollowLogsViaDashboard(t *testing.T) {
6076
done <- executor.followLogsViaDashboard(ctx, mockClient, nil, LogLevelAll, nil, &buf)
6177
}()
6278

63-
time.Sleep(100 * time.Millisecond)
79+
// Poll for output instead of fixed sleep
80+
if !waitForOutput(&buf, "Log", 2*time.Second) {
81+
t.Fatal("timed out waiting for log output")
82+
}
6483
cancel()
6584

6685
err := <-done
@@ -98,7 +117,10 @@ func TestLogsExecutor_FollowLogsViaDashboard(t *testing.T) {
98117
done <- executor.followLogsViaDashboard(ctx, mockClient, nil, service.LogLevelError, nil, &buf)
99118
}()
100119

101-
time.Sleep(100 * time.Millisecond)
120+
// Wait for the error log to appear (it passes the filter)
121+
if !waitForOutput(&buf, "Error log", 2*time.Second) {
122+
t.Log("Warning: no filtered output appeared within timeout")
123+
}
102124
cancel()
103125

104126
<-done
@@ -133,7 +155,10 @@ func TestLogsExecutor_FollowLogsViaDashboard(t *testing.T) {
133155
done <- executor.followLogsViaDashboard(ctx, mockClient, []string{"api", "worker"}, LogLevelAll, nil, &buf)
134156
}()
135157

136-
time.Sleep(100 * time.Millisecond)
158+
// Wait for the API log to appear (it passes the service filter)
159+
if !waitForOutput(&buf, "API log", 2*time.Second) {
160+
t.Log("Warning: no filtered output appeared within timeout")
161+
}
137162
cancel()
138163

139164
<-done
@@ -153,17 +178,21 @@ func TestLogsExecutor_FollowLogsViaDashboard(t *testing.T) {
153178

154179
ctx := context.Background()
155180

156-
done := make(chan error)
181+
done := make(chan error, 1)
157182
go func() {
158183
done <- executor.followLogsViaDashboard(ctx, mockClient, nil, LogLevelAll, nil, &buf)
159184
}()
160185

161-
time.Sleep(10 * time.Millisecond)
186+
// Buffered channel - signal will be picked up when goroutine reaches select
162187
sigChan <- os.Interrupt
163188

164-
err := <-done
165-
if err != nil {
166-
t.Errorf("Expected nil error on signal, got: %v", err)
189+
select {
190+
case err := <-done:
191+
if err != nil {
192+
t.Errorf("Expected nil error on signal, got: %v", err)
193+
}
194+
case <-time.After(5 * time.Second):
195+
t.Fatal("timed out waiting for signal to interrupt streaming")
167196
}
168197
})
169198
}
@@ -177,17 +206,20 @@ func TestLogsExecutor_FollowLogsInMemory(t *testing.T) {
177206
subscriptions := make(map[string]chan service.LogEntry)
178207
mockLM := newMockLogManager()
179208

180-
done := make(chan error)
209+
done := make(chan error, 1)
181210
go func() {
182211
done <- executor.followLogsInMemory(subscriptions, mockLM, LogLevelAll, nil, &buf)
183212
}()
184213

185-
time.Sleep(10 * time.Millisecond)
186214
sigChan <- os.Interrupt
187215

188-
err := <-done
189-
if err != nil {
190-
t.Errorf("Expected nil error on signal, got: %v", err)
216+
select {
217+
case err := <-done:
218+
if err != nil {
219+
t.Errorf("Expected nil error on signal, got: %v", err)
220+
}
221+
case <-time.After(5 * time.Second):
222+
t.Fatal("timed out waiting for signal to interrupt streaming")
191223
}
192224
})
193225

@@ -217,7 +249,10 @@ func TestLogsExecutor_FollowLogsInMemory(t *testing.T) {
217249
now := time.Now()
218250
logChan <- service.LogEntry{Service: "api", Level: service.LogLevelInfo, Message: "Test message", Timestamp: now}
219251

220-
time.Sleep(50 * time.Millisecond)
252+
// Poll for output instead of fixed sleep
253+
if !waitForOutput(&buf, "Test message", 2*time.Second) {
254+
t.Fatal("timed out waiting for log output")
255+
}
221256
sigChan <- os.Interrupt
222257

223258
<-done
@@ -253,7 +288,10 @@ func TestLogsExecutor_FollowLogsInMemory(t *testing.T) {
253288
logChan <- service.LogEntry{Service: "api", Level: service.LogLevelInfo, Message: "Info msg", Timestamp: now}
254289
logChan <- service.LogEntry{Service: "api", Level: service.LogLevelError, Message: "Error msg", Timestamp: now}
255290

256-
time.Sleep(50 * time.Millisecond)
291+
// Poll for the error message to appear (it passes the filter)
292+
if !waitForOutput(&buf, "Error msg", 2*time.Second) {
293+
t.Log("Warning: no filtered output appeared within timeout")
294+
}
257295
sigChan <- os.Interrupt
258296

259297
<-done
@@ -276,17 +314,21 @@ func TestLogsExecutor_FollowLogsInMemory(t *testing.T) {
276314
buf3, _ := service.NewLogBuffer("api", 100, false, "")
277315
mockLM.buffers["api"] = buf3
278316

279-
done := make(chan error)
317+
done := make(chan error, 1)
280318
go func() {
281319
done <- executor.followLogsInMemory(subscriptions, mockLM, LogLevelAll, nil, &buf)
282320
}()
283321

284-
time.Sleep(10 * time.Millisecond)
322+
// Close channel - buffered done chan ensures we don't block
285323
close(logChan)
286324

287-
err := <-done
288-
if err != nil {
289-
t.Errorf("Expected nil error when channels close, got: %v", err)
325+
select {
326+
case err := <-done:
327+
if err != nil {
328+
t.Errorf("Expected nil error when channels close, got: %v", err)
329+
}
330+
case <-time.After(5 * time.Second):
331+
t.Fatal("timed out waiting for close to propagate")
290332
}
291333
})
292334

@@ -311,7 +353,9 @@ func TestLogsExecutor_FollowLogsInMemory(t *testing.T) {
311353
now := time.Now()
312354
logChan <- service.LogEntry{Service: "api", Level: service.LogLevelInfo, Message: "JSON test", Timestamp: now}
313355

314-
time.Sleep(50 * time.Millisecond)
356+
if !waitForOutput(&buf, "JSON test", 2*time.Second) {
357+
t.Fatal("timed out waiting for JSON output")
358+
}
315359
sigChan <- os.Interrupt
316360

317361
<-done
@@ -330,16 +374,19 @@ func TestLogsExecutor_FollowLogs(t *testing.T) {
330374
executor := newTestExecutor(&buf, sigChan, &logsOptions{format: "text"})
331375

332376
mockClient := &mockDashboardClient{}
333-
done := make(chan error)
377+
done := make(chan error, 1)
334378
go func() {
335379
done <- executor.followLogsViaDashboard(context.Background(), mockClient, nil, LogLevelAll, nil, &buf)
336380
}()
337381

338-
time.Sleep(10 * time.Millisecond)
339382
sigChan <- os.Interrupt
340-
err := <-done
341-
if err != nil {
342-
t.Errorf("Expected nil error, got: %v", err)
383+
select {
384+
case err := <-done:
385+
if err != nil {
386+
t.Errorf("Expected nil error, got: %v", err)
387+
}
388+
case <-time.After(5 * time.Second):
389+
t.Fatal("timed out waiting for completion")
343390
}
344391
})
345392

@@ -356,16 +403,19 @@ func TestLogsExecutor_FollowLogs(t *testing.T) {
356403
buf6, _ := service.NewLogBuffer("api", 100, false, "")
357404
mockLM.buffers["api"] = buf6
358405

359-
done := make(chan error)
406+
done := make(chan error, 1)
360407
go func() {
361408
done <- executor.followLogsInMemory(subscriptions, mockLM, LogLevelAll, nil, &buf)
362409
}()
363410

364-
time.Sleep(10 * time.Millisecond)
365411
sigChan <- os.Interrupt
366-
err := <-done
367-
if err != nil {
368-
t.Errorf("Expected nil error, got: %v", err)
412+
select {
413+
case err := <-done:
414+
if err != nil {
415+
t.Errorf("Expected nil error, got: %v", err)
416+
}
417+
case <-time.After(5 * time.Second):
418+
t.Fatal("timed out waiting for completion")
369419
}
370420
})
371421
}
@@ -390,16 +440,19 @@ func TestLogsExecutor_FollowLogsOrchestration(t *testing.T) {
390440
pingErr: context.DeadlineExceeded,
391441
}
392442

393-
done := make(chan error)
443+
done := make(chan error, 1)
394444
go func() {
395445
done <- executor.followLogs(context.Background(), tmpDir, mockLM, mockClient, nil, LogLevelAll, nil, &buf)
396446
}()
397447

398-
time.Sleep(10 * time.Millisecond)
399448
sigChan <- os.Interrupt
400-
err := <-done
401-
if err != nil {
402-
t.Errorf("Expected nil error, got: %v", err)
449+
select {
450+
case err := <-done:
451+
if err != nil {
452+
t.Errorf("Expected nil error, got: %v", err)
453+
}
454+
case <-time.After(5 * time.Second):
455+
t.Fatal("timed out waiting for completion")
403456
}
404457
})
405458

@@ -414,16 +467,19 @@ func TestLogsExecutor_FollowLogsOrchestration(t *testing.T) {
414467

415468
mockClient := &mockDashboardClient{}
416469

417-
done := make(chan error)
470+
done := make(chan error, 1)
418471
go func() {
419472
done <- executor.followLogs(context.Background(), tmpDir, mockLM, mockClient, []string{"api"}, LogLevelAll, nil, &buf)
420473
}()
421474

422-
time.Sleep(10 * time.Millisecond)
423475
sigChan <- os.Interrupt
424-
err := <-done
425-
if err != nil {
426-
t.Errorf("Expected nil error, got: %v", err)
476+
select {
477+
case err := <-done:
478+
if err != nil {
479+
t.Errorf("Expected nil error, got: %v", err)
480+
}
481+
case <-time.After(5 * time.Second):
482+
t.Fatal("timed out waiting for completion")
427483
}
428484
})
429485

@@ -435,16 +491,19 @@ func TestLogsExecutor_FollowLogsOrchestration(t *testing.T) {
435491
mockLM := newMockLogManager()
436492
mockClient := &mockDashboardClient{}
437493

438-
done := make(chan error)
494+
done := make(chan error, 1)
439495
go func() {
440496
done <- executor.followLogs(context.Background(), tmpDir, mockLM, mockClient, nil, LogLevelAll, nil, &buf)
441497
}()
442498

443-
time.Sleep(10 * time.Millisecond)
444499
sigChan <- os.Interrupt
445-
err := <-done
446-
if err != nil {
447-
t.Errorf("Expected nil error, got: %v", err)
500+
select {
501+
case err := <-done:
502+
if err != nil {
503+
t.Errorf("Expected nil error, got: %v", err)
504+
}
505+
case <-time.After(5 * time.Second):
506+
t.Fatal("timed out waiting for completion")
448507
}
449508
})
450509

@@ -459,16 +518,19 @@ func TestLogsExecutor_FollowLogsOrchestration(t *testing.T) {
459518

460519
mockClient := &mockDashboardClient{}
461520

462-
done := make(chan error)
521+
done := make(chan error, 1)
463522
go func() {
464523
done <- executor.followLogs(context.Background(), tmpDir, mockLM, mockClient, []string{"nonexistent"}, LogLevelAll, nil, &buf)
465524
}()
466525

467-
time.Sleep(10 * time.Millisecond)
468526
sigChan <- os.Interrupt
469-
err := <-done
470-
if err != nil {
471-
t.Errorf("Expected nil error, got: %v", err)
527+
select {
528+
case err := <-done:
529+
if err != nil {
530+
t.Errorf("Expected nil error, got: %v", err)
531+
}
532+
case <-time.After(5 * time.Second):
533+
t.Fatal("timed out waiting for completion")
472534
}
473535
})
474536
}

0 commit comments

Comments
 (0)