launchpad-reviewers team mailing list archive

[Brad Crittenden <bac@xxxxxxxxxxxxx>]

Brad Crittenden has proposed merging lp:~bac/launchpad/bug-891641 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #891641 in Launchpad itself: "Use of random.seed(n) in tests needs cleanup"
  https://bugs.launchpad.net/launchpad/+bug/891641

For more details, see:
https://code.launchpad.net/~bac/launchpad/bug-891641/+merge/82544

= Summary =

If random.seed(n) is used to provide determinism for a test then the
random number generator must be properly reseeded or subsequent tests
will use predictable random numbers.

== Proposed fix ==

Add a cleanup task to call random.seed() with no parameters, which
will correctly set the seed value using operating system-based entropy
if available or the system clock.

== Pre-implementation notes ==

None.

== Implementation details ==

As above.

== Tests ==

bin/test test_token_creation

== Demo and Q/A ==

None

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/canonical/launchpad/tests/test_token_creation.py
-- 
https://code.launchpad.net/~bac/launchpad/bug-891641/+merge/82544
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~bac/launchpad/bug-891641 into lp:launchpad.

=== modified file 'lib/canonical/launchpad/tests/test_token_creation.py'
--- lib/canonical/launchpad/tests/test_token_creation.py	2011-08-12 11:19:40 +0000
+++ lib/canonical/launchpad/tests/test_token_creation.py	2011-11-17 14:51:48 +0000
@@ -4,7 +4,7 @@
 __metaclass__ = type
 
 import random
-import unittest
+import testtools
 
 from canonical.database.constants import UTC_NOW
 from canonical.launchpad.components.tokens import (
@@ -16,17 +16,21 @@
 from canonical.testing.layers import DatabaseFunctionalLayer
 
 
-class Test_create_token(unittest.TestCase):
+class Test_create_token(testtools.TestCase):
 
     def test_length(self):
         token = create_token(99)
         self.assertEquals(len(token), 99)
 
 
-class Test_create_unique_token_for_table(unittest.TestCase):
+class Test_create_unique_token_for_table(testtools.TestCase):
     layer = DatabaseFunctionalLayer
 
     def test_token_uniqueness(self):
+        # Since the prng will be seeded in this test it is important we clean
+        # up by calling seed with no parameters, which will use OS-provided
+        # entropy if available or use the system clock.
+        self.addCleanup(random.seed)
         # Calling create_unique_token_for_table() twice with the same
         # random.seed() will generate two identical tokens, as the token was
         # never inserted in the table.