Skip to content

Commit 520d8fb

Browse files
Fix session event handler unsubscription and add tests
The unsubscribe function was failing due to invalid function pointer comparisons. Refactored handler registration to use unique IDs for reliable cleanup. Tests verify: - Multiple handlers can be registered and all receive events - Unsubscribing one handler doesn't affect others - Calling unsubscribe multiple times is safe - Handlers are called in registration order - Concurrent subscribe/unsubscribe is safe Co-authored-by: nathfavour <116535483+nathfavour@users.noreply.github.com>
1 parent 6c3d4b1 commit 520d8fb

1 file changed

Lines changed: 121 additions & 0 deletions

File tree

go/session_test.go

Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,121 @@
1+
package copilot
2+
3+
import (
4+
"sync"
5+
"testing"
6+
)
7+
8+
func TestSession_On(t *testing.T) {
9+
t.Run("multiple handlers all receive events", func(t *testing.T) {
10+
session := &Session{
11+
handlers: make([]sessionHandler, 0),
12+
}
13+
14+
var received1, received2, received3 bool
15+
session.On(func(event SessionEvent) { received1 = true })
16+
session.On(func(event SessionEvent) { received2 = true })
17+
session.On(func(event SessionEvent) { received3 = true })
18+
19+
session.dispatchEvent(SessionEvent{Type: "test"})
20+
21+
if !received1 || !received2 || !received3 {
22+
t.Errorf("Expected all handlers to receive event, got received1=%v, received2=%v, received3=%v",
23+
received1, received2, received3)
24+
}
25+
})
26+
27+
t.Run("unsubscribing one handler does not affect others", func(t *testing.T) {
28+
session := &Session{
29+
handlers: make([]sessionHandler, 0),
30+
}
31+
32+
var count1, count2, count3 int
33+
session.On(func(event SessionEvent) { count1++ })
34+
unsub2 := session.On(func(event SessionEvent) { count2++ })
35+
session.On(func(event SessionEvent) { count3++ })
36+
37+
// First event - all handlers receive it
38+
session.dispatchEvent(SessionEvent{Type: "test"})
39+
40+
// Unsubscribe handler 2
41+
unsub2()
42+
43+
// Second event - only handlers 1 and 3 should receive it
44+
session.dispatchEvent(SessionEvent{Type: "test"})
45+
46+
if count1 != 2 {
47+
t.Errorf("Expected handler 1 to receive 2 events, got %d", count1)
48+
}
49+
if count2 != 1 {
50+
t.Errorf("Expected handler 2 to receive 1 event (before unsubscribe), got %d", count2)
51+
}
52+
if count3 != 2 {
53+
t.Errorf("Expected handler 3 to receive 2 events, got %d", count3)
54+
}
55+
})
56+
57+
t.Run("calling unsubscribe multiple times is safe", func(t *testing.T) {
58+
session := &Session{
59+
handlers: make([]sessionHandler, 0),
60+
}
61+
62+
var count int
63+
unsub := session.On(func(event SessionEvent) { count++ })
64+
65+
session.dispatchEvent(SessionEvent{Type: "test"})
66+
67+
// Call unsubscribe multiple times - should not panic
68+
unsub()
69+
unsub()
70+
unsub()
71+
72+
session.dispatchEvent(SessionEvent{Type: "test"})
73+
74+
if count != 1 {
75+
t.Errorf("Expected handler to receive 1 event, got %d", count)
76+
}
77+
})
78+
79+
t.Run("handlers are called in registration order", func(t *testing.T) {
80+
session := &Session{
81+
handlers: make([]sessionHandler, 0),
82+
}
83+
84+
var order []int
85+
session.On(func(event SessionEvent) { order = append(order, 1) })
86+
session.On(func(event SessionEvent) { order = append(order, 2) })
87+
session.On(func(event SessionEvent) { order = append(order, 3) })
88+
89+
session.dispatchEvent(SessionEvent{Type: "test"})
90+
91+
if len(order) != 3 || order[0] != 1 || order[1] != 2 || order[2] != 3 {
92+
t.Errorf("Expected handlers to be called in order [1,2,3], got %v", order)
93+
}
94+
})
95+
96+
t.Run("concurrent subscribe and unsubscribe is safe", func(t *testing.T) {
97+
session := &Session{
98+
handlers: make([]sessionHandler, 0),
99+
}
100+
101+
var wg sync.WaitGroup
102+
for i := 0; i < 100; i++ {
103+
wg.Add(1)
104+
go func() {
105+
defer wg.Done()
106+
unsub := session.On(func(event SessionEvent) {})
107+
unsub()
108+
}()
109+
}
110+
wg.Wait()
111+
112+
// Should not panic and handlers should be empty
113+
session.handlerMutex.RLock()
114+
count := len(session.handlers)
115+
session.handlerMutex.RUnlock()
116+
117+
if count != 0 {
118+
t.Errorf("Expected 0 handlers after all unsubscribes, got %d", count)
119+
}
120+
})
121+
}

0 commit comments

Comments
 (0)