[Python-checkins] bpo-33144: Fix choosing random.Random._randbelow implementation. (GH-6563)

[Serhiy Storchaka]

https://github.com/python/cpython/commit/ec1622d56c80d15740f7f8459c9a79fd55f5d3c7
commit: ec1622d56c80d15740f7f8459c9a79fd55f5d3c7
branch: master
author: Serhiy Storchaka <storchaka at gmail.com>
committer: GitHub <noreply at github.com>
date: 2018-05-08T15:45:15+03:00
summary:

bpo-33144: Fix choosing random.Random._randbelow implementation. (GH-6563)

random() takes precedence over getrandbits() if defined later
in the class tree.

files:
M Lib/random.py
M Lib/test/test_random.py

diff --git a/Lib/random.py b/Lib/random.py
index 0ed5511e9f63..1e0dcc87ed4a 100644
--- a/Lib/random.py
+++ b/Lib/random.py
@@ -102,18 +102,16 @@ def __init_subclass__(cls, **kwargs):
         ranges.
         """
 
-        if (cls.random is _random.Random.random) or (
-            cls.getrandbits is not _random.Random.getrandbits):
-            # The original random() builtin method has not been overridden
-            # or a new getrandbits() was supplied.
-            # The subclass can use the getrandbits-dependent implementation
-            # of _randbelow().
-            cls._randbelow = cls._randbelow_with_getrandbits
-        else:
-            # There's an overridden random() method but no new getrandbits(),
-            # so the subclass can only use the getrandbits-independent
-            # implementation of _randbelow().
-            cls._randbelow = cls._randbelow_without_getrandbits
+        for c in cls.__mro__:
+            if '_randbelow' in c.__dict__:
+                # just inherit it
+                break
+            if 'getrandbits' in c.__dict__:
+                cls._randbelow = cls._randbelow_with_getrandbits
+                break
+            if 'random' in c.__dict__:
+                cls._randbelow = cls._randbelow_without_getrandbits
+                break
 
     def seed(self, a=None, version=2):
         """Initialize internal state from hashable object.
diff --git a/Lib/test/test_random.py b/Lib/test/test_random.py
index d91908b03d02..e7ef68ba3d26 100644
--- a/Lib/test/test_random.py
+++ b/Lib/test/test_random.py
@@ -5,7 +5,6 @@
 import time
 import pickle
 import warnings
-import logging
 from functools import partial
 from math import log, exp, pi, fsum, sin, factorial
 from test import support
@@ -940,6 +939,7 @@ def test_betavariate_return_zero(self, gammavariate_mock):
         gammavariate_mock.return_value = 0.0
         self.assertEqual(0.0, random.betavariate(2.71828, 3.14159))
 
+
 class TestRandomSubclassing(unittest.TestCase):
     def test_random_subclass_with_kwargs(self):
         # SF bug #1486663 -- this used to erroneously raise a TypeError
@@ -958,30 +958,80 @@ def test_subclasses_overriding_methods(self):
         # randrange
         class SubClass1(random.Random):
             def random(self):
-                return super().random()
+                called.add('SubClass1.random')
+                return random.Random.random(self)
 
             def getrandbits(self, n):
-                logging.getLogger('getrandbits').info('used getrandbits')
-                return super().getrandbits(n)
-        with self.assertLogs('getrandbits'):
-            SubClass1().randrange(42)
+                called.add('SubClass1.getrandbits')
+                return random.Random.getrandbits(self, n)
+        called = set()
+        SubClass1().randrange(42)
+        self.assertEqual(called, {'SubClass1.getrandbits'})
 
         # subclass providing only random => can only use random for randrange
         class SubClass2(random.Random):
             def random(self):
-                logging.getLogger('random').info('used random')
-                return super().random()
-        with self.assertLogs('random'):
-            SubClass2().randrange(42)
+                called.add('SubClass2.random')
+                return random.Random.random(self)
+        called = set()
+        SubClass2().randrange(42)
+        self.assertEqual(called, {'SubClass2.random'})
 
         # subclass defining getrandbits to complement its inherited random
         # => can now rely on getrandbits for randrange again
         class SubClass3(SubClass2):
             def getrandbits(self, n):
-                logging.getLogger('getrandbits').info('used getrandbits')
-                return super().getrandbits(n)
-        with self.assertLogs('getrandbits'):
-            SubClass3().randrange(42)
+                called.add('SubClass3.getrandbits')
+                return random.Random.getrandbits(self, n)
+        called = set()
+        SubClass3().randrange(42)
+        self.assertEqual(called, {'SubClass3.getrandbits'})
+
+        # subclass providing only random and inherited getrandbits
+        # => random takes precedence
+        class SubClass4(SubClass3):
+            def random(self):
+                called.add('SubClass4.random')
+                return random.Random.random(self)
+        called = set()
+        SubClass4().randrange(42)
+        self.assertEqual(called, {'SubClass4.random'})
+
+        # Following subclasses don't define random or getrandbits directly,
+        # but inherit them from classes which are not subclasses of Random
+        class Mixin1:
+            def random(self):
+                called.add('Mixin1.random')
+                return random.Random.random(self)
+        class Mixin2:
+            def getrandbits(self, n):
+                called.add('Mixin2.getrandbits')
+                return random.Random.getrandbits(self, n)
+
+        class SubClass5(Mixin1, random.Random):
+            pass
+        called = set()
+        SubClass5().randrange(42)
+        self.assertEqual(called, {'Mixin1.random'})
+
+        class SubClass6(Mixin2, random.Random):
+            pass
+        called = set()
+        SubClass6().randrange(42)
+        self.assertEqual(called, {'Mixin2.getrandbits'})
+
+        class SubClass7(Mixin1, Mixin2, random.Random):
+            pass
+        called = set()
+        SubClass7().randrange(42)
+        self.assertEqual(called, {'Mixin1.random'})
+
+        class SubClass8(Mixin2, Mixin1, random.Random):
+            pass
+        called = set()
+        SubClass8().randrange(42)
+        self.assertEqual(called, {'Mixin2.getrandbits'})
+
 
 class TestModule(unittest.TestCase):
     def testMagicConstants(self):