-
Notifications
You must be signed in to change notification settings - Fork 13.4k
exact_div: add tests #141939
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
exact_div: add tests #141939
Conversation
This comment has been minimized.
This comment has been minimized.
00e7551
to
2c4e592
Compare
This comment has been minimized.
This comment has been minimized.
2c4e592
to
b72cf24
Compare
This comment has been minimized.
This comment has been minimized.
b72cf24
to
e87f138
Compare
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.
ah, np. I should've nagged you for them.
|
||
// failures | ||
assert_eq_const_safe!(Option<$T>: <$T>::checked_exact_div(1, 2), None); | ||
assert_eq_const_safe!(Option<$T>: <$T>::checked_exact_div(0, 0), None); |
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 suppose we could test (<$T>::MAX >> 1) + 1
divided by -1 as _
for unsigned integers, but it would just also be None
, for the same reason 1, 2
is.
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'm not sure what, if anything, you want me to change here.
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.
Nada, just thought process effluvia.
feels weird to test signed integers without testing negative cases @rustbot author |
Reminder, once the PR becomes ready for a review, use |
@rustbot ready |
@bors r+ rollup |
assert_eq_const_safe!(Option<$T>: <$T>::checked_exact_div(EXACT_DIV_SUCCESS_DIVIDEND1, EXACT_DIV_SUCCESS_DIVISOR1), Some(EXACT_DIV_SUCCESS_QUOTIENT1)); | ||
assert_eq_const_safe!($T: <$T>::exact_div(EXACT_DIV_SUCCESS_DIVIDEND1, EXACT_DIV_SUCCESS_DIVISOR1), EXACT_DIV_SUCCESS_QUOTIENT1); | ||
|
||
// 18 / 3 | ||
assert_eq_const_safe!(Option<$T>: <$T>::checked_exact_div(EXACT_DIV_SUCCESS_DIVIDEND2, EXACT_DIV_SUCCESS_DIVISOR2), Some(EXACT_DIV_SUCCESS_QUOTIENT2)); |
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.
Not to necessarily block anything, but maybe want to hand wrap these so they don't wrap in the UI?
…=workingjubilee exact_div: add tests tracking issue: rust-lang#139911 I neglected to add tests in my last PR (rust-lang#141237), so I've added them here. r? `@workingjubilee` (Feel free to reroll, I just picked you since you reviewed the last one.)
Rollup of 9 pull requests Successful merges: - #141271 (Streamline some attr parsing APIs) - #141570 (Fix incorrect eq_unspanned in TokenStream) - #141857 (coretests: move float tests from num to floats module and use a more flexible macro to generate them) - #141893 (remove `f16: From<u16>`) - #141924 (Lightly tweak docs for BTree{Map,Set}::extract_if) - #141939 (exact_div: add tests) - #141959 (Add more missing 2015 edition directives) - #142002 (redesign stage 0 std follow-ups part2) - #142007 (Improve some `Visitor` comments.) r? `@ghost` `@rustbot` modify labels: rollup
Rollup of 7 pull requests Successful merges: - #141271 (Streamline some attr parsing APIs) - #141570 (Fix incorrect eq_unspanned in TokenStream) - #141893 (remove `f16: From<u16>`) - #141924 (Lightly tweak docs for BTree{Map,Set}::extract_if) - #141939 (exact_div: add tests) - #141959 (Add more missing 2015 edition directives) - #142007 (Improve some `Visitor` comments.) r? `@ghost` `@rustbot` modify labels: rollup
tracking issue: #139911
I neglected to add tests in my last PR (#141237), so I've added them here.
r? @workingjubilee (Feel free to reroll, I just picked you since you reviewed the last one.)