Skip to content

Commit 2b7d596

Browse files
committed
util/retry: remove maxExpectedTimeSpent from tests
With the two-stage select implementation prioritizing context/closer cancellation, timing is now deterministic. Remove non-deterministic timing assertions and unused skip package dependency. Release note: None
1 parent a7bf63c commit 2b7d596

File tree

2 files changed

+5
-17
lines changed

2 files changed

+5
-17
lines changed

pkg/util/retry/BUILD.bazel

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ go_test(
2525
],
2626
embed = [":retry"],
2727
deps = [
28-
"//pkg/testutils/skip",
2928
"//pkg/util/log",
3029
"//pkg/util/timeutil",
3130
"@com_github_cockroachdb_errors//:errors",

pkg/util/retry/retry_test.go

Lines changed: 5 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import (
1010
"testing"
1111
"time"
1212

13-
"github.com/cockroachdb/cockroach/pkg/testutils/skip"
1413
"github.com/cockroachdb/cockroach/pkg/util/timeutil"
1514
"github.com/cockroachdb/errors"
1615
"github.com/stretchr/testify/require"
@@ -402,10 +401,7 @@ func TestRetryWithMaxDuration(t *testing.T) {
402401
retryFunc func(attemptNum int) error
403402
preRetryFunc func()
404403
expectedTimeSpent time.Duration
405-
// For cases where the amount of time spent is not deterministic, we can set
406-
// an upper bound instead (e.g. for context or closer).
407-
maxExpectedTimeSpent time.Duration
408-
expectedErr bool
404+
expectedErr bool
409405
}
410406

411407
testCases := []testcase{
@@ -473,8 +469,8 @@ func TestRetryWithMaxDuration(t *testing.T) {
473469
preRetryFunc: func() {
474470
cancelCtxFunc()
475471
},
476-
maxExpectedTimeSpent: time.Millisecond * 20,
477-
expectedErr: true,
472+
expectedTimeSpent: time.Millisecond,
473+
expectedErr: true,
478474
},
479475
{
480476
name: "errors with opt.Closer that is closed",
@@ -490,8 +486,8 @@ func TestRetryWithMaxDuration(t *testing.T) {
490486
preRetryFunc: func() {
491487
close(closeCh)
492488
},
493-
maxExpectedTimeSpent: time.Millisecond * 20,
494-
expectedErr: true,
489+
expectedTimeSpent: time.Millisecond,
490+
expectedErr: true,
495491
},
496492
}
497493

@@ -538,13 +534,6 @@ func TestRetryWithMaxDuration(t *testing.T) {
538534
t, tc.expectedTimeSpent, timeSource.Since(start), "expected time does not match actual spent time",
539535
)
540536
}
541-
542-
if tc.maxExpectedTimeSpent != 0 {
543-
require.LessOrEqual(
544-
t, timeSource.Since(start), tc.maxExpectedTimeSpent,
545-
"expected time spent to be less than or equal to max expected time spent",
546-
)
547-
}
548537
})
549538
}
550539
}

0 commit comments

Comments
 (0)