Skip to content

Commit 124f252

Browse files
deansheatherdeansheather
committed
fix: fix races in loadtest tests
1 parent df7866c commit 124f252

File tree

4 files changed

+27
-27
lines changed

4 files changed

+27
-27
lines changed

‎loadtest/harness/harness.go

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ type ExecutionStrategy interface {
2020
// TestHarness runs a bunch of registered test runs using the given
2121
// ExecutionStrategy.
2222
type TestHarness struct {
23-
execStrat ExecutionStrategy
23+
strategy ExecutionStrategy
2424

2525
mut *sync.Mutex
2626
runIDs map[string]struct{}
@@ -30,13 +30,13 @@ type TestHarness struct {
3030
}
3131

3232
// NewTestHarness creates a new TestHarness with the given ExecutionStrategy.
33-
func NewTestHarness(execStrat ExecutionStrategy) *TestHarness {
33+
func NewTestHarness(strategy ExecutionStrategy) *TestHarness {
3434
return &TestHarness{
35-
execStrat: execStrat,
36-
mut: new(sync.Mutex),
37-
runIDs: map[string]struct{}{},
38-
runs: []*TestRun{},
39-
done: make(chan struct{}),
35+
strategy: strategy,
36+
mut: new(sync.Mutex),
37+
runIDs: map[string]struct{}{},
38+
runs: []*TestRun{},
39+
done: make(chan struct{}),
4040
}
4141
}
4242

@@ -63,7 +63,7 @@ func (h *TestHarness) Run(ctx context.Context) (err error) {
6363
}
6464
}()
6565

66-
err = h.execStrat.Execute(ctx, h.runs)
66+
err = h.strategy.Execute(ctx, h.runs)
6767
//nolint:revive // we use named returns because we mutate it in a defer
6868
return
6969
}

‎loadtest/harness/harness_test.go

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import (
44
"context"
55
"io"
66
"testing"
7-
"time"
87

98
"github.com/stretchr/testify/assert"
109
"github.com/stretchr/testify/require"
@@ -180,12 +179,14 @@ func Test_TestHarness(t *testing.T) {
180179
t.Parallel()
181180

182181
var (
182+
started = make(chan struct{})
183183
endRun = make(chan struct{})
184184
testsEnded = make(chan struct{})
185185
)
186186
h := harness.NewTestHarness(harness.LinearExecutionStrategy{})
187187
_ = h.AddRun("test", "1", testFns{
188188
RunFn: func(_ context.Context, _ string, _ io.Writer) error {
189+
close(started)
189190
<-endRun
190191
return nil
191192
},
@@ -196,12 +197,12 @@ func Test_TestHarness(t *testing.T) {
196197
assert.NoError(t, err)
197198
}()
198199

199-
time.Sleep(100 * time.Millisecond)
200+
<-started
200201
require.Panics(t, func() {
201202
h.Results()
202203
})
203-
204204
close(endRun)
205+
205206
<-testsEnded
206207
_ = h.Results()
207208
})
@@ -220,12 +221,14 @@ func Test_TestHarness(t *testing.T) {
220221
t.Parallel()
221222

222223
var (
224+
started = make(chan struct{})
223225
endRun = make(chan struct{})
224226
testsEnded = make(chan struct{})
225227
)
226228
h := harness.NewTestHarness(harness.LinearExecutionStrategy{})
227229
_ = h.AddRun("test", "1", testFns{
228230
RunFn: func(_ context.Context, _ string, _ io.Writer) error {
231+
close(started)
229232
<-endRun
230233
return nil
231234
},
@@ -236,14 +239,13 @@ func Test_TestHarness(t *testing.T) {
236239
assert.NoError(t, err)
237240
}()
238241

239-
time.Sleep(100 * time.Millisecond)
242+
<-started
240243
require.Panics(t, func() {
241244
h.Cleanup(context.Background())
242245
})
243-
244246
close(endRun)
245-
<-testsEnded
246247

248+
<-testsEnded
247249
err := h.Cleanup(context.Background())
248250
require.NoError(t, err)
249251
})

‎loadtest/harness/strategies.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,15 +60,16 @@ var _ ExecutionStrategy = ParallelExecutionStrategy{}
6060
func (p ParallelExecutionStrategy) Execute(ctx context.Context, runs []*TestRun) error {
6161
var wg sync.WaitGroup
6262
sem := make(chan struct{}, p.Limit)
63+
defer close(sem)
6364

6465
for _, run := range runs {
6566
run := run
6667

6768
wg.Add(1)
6869
go func() {
69-
defer wg.Done()
7070
defer func() {
7171
<-sem
72+
wg.Done()
7273
}()
7374
sem <- struct{}{}
7475
_ = run.Run(ctx)

‎loadtest/harness/strategies_test.go

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,8 @@ import (
1616
"github.com/coder/coder/loadtest/harness"
1717
)
1818

19+
//nolint:paralleltest // this tests uses timings to determine if it's working
1920
func Test_LinearExecutionStrategy(t *testing.T) {
20-
t.Parallel()
21-
2221
var (
2322
lastSeenI int64 = -1
2423
count int64
@@ -27,6 +26,7 @@ func Test_LinearExecutionStrategy(t *testing.T) {
2726
atomic.AddInt64(&count, 1)
2827
swapped := atomic.CompareAndSwapInt64(&lastSeenI, int64(i-1), int64(i))
2928
assert.True(t, swapped)
29+
time.Sleep(2 * time.Millisecond)
3030
return nil
3131
})
3232
strategy := harness.LinearExecutionStrategy{}
@@ -42,9 +42,8 @@ func Test_LinearExecutionStrategy(t *testing.T) {
4242
}
4343
}
4444

45+
//nolint:paralleltest // this tests uses timings to determine if it's working
4546
func Test_ConcurrentExecutionStrategy(t *testing.T) {
46-
t.Parallel()
47-
4847
runs := strategyTestData(10, func(_ context.Context, i int, _ io.Writer) error {
4948
time.Sleep(1 * time.Second)
5049
return nil
@@ -67,9 +66,8 @@ func Test_ConcurrentExecutionStrategy(t *testing.T) {
6766
}
6867
}
6968

69+
//nolint:paralleltest // this tests uses timings to determine if it's working
7070
func Test_ParallelExecutionStrategy(t *testing.T) {
71-
t.Parallel()
72-
7371
runs := strategyTestData(10, func(_ context.Context, _ int, _ io.Writer) error {
7472
time.Sleep(1 * time.Second)
7573
return nil
@@ -79,6 +77,7 @@ func Test_ParallelExecutionStrategy(t *testing.T) {
7977
}
8078

8179
startTime := time.Now()
80+
time.Sleep(time.Millisecond)
8281
err := strategy.Execute(context.Background(), runs)
8382
require.NoError(t, err)
8483

@@ -111,10 +110,9 @@ func Test_ParallelExecutionStrategy(t *testing.T) {
111110
require.Equal(t, 5, withinRange)
112111
}
113112

113+
//nolint:paralleltest // this tests uses timings to determine if it's working
114114
func Test_TimeoutExecutionStrategy(t *testing.T) {
115-
t.Parallel()
116-
117-
runs := strategyTestData(10, func(ctx context.Context, _ int, _ io.Writer) error {
115+
runs := strategyTestData(1, func(ctx context.Context, _ int, _ io.Writer) error {
118116
ticker := time.NewTicker(500 * time.Millisecond)
119117
defer ticker.Stop()
120118

@@ -127,7 +125,7 @@ func Test_TimeoutExecutionStrategy(t *testing.T) {
127125
})
128126
strategy := harness.TimeoutExecutionStrategyWrapper{
129127
Timeout: 100 * time.Millisecond,
130-
Inner: harness.ConcurrentExecutionStrategy{},
128+
Inner: harness.LinearExecutionStrategy{},
131129
}
132130

133131
err := strategy.Execute(context.Background(), runs)
@@ -138,9 +136,8 @@ func Test_TimeoutExecutionStrategy(t *testing.T) {
138136
}
139137
}
140138

139+
//nolint:paralleltest // this tests uses timings to determine if it's working
141140
func Test_ShuffleExecutionStrategyWrapper(t *testing.T) {
142-
t.Parallel()
143-
144141
runs := strategyTestData(100000, func(_ context.Context, i int, _ io.Writer) error {
145142
// t.Logf("run %d", i)
146143
return nil

0 commit comments

Comments
 (0)