armstrong-numbers: add larger testcases #1601
Conversation
Add some extra Armstrong number tests for larger numbers. In particular, the ten-digit solutions can cause naive solutions to overflow; it might be reasonable to gate them behind a feature flag or bonus exercise. The test case 4106098957 is particularly interesting: if the addition is done using `wrapping_add`, this number will appear as an Armstrong number, even though it really isn't. This is because the sum is equal to exactly 2^32 + 4106098957. This might be a worthwhile testcase to add to other languages too to check for overflow bugs. (Rust should panic in debug mode, but this will also catch folks who try to use wrapping_add to get around the panic).
There was a problem hiding this comment.
in times of peace, these tests should be submitted to https://github.com/exercism/problem-specifications/blob/main/exercises/armstrong-numbers/canonical-data.json. But, https://github.com/exercism/problem-specifications/blob/main/.github/workflows/pause-community-contributions.yml is going to close it and redirect it to https://forum.exercism.org. I do not know the exact process to do that, but I see a previous thread in that vein: https://forum.exercism.org/t/canonical-resistor-color-trio-testing-gap-values-over-1000-which-are-not-kiloohms/1434. I encourage you to seek out this avenue to get your tests added to https://github.com/exercism/problem-specifications/blob/main/exercises/armstrong-numbers/canonical-data.json, but I cannot force you to.
For the Rust track specifically, let's make it clear what "fake" means, and let's go with what you have now.
|
|
||
| #[test] | ||
| #[ignore] | ||
| fn test_ten_digit_fake_armstrong_number() { |
There was a problem hiding this comment.
let's make it clearer what "fake" means - your commit message already explains it may appear as an armstrong number under wrapping_add, but this commit message wouldn't be visible to a student solving this exercise without knowledge of this repo. So I would think this test should be renamed or a comment should be added
There was a problem hiding this comment.
OK, I updated the comment and test case name.
Adding a comment and updating the test case's name to better reflect what the "fake" case is testing.
|
Thanks for the feedback! Once this is merged, I'll propose adding the test cases to the canonical file. |
|
I must have forgotten to check this earlier. https://github.com/exercism/rust/blob/main/exercises/practice/armstrong-numbers/.meta/example.rs will need to be updated to handle these cases. Let's get that done, and that will be all that's needed. |
Use checked_add to gracefully handle the case where the sum overflows. Note that the individual powers cannot overflow (9^10 < 2^32).
|
Done. |
|
thanks! |
Add some extra Armstrong number tests for larger numbers. In particular, the ten-digit solutions can cause naive solutions to overflow; it might be reasonable to gate them behind a feature flag or bonus exercise.
The test case 4106098957 is particularly interesting: if the addition is done using
wrapping_add, this number will appear as an Armstrong number, even though it really isn't. This is because the sum is equal to exactly 2^32 + 4106098957. This might be a worthwhile testcase to add to other languages too to check for overflow bugs. (Rust should panic in debug mode, but this will also catch folks who try to use wrapping_add to get around the panic).