Skip to content

Commit 1dca2c3

Browse files
johnstcnjohnstcn
committed
address PR comments
1 parent 32b578e commit 1dca2c3

File tree

3 files changed

+37
-25
lines changed

3 files changed

+37
-25
lines changed

‎coderd/coderdtest/coderdtest.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -411,9 +411,9 @@ func AwaitTemplateVersionJob(t *testing.T, client *codersdk.Client, version uuid
411411

412412
t.Logf("waiting for template version job %s", version)
413413
var templateVersion codersdk.TemplateVersion
414-
require.True(t, testutil.EventuallyShort(t, func() bool {
414+
require.True(t, testutil.EventuallyShort(t, func(ctx context.Context) bool {
415415
var err error
416-
templateVersion, err = client.TemplateVersion(context.Background(), version)
416+
templateVersion, err = client.TemplateVersion(ctx, version)
417417
return assert.NoError(t, err) && templateVersion.Job.CompletedAt != nil
418418
}))
419419
return templateVersion
@@ -425,8 +425,8 @@ func AwaitWorkspaceBuildJob(t *testing.T, client *codersdk.Client, build uuid.UU
425425

426426
t.Logf("waiting for workspace build job %s", build)
427427
var workspaceBuild codersdk.WorkspaceBuild
428-
require.True(t, testutil.EventuallyShort(t, func() bool {
429-
workspaceBuild, err := client.WorkspaceBuild(context.Background(), build)
428+
require.True(t, testutil.EventuallyShort(t, func(ctx context.Context) bool {
429+
workspaceBuild, err := client.WorkspaceBuild(ctx, build)
430430
return assert.NoError(t, err) && workspaceBuild.Job.CompletedAt != nil
431431
}))
432432
return workspaceBuild
@@ -438,9 +438,9 @@ func AwaitWorkspaceAgents(t *testing.T, client *codersdk.Client, build uuid.UUID
438438

439439
t.Logf("waiting for workspace agents (build %s)", build)
440440
var resources []codersdk.WorkspaceResource
441-
require.True(t, testutil.EventuallyLong(t, func() bool {
441+
require.True(t, testutil.EventuallyLong(t, func(ctx context.Context) bool {
442442
var err error
443-
resources, err = client.WorkspaceResourcesByBuild(context.Background(), build)
443+
resources, err = client.WorkspaceResourcesByBuild(ctx, build)
444444
if !assert.NoError(t, err) {
445445
return false
446446
}

‎testutil/eventually.go

Lines changed: 22 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4,16 +4,27 @@ import (
44
"context"
55
"testing"
66
"time"
7+
8+
"github.com/stretchr/testify/assert"
79
)
810

9-
// Eventually is like require.Eventually except it takes a context.
10-
// If ctx times out, the test will fail.
11-
// ctx must have a deadline set or this will panic.
12-
func Eventually(ctx context.Context, t testing.TB, condition func() bool, tick time.Duration) bool {
11+
// Eventually is like require.Eventually except it allows passing
12+
// a context into the condition. It is safe to use with `require.*`.
13+
//
14+
// If ctx times out, the test will fail, but not immediately.
15+
// It is the caller's responsibility to exit early if required.
16+
//
17+
// It is the caller's responsibility to ensure that ctx has a
18+
// deadline or timeout set. Eventually will panic if this is not
19+
// the case in order to avoid potentially waiting forever.
20+
//
21+
// condition is not run in a goroutine; use the provided
22+
// context argument for cancellation if required.
23+
func Eventually(ctx context.Context, t testing.TB, condition func(context.Context) bool, tick time.Duration) bool {
1324
t.Helper()
1425

1526
if _, ok := ctx.Deadline(); !ok {
16-
panic("developer error: must set deadline on ctx")
27+
panic("developer error: must set deadline or timeout on ctx")
1728
}
1829

1930
ch := make(chan bool, 1)
@@ -22,13 +33,14 @@ func Eventually(ctx context.Context, t testing.TB, condition func() bool, tick t
2233
for tick := ticker.C; ; {
2334
select {
2435
case <-ctx.Done():
25-
t.Errorf("Await timed out")
36+
assert.NoError(t, ctx.Err(), "Eventually timed out")
2637
return false
2738
case <-tick:
2839
tick = nil
29-
go func() { ch <- condition() }()
40+
ch <- condition(ctx)
3041
case v := <-ch:
3142
if v {
43+
close(ch)
3244
return true
3345
}
3446
tick = ticker.C
@@ -38,23 +50,23 @@ func Eventually(ctx context.Context, t testing.TB, condition func() bool, tick t
3850

3951
// EventuallyShort is a convenience function that runs Eventually with
4052
// IntervalFast and times out after WaitShort.
41-
func EventuallyShort(t testing.TB, condition func() bool) bool {
53+
func EventuallyShort(t testing.TB, condition func(context.Context) bool) bool {
4254
ctx, cancel := context.WithTimeout(context.Background(), WaitShort)
4355
defer cancel()
4456
return Eventually(ctx, t, condition, IntervalFast)
4557
}
4658

4759
// EventuallyMedium is a convenience function that runs Eventually with
4860
// IntervalMedium and times out after WaitMedium.
49-
func EventuallyMedium(t testing.TB, condition func() bool) bool {
61+
func EventuallyMedium(t testing.TB, condition func(context.Context) bool) bool {
5062
ctx, cancel := context.WithTimeout(context.Background(), WaitMedium)
5163
defer cancel()
5264
return Eventually(ctx, t, condition, IntervalMedium)
5365
}
5466

5567
// EventuallyLong is a convenience function that runs Eventually with
5668
// IntervalSlow and times out after WaitLong.
57-
func EventuallyLong(t testing.TB, condition func() bool) bool {
69+
func EventuallyLong(t testing.TB, condition func(context.Context) bool) bool {
5870
ctx, cancel := context.WithTimeout(context.Background(), WaitLong)
5971
defer cancel()
6072
return Eventually(ctx, t, condition, IntervalSlow)

‎testutil/eventually_test.go

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -19,26 +19,26 @@ func TestEventually(t *testing.T) {
1919
t.Run("OK", func(t *testing.T) {
2020
t.Parallel()
2121
state := 0
22-
condition := func() bool {
22+
condition := func(_ context.Context) bool {
2323
defer func() {
2424
state++
2525
}()
2626
return state > 2
2727
}
2828
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitShort)
2929
defer cancel()
30-
assert.True(t, testutil.Eventually(ctx, t, condition, testutil.IntervalFast))
30+
testutil.Eventually(ctx, t, condition, testutil.IntervalFast)
3131
})
3232

3333
t.Run("Timeout", func(t *testing.T) {
3434
t.Parallel()
35-
condition := func() bool {
35+
condition := func(_ context.Context) bool {
3636
return false
3737
}
3838
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitShort)
3939
defer cancel()
4040
mockT := new(testing.T)
41-
assert.False(t, testutil.Eventually(ctx, mockT, condition, testutil.IntervalFast))
41+
testutil.Eventually(ctx, mockT, condition, testutil.IntervalFast)
4242
assert.True(t, mockT.Failed())
4343
})
4444

@@ -47,24 +47,24 @@ func TestEventually(t *testing.T) {
4747

4848
panicky := func() {
4949
mockT := new(testing.T)
50-
condition := func() bool { return true }
51-
assert.False(t, testutil.Eventually(context.Background(), mockT, condition, testutil.IntervalFast))
50+
condition := func(_ context.Context) bool { return true }
51+
testutil.Eventually(context.Background(), mockT, condition, testutil.IntervalFast)
5252
}
5353
assert.Panics(t, panicky)
5454
})
5555

5656
t.Run("Short", func(t *testing.T) {
5757
t.Parallel()
58-
assert.True(t, testutil.EventuallyShort(t, func() bool { return true }))
58+
testutil.EventuallyShort(t, func(_ context.Context) bool { return true })
5959
})
6060

6161
t.Run("Medium", func(t *testing.T) {
6262
t.Parallel()
63-
assert.True(t, testutil.EventuallyMedium(t, func() bool { return true }))
63+
testutil.EventuallyMedium(t, func(_ context.Context) bool { return true })
6464
})
6565

6666
t.Run("Long", func(t *testing.T) {
6767
t.Parallel()
68-
assert.True(t, testutil.EventuallyLong(t, func() bool { return true }))
68+
testutil.EventuallyLong(t, func(_ context.Context) bool { return true })
6969
})
7070
}

0 commit comments

Comments
 (0)