Skip to content
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

Make SignedShrinker bounded for <$ty>::MIN #296

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

neithernut
Copy link

@neithernut neithernut commented Aug 9, 2021

Previously, the SignedShrinker was guaranteed to yield a result if initialized with <$ty>::MIN (e.g. i32::MIN for shrinking i32 and f32). This would result in endless shrinking if a test would happen to fail for these values.

Shrinking signed values is done by halfing a second value i and subtracting it from the original x. Thus i will be equal to 0 at some point. We choose to halt the iteration in this state, i.e. not yield any new values. Yielding x - 0 would be pointless anyway, since that obviously equals the original value which was the initial witness for the test failure.


Closes #295, #301.

Previously, the `SignedShrinker` was guaranteed to yield a result if
initialized with `<$ty>::MIN` (e.g. `i32::MIN` for shrinking `i32` and
`f32`). This would result in endless shrinking if a test would happen to
fail for these values.

Shrinking signed values is done by halfing a second value `i` and
subtracting it from the original `x`. Thus `i` will be equal to `0` at
some point. We choose to halt the iteration in this state, i.e. not
yield any new values. Yielding `x - 0` would be pointless anyway, since
that obviously equals the original value which was the initial witness
for the test failure.
@neithernut
Copy link
Author

Maybe it makes sense to introduce shrinking tests which assert that the shrinkers included in this crate yield at most $number values?

@neithernut neithernut marked this pull request as ready for review August 10, 2021 06:18
The condition `(self.x - self.i).abs() < self.x.abs()` is true if
`self.i` is not `0` and has the same sign than `self.x`. The latter is
always the case due to the initialization, the former was recently
introduced to bound the iteration in the case of `self.x == <$ty>::MIN`.
Thus, both of these conditions became redundant.
For floating point values, we use a shrinker for signed integers and
convert the output to the float's type. The underlying shrinker is
bounded and never includes the original value in the output, i.e. it
behaves correctly. However, when converting the value to the target type
we may loose some information and thus yield the original value, again.

If the test happens to fail for such a value, we may end up endlessly
repeating the test with a sequence of values including the original one
over and over again. This change avoids the issue by applying an
additional filter.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Infinite Repetition/Never Ending Test with f32 and f64.
1 participant