-
Notifications
You must be signed in to change notification settings - Fork 2k
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
helper: guard against negative inputs into random stagger #14497
Conversation
This PR modifies RandomStagger to protect against negative input values. If the given interval is negative, the value returned will be somewhere in the stratosphere. Instead, treat negative inputs like zero, returning zero.
d3e3dd1
to
75b30c2
Compare
must.GreaterEq(t, result, 0) | ||
must.LessEq(t, result, abs(tc.input)) |
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.
Just now realizing I got these parameters backwards, shoenig/test#33
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.
fwiw, having the parameters in the same "direction" as the operator feels more intuitive to me. ex. must.GreaterEq(t, a, b)
reads like it should be checking a > b
. Whereas for something like must.Eq
there isn't an obvious expectation vs result in the way the parameters are named anyways.
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
I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions. |
This PR modifies
RandomStagger
to protect against negative inputvalues. If the given interval is negative, the value returned will
be somewhere in the stratosphere. Instead, treat negative inputs
like zero, returning zero.
Before: