- Sponsor
-
Notifications
You must be signed in to change notification settings - Fork 471
Description
Background
What is your motivation?
I want to test code that uses rand module. Ideally, I would seed the mock with the random entries I want to produce, so I can confirm the end result is working as expected.
What type of application is this? (E.g. cryptography, game, numerical simulation)
game
Feature request
I am writing a small program that randomly selects an entry from an enum. Sample code:
#[derive(Debug)]
enum SettlementSize {
VILLAGE,
TOWN,
CITY
}
impl Distribution<SettlementSize> for Standard {
fn sample<R: Rng + ?Sized>(&self, rng: &mut R) -> SettlementSize {
let res = rng.gen_range(0, 3);
match res {
0 => SettlementSize::VILLAGE,
1 => SettlementSize::TOWN,
2 => SettlementSize::CITY,
_ => panic!("Unknown value!")
}
}
}
fn get_settlement_size(mut rng: impl RngCore) -> SettlementSize {
let size: SettlementSize = rng.gen();
size
}
Now, of course I want to test it. That's why get_settlement_size takes the rng value.
#[test]
fn random_human_readable() {
let rng = StepRng::new(1, 1);
assert_eq!("Town", get_settlement_size(rng).human_readable());
}
Unfortunately, this doesn't work. When I added some printlns, the value returned from:
rng.gen_range(0, 3);
is always 0. I copied StepRng code into my test module to add println inside and I see next_u32 and next_u64 called. However, later the code disappears into UniformSampler and at that point it becomes too hard for me to follow.
Ideally, I would mock gen_range
directly.
Activity
newpavlov commentedon Aug 20, 2020
Is there a reason why you can't use PRNG seeded with a constant value, e.g. PCG? I guess a "straightforward" fix be to use
StepRng::new(1, 1<<30)
, but this solution is quite fragile and will not produce all values for different ranges such asgen_range(0, 6)
.gruszczy commentedon Aug 20, 2020
@newpavlov Thanks for the response! I was not aware of alternative solutions. Is the suggestion that instead of mocking, I use a fixed seed in the unit test, so while I don't control what will be returned I will get the same values on the same run?
newpavlov commentedon Aug 20, 2020
Yes. Note that you do control return values to a certain extent, i.e. you can play with the constant seed to get a better sequence of test values (e.g. which provide full coverage faster).
dhardy commentedon Aug 21, 2020
Indeed, the design of
Rng
trait makes it impossible for a mock RNG to override methods likeRng::gen_range
, so unless you use custom wrappers in your API you cannot directly control these with a mock RNG. I don't think we want a redesign just to allow mocking here, so probably we'll close this without action.gruszczy commentedon Aug 21, 2020
Wrapping Rng in my own class was another approach I considered, but I was hoping that the crate would provide that natively. I am a little apprehensive of having to create my own wrapper class to allow faking/mocking (reminds of wrapping C++ chrono clocks so I could fake them in tests).
Naively (I know nothing): couldn't Rng become a trait with default implementation, so I could fully re-implement Rng with faked behavior?
dhardy commentedon Aug 21, 2020
Rng
is an auto-implemented extension trait.Originally
Rng
was the main (only) generator trait, which would have allowed this. The extension design is cleaner in a couple of ways:RngCore
can be pushed up torand_core
whileRng
can still make use of other bits of machinery inrand
, and it neatly separates the work of generating "random data" and "random samples from a distribution".Overriding
Rng
methods likegen_range
wouldn't in any case allow complete mocking since the distribution objects are constructed directly. Theoretically this allows another path to mocking: have aDistributionBuilder
with methods likebuilder.range(1..=6)
yielidng an instance of theUniform
distribution — or a mock distribution. Of course this requires usingRng::sample
instead ofRng::gen_range
, so not quite the same.dhardy commentedon Aug 28, 2020
We can discuss further if you like, but I believe we will not change this to support mocking.
gruszczy commentedon Aug 28, 2020
Spending some time working with this and using fixed seed, I don't think that will be sufficient. If I need to write a test that addresses a bug that happens in a very specific circumstances of random execution, I can't be guessing a seed to try to reproduce. The random process takes multiple steps[1], each with a random component. If the bug manifests only in a certain random events, I need to be able to set exactly that.
As a result my only option now will be to create a wrapper around Rng to be able to fake it and feed it concrete results. This seems unnecessary and will likely now prevent me from using things like
impl Distribution for Standard {
let size: SettlementSize = rng.gen();
Because Instead I will be calling:
DiceRoller::roll(1, 3) instead, which will not be related to Distribution, Standard or other class defined in random crates.
[1] If you are familiar with tabletop role playing games I am making random choices on several steps of random tables to generate content.
dhardy commentedon Aug 28, 2020
If you are doing things like
impl Distribution<SettlementSize> for Standard
to allowrng.gen()
, there is another option: a mockSettlementSize
.Another option in your case would be to create a sampling helper like this:
jhpratt commentedon Sep 14, 2020
For what it's worth I was quite surprised when I discovered that
gen_range
didn't work as expected. Given that I'm mocking a RNG, I would've expected the number to increment in a wrapping manner, as is documented. I shouldn't need to pull in a proper RNG (even if it is seeded) just to ensure all behavior is covered.Basically, I'd expect the following to print
[0, 1, 2, 3, 4, 5, 6, 0]
The failure of the current behavior is surprising at beset.
dhardy commentedon Sep 14, 2020
We've had quite a few people say this — lots of people expect
gen_range
to work like the modulo operator they are used to, but should it? As O'Neill summarises, there are quite a few possible ways to implement this; we use a variant of de-biased integer multiplication.jhpratt commentedon Sep 14, 2020
With my understanding of the way the traits interact, I don't believe it would be possible to override the behavior for this struct specifically, at least until specialization lands on stable, correct?
For my testing purposes (presumably others' as well), bias is fine. That's kind of the point of mocking, after all! Admittedly, I'm not sure how the behavior could change, but I (obviously) wish it were different.
dhardy commentedon Sep 14, 2020
You're correct: it is not currently possible to override the behaviour, but might be with specialisation. Is it possible with min_specialization? If so we could implement a nightly-only feature for this.