-
Notifications
You must be signed in to change notification settings - Fork 4k
jobs: fix TestJitterCalculation failed #157951
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
jobs: fix TestJitterCalculation failed #157951
Conversation
pkg/jobs/registry_test.go
Outdated
| // where the test flakes because the random value rounds to 1/2. This is more | ||
| // common than you would expect because its probability is based on the width | ||
| // of the duration, not the width of the random float. | ||
| rand.Seed(4) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was a little concerned that setting this global seed might impact other randomized tests. But then I read that perhaps this isn't doing what we want in the first place:
// As of Go 1.24 [Seed] is a no-op. To restore the previous behavior set
// GODEBUG=randseednop=0.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point on both counts. I switched strategies and rewrote the test to retry cases where input == output.
044859b to
83f8447
Compare
Potential Bug(s) DetectedThe three-stage Claude Code analysis has identified potential bug(s) in this PR that may warrant investigation. Next Steps: Note: When viewing the workflow output, scroll to the bottom to find the Final Analysis Summary. After you review the findings, please tag the issue as follows:
|
83f8447 to
4751283
Compare
stevendanna
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
Might be worth a comment on why we might need to retry.
This test will sometimes fail due to a randomly picking a value that rounds to 1/2. The frequency of this is a bit higher than one would expect because the rounding behavior depends on the width of the duration, not the width of a float64. The test was adjusted to retry if the jittered value equals the input. Release note: none Fixes: cockroachdb#128381 Fixes: cockroachdb#157113
4751283 to
4968c09
Compare
|
bors r+ |
|
Thanks for a review! I added a comment. |
|
Build succeeded: |
This test will sometimes fail due to a randomly picking a value that
rounds to 1/2. The frequency of this is a bit higher than one would
expect because the rounding behavior depends on the width of the
duration, not the width of a float64. The test was adjusted to retry
if the jittered value equals the input.
Release note: none
Fixes: #128381
Fixes: #157113