Skip to content

Commit f5c9701

Browse files
committed
chore: use sloghuman.Sink and exported SlowFlushThreshold in tests
Apply reviewer feedback: - Replace custom captureSink with sloghuman.Sink(&strings.Builder{}) - Use exported SlowFlushThreshold constant instead of hardcoded 600ms
1 parent cdd4c73 commit f5c9701

2 files changed

Lines changed: 14 additions & 39 deletions

File tree

intercept/eventstream/eventstream.go

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,10 @@ import (
2121
var ErrEventStreamClosed = xerrors.New("event stream closed")
2222

2323
const (
24-
pingInterval = time.Second * 10
25-
slowFlushThreshold = time.Millisecond * 500
24+
pingInterval = time.Second * 10
25+
// SlowFlushThreshold is the duration after which a flush to the client is
26+
// considered slow and a warning is logged.
27+
SlowFlushThreshold = time.Millisecond * 500
2628
)
2729

2830
type event []byte
@@ -142,7 +144,7 @@ func (s *EventStream) Start(w http.ResponseWriter, r *http.Request) {
142144
s.logger.Warn(ctx, "failed to flush event stream", slog.Error(err))
143145
return
144146
}
145-
if d := s.clk.Since(flushStart); d > slowFlushThreshold {
147+
if d := s.clk.Since(flushStart); d > SlowFlushThreshold {
146148
s.logger.Warn(ctx, "slow client detected", slog.F("flush_duration", d))
147149
}
148150

intercept/eventstream/eventstream_test.go

Lines changed: 9 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -6,45 +6,20 @@ import (
66
"net"
77
"net/http"
88
"net/http/httptest"
9-
"sync"
9+
"strings"
1010
"testing"
1111
"time"
1212

1313
"github.com/stretchr/testify/require"
1414

1515
"cdr.dev/slog/v3"
16+
"cdr.dev/slog/v3/sloggers/sloghuman"
1617
"cdr.dev/slog/v3/sloggers/slogtest"
1718

1819
"github.com/coder/aibridge/intercept/eventstream"
1920
"github.com/coder/quartz"
2021
)
2122

22-
// captureSink collects log entries for assertions in tests.
23-
type captureSink struct {
24-
mu sync.Mutex
25-
entries []slog.SinkEntry
26-
}
27-
28-
func (s *captureSink) LogEntry(_ context.Context, e slog.SinkEntry) {
29-
s.mu.Lock()
30-
defer s.mu.Unlock()
31-
s.entries = append(s.entries, e)
32-
}
33-
34-
func (*captureSink) Sync() {}
35-
36-
func (s *captureSink) warns() []slog.SinkEntry {
37-
s.mu.Lock()
38-
defer s.mu.Unlock()
39-
var out []slog.SinkEntry
40-
for _, e := range s.entries {
41-
if e.Level == slog.LevelWarn {
42-
out = append(out, e)
43-
}
44-
}
45-
return out
46-
}
47-
4823
// clockAdvancingFlusher wraps httptest.ResponseRecorder and advances the mock
4924
// clock on each Flush call, simulating a slow client without real sleeping.
5025
type clockAdvancingFlusher struct {
@@ -66,8 +41,8 @@ func (*clockAdvancingFlusher) Hijack() (net.Conn, *bufio.ReadWriter, error) {
6641
func TestEventStream_LogsWarning_WhenFlushIsSlow(t *testing.T) {
6742
t.Parallel()
6843

69-
sink := &captureSink{}
70-
logger := slogtest.Make(t, nil).AppendSinks(sink).Leveled(slog.LevelWarn)
44+
var buf strings.Builder
45+
logger := slogtest.Make(t, nil).AppendSinks(sloghuman.Sink(&buf)).Leveled(slog.LevelWarn)
7146
ctx := context.Background()
7247
clk := quartz.NewMock(t)
7348

@@ -76,7 +51,7 @@ func TestEventStream_LogsWarning_WhenFlushIsSlow(t *testing.T) {
7651
w := &clockAdvancingFlusher{
7752
ResponseRecorder: httptest.NewRecorder(),
7853
clk: clk,
79-
advance: 600 * time.Millisecond, // exceeds slowFlushThreshold (500ms)
54+
advance: eventstream.SlowFlushThreshold + time.Millisecond,
8055
}
8156

8257
req, err := http.NewRequestWithContext(ctx, http.MethodGet, "/", nil)
@@ -93,16 +68,14 @@ func TestEventStream_LogsWarning_WhenFlushIsSlow(t *testing.T) {
9368
require.NoError(t, stream.Shutdown(ctx))
9469
<-done
9570

96-
warns := sink.warns()
97-
require.Len(t, warns, 1)
98-
require.Equal(t, "slow client detected", warns[0].Message)
71+
require.Contains(t, buf.String(), "slow client detected")
9972
}
10073

10174
func TestEventStream_NoWarning_WhenFlushIsFast(t *testing.T) {
10275
t.Parallel()
10376

104-
sink := &captureSink{}
105-
logger := slogtest.Make(t, nil).AppendSinks(sink).Leveled(slog.LevelWarn)
77+
var buf strings.Builder
78+
logger := slogtest.Make(t, nil).AppendSinks(sloghuman.Sink(&buf)).Leveled(slog.LevelWarn)
10679
ctx := context.Background()
10780
clk := quartz.NewMock(t)
10881

@@ -129,5 +102,5 @@ func TestEventStream_NoWarning_WhenFlushIsFast(t *testing.T) {
129102
require.NoError(t, stream.Shutdown(ctx))
130103
<-done
131104

132-
require.Empty(t, sink.warns())
105+
require.Empty(t, buf.String())
133106
}

0 commit comments

Comments
 (0)