quickcheck icon indicating copy to clipboard operation
quickcheck copied to clipboard

Durations's Arbitrary instance is dependant on Gen's size

Open avandecreme opened this issue 1 year ago • 1 comments

According to Gen::new doc:

The size parameter controls the size of random values generated. For example, it specifies the maximum length of a randomly generated vector, but is and should not be used to control the range of a randomly generated number. (Unless that number is used to control the size of a data structure.)

However Duration's Arbitrary is defined like this: https://github.com/BurntSushi/quickcheck/blob/aa968a94650b5d4d572c4ef581a7f5eb259aa0d2/src/arbitrary.rs#L1068-L1072

The way seconds is generated seems in violation of the above rule to me and has the consequence of generating very short duration only (with the default size of 100).

According to https://doc.rust-lang.org/nightly/core/time/struct.Duration.html#method.new it seems that we should be able to use the full u64 range without issue for seconds since we are already capping the nano seconds to one billion.

However from my testing this break SystemTime's instance because we can get overflows in the following implementation: https://github.com/BurntSushi/quickcheck/blob/aa968a94650b5d4d572c4ef581a7f5eb259aa0d2/src/arbitrary.rs#L1104-L1112

Any idea how we should fix this?

avandecreme avatar Dec 11 '23 17:12 avandecreme

We could use SystemTime::checked_add and checked_sub, and divide the duration by two if that fails. Maybe just do that in a loop until we end up with a valid SystemTime.

neithernut avatar Dec 12 '23 13:12 neithernut