launchpad-dev team mailing list archive

[Brad Crittenden <brad.crittenden@xxxxxxxxxxxxx>]

On Nov 17, 2011, at 17:59 , Martin Pool wrote:

> On 18 November 2011 01:53, Brad Crittenden
> <brad.crittenden@xxxxxxxxxxxxx> wrote:
>> 
>> On Nov 16, 2011, at 22:49 , Martin Pool wrote:
>>> 
>>> Why would it return the same values?  Well, there are several tests or
>>> functions reached from tests that reset the random seed, for instance
>>> test_token_creation just sets it flat out to zero.
>> 
>> Actually test_token_creation is the only test that resets the seed and what you have discovered is an error in that test.  Anytime we set the seed to a specific value it should be reseeded when done so subsequent calls to random are not predictable.  Calling random.seed() with no parameter takes pains to do the right thing.
>> 
>> I've created a branch to fix test_token_creation.
>> 
>> https://code.launchpad.net/~bac/launchpad/bug-891641/+merge/82544
> 
> Actually no, there is other code reachable from the tests that resets
> the seed, including Mailman/LockFile.py, and model/person.py.  The
> difference is that they save the state and restore it which, if I had
> the chance, is what I would have advised you to do here rather than
> reseeding from OS randomness.
> 
> Injecting more randomness in to tests is not a good idea!

Thanks for bringing that up Martin.  I agree with you getting and later restoring the prng state is a better way to go, but for a different reason.  Reseeding interferes with the period of the prng thus making it more likely that generated numbers repeat.  And, should we ever decide we wanted to start our test with a known seed the reseeding in this test would thwart that approach (though I'm not advocating such a thing).

The best way to handle the needs of that test would be to instantiate an independent generator (random.Random()) rather than monkeying with the global one.

> 
> It is better if things fail deterministically so that they can be
> understood and debugged.

Reseeding vs restoring prng state will have no noticeable effect on that goal.

> 
> Even better would be to update the test so that it does not rely on the prng.
> 
> -- 
> Martin

Best,

Brad

Attachment: signature.asc
Description: Message signed with OpenPGP using GPGMail