Skip to content

Commit 3cb895f

Browse files
stephentoubCopilot
andcommitted
Fix data race in TestStreamingFidelity
Add sync.Mutex to protect concurrent access to the events slice in all three subtests, matching the pattern used throughout the rest of the E2E test suite. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent d979761 commit 3cb895f

1 file changed

Lines changed: 31 additions & 6 deletions

File tree

go/internal/e2e/streaming_fidelity_test.go

Lines changed: 31 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package e2e
22

33
import (
44
"strings"
5+
"sync"
56
"testing"
67

78
copilot "github.com/github/copilot-sdk/go"
@@ -25,18 +26,26 @@ func TestStreamingFidelity(t *testing.T) {
2526
}
2627

2728
var events []copilot.SessionEvent
29+
var mu sync.Mutex
2830
session.On(func(event copilot.SessionEvent) {
31+
mu.Lock()
2932
events = append(events, event)
33+
mu.Unlock()
3034
})
3135

3236
_, err = session.SendAndWait(t.Context(), copilot.MessageOptions{Prompt: "Count from 1 to 5, separated by commas."})
3337
if err != nil {
3438
t.Fatalf("Failed to send message: %v", err)
3539
}
3640

41+
mu.Lock()
42+
snapshot := make([]copilot.SessionEvent, len(events))
43+
copy(snapshot, events)
44+
mu.Unlock()
45+
3746
// Should have streaming deltas before the final message
3847
var deltaEvents []copilot.SessionEvent
39-
for _, e := range events {
48+
for _, e := range snapshot {
4049
if e.Type == "assistant.message_delta" {
4150
deltaEvents = append(deltaEvents, e)
4251
}
@@ -54,7 +63,7 @@ func TestStreamingFidelity(t *testing.T) {
5463

5564
// Should still have a final assistant.message
5665
hasAssistantMessage := false
57-
for _, e := range events {
66+
for _, e := range snapshot {
5867
if e.Type == "assistant.message" {
5968
hasAssistantMessage = true
6069
break
@@ -67,7 +76,7 @@ func TestStreamingFidelity(t *testing.T) {
6776
// Deltas should come before the final message
6877
firstDeltaIdx := -1
6978
lastAssistantIdx := -1
70-
for i, e := range events {
79+
for i, e := range snapshot {
7180
if e.Type == "assistant.message_delta" && firstDeltaIdx == -1 {
7281
firstDeltaIdx = i
7382
}
@@ -92,18 +101,26 @@ func TestStreamingFidelity(t *testing.T) {
92101
}
93102

94103
var events []copilot.SessionEvent
104+
var mu sync.Mutex
95105
session.On(func(event copilot.SessionEvent) {
106+
mu.Lock()
96107
events = append(events, event)
108+
mu.Unlock()
97109
})
98110

99111
_, err = session.SendAndWait(t.Context(), copilot.MessageOptions{Prompt: "Say 'hello world'."})
100112
if err != nil {
101113
t.Fatalf("Failed to send message: %v", err)
102114
}
103115

116+
mu.Lock()
117+
snapshot := make([]copilot.SessionEvent, len(events))
118+
copy(snapshot, events)
119+
mu.Unlock()
120+
104121
// No deltas when streaming is off
105122
var deltaEvents []copilot.SessionEvent
106-
for _, e := range events {
123+
for _, e := range snapshot {
107124
if e.Type == "assistant.message_delta" {
108125
deltaEvents = append(deltaEvents, e)
109126
}
@@ -114,7 +131,7 @@ func TestStreamingFidelity(t *testing.T) {
114131

115132
// But should still have a final assistant.message
116133
var assistantEvents []copilot.SessionEvent
117-
for _, e := range events {
134+
for _, e := range snapshot {
118135
if e.Type == "assistant.message" {
119136
assistantEvents = append(assistantEvents, e)
120137
}
@@ -153,8 +170,11 @@ func TestStreamingFidelity(t *testing.T) {
153170
}
154171

155172
var events []copilot.SessionEvent
173+
var mu sync.Mutex
156174
session2.On(func(event copilot.SessionEvent) {
175+
mu.Lock()
157176
events = append(events, event)
177+
mu.Unlock()
158178
})
159179

160180
answer, err := session2.SendAndWait(t.Context(), copilot.MessageOptions{Prompt: "Now if you double that, what do you get?"})
@@ -167,9 +187,14 @@ func TestStreamingFidelity(t *testing.T) {
167187
t.Errorf("Expected answer to contain '18', got %v", answer)
168188
}
169189

190+
mu.Lock()
191+
snapshot := make([]copilot.SessionEvent, len(events))
192+
copy(snapshot, events)
193+
mu.Unlock()
194+
170195
// Should have streaming deltas before the final message
171196
var deltaEvents []copilot.SessionEvent
172-
for _, e := range events {
197+
for _, e := range snapshot {
173198
if e.Type == "assistant.message_delta" {
174199
deltaEvents = append(deltaEvents, e)
175200
}

0 commit comments

Comments
 (0)